Repository navigation
Scrub the legacy plan-force-actions detail state_unavailable line (#4346) - #4384
Merged
Merged
Conversation
) One-time startup scrub: PlanForceActionDetailScrub reads collect.plan_force_actions.detail raw (the one named exemption in PlanForceActionAuditRedactionTests' RawDetailReaderExemptions, #4377), runs it through PgPlanForceActionStore.SanitizeDetailForAudit (no second copy of the allow-list), and rewrites only rows whose sanitized text differs. Marker in collect.collector_state (V44, fleet-sentinel server_id) - no new migration rung. Batched by row count (plan_force_actions is a plain table, not a hypertable); a batch failure is logged (SQLSTATE only) and the marker withheld so the next start retries. Wired into DarlingWorker alongside the #4348 setting scrub.
…4346) PlanForceActionAuditRedactionTests.EnclosingMethodFqName picked the enclosing type/method by the textually nearest preceding class/method keyword, not by which declaration's braces actually enclose the SQL site. PlanForceActionDetailScrub declares a nested Summary class above RunAsync, so the scrub's raw SELECT was attributed to Summary.Summary and the one exemption in RawDetailReaderExemptions never matched. Rewritten to use CSharpSourceWalker.BraceBalanced to find the innermost type/method whose body actually contains the SQL position, with a fallback for a query built as a const string field (this scrub's own shape) that finds the next method declared in the same type. Added a round-trip self-check pin.
…unrelated CandidateSql fields (#4346/#4376) - PlanForceActionDetailScrub's raw-detail SQL field is renamed from the bare CandidateSql to LegacyDetailCandidateSql. PgSettingScrub.cs and QueryStoreBackfill.cs each declare their own unrelated CandidateSql field, so a guard on the bare name failed on the real tree before the census even ran; the new name cannot collide with either. - PlanForceActionAuditRedactionTests' exemption list, its round-trip pin, and its synthetic positive/negative controls are updated to the new field name. - Fixed the scrub's remarks: a failed batch STOPS the run rather than continuing with the next batch, matching the actual break in RunAsync. - Fixed the scrub's type remarks to state plainly that JournalAsync does not sanitize at write time; a new row is safe only because the #4326/#4363 producers that build Detail are, not because the store enforces it on the way in. - DarlingWorker's catch around the scrub now logs the exception's type and SQLSTATE only, matching the type-and-SQLSTATE-only shape used elsewhere in this area, rather than the exception's own message. Darling.Tests builds clean (-p:EnableWindowsTargeting=true, 0 Warning(s), 0 Error(s)).
…l count Deletes the #4346/#4376 enclosing-method-attribution scan (NoOtherProductionCode_ReadsTheDetailColumnDirectly and its helpers) and its now-unused controls, and replaces it with PlanForceActionDetailCensus: a small C# tokenizer that joins consecutive string literals (regular, verbatim, raw, interpolated) the way the compiler folds a +-concatenation, and counts joined literals naming both plan_force_actions and the whole word detail. PlanForceActionDetailCensusTests pins the exact real counts per file (PgPlanForceActionStore.cs, PlanForceActionDetailScrub.cs, and PgMigrations.cs' V107 DDL) plus synthetic controls for every literal shape and an M1 check that LegacyDetailCandidateSql is declared once and used exactly once. Refs #4346, #4376.
erikdarlingdata
marked this pull request as ready for review
September 26, 2026 06:20
This was referenced Sep 26, 2026
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.
Refs #4346.
What this does
PlanForceActionDetailScrub, rewrites legacycollect.plan_force_actions.detailaudit lines written by older builds to the same safe form thatPgPlanForceActionStore.SanitizeDetailForAuditalready applies on every read.collect.collector_stateand is written only when every row succeeded. A failed batch logs its SQLSTATE and retries at the next start.detailcensus (PlanForceActionDetailCensus) pins two exact per-file maps over production code: every string literal namingplan_force_actions, and every literal naming it together withdetail. Any other file must count zero. This job is the one exempted raw reader, and its candidate filter constant is used exactly once.Test plan
PlanForceActionDetailScrubLiveTests, gated onDARLING_TEST_PG):$$raw,+-joined).CHANGELOG entry
SECTION: Security
ENTRY:
REF:
[Scrub the legacy plan-force-actions detail state_unavailable line (#4346) #4384]: Scrub the legacy plan-force-actions detail state_unavailable line (#4346) #4384