fix: speed up hakeeper bootstrap - #26760
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
There was a problem hiding this comment.
Pull request overview
This PR aims to speed up HAKeeper bootstrap (especially in test environments) by making HAKeeper state checks more responsive during bootstrap phases and by triggering earlier heartbeats when bootstrap progress is made.
Changes:
- Make HAKeeper check interval adaptive (faster during bootstrap, normal when running) and wire it into the store ticker loop.
- Trigger logservice heartbeats on key bootstrap events to accelerate state propagation.
- Add/adjust unit tests for the new bootstrap/check-interval behaviors and leader selection in test clusters.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/tests/service/service.go | Passes context into initial cluster-info initialization during logservice startup. |
| pkg/tests/service/logservice.go | Updates cluster-info initialization to target the HAKeeper leader. |
| pkg/tests/service/service_test.go | Adds a unit test ensuring initial cluster info is set via the HAKeeper leader. |
| pkg/logservice/store.go | Replaces fixed HAKeeper check ticker with an adaptive timer; adds bootstrap callbacks/state for responsiveness. |
| pkg/logservice/store_hakeeper_check.go | Introduces adaptive check interval logic and returns state from hakeeperCheck for scheduling decisions. |
| pkg/logservice/store_hakeeper_check_test.go | Adds tests for adaptive interval logic and bootstrap callback invocation. |
| pkg/logservice/service.go | Adds heartbeat trigger channel wiring via bootstrap callback. |
| pkg/logservice/service_commands.go | Adds an explicit on-demand heartbeat trigger path. |
| pkg/logservice/service_bootstrap.go | Waits for leader readiness before proposing initial cluster info; requests heartbeat after proposal. |
| pkg/embed/operator.go | Speeds up polling loops (smaller sleep) and throttles logs while waiting for cluster conditions. |
| pkg/embed/operator_test.go | Adds a unit test for the new polling interval. |
| func (l *store) nextHAKeeperCheckInterval(state *pb.CheckerState) time.Duration { | ||
| interval := l.cfg.HAKeeperCheckInterval.Duration | ||
| if state == nil || state.State != pb.HAKeeperRunning { | ||
| if interval > bootstrapHAKeeperCheckInterval { | ||
| return bootstrapHAKeeperCheckInterval | ||
| } | ||
| } | ||
| return interval | ||
| } |
There was a problem hiding this comment.
Fixed in 33cdfa7. nil checker state now keeps the configured HAKeeper check interval, so non-leader LogServices do not stay on the 100ms bootstrap cadence. Verified with GOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^(TestServiceBootstrapRestoresHAKeeperAndWAL|TestSetInitialClusterInfo|TestNextHAKeeperCheckIntervalUsesFastBootstrapInterval|TestBootstrap|TestFailedBootstrap)$' -count=1 -timeout=180s ./pkg/logservice.
| func TestNextHAKeeperCheckIntervalUsesFastBootstrapInterval(t *testing.T) { | ||
| s := &store{ | ||
| cfg: Config{ | ||
| HAKeeperCheckInterval: toml.Duration{Duration: 3 * time.Second}, | ||
| }, | ||
| } | ||
|
|
||
| require.Equal(t, bootstrapHAKeeperCheckInterval, s.nextHAKeeperCheckInterval(nil)) | ||
| require.Equal(t, bootstrapHAKeeperCheckInterval, s.nextHAKeeperCheckInterval(&pb.CheckerState{ | ||
| State: pb.HAKeeperBootstrapping, | ||
| })) | ||
| require.Equal(t, bootstrapHAKeeperCheckInterval, s.nextHAKeeperCheckInterval(&pb.CheckerState{ | ||
| State: pb.HAKeeperBootstrapCommandsReceived, | ||
| })) | ||
| require.Equal(t, 3*time.Second, s.nextHAKeeperCheckInterval(&pb.CheckerState{ | ||
| State: pb.HAKeeperRunning, | ||
| })) | ||
| } |
There was a problem hiding this comment.
Fixed in 33cdfa7. The test now asserts nil uses the configured interval, adds an explicit HAKeeperCreated bootstrap case, and checks the separate initial fast interval helper. Verified with GOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^(TestServiceBootstrapRestoresHAKeeperAndWAL|TestSetInitialClusterInfo|TestNextHAKeeperCheckIntervalUsesFastBootstrapInterval|TestBootstrap|TestFailedBootstrap)$' -count=1 -timeout=180s ./pkg/logservice.
| case pb.HAKeeperCreated: | ||
| l.runtime.Logger().Warn("waiting for initial cluster info to be set, check skipped") | ||
| return | ||
| return state |
There was a problem hiding this comment.
Fixed in 33cdfa7. The HAKeeperCreated warning is now throttled to once per second using the existing bootstrap log timestamp. Verified with GOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^(TestServiceBootstrapRestoresHAKeeperAndWAL|TestSetInitialClusterInfo|TestNextHAKeeperCheckIntervalUsesFastBootstrapInterval|TestBootstrap|TestFailedBootstrap)$' -count=1 -timeout=180s ./pkg/logservice.
| stopper *stopper.Stopper | ||
| haClient LogHAKeeperClient | ||
| fileService fileservice.FileService | ||
| heartbeatC chan struct{} | ||
| shutdownC chan struct{} |
There was a problem hiding this comment.
Fixed. Updated the PR title to fix: speed up hakeeper bootstrap and adjusted the body to describe the runtime bootstrap responsiveness and steady-state interval behavior.
| haTimer := time.NewTimer(l.nextHAKeeperCheckInterval(nil)) | ||
| defer haTimer.Stop() |
There was a problem hiding this comment.
Fixed in 33cdfa7. The ticker now starts with a separate initial fast interval, while subsequent nil states use the configured interval. Verified with GOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^(TestServiceBootstrapRestoresHAKeeperAndWAL|TestSetInitialClusterInfo|TestNextHAKeeperCheckIntervalUsesFastBootstrapInterval|TestBootstrap|TestFailedBootstrap)$' -count=1 -timeout=180s ./pkg/logservice.
| leader := c.WaitHAKeeperLeader(ctx) | ||
| err = leader.SetInitialClusterInfo( | ||
| c.opt.initial.logShardNum, |
There was a problem hiding this comment.
Fixed in 33cdfa7. setInitialClusterInfo now caches the once.Do result on the test cluster instead of waiting on a per-call channel, and TestSetInitialClusterInfoCanBeCalledAgain covers the second-call path. Verified with GOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^(TestSetInitialClusterInfoUsesHAKeeperLeader|TestSetInitialClusterInfoCanBeCalledAgain)$' -count=1 -timeout=60s ./pkg/tests/service.
8438e79 to
56ed0e6
Compare
volgariver6
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
pkg/tests/service/logservice.go:243
- setInitialClusterInfo now calls WaitHAKeeperLeader(), which polls every 100ms. WaitHAKeeperLeader/getHAKeeperLeader currently emit an Info log ("hakeeper state") on every poll for each started HAKeeper service (see pkg/tests/service/service.go:1613-1617). During slow leader election this can spam CI logs and slow tests. Consider throttling that log (e.g., once per second) or reducing it to Debug during leader-wait loops.
c.logger.Info("initialize cluster information")
leader := c.WaitHAKeeperLeader(ctx)
err = leader.SetInitialClusterInfo(
What type of PR is this?
Which issue(s) this PR fixes:
issue #26699
What this PR does / why we need it:
This reduces HAKeeper bootstrap latency by making bootstrap-phase checks and heartbeats more responsive while preserving the configured steady-state check interval. The previous path waited on coarse polling intervals while HAKeeper elected a leader, accepted initial cluster info, generated bootstrap commands, and reported running state. That made tests such as
TestSpeedupAbortAllTxnspend unnecessary time before the SQL workload started.Changes
Tests
GOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^TestSpeedupAbortAllTxn$' -count=1 -timeout=180s ./pkg/tests/issuesGOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^(TestClusterConditionCheckInterval|Test_waitHAKeeperRunningLocked|Test_waitAnyShardReadyLocked|TestHAKeeperRunningTimeout|TestWaitClusterConditionClosesHAKeeperClient)$' -count=1 -timeout=120s ./pkg/embedGOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^TestSetInitialClusterInfoUsesHAKeeperLeader$' -count=1 -timeout=60s ./pkg/tests/serviceGOPROXY=https://goproxy.cn,direct .agents/skills/mo-dev/scripts/mo-cgo-test -v -run '^(TestServiceBootstrapRestoresHAKeeperAndWAL|TestSetInitialClusterInfo|TestNextHAKeeperCheckIntervalUsesFastBootstrapInterval|TestBootstrap|TestFailedBootstrap)$' -count=1 -timeout=180s ./pkg/logservicegit diff --check HEAD