Skip to content

Make proxy lifecycle tests deterministic - #330

Merged
PaxMachinaOne merged 2 commits into
KafScale:mainfrom
novatechflow:fix/listener-ttl-close-regression
Nov 14, 2026
Merged

PaxMachinaOne merged 2 commits into
KafScale:mainfrom
novatechflow:fix/listener-ttl-close-regression

Conversation

@novatechflow

@novatechflow novatechflow commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fix the main CI flake in TestLFSCanceledLockWaitHasBoundedCleanup: its 250ms request budget could expire during lease setup before the injected cleanup failure was reached. Wait for contended cleanup to begin, then cancel and measure the return bound. A deliberate lease-grant delay ensures slow setup cannot bypass the cleanup assertions, including the bounded release attempt.

Also strengthen the #319 listener regression: assert EOF after stale-cache rejection on every request path and limit cleanup to five seconds after client disconnect. This catches a missing server close without consuming the package timeout.

Validation

  • The original FindCoordinator subtest passes 10 race-detector runs on main; the final TTL suite passes five repeated runs.
  • Removing the listener socket close makes the new assertion fail after five seconds.
  • Restoring the old 250ms budget with delayed lease setup reproduces the main CI failure. The synchronized cancellation test passes 20 race-detector runs.
  • The full proxy race suite passes on e24dd6c. Deliberately unbounded release cleanup is still detected within the two-second cancellation bound.
  • Hosted CI and CodeQL pass on e24dd6c, including the full race suite, authentication coverage, and end-to-end checks.

These changes affect tests only. The unreleased notes already document listener closure after cache expiry, so no additional release-note entry is needed.

@novatechflow novatechflow changed the title Assert listener closure after cache expiry Make proxy lifecycle tests deterministic Nov 14, 2026
@novatechflow
novatechflow marked this pull request as ready for review November 14, 2026 10:56

@novatechflow novatechflow left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed e24dd6c; no remaining blockers.

The #319 setup timeout fix is already on main, and the formerly hanging FindCoordinator subtest passes 10 race-detector runs. This PR adds explicit EOF assertions after stale-cache rejection and bounds disconnected-listener cleanup to five seconds. Omitting the server socket close makes the new assertion fail promptly.

The current main CI failure was a separate multipart cancellation fixture: the 250ms deadline could expire before reaching cleanup. Cancellation is now synchronized with contended cleanup, including a delayed lease grant. The old deadline reproduces the failure; the corrected test passes 20 repeated race runs and still detects deliberately unbounded release cleanup.

Validation: full proxy race suite, five repeated TTL suite runs, hosted CI, and CodeQL pass on this head.

Release notes: no new entry is needed because this changes tests only. The existing unreleased notes already describe closing client connections after cache expiry.

@PaxMachinaOne
PaxMachinaOne merged commit 51f157a into KafScale:main Nov 14, 2026
14 checks passed
@novatechflow
novatechflow deleted the fix/listener-ttl-close-regression branch November 14, 2026 11:07
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.

2 participants