Skip to content

test: make the heartbeat recovery test independent of write speed - #101

Merged
cardmagic merged 1 commit into
mainfrom
test/heartbeat-recovery-clock
Oct 19, 2026
Merged

cardmagic merged 1 commit into
mainfrom
test/heartbeat-recovery-clock

Conversation

@cardmagic

Copy link
Copy Markdown
Owner

Summary

  • EffectRecoveryTest#test_a_running_effect_keeps_its_process_heartbeat_fresh failed three times on MySQL (trilogy and mysql2) in the last 30 failed CI runs, with Expected: "processing", Actual: "completed".
  • Cause, in the test: it sets process_alive_threshold = 0.1 and runs the recovery pass right after the 8th heartbeat. One slow heartbeat commit makes the stored heartbeat older than 0.1 s, so the pass retires the effect, which is the designed behavior for a stale owner (docs/effect-recovery.md). With the defaults (15 s interval, 60 s threshold), a heartbeat must stall for about 45 s.
  • Fix, test only: the test waits until the stored heartbeat is more than one threshold past the claim, then runs the recovery pass with database_clock_now set half a threshold after that heartbeat. The heartbeat thread and the database writes stay real.
  • Test first: a new case pauses 0.15 s after each heartbeat write. It failed with the CI message before the change and passes after it. Both cases fail with Timeout::Error when ProcessHeartbeat#start is disabled, so they still catch a missing heartbeat.

Checks

  • bundle exec rake passes on SQLite (984 runs, 0 failures).
  • The two cases passed 10 times in a row.

The test set a 0.1 second alive threshold and ran the recovery pass
right after the 8th heartbeat. One slow heartbeat commit on a busy CI
runner made the stored heartbeat older than the threshold, so the pass
retired the running effect as designed, and the test read "completed".
It failed three times on MySQL in the last 30 failed CI runs.

The test now waits until the stored heartbeat is more than one
threshold past the claim, and runs the recovery pass with its clock
set half a threshold after that heartbeat. A slow write can only make
the test slower. A new case adds a 0.15 second pause after each
heartbeat write: it failed with the CI message before this change and
passes after it. Both cases still fail when the heartbeat thread is off.
@greptile-apps

greptile-apps Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The test-only change appears safe to merge.

Summary

Makes the heartbeat recovery test independent of database write speed.

  • The heartbeat test checks recovery at a controlled time.

Reviews (1) · Last reviewed commit: "test: make the heartbeat recovery test i..." · Reviewed by Greptile

@cardmagic
cardmagic merged commit b2031d2 into main Oct 19, 2026
44 checks passed
@cardmagic
cardmagic deleted the test/heartbeat-recovery-clock branch October 19, 2026 00:43
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