Skip to content

Follow-ups from the coordinator session-loop review #3

Open

Description

@velo

Non-blocking items carried out of the review of PR #2. None affects correctness of a verdict; all were consciously left rather than missed.

  • No test for the conflicting-hash replay refusal. CoordinatorCore.replayRegistered refuses to boot when one session id carries two different testSetHash values. The refusal path has no coverage, and it is a path that only runs at boot — so a regression there is invisible until a real conflicting log appears.

  • result() and nack() do not join(). A shard whose lease expires mid-test is marked departed, and un-marks itself on its next claim or register. But a shard actively posting accepted results for its other valid leases still reads as departed in mid-run views until it next claims. Cosmetic — the CI verdict's single post-run GET is unaffected, and PASSED/FAILED ignore the roster entirely — but a join() inside applyResult would close the window.

  • Directory fsync throws on Windows. bumpIncarnation does FileChannel.open(dir, READ).force(true) after the atomic move, which is correct on Linux and throws on Windows — and here that propagates into a refused start. Fine for the containerized target; it means the coordinator cannot run on a Windows dev box. A tolerant catch would fix it.

  • No saveIfDirty at shutdown. The 30s debounced snapshot flusher is stopped with shutdownNow, discarding a pending flush. Harmless — the snapshot is a boot accelerator and the rows are already in the history log — but a shutdown hook would avoid the needless cold fold on next start.

  • Maintenance task uses Clock.systemUTC() directly rather than the injected clock. Testability only.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions