Repository navigation
Flake batch 38: Windows base-reclaim and call-graph read-bound tests - #1346
Merged
Merged
Conversation
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
The regression used to require two collector visits to reclaim a base, but lock acquisition and registry setup share the visit budget and can defer a reference sweep entirely. Keep the 250 ms budget and forced owed accounting; retry at most 16 visits, retaining pin/reference checks on every visit and verifying the directory is removed. Inject one slow registry open so the test covers a deferred reference sweep instead of relying on a fast runner. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Record test-only pin, zoom observation, context selection/gap/accounting, query-lock, submission, receiver-budget and completion phases. Keep the existing end-to-end assertion unchanged to diagnose the reported overrun before choosing a fix. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Native Windows solo and parallel configure-module instrumentation did not reproduce the historical extra second: observed reader windows stayed about 3004-3007 ms, with millisecond parsing/rendering overhead. Preserve end-to-end timing as diagnostics, but prove the actual contract through unchanged absolute deadlines, remaining query/receive budgets, and timeout outcomes while the worker stays gated. Use a cleanup-only watchdog, not a larger production timeout. Add a controlled 1250 ms post-timeout delivery gate: replies deliberately exceed the former wall-clock envelope while wait arguments and Building responses remain correct. All observers and delay controls are test-only. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…s on slow runners Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two test-only fixes for Windows failures. Both turned main red or failed unrelated PRs' runs. No production behaviour changes.
1. View base reclaim test (storage retention)
retention_reclaims_unreferenced_bases_even_when_accounting_is_owedassumed an unused commit base is reclaimed on exactly the second cleanup pass. Reference reclaim runs before the byte-accounting walk and its progress persists across passes. But a pass that spends its whole 250 ms budget opening the registry does no reference sweep, so reclaim lands one pass later. A one-time, test-only registry-setup delay reproduces the CI failure natively on Windows. On main it failed 1 of 19 Windows runs, and never on Linux or macOS. The test now waits up to 16 passes for reclaim. It keeps the 250 ms budget and the 500 ms accounting delay, and still checks after every pass that held and referenced bases are never touched.2. Call-graph read bound test (#1338)
callgraph_views_rematerialization_bounds_zoom_then_serves_publicationasserted end-to-end zoom time under 3 s, with no headroom for runner overhead. On GitHub's Windows runner it saw 4.05 s. Natively on Windows, the bounded waits measured 3.006 s alone and in parallel, and the extra second didn't reproduce. The test now asserts the wait contract that is actually promised: the unchanged absolute deadline, the requested query-lock and receive timeouts, and both timeout outcomes while materialization is held. A new case delays the reply after the timeout to prove reply rendering isn't counted. End-to-end timings stay as diagnostics. The 3 s production bound and the replies are unchanged. The timing hooks are#[cfg(test)]only.Gate: 264 call-graph view, configure and storage-retention tests on Linux, formatting and the Windows check. Both fixes passed natively on Windows in their workers' runs.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes two test flakes that failed Windows CI runs. No production behaviour changes.
Storage retention regression
Call-graph read bound test
#[cfg(test)]only and skip when a test context has no cache root, and the acquisition-token test now allows 30 s for positive waits on loaded runners.Written for commit 05e54d2. Summary will update on new commits.
Follow-ups after the first CI run:
canonical_cache_root(), which panics in test contexts that never ran configure, so about fifteen zoom tests failed on all platforms. They now usecanonical_cache_root_opt()and skip when there is no root. Confirmed natively on Windows (panic atcontext.rs:9892before the fix), and 95 zoom and read-bound tests pass on Linux after it.cancel_reaches_completed_and_running_acquisition_tokens(Views: call-graph reads answer within seconds instead of waiting for a full rebuild #1338): it waited only 2 s for each acquisition token, which a loaded Windows runner missed. Those positive waits are now 30 s. The 250 ms cancellation check is unchanged.