Skip to content

Skip obsolete queued benchmark previews before loading the site - #1137

Merged
helly25 merged 6 commits into
mainfrom
fix/stale-preview-writer-admission
Nov 7, 2026
Merged

helly25 merged 6 commits into
mainfrom
fix/stale-preview-writer-admission

Conversation

@helly25

@helly25 helly25 commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Stop obsolete PR benchmark previews from loading and processing the complete retained site after a long queue wait. Recheck freshness inside the writer job, and use collection's final result before setting up the renderer or publishing.

AG;DR

A preview's comparison waiter can succeed while its PR head is current, then spend hours waiting for the shared writer lock. The existing collector correctly returns no data for the superseded head, but the workflow still expanded retained inputs, built the renderer, refreshed/compacted reports, and attempted an unchanged push. One observed publisher reported zero decorated pages and no branch changes after substantial preparation work.

  • Add a read-only current operation to the trusted preview helper, using the existing source-attempt, Test/PR-event, repository, open-PR, and current-head validation. It needs source identity but no retained-site root or artifact listing.
  • Run that check after acquiring the writer lock and checking out trusted main code, before retained-site checkout/expansion. An explicitly obsolete preview skips site loading and collection.
  • Gate Node setup, renderer build, and refresh/push on the collector's result, catching a PR that becomes obsolete during site loading. Derive incremental preview mode from that same collection-time output rather than the earlier waiter output.
  • Leave main, release, backfill, and manual publication admission and reconciliation intact. Keep the serialized non-canceling writer queue, artifact validation, retention, deployment separation, and current-head comments.
  • API failures remain failures, with the existing bounded read retries; they cannot silently become an obsolete result. Source operations still require run and attempt arguments.

Validation: 82 direct regression tests passed on both macOS and Linux over SSH, with all 199 transferred source hashes checked. All five affected Mac Bazel targets passed; the final API-failure regression was also executed in the hermetic preview target. Tests cover current versus superseded attempts/heads, closed or foreign-repository PRs, no site/artifact access during the check, failure propagation, required arguments, placement after lock acquisition, and both admission-time and collection-time workflow guards. Normal pre-commit (local clang-tidy skipped under the standing instruction), actionlint, Trunk, and diff checks passed for all seven changed files. Public-head CI remains required.

This is a separate follow-up to #1135/#1136 and does not modify their heads. Fresh previews pay one additional bounded source/PR API check. No elapsed-time speedup is asserted. Already queued legacy workflow definitions retain their original steps; earlier queue-full source rejections still need recovery. Obsolete preview cleanup is deferred to the next normal metadata refresh, and a successful no-op producer can still send a notification to the coalesced latest-snapshot deployer. No incoming valid measurements are canceled or discarded.

Public integration and current qualification

#1135 and #1136 are merged. Current signed head 737795b2050af4f10d912e5322a41e2ed8fea618 incorporates actual main 1f25ba6d4ae0c9d69d6a2143940f08c2ed41acbe, including the template-stat runtime and all intervening reports. The normal combined merge resolves eight repeated-ancestry/context conflicts. It preserves the exact qualified preview helper (fcc75edb94a83b9b61c1cabd6878ceb53b8af96554f51ae19234e552cbd39b64) and its test source, the original stale-admission workflow conditions, every newer main implementation outside this seven-file delta, current fifth-candidate policy regressions, canonical documentation, exact evidence exemptions and site mappings. Combined tree: 16a968ea59bc72999c619705369a337339a08984.

Tables and diff checks pass; all 32 integrated workflow/history tests pass under Python 3.13. An initial old-default-Python import failure occurred before test execution and was resolved by using the installed supported interpreter, with the failure retained separately. Normal signed commit and immediate push hooks passed; normal merge hooks intentionally skip redundant Trunk, and no local clang-tidy ran. Fresh own current-main Test 42399241252 completed successfully on this actual pushed head, including every product/lint/sanitizer/coverage check, all six initial shards, the required fourth Mac candidate, agreement, final best-seven-of-nine comparison and done; fifth candidates skipped normally after agreement. Exact predecessor-main Test 42398990563 also completed successfully. GitHub recognizes this head as current and mergeable; earlier stacked-head checks do not establish readiness.

This is the remaining queue root, with #1138 as its immediate child. Merge this PR unchanged when ready, then synchronize the retargeted child. The first #1135 hosted deployment verified its live index, but missing-source replay, obsolete legacy queued definitions and broader retained-site storage/rendering work remain separate open items.

Base automatically changed from perf/reuse-raw-benchmark-archives to main November 7, 2026 17:59
@helly25
helly25 merged commit 3b8a8a3 into main Nov 7, 2026
32 checks passed
@helly25
helly25 deleted the fix/stale-preview-writer-admission branch November 7, 2026 18:58
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