Skip to content
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.

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

feat: also gracefully shutdown on SIGTERM #17802

Merged
merged 2 commits into from
Jul 30, 2024
Merged

feat: also gracefully shutdown on SIGTERM #17802

merged 2 commits into from
Jul 30, 2024

Conversation

BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Jul 24, 2024

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

...because Kubernetes will send SIGTERM when killing a pod.

As described in #17662, this can empower more seamless scaling-in in Kubernetes deployments. (correct me if I'm wrong)

After this PR, I suppose we can get rid of the extra manual step of risingwave ctl meta unregister-worker when scaling-in, as described in the doc.

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

  • My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

The step 1 of manual unregistration of the compute node when scaling-in (doc) is not necessary any more. Users can directly apply the new yaml file to trigger a graceful scale-in.

@@ -96,24 +97,26 @@ where
spawn_prof_thread(profile_path);
}

let mut sigint = tokio::signal::unix::signal(SignalKind::interrupt()).unwrap();
Copy link
Member Author

Choose a reason for hiding this comment

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

We don't support Windows, so directly use unix-only features here.

// Watch SIGINT, typically originating from user pressing ctrl-c.
// Attempt to shutdown gracefully and force shutdown on the next signal.
_ = sigint.recv() => {
tracing::info!("received ctrl-c, shutting down... (press ctrl-c again to force shutdown)");
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@BugenZhao
Copy link
Member Author

Tested with risingwave-operator and it works like a charm. 🥰

compute:

2024-07-30T09:10:05.276715082Z  INFO risingwave_rt: received SIGTERM, shutting down...
2024-07-30T09:10:05.283413439Z  INFO risingwave_rpc_client::meta_client: successfully unregistered from meta service worker_id=2004
2024-07-30T09:10:05.283480937Z  WARN risingwave_stream::task::barrier_manager: shutdown with running actors, scaling or migration will be triggered
2024-07-30T09:10:05.283581726Z  INFO risingwave_stream::task::barrier_manager: waiting for meta service to close control stream...
2024-07-30T09:10:05.285297803Z  INFO risingwave_rpc_client::meta_client: Heartbeat loop is stopped
2024-07-30T09:10:05.285344052Z  INFO risingwave_common::telemetry::report: Telemetry exit

meta:

2024-07-30T09:10:05.283791678Z  WARN risingwave_meta::barrier::info: node with running actors is deleted node_id=2004 node=Some(WorkerNode { id: 2004, r#type: ComputeNode, host: Some(HostAddress { host: "risingwave-compute-2.risingwave-compute", port: 5688 }), state: Running, property: Some(Property { is_streaming: true, is_serving: true, is_unschedulable: false }), transactional_id: Some(4), resource: None, started_at: None, parallelism: 1 }) actors={2007, 2008}
2024-07-30T09:10:05.283967173Z  INFO failure_recovery{error=worker node 2004 is shutting down prev_epoch=6887532391759872}: risingwave_meta::barrier::recovery: recovery start!
2024-07-30T09:10:05.287488493Z  INFO risingwave_meta::hummock::manager::worker: Released hummock context 2004
2024-07-30T09:10:05.288955702Z  INFO failure_recovery{error=worker node 2004 is shutting down prev_epoch=6887532391759872}:recovery_attempt: risingwave_meta::manager::catalog::fragment: cleaning dirty downstream merge nodes for table sink
2024-07-30T09:10:05.293052214Z  INFO failure_recovery{error=worker node 2004 is shutting down prev_epoch=6887532391759872}:recovery_attempt: risingwave_meta::barrier::recovery: recovering mview progress
2024-07-30T09:10:05.293080297Z  INFO failure_recovery{error=worker node 2004 is shutting down prev_epoch=6887532391759872}:recovery_attempt: risingwave_meta::barrier::recovery: recovered mview progress
2024-07-30T09:10:05.304094784Z  INFO failure_recovery{error=worker node 2004 is shutting down prev_epoch=6887532391759872}:recovery_attempt: risingwave_meta::barrier::recovery: offline rescheduling for job 2001 in recovery is done

@BugenZhao BugenZhao removed the request for review from a team July 30, 2024 09:17
@BugenZhao BugenZhao added this pull request to the merge queue Jul 30, 2024
@BugenZhao BugenZhao added the user-facing-changes Contains changes that are visible to users label Jul 30, 2024
Merged via the queue into main with commit b8e08c7 Jul 30, 2024
35 of 37 checks passed
@BugenZhao BugenZhao deleted the bz/shutdown-sigterm branch July 30, 2024 09:44
@fuyufjh
Copy link
Member

fuyufjh commented Jul 31, 2024

cc. @arkbriar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature user-facing-changes Contains changes that are visible to users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants