Skip to content

test: the streams test finds a delivery by its payload, not its place in the list - #42

Merged
giraffesyo merged 1 commit into
canaryfrom
test/deflake-sweep
Oct 2, 2026
Merged

giraffesyo merged 1 commit into
canaryfrom
test/deflake-sweep

Conversation

@giraffesyo

Copy link
Copy Markdown
Member

Summary

The Streams conformance test assumed that two deliveries are listed in the order they were made. JobList returns jobs in ID order, and on Postgres 14 to 17 two IDs generated in the same millisecond sort randomly (hopper_uuidv7 there is a millisecond timestamp plus random bits). When both pumps land in one millisecond the test looked at the wrong delivery and failed at stream.go:152 (audit deliveries = [...]). No library code changes.

The test now picks the delivery of the unkeyed event by its payload, as the earlier part of the same test already does.

This came out of a sweep for timing assumptions: the suite run in a CPU-limited container against a CPU-limited Postgres 17, then with network delay added to Postgres's replies.

Round Tests Postgres Passes Result
1 1 CPU, packages in turn 0.5 CPU 3 this test failed once per driver
2 1 CPU, packages at once 0.5 CPU 4 clean
3 0.5 CPU, packages at once 0.25 CPU 4 clean
4 1 CPU, packages at once 0.5 CPU, 20 ms ± 20 ms delay 2 clean
5 1 CPU, packages at once 0.5 CPU, 60 ms ± 60 ms delay 1 four tests failed

Round 5's four failures are left alone. TestConcurrentClientsFinalizeExactlyOnce, TestGlobalLimitHoldsAcrossClients and TestUpgradeUnderTraffic ran out of their 15-second budgets, and TestPeriodicJobsRunOnceAcrossLeaders saw a leader change inside its 3.5-second window, which needs the leader to stall for its whole 2-second lease. None is a wrong result, and nothing on the CI runners has come close to that delay.

Testing

  • Round 1 above reproduced the failure: 2 of 6 runs. It did not reproduce from the host, where a round trip to Postgres takes longer than a millisecond (0 of 80 runs of the old test).
  • go test -race -count=40 -run TestConformance/Streams ./driver/hopperpgx ./driver/hoppersql passes with the fix.
  • golangci-lint: 0 issues.

Only a test changed, so no hopperbench results.

Checklist

  • make check passes — lint and the changed test run locally; the full suite runs in CI here
  • No cryptography was added (tests run with GODEBUG=fips140=only)
  • New or changed SQL in the claim, finalize, rescue or leader paths has a concurrency or chaos test — no SQL changed
  • Behavior changes are reflected in docs/PLAN.md — none

@giraffesyo
giraffesyo merged commit 4c522f9 into canary Oct 2, 2026
8 checks passed
@giraffesyo
giraffesyo deleted the test/deflake-sweep branch October 2, 2026 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant