Repository navigation
*: refine exit code handling - #68096
Conversation
ti-chi-bot
Bot
added
do-not-merge/needs-linked-issue
do-not-merge/needs-tests-checked
release-note-none
|
Hi @ChangRui-Ryan. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. DetailsInstructions 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughShutdown handlers now receive the actual ChangesSignal & shutdown
Sequence DiagramsequenceDiagram
participant OS as Operating System
participant Handler as Signal Handler
participant Main as Main
participant Server as Server/Resources
participant Logger as syncLog
OS->>Handler: termination signal (SIGINT/SIGTERM)
Handler->>Handler: log received signal
Handler->>Main: shutdownFunc(sig)
Main->>Main: set exitCode (SIGINT → interrupt code)
Main->>Server: perform shutdown sequence
Server-->>Main: shutdown complete
Main->>Logger: call syncLog()
Logger->>Logger: fsync logs (ignore /dev/stdout EINVAL)
Logger-->>Main: return error or nil
Main->>Main: adjust exitCode based on syncLog result
Main->>OS: exit with determined exitCode (only if non-OK)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #68096 +/- ##
================================================
- Coverage 77.7489% 76.9862% -0.7627%
================================================
Files 1990 1972 -18
Lines 551632 556294 +4662
================================================
- Hits 428888 428270 -618
- Misses 121824 127800 +5976
+ Partials 920 224 -696
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/util/signal/signal_posix.go (1)
73-85:⚠️ Potential issue | 🔴 CriticalKeep
SetupSignalHandleraligned across all build tags.The POSIX implementation now takes
func(sig os.Signal), but the Windows and WASM implementations have different signatures: Windows usesfunc()and WASM usesfunc(bool). This breaks the exported API contract and prevents portable code from using this function across platforms.All three implementations must be updated to share the same signature:
pkg/util/signal/signal_posix.go:74—func(sig os.Signal)pkg/util/signal/signal_windows.go:31—func()pkg/util/signal/signal_wasm.go:21—func(bool)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/util/signal/signal_posix.go` around lines 73 - 85, The exported SetupSignalHandler signatures are inconsistent across platforms; update the Windows and WASM implementations to match the POSIX signature SetupSignalHandler(shutdownFunc func(sig os.Signal)). Edit the SetupSignalHandler declaration in signal_windows.go and signal_wasm.go to accept func(sig os.Signal), and update their internal calls to shutdownFunc to pass an os.Signal value (use nil if no concrete signal is available on that platform) so the API is portable and caller-compatible with the POSIX implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@pkg/util/signal/signal_posix.go`:
- Around line 73-85: The exported SetupSignalHandler signatures are inconsistent
across platforms; update the Windows and WASM implementations to match the POSIX
signature SetupSignalHandler(shutdownFunc func(sig os.Signal)). Edit the
SetupSignalHandler declaration in signal_windows.go and signal_wasm.go to accept
func(sig os.Signal), and update their internal calls to shutdownFunc to pass an
os.Signal value (use nil if no concrete signal is available on that platform) so
the API is portable and caller-compatible with the POSIX implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2d697f19-98d7-4ce6-a42c-63e368a1817d
📒 Files selected for processing (2)
cmd/tidb-server/main.gopkg/util/signal/signal_posix.go
ti-chi-bot
Bot
removed
do-not-merge/needs-tests-checked
do-not-merge/needs-linked-issue
labels
|
I think it breaks backward compatibility. For example, some user scripts may have rely on the error code. Please check with PM about this change? |
687c0dd to
596f50e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/util/signal/signal_wasm.go (1)
22-24: Document that this handler is intentionally a no-op on WASM.
SetupSignalHandleris exported, but this implementation never invokes the callback. A short doc comment calling out the WASM no-op behavior would make the platform contract much clearer for callers.As per coding guidelines, "Comments SHOULD explain non-obvious intent, constraints, invariants, concurrency guarantees, SQL/compatibility contracts, or important performance trade-offs" and "Keep exported-symbol doc comments, and prefer semantic constraints over name restatement."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/util/signal/signal_wasm.go` around lines 22 - 24, Add an explanatory doc comment to the exported SetupSignalHandler function indicating that on WASM this implementation is intentionally a no-op and will not invoke the provided shutdownFunc callback; update the comment above SetupSignalHandler to state the platform constraint and the expected behavior callers should rely on so it's clear why the function body is empty.cmd/tidb-server/main.go (1)
149-153: Explain the130convention inline.
exitCodeIntreads like a magic number unless the reader already knows the shell convention. A brief note like128 + SIGINTwould make the intent obvious during future refactors.As per coding guidelines, "Code SHOULD be self-documenting through clear naming and structure" and "Comments SHOULD explain non-obvious intent, constraints, invariants, concurrency guarantees, SQL/compatibility contracts, or important performance trade-offs."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/tidb-server/main.go` around lines 149 - 153, Add an inline explanation for the magic number used by exitCodeInt so readers know it represents a signal-based exit (130 == 128 + SIGINT); update the const block for exitCodeInt to include a brief comment like "130 == 128 + SIGINT" and optionally compute it from syscall.SIGINT (e.g., 128 + int(syscall.SIGINT)) using the exitCodeInt symbol to avoid the magic number and make intent explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/util/signal/signal_windows.go`:
- Around line 47-57: TiDBExit currently swallows failures on Windows because
os.Process.Signal only supports os.Kill; change TiDBExit to return an error
instead of hiding it and explicitly handle unsupported signals on Windows: in
TiDBExit, after os.FindProcess, detect if sig is not syscall.SIGKILL (or
os.Kill) and return a clear error (e.g., fmt.Errorf("signal %v not supported on
Windows", sig)) rather than just logging, and update callers to handle the
returned error; reference the TiDBExit function, os.FindProcess, and p.Signal
when making these changes.
---
Nitpick comments:
In `@cmd/tidb-server/main.go`:
- Around line 149-153: Add an inline explanation for the magic number used by
exitCodeInt so readers know it represents a signal-based exit (130 == 128 +
SIGINT); update the const block for exitCodeInt to include a brief comment like
"130 == 128 + SIGINT" and optionally compute it from syscall.SIGINT (e.g., 128 +
int(syscall.SIGINT)) using the exitCodeInt symbol to avoid the magic number and
make intent explicit.
In `@pkg/util/signal/signal_wasm.go`:
- Around line 22-24: Add an explanatory doc comment to the exported
SetupSignalHandler function indicating that on WASM this implementation is
intentionally a no-op and will not invoke the provided shutdownFunc callback;
update the comment above SetupSignalHandler to state the platform constraint and
the expected behavior callers should rely on so it's clear why the function body
is empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 14701fe4-936a-4c28-96ae-ca564b2b40f3
📒 Files selected for processing (5)
cmd/tidb-server/main.gopkg/util/signal/exit.gopkg/util/signal/signal_posix.gopkg/util/signal/signal_wasm.gopkg/util/signal/signal_windows.go
✅ Files skipped from review due to trivial changes (1)
- pkg/util/signal/exit.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/util/signal/signal_posix.go
| // TiDBExit sends a signal to the current process. | ||
| func TiDBExit(sig syscall.Signal) { | ||
| p, err := os.FindProcess(os.Getpid()) | ||
| if err != nil { | ||
| logutil.BgLogger().Error("failed to find current process", zap.Error(err), zap.Stringer("signal", sig)) | ||
| return | ||
| } | ||
| if err := p.Signal(sig); err != nil { | ||
| logutil.BgLogger().Error("failed to send signal", zap.Error(err), zap.Stringer("signal", sig)) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the current Go documentation for Windows, does os.Process.Signal support delivering syscall.SIGINT or syscall.SIGTERM to the current process? Please cite the relevant os.Process.Signal / os.Interrupt documentation and Windows-specific caveats.
💡 Result:
No, in the current Go documentation for Windows, os.Process.Signal does not support delivering syscall.SIGINT or syscall.SIGTERM to the current process (or any process). It only supports os.Kill, which forces termination without graceful handling. Sending os.Interrupt (equivalent to SIGINT) is explicitly not implemented on Windows and returns an error. The os package documentation for Process.Signal states: "The only signal values guaranteed to be present in the os package on all systems are os.Interrupt (send the process an interrupt) and os.Kill (force the process to exit). On Windows, sending os.Interrupt to a process with os.Process.Signal is not implemented; it will return an error instead of sending a signal." (cited from multiple GitHub issues #46345, #6720, #27854 quoting the docs, and StackOverflow answers referencing it). syscall.SIGINT and syscall.SIGTERM exist as constants on Windows (SIGINT=2, SIGTERM=15), but os.Process.Signal does not support arbitrary syscall.Signal values on Windows; it only handles Kill and would return EWINDOWS for others. Windows-specific caveats: Windows lacks Unix-style signals. os.Interrupt can be received via os/signal.Notify (e.g., Ctrl+C maps to os.Interrupt), but cannot be sent programmatically to a specific process via Process.Signal due to API limitations (GenerateConsoleCtrlEvent sends to console process groups, not individual processes, and only for attached console processes created with specific flags). Issues #6720 and #46345 discuss this in detail, closed as impossible without changing semantics. For child processes from exec.Cmd, if not in the same console group, it won't work reliably. os/signal.NotifyContext documentation notes os.Interrupt reception on Windows but does not claim sending support. No changes noted up to Go 1.25.6 or recent issues as of 2026.
Citations:
- 1: https://pkg.go.dev/os/signal@go1.25.6
- 2: https://pkg.go.dev/os/signal
- 3: os: support Process.Signal(Interrupt) on Windows golang/go#46345
- 4: os: Interrupt is not sendable on Windows golang/go#6720
- 5: https://go.dev/issue/27854
- 6: https://pkg.go.dev/os/signal/
- 7: https://go.pact.im/doc/use/os..Process.Signal.html
- 8: https://docs-go.hexacode.org/src/os/signal/doc.go
- 9: NOT SUPPORT send SIGINT to other process on windows? golang/go#28498
- 10: https://www.go.dev
- 11: https://www.reclaimedcanada.com
TiDBExit silently fails on Windows for non-Kill signals.
On Windows, os.Process.Signal() does not support Unix-style signals like syscall.SIGINT or syscall.SIGTERM—it only supports os.Kill. Per Go's os package documentation and GitHub issues #46345 and #6720, sending os.Interrupt via Process.Signal returns an error on Windows instead of delivering the signal. This function logs the error and returns, leaving the process running despite the expectation of a signal handler effect. Either document the Windows limitation explicitly, return an error to the caller, or implement platform-appropriate shutdown logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/util/signal/signal_windows.go` around lines 47 - 57, TiDBExit currently
swallows failures on Windows because os.Process.Signal only supports os.Kill;
change TiDBExit to return an error instead of hiding it and explicitly handle
unsupported signals on Windows: in TiDBExit, after os.FindProcess, detect if sig
is not syscall.SIGKILL (or os.Kill) and return a clear error (e.g.,
fmt.Errorf("signal %v not supported on Windows", sig)) rather than just logging,
and update callers to handle the returned error; reference the TiDBExit
function, os.FindProcess, and p.Signal when making these changes.
There was a problem hiding this comment.
IMHO it is fine since TiDB is rarely deployed(not officially supported) on Windows.
There was a problem hiding this comment.
@bb7133, understood — that's a fair point. Since TiDB is not officially supported on Windows, the best-effort semantics of TiDBExit are acceptable there. I'll note this for future reviews.
✏️ Learnings added
Learnt from: bb7133
Repo: pingcap/tidb PR: 68096
File: pkg/util/signal/signal_windows.go:47-55
Timestamp: 2026-05-06T06:23:01.302Z
Learning: TiDB is rarely deployed and not officially supported on Windows. Windows-specific limitations (e.g., os.Process.Signal not supporting Unix-style signals like SIGINT/SIGTERM) are acceptable in pkg/util/signal/signal_windows.go since Windows is not an officially supported deployment target for TiDB.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: joechenrh
Repo: pingcap/tidb PR: 66878
File: pkg/planner/core/logical_plan_builder.go:6455-6472
Timestamp: 2026-03-11T06:29:00.122Z
Learning: Ensure code reviews verify the VirtualAssignmentsOffset semantics: the planner sets Update.VirtualAssignmentsOffset = len(update.List). The executor should only apply OrderedList[:VirtualAssignmentsOffset] when composing new rows, and only after the 'changed' check should it group/evaluate OrderedList[VirtualAssignmentsOffset:] per table. This pattern applies to files under pkg/planner/core and pkg/executor (e.g., common_plans.go and update.go). Reviewers should check that updates respect slicing behavior, that the offset is consistently derived from the planner, and that downstream code does not bypass the offset when creating new rows. Add tests validating both branches: the slice before the offset for new rows, and the per-table handling of the slice after the offset.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/tidb-server/main.go (1)
428-433: Consider documenting exit-code precedence in-line.At Line 428–Line 430,
syncLog()failure overwrites a prior SIGINT code (130) with1. A brief comment here would make this contract obvious to future maintainers.Suggested inline clarification
<-exited + // Exit-code precedence: + // 1) log sync failure -> 1 + // 2) otherwise keep signal-derived code (e.g. SIGINT -> 130) if err := syncLog(); err != nil { exitCode = exitCodeErr }As per coding guidelines "Comments SHOULD explain non-obvious intent, constraints, invariants, concurrency guarantees, SQL/compatibility contracts, or important performance trade-offs, and SHOULD NOT restate what the code already makes clear."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/tidb-server/main.go` around lines 428 - 433, Add a short inline comment explaining exit-code precedence: when syncLog() returns an error it sets exitCode = exitCodeErr (overriding any prior signal-derived value such as SIGINT/130), and then the later check if exitCode != exitCodeOK causes os.Exit(exitCode); update the block around syncLog(), exitCode, exitCodeErr, exitCodeOK to include this clarification so future maintainers understand that syncLog() failures take precedence over earlier signal codes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/tidb-server/main.go`:
- Around line 428-433: Add a short inline comment explaining exit-code
precedence: when syncLog() returns an error it sets exitCode = exitCodeErr
(overriding any prior signal-derived value such as SIGINT/130), and then the
later check if exitCode != exitCodeOK causes os.Exit(exitCode); update the block
around syncLog(), exitCode, exitCodeErr, exitCodeOK to include this
clarification so future maintainers understand that syncLog() failures take
precedence over earlier signal codes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b1bbcdf9-ef85-47af-8efc-add3767b55d2
📒 Files selected for processing (3)
cmd/tidb-server/main.gopkg/util/signal/signal_wasm.gopkg/util/signal/signal_windows.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/util/signal/signal_windows.go
- pkg/util/signal/signal_wasm.go
5a4ff81 to
fec3785
Compare
Thanks for raising this. I think the compatibility impact should be acceptable here. Before this change, these abnormal shutdown paths did not expose useful information through the process exit status. Whether TiDB was interrupted by a signal or hit an error during shutdown, the observable exit code could still be This PR keeps the normal successful exit path returning So if a user previously checked the exit code, they could not distinguish these abnormal cases anyway. The new behavior gives them more accurate information instead of preserving an indistinguishable |
|
/retest |
|
@ChangRui-Ryan: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
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. |
|
/retest |
|
@ChangRui-Ryan: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
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. |
|
/retest |
|
@ChangRui-Ryan: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
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. |
| <-exited | ||
| syncLog() | ||
| if err := syncLog(); err != nil { | ||
| exitCode = exitCodeErr |
There was a problem hiding this comment.
This looks reasonable to me...but how about adding a comment here for this?
ti-chi-bot
Bot
added
approved
needs-1-more-lgtm
|
@iosmanthus: adding LGTM is restricted to approvers and reviewers in OWNERS files. DetailsIn response to this: 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. |
fec3785 to
a50e843
Compare
a50e843 to
c571849
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/util/signal/signal_windows.go (1)
47-55: ⚡ Quick winLog the
FindProcesserror before silently returning.The
p.Signalerror is intentionally discarded and explained by the "best effort" comment, but theFindProcesserror path is a silent no-op with no trace. On the rare occasionFindProcessfails, there is no observable evidence and the caller has no way to detect that no signal was delivered. A single log line would make post-mortem debugging possible without changing behaviour.♻️ Proposed fix
func TiDBExit(sig syscall.Signal) { p, err := os.FindProcess(os.Getpid()) if err != nil { + logutil.BgLogger().Error("TiDBExit: failed to find current process", zap.Error(err)) return } // Best effort; Windows does not support POSIX signal shutdown semantics. _ = p.Signal(sig) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/util/signal/signal_windows.go` around lines 47 - 55, TiDBExit currently silently returns if os.FindProcess(os.Getpid()) fails; change it to log the error before returning so failures are observable. In the TiDBExit function, after the os.FindProcess(...) call, if err != nil then emit a single log entry including the error (e.g., "FindProcess failed" + err) using the package's logger or log.Printf, then return; keep the rest of the function (the best-effort call to p.Signal) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/util/signal/signal_windows.go`:
- Around line 47-55: TiDBExit currently silently returns if
os.FindProcess(os.Getpid()) fails; change it to log the error before returning
so failures are observable. In the TiDBExit function, after the
os.FindProcess(...) call, if err != nil then emit a single log entry including
the error (e.g., "FindProcess failed" + err) using the package's logger or
log.Printf, then return; keep the rest of the function (the best-effort call to
p.Signal) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3e5af844-bd36-40e7-8403-e4637c8065b8
📒 Files selected for processing (6)
cmd/tidb-server/main.gocmd/tidb-server/main_test.gopkg/util/signal/exit.gopkg/util/signal/signal_posix.gopkg/util/signal/signal_wasm.gopkg/util/signal/signal_windows.go
✅ Files skipped from review due to trivial changes (1)
- pkg/util/signal/exit.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/util/signal/signal_posix.go
- pkg/util/signal/signal_wasm.go
- cmd/tidb-server/main.go
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bb7133, iosmanthus, wjhuang2016 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
Resolve conflicts against ~3.5 months of master drift: - pkg/util/codec, pkg/tablecodec: rebase the DESC-aware encoding onto the explicit-collation Encoder introduced by pingcap#69566. EncodeKeyWithDesc becomes an Encoder method (with the package-level wrapper kept for range/probe-key callers), and GenIndexKey dispatches through the caller-supplied encoder on both the ASC fast path and the DESC path. - pkg/executor: thread indexColDescFlags through the refactored buildKeyRanges signature (new rangeMemTracker) and the new unified IndexMergeReaderExecutor.buildPartialWorkerKVRanges. - Resolve files touched only by interim cherry-picks (pingcap#68096, pingcap#68137, pingcap#68148, pingcap#68168) to their canonical upstream versions. - BUILD.bazel: union of dependency edits; adopt upstream shard_count. Verified with go vet + unit tests on all touched packages, the pkg/ddl DESC feature tests (-tags=intest), and a full bazel CI build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: takaidohigasi <takaidohigasi@gmail.com>
5 participants
What problem does this PR solve?
Issue Number: ref #67765
Problem Summary:
TiDB handles shutdown signals itself in order to perform graceful shutdown. As a result, when TiDB receives SIGINT, the process can finish cleanup and exit with code 0, which makes an interrupted shutdown indistinguishable from a normal successful exit for scripts, supervisors, or operators checking the process exit status.
SIGINT is conventionally reported as exit code 130 (
128 + SIGINT). TiDB should preserve this convention after graceful shutdown, while keeping SIGTERM/SIGHUP/SIGQUIT successful graceful shutdown behavior unchanged.What changed and how does it work?
This PR passes the received signal into the TiDB shutdown callback. During shutdown,
tidb-serverrecords the intended exit code and sets it to 130 when the received signal is SIGINT. Other shutdown signals continue to use the normal successful exit path if cleanup succeeds.This PR also changes
syncLog()to return an error instead of callingos.Exit()internally, so the final exit code is decided in one place inmain(). If log sync fails, TiDB exits with code 1; otherwise, SIGINT exits with 130 and normal graceful shutdown exits with 0.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit