Skip to content

Commit b573988

Browse files
ubettigoleclaude
andauthored
Fixing flaky integration test: TestCrashAfterRejectDoesNotLoseMessages (#274)
## Summary TestCrashAfterRejectDoesNotLoseMessages was flaky because it asserted consumer lag immediately after Ack(), but Ack() only marks the delivery state — watermark advancement (which updates offset_acked) is deferred to the next poll loop tick. The test raced against the poll loop: if the lag check ran before advanceWatermark, offset_acked was still stale and lag was non-zero. Fix by adding OnSignal to worker-2's queue and calling waitForSignal after acks, ensuring the poll loop has run advanceWatermark before we check lag. This matches the pattern already used by TestWatermarkAdvancesContiguously. ## Test Plan Ran the test 50 times to confirm it's no longer flaky: ``` ./tool/bazel test //test/integration/extension/messagequeue/mysql:mysql_test --test_filter='TestSQLQueueIntegration/TestCrashAfterRejectDoesNotLoseMessages' --runs_per_test=50 --test_output=summary --jobs=4 INFO: Found 1 test target... Target //test/integration/extension/messagequeue/mysql:mysql_test up-to-date: bazel-bin/test/integration/extension/messagequeue/mysql/mysql_test_/mysql_test INFO: Elapsed time: 205.664s, Critical Path: 20.62s INFO: 51 processes: 9 action cache hit, 1 internal, 50 darwin-sandbox. INFO: Build completed successfully, 51 total actions //test/integration/extension/messagequeue/mysql:mysql_test PASSED in 20.6s Stats over 50 runs: max = 20.6s, min = 10.0s, avg = 16.0s, dev = 2.0s Executed 1 out of 1 test: 1 test passes. ``` ## Issue <!-- Link the issue here. - Use 'Closes #123' if this is the final fix. - Use 'Part of #123' or just '#123' if the feature is still in progress. --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0475631 commit b573988

1 file changed

Lines changed: 8 additions & 1 deletion

File tree

‎test/integration/extension/messagequeue/mysql/queue_test.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2221,10 +2221,12 @@ func (s *SQLQueueIntegrationSuite) TestCrashAfterRejectDoesNotLoseMessages() {
22212221

22222222
// Start worker-2 with same consumer group — it polls and finds msg-C
22232223
// after lease + visibility expire in the DB
2224+
signalCh := make(chan queueMySQL.HookSignal, 100)
22242225
q2, err := queueMySQL.NewQueue(queueMySQL.Params{
22252226
DB: s.db,
22262227
Logger: zaptest.NewLogger(t),
22272228
MetricsScope: tally.NoopScope,
2229+
OnSignal: signalCh,
22282230
})
22292231
require.NoError(t, err)
22302232
defer q2.Close()
@@ -2243,6 +2245,9 @@ func (s *SQLQueueIntegrationSuite) TestCrashAfterRejectDoesNotLoseMessages() {
22432245
require.NoError(t, delivery.Ack(s.ctx))
22442246
t.Logf("Worker-2 recovered msg-C (attempt=%d)", delivery.Attempt())
22452247

2248+
// Wait for the poll loop to advance the watermark after acking msg-C.
2249+
waitForSignal(t, signalCh, queueMySQL.SignalDeliveryCheck)
2250+
22462251
// Verify DLQ contains msg-B
22472252
dlqTopic := topic + subConfig.DLQ.TopicSuffix
22482253
dlqConfig := extqueue.DefaultSubscriptionConfig("worker-2", "crash-reject-cg")
@@ -2254,7 +2259,9 @@ func (s *SQLQueueIntegrationSuite) TestCrashAfterRejectDoesNotLoseMessages() {
22542259
assert.Equal(t, "msg-B", dlqDelivery.Message().ID, "msg-B should be in DLQ")
22552260
require.NoError(t, dlqDelivery.Ack(s.ctx))
22562261

2257-
// Verify consumer lag is 0
2262+
// Verify consumer lag is 0.
2263+
// Wait for the poll loop so advanceWatermark has run after all acks.
2264+
waitForSignal(t, signalCh, queueMySQL.SignalDeliveryCheck)
22582265
admin := queueAdmin.NewAdminStore(s.db)
22592266
lags, err := admin.ConsumerLag(s.ctx, topic)
22602267
require.NoError(t, err)

0 commit comments

Comments
 (0)