Merged
Conversation
Contributor
driv3r
commented
Apr 14, 2026
driv3r
commented
- align default zerolog log level with logrus (info vs trace)
- introduce slog handler wrapper around our Logger interface which we can pass into BinlogStreamer
zerolog's own ctx.go init() sets SetGlobalLevel(TraceLevel), which passes all messages — including Debug — when no explicit level is configured. logrus defaults to InfoLevel, so switching to the zerolog backend produced unexpected debug noise. Use a package-level var initializer in zerolog_logger.go to set InfoLevel before ghostferry's own init() runs, ensuring env-var and Config.LogLevel overrides still take effect after the default is applied. Also fix the test teardown helper to explicitly reset zerolog's global level on every teardown: because zerolog's level is a separate atomic from logrus's, restoring the logrus backend without also resetting zerolog's level left it contaminated for subsequent zerolog tests.
…ostferry Introduce slog_handler.go with a backend-agnostic slog.Handler that wraps the ghostferry Logger interface. It maps slog levels to Logger.Debug/Info/ Warn/Error, flattens slog attributes and groups into structured fields via WithField, and delegates level filtering to the active backend. Works with both zerolog and logrus without any backend-specific code. Wire it into BinlogStreamer.createBinlogSyncer via BinlogSyncerConfig.Logger so that go-mysql-org/go-mysql's BinlogSyncer logs flow through the same tagged ghostferry logger as the rest of the streamer, instead of bypassing it via slog.Default().
The only usage was a single SetDefaultLogger(NullHandler{}) call in
ferry.go whose purpose was to silence go-log output. No vendored runtime
dependency imports go-log anymore — go-mysql-org/go-mysql switched to
log/slog — so the suppression call and the import are both dead code.
Remove the import and call, run go mod tidy and go mod vendor to drop
the module from go.mod, go.sum and vendor/.
test_interrupt_resume_idempotence_with_uuid_table (and similar tests) failed spuriously when GHOSTFERRY_LOG_BACKEND=zerolog. The failure was a test-harness race, not a product bug. Root cause: Ghostferry panics on SIGTERM before cutover (the intended behaviour). send_signal snapshots @pid before checking it, but the subprocess thread can set @pid = 0 and the OS can reap the child between the snapshot and Process.kill. This raises Errno::ESRCH, which propagates out of the WEBrick status handler and fails the test. The uuid-table variant is more susceptible because it has two tables in scope and DataIterationConcurrency defaults to 4, so multiple concurrent AFTER_ROW_COPY callbacks can arrive while the process is already shutting down. Fix 1 — ghostferry_helper.rb send_signal: snapshot @pid atomically, guard against pid == 0, and rescue Errno::ESRCH as benign. Fix 2 — test_helper.rb new_ghostferry_with_interrupt_after_row_copy: add an `interrupted` flag so TERM is sent at most once per instance, regardless of how many AFTER_ROW_COPY callbacks fire. Add ghostferry_helper_test.rb with two deterministic regression tests that verify send_signal is a no-op when @pid == 0 and when the target process has already exited.
The previous commit introduced an `interrupted` one-shot guard in new_ghostferry_with_interrupt_after_row_copy that changed the threshold from `>=` to `>`, shifting the interrupt to one batch later than intended. Tests such as test_interrupt_resume_idempotence_with_multiple_interrupts and _with_multiple_interrupts_and_writes_to_source resume from a state where few batches remain; with the off-by-one the process completed normally instead of being interrupted, causing run_expecting_interrupt to raise "Ghostferry did not get interrupted" consistently. Revert new_ghostferry_with_interrupt_after_row_copy to the original `>=` semantics with no one-shot guard. The Errno::ESRCH flake is already handled by the ESRCH rescue in send_signal, so the guard is not needed.
austenLacy
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.