From 2b795807df16e66aeda6e59adfb7f815e56dc03f Mon Sep 17 00:00:00 2001 From: bootjp Date: Fri, 11 Sep 2026 20:34:31 +0900 Subject: [PATCH] kv: fix the TSO shadow test's flaky two-sample ceiling TestShadowAndCutoverAllocatorsSerializeMigrationOnGroupZero took its two HLC ceilings from two separate time.Now() calls. When those calls straddle a millisecond boundary, legacyClock's ceiling lands 1 ms above tsoClock's, and the test fails with tso shadow validation: reserve local TSO window: tso minimum N with batch 1 rolls to physical P beyond committed ceiling P-1 HLC.fencedNowMillis returns the ceiling as "now" whenever the ceiling is in the future, so every timestamp legacyClock issues sits exactly at legacyClock's ceiling. The shadow path hands that value to the group-0 allocator as a minimum, and rejectMinimumWindowBeyondCeiling correctly refuses a minimum above tsoClock's committed ceiling. The rejection is right; the fixture was wrong. Production wires one *HLC -- coordinate.Clock() -- into both the dedicated allocator and the legacy path (main.go configureDedicatedCoordinatorTSO), so the two ceilings are the same value by construction and cannot drift. Taking both from one sample makes the test model that. Measured: ~15-22 failures per 20000 runs before, 0 after. Claude-Session: https://claude.ai/code/session_013rNHooj7NF3giihWVba8QE --- kv/tso_raft_test.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/kv/tso_raft_test.go b/kv/tso_raft_test.go index 834afc8ef..f88f68e8e 100644 --- a/kv/tso_raft_test.go +++ b/kv/tso_raft_test.go @@ -608,8 +608,23 @@ func TestShadowTimestampAllocatorBypassesLegacyAfterObservedCutover(t *testing.T } func TestShadowAndCutoverAllocatorsSerializeMigrationOnGroupZero(t *testing.T) { + // Both clocks must take their ceiling from ONE wall-clock sample. + // + // HLC.fencedNowMillis jumps the physical half straight to the ceiling + // whenever the ceiling is in the future, so every timestamp this legacy + // clock issues sits exactly at legacyClock's ceiling. The shadow path + // hands that timestamp to the group-0 allocator as a minimum, and + // rejectMinimumWindowBeyondCeiling correctly refuses a minimum above + // tsoClock's committed ceiling. Two separate time.Now() calls can + // straddle a millisecond boundary, which puts legacyClock's ceiling 1 ms + // above tsoClock's and fails the shadow validation -- a fixture artifact, + // not a real condition: production wires one *HLC (coordinate.Clock()) + // into both the dedicated allocator and the legacy path, so the two + // ceilings there are the same value by construction. + ceilingMs := time.Now().Add(testTSOFutureCeiling).UnixMilli() + tsoClock := NewHLC() - tsoClock.SetPhysicalCeiling(time.Now().Add(testTSOFutureCeiling).UnixMilli()) + tsoClock.SetPhysicalCeiling(ceilingMs) fsm := NewTSOStateMachine(tsoClock) engine := &recordingTSOEngine{ state: raftengine.StateLeader, @@ -621,7 +636,7 @@ func TestShadowAndCutoverAllocatorsSerializeMigrationOnGroupZero(t *testing.T) { require.NoError(t, err) legacyClock := NewHLC() - legacyClock.SetPhysicalCeiling(time.Now().Add(testTSOFutureCeiling).UnixMilli()) + legacyClock.SetPhysicalCeiling(ceilingMs) shadowRoute, err := NewLeaderRoutedTSOAllocator(local, engine, WithTSORoutedClock(legacyClock)) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, shadowRoute.Close()) })