From 54c6cd56e657c98a2b9fb823be64772db01438d0 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Sun, 24 May 2026 06:47:58 +0900 Subject: [PATCH 1/2] test(sqs): slow throttle refill on no-op SetQueueAttributes test to fix CI flake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestSQSServer_Throttle_NoOpSetQueueAttributesPreservesBucket intermittently fails with 'expected 400, got 200' on CI runs (observed multiple times today on unrelated PRs #813, etc). Root cause: 1-token-per-second refill races the test wall-clock. The test sequence — 10 drain sends → sanity send → SetQueueAttributes → post-no-op send — pushes 12 Raft writes through the SQS adapter. Under -race on slow CI runners each write's propose+apply takes 100-250ms; total elapsed before the final assertion can reach 1.5-2.5s. At 1 token/sec refill, by that point the bucket has accumulated 1+ tokens and the post-no-op send returns 200 instead of the expected 400 — falsely indicating a no-op invalidate-bypass regression. Fix: drop refill from 1/sec to 0.01/sec (1 token per 100s) so no test-window wall-clock can accumulate to a whole token. The test's intent — verify that a no-op SetQueueAttributes does not reset the bucket state — is independent of the refill rate, so widening the safety margin is in scope. Tested locally with -race -count=2 (-2.1s wall, both pass). Lint clean. Caller audit: test-only change. The throttle config validator accepts fractional float64 refill rates (sqs_catalog.go:163 SendRefillPerSecond float64). 0.01 is non-zero so IsEmpty (line 172) returns false and throttling stays enabled — the test still exercises the throttle path. --- adapter/sqs_throttle_integration_test.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/adapter/sqs_throttle_integration_test.go b/adapter/sqs_throttle_integration_test.go index 26f0b688..d1156bdc 100644 --- a/adapter/sqs_throttle_integration_test.go +++ b/adapter/sqs_throttle_integration_test.go @@ -325,9 +325,22 @@ func TestSQSServer_Throttle_NoOpSetQueueAttributesPreservesBucket(t *testing.T) node := sqsLeaderNode(t, nodes) url := mustCreateQueue(t, node, "throttle-noop-set") + // Refill rate is intentionally slow (0.01/sec = 1 token per + // 100 seconds) so the test's wall-clock window between drain + // and the post-no-op send cannot accumulate to a whole token. + // At 1/sec the test raced its own refill on slow CI runners: + // 10 drain sends + sanity send + SetQueueAttributes (each + // going through Raft propose+apply at ~100-250ms under -race) + // elapses ~1.5-2.5s before the post-no-op send fires, by which + // time the bucket has refilled 1+ tokens and the send returns + // 200 instead of the expected 400. The test's intent — verify + // that a no-op SetQueueAttributes does not reset the bucket — + // is independent of the refill rate, so slowing it down is + // the right scope. + const slowRefill = "0.01" mustSetQueueAttributes(t, node, url, map[string]string{ "ThrottleSendCapacity": "10", - "ThrottleSendRefillPerSecond": "1", + "ThrottleSendRefillPerSecond": slowRefill, }) // Drain the bucket so the next charge would only succeed if the // bucket was reset to a fresh full-capacity replacement. @@ -351,7 +364,7 @@ func TestSQSServer_Throttle_NoOpSetQueueAttributesPreservesBucket(t *testing.T) // a fresh full bucket. mustSetQueueAttributes(t, node, url, map[string]string{ "ThrottleSendCapacity": "10", - "ThrottleSendRefillPerSecond": "1", + "ThrottleSendRefillPerSecond": slowRefill, }) // Bucket must still be drained — no-op SetQueueAttributes must not // reset the rate limiter. From 6e5ad3f90199727080a45cc697cc36a380b6dfde Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Sun, 24 May 2026 06:55:25 +0900 Subject: [PATCH 2/2] test(sqs): extract throttleAttrs map for no-op test (PR #819 r1) Gemini medium x2: define the attribute map once and reuse it on both mustSetQueueAttributes calls so the 'no-op' intent is structurally visible. A future drift between the two map literals (typo, stray edit) would now be obviously wrong since they share the same source variable. No behavior change. go test -race -count=1 -timeout=120s ./adapter/... in scope: passes golangci-lint: clean --- adapter/sqs_throttle_integration_test.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/adapter/sqs_throttle_integration_test.go b/adapter/sqs_throttle_integration_test.go index d1156bdc..73655e33 100644 --- a/adapter/sqs_throttle_integration_test.go +++ b/adapter/sqs_throttle_integration_test.go @@ -338,10 +338,16 @@ func TestSQSServer_Throttle_NoOpSetQueueAttributesPreservesBucket(t *testing.T) // is independent of the refill rate, so slowing it down is // the right scope. const slowRefill = "0.01" - mustSetQueueAttributes(t, node, url, map[string]string{ + // Define the attribute map once and reuse on both calls so the + // "no-op" intent is structurally visible: the two + // mustSetQueueAttributes calls share the same map literal, so + // a future drift between them (e.g., a typo or a stray edit) + // would be obviously wrong (Gemini medium on PR #819). + throttleAttrs := map[string]string{ "ThrottleSendCapacity": "10", "ThrottleSendRefillPerSecond": slowRefill, - }) + } + mustSetQueueAttributes(t, node, url, throttleAttrs) // Drain the bucket so the next charge would only succeed if the // bucket was reset to a fresh full-capacity replacement. for range 10 { @@ -362,10 +368,7 @@ func TestSQSServer_Throttle_NoOpSetQueueAttributesPreservesBucket(t *testing.T) // Re-submit identical Throttle* values. Old code invalidated on // key presence and the next send would have been allowed against // a fresh full bucket. - mustSetQueueAttributes(t, node, url, map[string]string{ - "ThrottleSendCapacity": "10", - "ThrottleSendRefillPerSecond": slowRefill, - }) + mustSetQueueAttributes(t, node, url, throttleAttrs) // Bucket must still be drained — no-op SetQueueAttributes must not // reset the rate limiter. status, _ = callSQS(t, node, sqsSendMessageTarget, map[string]any{