server: graceful shutdown tikv-impl by hujiatao0 · Pull Request #18930 · tikv/tikv

hujiatao0 · 2025-09-04T03:42:40Z

What is changed and how it works?

Issue Number: Close #17221

What's Changed:

When a SIGTERM signal is received, TiKV tells PD it's stopping by StoreHeartbeat. PD then try to move all the leaders from that TiKV instance before it fully shuts down.

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Release note

Support graceful shutdown for TiKV upon receiving a SIGTERM signal.

ti-chi-bot · 2025-09-04T03:42:50Z

Hi @hujiatao0. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

overvenus · 2025-09-08T02:13:02Z

cmd/tikv-server/src/main.rs

+        .arg(
+            Arg::with_name("enable-graceful-shutdown")
+                .long("enable-graceful-shutdown")
+                .takes_value(true)
+                .value_name("BOOL")
+                .help("Enable graceful shutdown for TiKV server")
+                .long_help(
+                    "Enable graceful shutdown operations like leader eviction before terminating. \
+                     Defaults to true unless explicitly set to false.",
+                ),
+        )
+        .arg(
+            Arg::with_name("evict-leader-timeout")
+                .long("evict-leader-timeout")
+                .takes_value(true)
+                .value_name("DURATION")
+                .help("Timeout for leader eviction during graceful shutdown")
+                .long_help(
+                    "Timeout for leader eviction during graceful shutdown. \
+                     After this timeout, TiKV will proceed with shutdown even if some regions \
+                     haven't completed leader transfer. Defaults to 20s.",
+                ),
+        )


Can we remove them from args? It seems unnecessary in args after all they can be configured via config file.

overvenus · 2025-09-08T02:14:20Z

src/server/config.rs

+    /// Timeout for leader eviction during graceful shutdown.
+    /// After this timeout, TiKV will proceed with shutdown even if
+    /// some regions haven't completed leader transfer.
+    #[online_config(skip)]


Please support online config change.

overvenus · 2025-09-08T02:14:58Z

src/server/config.rs

+    /// When enabled, TiKV will perform graceful shutdown operations like
+    /// leader eviction before terminating.
+    #[online_config(skip)]
+    pub enable_graceful_shutdown: bool,


It seems unnecessary, we can disable graceful shutdown if evict_leader_timeout is zero.

overvenus · 2025-09-08T02:15:17Z

src/server/config.rs

+    /// After this timeout, TiKV will proceed with shutdown even if
+    /// some regions haven't completed leader transfer.
+    #[online_config(skip)]
+    pub evict_leader_timeout: ReadableDuration,


Suggested change

pub evict_leader_timeout: ReadableDuration,

pub graceful_shutdown_timeout: ReadableDuration,

How about graceful_shutdown_timeout? It's more intuitive.

overvenus · 2025-09-08T02:20:59Z

components/server/src/signal_handler.rs

        for signal in &mut signals {
            match signal {
-                SIGTERM | SIGINT | SIGHUP => {
+                SIGTERM => {


What happens if tikv receives SIGTERM twice within graceful shutdown timeout?

The receipt of the first SIGTERM signal triggers a graceful shutdown and forces an exit from the wait_for_signal loop using a break statement. The process will be unable to handle another SIGTERM signal.

Does tikv ignores the second SIGTERM signal or tikv exits immediately?

Does tikv ignores the second SIGTERM signal or tikv exits immediately?

tikv will ignore the second SIGTERM

overvenus · 2025-09-08T02:22:29Z

components/server/src/server2.rs

+        let now = Instant::now();
+        self.set_state(true);
+        self.wait_for_leader_eviction(now);
+        self.set_state(false);


Why set it to false? Please add some comments.

When the set_state function sets the is_stopping status, it also triggers an immediate storeheartbeat. The PD heartbeat handler has logic to clean up the graceful shutdown evict-leader scheduler when a store's state is not is_stopping.

I explicitly set the state and trigger a final heartbeat after the leader eviction is complete, ensuring PD removes the scheduler promptly. While this isn't strictly required—as a node restart would eventually trigger the same cleanup via a regular heartbeat—this approach prevents a delay in the scheduler's removal.

I see, please leave the above comment in code, thanks!

overvenus

Rest LGTM

overvenus · 2025-09-12T09:52:31Z

components/server/src/signal_handler.rs

        for signal in &mut signals {
            match signal {
-                SIGTERM | SIGINT | SIGHUP => {
+                SIGTERM => {


Does tikv ignores the second SIGTERM signal or tikv exits immediately?

overvenus · 2025-09-12T09:54:03Z

components/server/src/server2.rs

+        let now = Instant::now();
+        self.set_state(true);
+        self.wait_for_leader_eviction(now);
+        self.set_state(false);


I see, please leave the above comment in code, thanks!

overvenus · 2025-09-12T09:58:23Z

To avoid blocking this PR from merging, please submit a separate PR that adds a test to verify that graceful shutdown evicts leaders.

hbisheng

Overall LGTM

Please add a release note, something like “Add graceful shutdown mechanism to evict leaders when receiving a SIGTERM signal.”

hbisheng · 2025-09-16T08:58:46Z

components/server/src/server.rs

+        self.set_state(true);
+        self.wait_for_leader_eviction(now);
+        self.set_state(false);
+        std::thread::sleep(Duration::from_millis(200));


nit: add a comment for why sleep is needed here (waiting for the final store heartbeat to be uploadedd)

Signed-off-by: hujiatao0 <hhjjtt110@gmail.com> change the signal handler Signed-off-by: hujiatao0 <hhjjtt110@gmail.com> remove enable config Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>

Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>

hbisheng · 2025-09-22T07:03:49Z

components/server/src/setup.rs

        warn!("metrics push is not supported any more.");
    }
+
+    if let Some(graceful_shutdown_timeout) = matches.value_of("graceful-shutdown-timeout") {


Is this necessary? Do we need to overwrite the config with command line args?

ti-chi-bot · 2025-09-24T10:36:38Z

[LGTM Timeline notifier]

Timeline:

2025-09-24 03:06:09.004149183 +0000 UTC m=+412379.074642866: ☑️ agreed by hbisheng.
2025-09-24 10:36:37.149399996 +0000 UTC m=+439407.219893656: ☑️ agreed by overvenus.

Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>

cfzjywxk · 2025-09-25T05:59:06Z

components/server/src/server2.rs

+    fn graceful_shutdown(&mut self) {
+        let now = Instant::now();
+        self.set_state(true);
+        self.wait_for_leader_eviction(now);


Should we wait for other possible ongoing tasks finishing like:

The cdc observer related handling

The backup restore task handling

The read write requests from kv-client, like the follower read requests

Besides, do we need a configuration for the graceful shutdow maxium wait time? From the product persipective.

Should we wait for other possible ongoing tasks finishing like:

The cdc observer related handling

The backup restore task handling

The read write requests from kv-client, like the follower read requests

Besides, do we need a configuration for the graceful shutdow maxium wait time? From the product persipective.

Actually, each TiKV node has an online-configurable graceful shutdown timeout. If this timeout is triggered while the node still has leaders that haven't been transferred, it will fall back to the standard shutdown process. The wait_for_leader_eviction step is just an additional waiting period for the leader transfer to complete. After this process finishes, it proceeds to the existing shutdown logic to handle other pre-shutdown operations. I think the operations you mentioned are already covered by this existing shutdown logic? Or these operations didn't exist previously. We should consider adding them as part of a new requirement in the future.

I think the operations you mentioned are already covered by this existing shutdown logic?

I am not quire sure, there are requests that may not require leadership like follower/stale read requests for example. Perhaps we could add a TODO about it since this needs to be merged quickly.

ti-chi-bot · 2025-09-25T08:03:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, hbisheng, overvenus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [cfzjywxk,overvenus]
~~src/server/OWNERS~~ [cfzjywxk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

close tikv#17221 When a SIGTERM signal is received, TiKV tells PD it's stopping by StoreHeartbeat. PD then try to move all the leaders from that TiKV instance before it fully shuts down. Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>

close #17221 When a SIGTERM signal is received, TiKV tells PD it's stopping by StoreHeartbeat. PD then try to move all the leaders from that TiKV instance before it fully shuts down. Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>

close tikv#17221 When a SIGTERM signal is received, TiKV tells PD it's stopping by StoreHeartbeat. PD then try to move all the leaders from that TiKV instance before it fully shuts down. Signed-off-by: hujiatao0 <hhjjtt110@gmail.com> Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

close tikv#17221 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2025-12-01T05:18:32Z

In response to a cherrypick label: new pull request created to branch release-8.5: #19157.
But this PR has conflicts, please resolve them!

close #17221 When a SIGTERM signal is received, TiKV tells PD it's stopping by StoreHeartbeat. PD then try to move all the leaders from that TiKV instance before it fully shuts down. Signed-off-by: hujiatao0 <hhjjtt110@gmail.com> Co-authored-by: hujiatao0 <hhjjtt110@gmail.com>

ti-chi-bot bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 4, 2025

hujiatao0 mentioned this pull request Sep 4, 2025

scheduler: graceful shutdown implement tikv/pd#9720

Merged

hujiatao0 changed the title ~~graceful shutdown tikv-impl~~ server: graceful shutdown tikv-impl Sep 4, 2025

overvenus reviewed Sep 8, 2025

View reviewed changes

overvenus reviewed Sep 12, 2025

View reviewed changes

overvenus requested a review from hbisheng September 16, 2025 03:55

hujiatao0 force-pushed the graceful-shutdown-impl branch from b7bc98f to 4a8804f Compare September 16, 2025 09:25

hbisheng reviewed Sep 16, 2025

View reviewed changes

hujiatao0 force-pushed the graceful-shutdown-impl branch 3 times, most recently from 2099e15 to 496fa96 Compare September 17, 2025 16:21

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 17, 2025

hujiatao0 added 2 commits September 23, 2025 17:34

graceful shutdown tikv-impl

d7defdd

Signed-off-by: hujiatao0 <hhjjtt110@gmail.com> change the signal handler Signed-off-by: hujiatao0 <hhjjtt110@gmail.com> remove enable config Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>

update cargo lock

d136b22

Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>

hujiatao0 force-pushed the graceful-shutdown-impl branch from 496fa96 to d136b22 Compare September 23, 2025 09:40

hujiatao0 added 2 commits September 23, 2025 17:52

update Cargo.toml

b0f742d

Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>

fix clippy

78ee27f

Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>

hbisheng approved these changes Sep 24, 2025

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Sep 24, 2025

overvenus approved these changes Sep 24, 2025

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 24, 2025

remove command line

84ad35d

Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>

cfzjywxk reviewed Sep 25, 2025

View reviewed changes

cfzjywxk approved these changes Sep 25, 2025

View reviewed changes

ti-chi-bot bot added the approved label Sep 25, 2025

ti-chi-bot bot merged commit 2870bde into tikv:master Sep 25, 2025
9 checks passed

ti-chi-bot bot added this to the Pool milestone Sep 25, 2025

overvenus added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Dec 1, 2025

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request Dec 1, 2025

This is an automated cherry-pick of tikv#18930

4d31b3c

close tikv#17221 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Dec 1, 2025

server: graceful shutdown tikv-impl (#18930) #19157

Merged

9 tasks

ti-chi-bot mentioned this pull request Dec 5, 2025

scheduler: graceful shutdown implement (#9720) tikv/pd#10000

Closed

	pub evict_leader_timeout: ReadableDuration,
	pub graceful_shutdown_timeout: ReadableDuration,

Conversation

hujiatao0 commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

ti-chi-bot bot commented Sep 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overvenus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overvenus commented Sep 12, 2025

Uh oh!

hbisheng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Sep 24, 2025

[LGTM Timeline notifier]

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hujiatao0 Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cfzjywxk Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Sep 25, 2025

Uh oh!

Uh oh!

ti-chi-bot commented Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hujiatao0 commented Sep 4, 2025 •

edited

Loading

hujiatao0 Sep 25, 2025 •

edited

Loading

cfzjywxk Sep 25, 2025 •

edited

Loading