fix(validation): pin celeris main dbbaaee and carry its two celeris#685 close-path counters to the artifacts (schema 5.18) - #454
Merged
Conversation
…85 close-path counters to the artifacts (schema 5.18) Pins celeris main dbbaaee in servers/celeris, the eight refapps and validation/refapp/internal/debugvars, and the observability refapp's middleware/metrics and middleware/otel sub-modules at the same commit. The pin carries the 16 PRs merged after 698bed6, including #793 (3e7abba), the celeris#685 fd-lifetime fix on the io_uring close paths. #793 added two engine.EngineMetrics fields (72 now). Both are carried through every hop the #389/#394 guards hold: - the debugvars publish list; - the enginekeys manifest (regenerated, 70 -> 72); - ParseDebugVars and DebugVarsKeys; - properties.Snapshot; - the report.EngineCounters tally; - the series: engine_close_fd_forced gets a column, appended; engine_close_fd_deferred is declared tally-only. CloseFDDeferred is a rate of the rule working. CloseFDForced, the release backstop closing a descriptor with an op still owed, is documented must-stay-zero; it is marked EngineCounter.MustStayZero, recorded but NOT gated, like the celeris#657 counters (probatorium#416 holds the gate decision). The engine_stale_recv_data_closed description is updated for #793. The schema moves from 5.17 to 5.18; the three hand pins were re-pinned and proven failing first.
FumingPower3925
added
the
dependencies
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: goceleris/probatorium/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (11)
📒 Files selected for processing (22)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (15)
🧰 Additional context used📓 Path-based instructions (5)This is the validation oracle.⚙️ CodeRabbit configuration file Files:
Magefiles carry //go:build mage: they are the bench driver, the release gate and the cluster orchestration, and `go test ./...`⚙️ CodeRabbit configuration file Files:
Competitor adapters for the benchmark.⚙️ CodeRabbit configuration file Files:
The report package defines the published result schema and the release gate.⚙️ CodeRabbit configuration file Files:
Source excerpt: [ ] If the report schema changed: `SchemaVersion` is bumped and re-pinned in `report/schema_test.go`, `mage_bench_sutenv_test.go` and `validation/runner_test.go`📄 CodeRabbit inference engine (.github/PULL_REQUEST_TEMPLATE.md) Files:
📝 SummarySummary by CodeRabbit
WalkthroughThe change adds deferred and forced io_uring descriptor-close counters to debug-variable collection, validation tallies, and series output. It documents the counters and advances the report schema to 5.18. Celeris dependency pins and schema-version test expectations are updated. ChangesClose-path counter reporting
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
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
1 participant
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.
Summary
Pins celeris main
dbbaaee(v1.5.12-0.20260928095652-dbbaaeebc86f, main at 09:56Z on 2026-09-28) in every probatorium module that requires celeris, and pins the observability refapp's celeris sub-modulesmiddleware/metricsandmiddleware/otelat the same commit. The previous pin was698bed6(#439).This pin includes #793 (
3e7abba), the celeris#685 fix: io_uring close paths no longer release a descriptor number while an op can still resolve it. It carries the 16 PRs merged since698bed6, in first-parent order: #765, #743, #747, #730, #748, #749, #745, #744, #746, #792, #774, #810, #773, #793, #768 and #766.celeris push CI on
dbbaaeeis green: CI run 36406691708 passed all 13 jobs, including the 7 required contexts, and Coverage, Push on main and Scorecard passed too. It was the head of celeris main when this PR was pinned.RULE 93: new
EngineMetricsfieldsengine.EngineMetricsgoes from 70 to 72 fields. Only #793 adds any; the other 15 PRs add no field. Both new fields are carried through every hop the #389/#394 guards hold, following the #410 precedent:CloseFDDeferredceleris.engine_close_fd_deferredCloseFDForcedceleris.engine_close_fd_forcedFor each field:
TestDebugVarsPublishesEveryEngineMetricsFieldcovers 72 of 72 fields.enginekeysmanifest is regenerated with-update-engine-keysand grows from 70 to 72 keys.ParseDebugVarsreads it, andDebugVarsKeyslists it.properties.SnapshotgetsEngineCloseFDDeferredandEngineCloseFDForced.report.EngineCounterswith the reasoning behind their Counts, Series and Why values, andrecordEngineCountersrecords both.engine_close_fd_forcedis appended as column 60. A nonzero reading is a defect. The backstop fires 5 s after the close it rescues, so the question is which close burst left an op owed. That means reading the column against theengine_close_countandadaptive_switchessteps from 5 s earlier.engine_close_fd_deferredis a ratio of totals againstengine_close_count, so it is tally-only.How
CloseFDForcedis carried, and why it is not gated. The harness has two classes for this:report.ZeroWitnessMeaning/engine_zero_witnessis the gate:report.Gatefails a cell on any nonzero entry.EngineCounter.MustStayZerorecords a counter's meaning without gating it.#410 put every new must-stay-zero diagnostic counter in the second class, and probatorium#416 holds the decision to move them into the gate until a nightly has shown what normal looks like.
engine_close_fd_forcedfollows that precedent. It is markedMustStayZero, listed indocumentedMustStayZero, and checked byTestEveryDocumentedMustStayZeroCounterIsRecordedButNotGated(8 counters now). It should join #416's list when that gate is built.Schema 5.17 → 5.18. Two keys are added to
Tier1Summary.EngineCountersand one series column is appended. The change is additive: older readers ignore every new field.RULE 48 applied by hand:
SchemaVersionwith its history entry,report/schema_test.go,mage_bench_sutenv_test.goandvalidation/runner_test.go. All three pins were first shown to fail withSchemaVersionat 5.18 and the pins still at 5.17.report/schema_v5.jsonhas no version constant and does not modeltier_1, so it is unchanged, as in #410.One existing description updated. celeris rewrote the
StaleRecvDataCloseddoc in #793. Since celeris#685, a live client's request can no longer land inengine_stale_recv_data_closed.report.EngineCountersnow says so, and says to read the counter with the old caveat on an older pin.Changes
servers/celeris, the eight refapps andvalidation/refapp/internal/debugvars:go get github.com/goceleris/celeris@dbbaaeebc86fandgo mod tidy. Only the celeris require line and its twogo.sumlines change in each module.validation/refapp/observability:middleware/metricsandmiddleware/otelare pinned atdbbaaeebc86f. Neither sub-module changed between698bed6anddbbaaee(emptygit diff), but both are pinned so every celeris module in every graph is at one commit.validation/refapp/internal/debugvars/debugvars.go: publishes the two keys.validation/internal/enginekeys/engine_metrics_keys.txt: regenerated.validation/checker/poll.go,validation/properties/snapshot.go,validation/checker/evaluator.go,report/engine_counter.go,validation/series.go: carry the two keys.report/schema.goand the three hand pins: schema 5.18.validation/checker/must_stay_zero_test.go,validation/series_test.go: the new counter and the new column.Proof
v1.5.12-0.20260928095652-dbbaaeebc86f. The 2 sub-module requires, both in observability, resolve to the same commit.698bed6anddbbaaee, the only additions areCloseFDDeferred uint64andCloseFDForced uint64, with no deprecations. A per-PR walk attributes both to #793. The same script reports9f4d89b..698bed6as identical, which matches deps: pin celeris main 698bed6, and record EngineMetrics.Throughput as deprecated #439.TestDebugVarsPublishesEveryEngineMetricsFieldreports 70 of 72 fields published, with 2 missing.TestParseDebugVarsReadsEveryPublishedEngineKey,TestEverySnapshotEngineFieldIsFedByAPublishedKey,TestEveryPublishedEngineKeyReachesTheTallyandTestEveryPublishedEngineKeyHasAColumnOrADeclaration.TestSeriesWriterRecordsEveryColumnchecks the new column by its own distinct value.Test Plan
go vet ./...,gofmt -l .andgolangci-lint run(v2.13.2) are clean on the root module, and golangci-lint reports 0 issues in the debugvars module.4ff95a5, run in sequence with-p 1:validation/refapp/internal/debugvars(-race): 30 PASS / 0 FAIL / 0 SKIP.servers/celeris(-race): 4 / 0 / 0.servers/celeris, the 8 refapps and debugvars build and vet withGOOS=linuxfor amd64 and arm64.go list -mreports celeris,middleware/metricsandmiddleware/otelatdbbaaeebc86f.go test -race -count=1 -p 1 ./...: 1364 PASS / 0 FAIL / 21 SKIP. With-tags mage: 1477 / 0 / 22. The skips are environmental on darwin: Linux-only tests, SSH, live targets, perf/PMU and the malformed skip-file case.Heads-up for the first cluster run on this pin
engine_close_fd_forcedis new and must stay 0. It is recorded and not gated, so a nonzero reading shows up in the tally and the series without failing a cell. The refapp log shows the same event as the WARN "releasing connState with kernel ops unaccounted for after backstop hold". celeris draft #813 (celeris#812) concerns a SEND_ZC buffer held past this backstop, and it is not in this pin.engine_close_fd_deferredwill be large on io_uring async-handler cells: celeris documents close to one per server-side close. That is the rule working.engine_stale_recv_data_closednow counts only a closed connection's own late bytes (#793).engine_transplant_double_claim: #765 (celeris#758) fixed the ~1-in-260 ordering flake that the deps: pin celeris main 698bed6, and record EngineMetrics.Throughput as deprecated #439 pin still carried, so the counter should read 0.engine_transplant_residual_busy: #766 (celeris#711) retracts a parked loop's or worker's residue. The stale Busy gauge that nightly 36247560882 showed after a post-sweep close should be gone, so the tally peak can drop against698bed6runs.engine_transplant_reap_unsupportedis now 0 wherever io_uring runs.Server.Shutdownwait for the drain, bounded by its context. The refapps callShutdownwith a 10 s context from their signal handler, andStartWithListenernow returns after thatShutdown. The refapps register noOnShutdownhooks. So the refapp still exits once the drain ends, and Tier 3's 5 sRefappStopGracekeeps its meaning.