Skip to content

fix: Add startupProbe to prevent Superset startup problems #654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: Add startupProbe to prevent Superset startup problems #654

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f26422
fix: Add startupProbe to prevent Superset startup problems
sbernauer Jul 31, 2025
91e4392
changelog
sbernauer Jul 31, 2025
8edc0a1
Update CHANGELOG.md
sbernauer Aug 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

## [Unreleased]

### Fixed

- Fix container not starting because Superset was starting too slow and was killed because a failing liveness probe.
We now add a proper startup probe, which allows Superset to take longer to start up ([#654]).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indenation

Suggested change
- Fix container not starting because Superset was starting too slow and was killed because a failing liveness probe.
We now add a proper startup probe, which allows Superset to take longer to start up ([#654]).
- Fix container not starting because Superset was starting too slow and was killed because a failing liveness probe.
We now add a proper startup probe, which allows Superset to take longer to start up ([#654]).

But I think it could be briefer:

Suggested change
- Fix container not starting because Superset was starting too slow and was killed because a failing liveness probe.
We now add a proper startup probe, which allows Superset to take longer to start up ([#654]).
- Add startup probe to give Superset longer to start up than the default ([#654]).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the current two sentences as they make a bit clearer what the problem was exactly and what the impact was.

than the default

There is no default, only before (like 60s) and after (~10 minutes) ^^

Copy link
Member

@NickLarsenNZ NickLarsenNZ Aug 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no default, only before (like 60s) and after (~10 minutes) ^^

Where was the 60 defined?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

15s initial_delay_seconds + failure_threshold (3) * period_seconds (15s)

Copy link
Member

@NickLarsenNZ NickLarsenNZ Aug 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where those are defined (in the current change) 😅

I feel like there should be a comment explaining how to derive the values for the next person who comes along (even better if it can be done using consts or some runtime vars.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created stackabletech/operator-rs#1078


[#654]: https://github.com/stackabletech/superset-operator/pull/654

## [25.7.0] - 2025-07-23

## [25.7.0-rc1] - 2025-07-18
Expand Down
45 changes: 30 additions & 15 deletions rust/operator-binary/src/superset_controller.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,21 +786,7 @@ fn build_server_rolegroup_statefulset(
create_vector_shutdown_file_command(STACKABLE_LOG_DIR),
}])
.resources(merged_config.resources.clone().into());
let probe = Probe {
http_get: Some(HTTPGetAction {
port: IntOrString::Int(APP_PORT.into()),
path: Some("/health".to_string()),
..HTTPGetAction::default()
}),
initial_delay_seconds: Some(15),
period_seconds: Some(15),
timeout_seconds: Some(1),
failure_threshold: Some(3),
success_threshold: Some(1),
..Probe::default()
};
superset_cb.readiness_probe(probe.clone());
superset_cb.liveness_probe(probe);
add_superset_container_probes(&mut superset_cb);

// listener endpoints will use persistent volumes
// so that load balancers can hard-code the target addresses and
Expand Down Expand Up @@ -932,6 +918,35 @@ fn build_server_rolegroup_statefulset(
})
}

fn add_superset_container_probes(superset_cb: &mut ContainerBuilder) {
let probe_action = HTTPGetAction {
port: IntOrString::Int(APP_PORT.into()),
path: Some("/health".to_string()),
..HTTPGetAction::default()
};
let common_probe = Probe {
http_get: Some(probe_action),
period_seconds: Some(5),
timeout_seconds: Some(5),
success_threshold: Some(1),
..Probe::default()
};
superset_cb.startup_probe(Probe {
failure_threshold: Some(10 /* minutes */ * 60 / 5),
..common_probe.clone()
});
// Remove it from the Service immediately
superset_cb.readiness_probe(Probe {
failure_threshold: Some(1),
..common_probe.clone()
});
// But only restart it after 3 failures
superset_cb.readiness_probe(Probe {
failure_threshold: Some(3),
..common_probe
});
}

fn add_authentication_volumes_and_volume_mounts(
auth_config: &SupersetClientAuthenticationDetailsResolved,
cb: &mut ContainerBuilder,
Expand Down
Loading