From ceebba5dab4867629243b5f2b5bd85236f1a3ba2 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:31:06 -0400 Subject: [PATCH 1/4] Size pg_index_bloat's ceiling and cycle budget against the measured block rate, not an assumed one PessimisticBlocksPerSecond asserted 2,000 blocks/s with no measurement behind it. The first SUCCESS row this collector has produced puts the rate at ~1,013, which is what the budget's own doc pre-registered as the figure that would settle this - and it argues the budget down rather than up. MeasuredBlocksPerSecond replaces it at 1,013, and both MeasureCeilingBytes and CycleMeasureBudgetBytes drop from 2 GiB to 1 GiB in lockstep. The ceiling had to move whatever the budget did: one index just under 2 GiB is 259 s in a single statement at the measured rate, past the whole 150 s allowance, so no cycle budget could have rescued it. That also settles the decoupling #3153 deferred - its goal was to keep the ceiling at 2 GiB while the budget fell, and ceiling + budget <= allowance has no solution. ThePerIndexCeiling_FitsTheDeadline_OnItsOwn asserts the ceiling's deadline cost directly instead of inheriting it from budget >= ceiling, because that inheritance is what a decoupling would silently remove. --- .../PgIndexBloatCollectorDefinitionTests.cs | 78 +++++++- .../PgIndexBloatCollector.cs | 184 ++++++++++++++---- 2 files changed, 210 insertions(+), 52 deletions(-) diff --git a/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs b/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs index a9374fa18c..e5dcc57867 100644 --- a/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs +++ b/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs @@ -478,8 +478,16 @@ public void TheBudgetLiterals_AgreeWithTheirConstants() /// "unmeasured" and "healthy" that skipped_reason exists to prevent, reintroduced one level up. /// /// Pinned as an inequality rather than as equality: raising the cycle budget above the ceiling is - /// a legitimate tuning move once a SUCCESS row supplies a real duration, and this must not stand in the - /// way of it. Only the floor is load-bearing. + /// a legitimate tuning move, and this must not stand in the way of it. Only the floor is + /// load-bearing. + /// + /// This pin is not sufficient on its own, which #3164 is what established. While + /// budget >= ceiling holds, + /// covers the CEILING's deadline cost for free — a near-ceiling index cannot cost more than the budget + /// it sits under. That transitive coverage disappears the moment anyone decouples the two, and its + /// disappearance is silent, so the ceiling now carries its own deadline assertion in + /// rather than inheriting one from this + /// inequality. /// [Fact] public void TheCycleBudget_IsNeverBelowThePerIndexCeiling() @@ -501,33 +509,83 @@ public void TheCycleBudget_IsNeverBelowThePerIndexCeiling() /// orders of magnitude, and that is the arithmetic error that a per-byte argument cannot see. /// /// Half the deadline, not all of it. The remainder pays for the catalog scan, connection - /// setup, and the tail index admitted while the running total was still just under budget — that index - /// is charged for itself, so the last admission can be almost a whole index past the point where the - /// budget was nearly spent. + /// setup, and being wrong about the rate — which is the whole of the margin, deliberately, since + /// is now what was measured rather than a + /// figure shaded downwards. Spending the margin in two places would leave neither meaning anything. /// - /// The rate is an assumption and is named as one. This pin does not make it true; it makes - /// raising the budget state a rate, and makes raising the rate state why. + /// The rate is a measurement now, and this pin is what made the trigger fire (#3164). The + /// figure was an assumed 2,000 blocks/s; the first SUCCESS row this collector ever produced put it at + /// ~1,013, and this assertion is what turned that into a required budget change rather than a note — + /// 2 GiB is 262,144 blocks, which at the measured rate is 259 s against a 150 s allowance. Its own + /// failure text named the two exits, "lower the budget, or argue the rate up and say on what + /// measurement", and the measurement argued it DOWN. The pin does not make the rate true; it makes the + /// budget answerable to it. /// [Fact] - public void TheCycleBudget_FitsTheDeadline_AtThePessimisticBlockRate() + public void TheCycleBudget_FitsTheDeadline_AtTheMeasuredBlockRate() { var deadlineSeconds = PgIndexBloatCollector.Instance.CommandTimeoutSecondsOverride; Assert.NotNull(deadlineSeconds); var blocks = PgIndexBloatCollector.CycleMeasureBudgetBytes / PgIndexBloatCollector.BlockSizeBytes; - var seconds = blocks / (double)PgIndexBloatCollector.PessimisticBlocksPerSecond; + var seconds = blocks / (double)PgIndexBloatCollector.MeasuredBlocksPerSecond; var allowed = deadlineSeconds.Value / 2.0; Assert.True( seconds <= allowed, $"a full cycle budget of {PgIndexBloatCollector.CycleMeasureBudgetBytes} bytes is {blocks} " - + $"blocks, which at {PgIndexBloatCollector.PessimisticBlocksPerSecond} blocks/s takes " + + $"blocks, which at {PgIndexBloatCollector.MeasuredBlocksPerSecond} blocks/s takes " + $"{seconds:F0}s — past the {allowed:F0}s that leaves half of the {deadlineSeconds}s command " + "deadline for everything else. Lower the budget, or argue the rate up and say on what " + "measurement"); } + /// + /// The PER-INDEX CEILING has to fit the deadline on its own, because one index just under it is the + /// largest amount of work a single statement can be asked to do. + /// + /// Why this is not redundant with + /// . Today the two are the same + /// arithmetic, because forces + /// budget >= ceiling and a near-ceiling index therefore cannot cost more than the budget it + /// sits under. That makes the ceiling's deadline cost covered TRANSITIVELY — and the transitive route + /// runs through an inequality whose own doc invites it to be relaxed. Decouple the two and the ceiling + /// silently loses the only assertion its cost ever had, with every other pin still green. + /// + /// It is also the assertion that decides the decoupling question, which is why #3164 added it + /// rather than filing it. #3153 deferred decoupling the budget from the ceiling — an unconditional + /// first-admission rule plus a deadline constraint restated as a SUM — so that the budget could fall to + /// fit a slower rate while the ceiling stayed at 2 GiB. This pin is what shows that goal to be + /// unreachable: at the measured rate a 2 GiB ceiling is 259 s in one statement, so it fails this + /// assertion before a budget is chosen at all, and ceiling + budget <= allowance has no + /// solution. The binding constraint was never the ordering between the two figures; it was the + /// ceiling's own deadline cost, which equal values had been hiding. + /// + /// Written against the ceiling and the deadline rather than against the budget, so it stays + /// meaningful under a decoupling instead of quietly becoming a second copy of the budget pin. + /// + [Fact] + public void ThePerIndexCeiling_FitsTheDeadline_OnItsOwn() + { + var deadlineSeconds = PgIndexBloatCollector.Instance.CommandTimeoutSecondsOverride; + + Assert.NotNull(deadlineSeconds); + + var blocks = PgIndexBloatCollector.MeasureCeilingBytes / PgIndexBloatCollector.BlockSizeBytes; + var seconds = blocks / (double)PgIndexBloatCollector.MeasuredBlocksPerSecond; + var allowed = deadlineSeconds.Value / 2.0; + + Assert.True( + seconds <= allowed, + $"one index just under the {PgIndexBloatCollector.MeasureCeilingBytes}-byte per-index ceiling " + + $"is {blocks} blocks, which at {PgIndexBloatCollector.MeasuredBlocksPerSecond} blocks/s takes " + + $"{seconds:F0}s in a SINGLE statement — past the {allowed:F0}s that leaves half of the " + + $"{deadlineSeconds}s command deadline for everything else. No cycle budget can fix this: the " + + "ceiling is what one statement can be asked to read. Lower the ceiling, or argue the rate up " + + "and say on what measurement"); + } + /* ---------------- rotation (#3153) ---------------- */ /// diff --git a/PerformanceMonitor.Collectors/PgIndexBloatCollector.cs b/PerformanceMonitor.Collectors/PgIndexBloatCollector.cs index 0ccb5447eb..6bd6282b87 100644 --- a/PerformanceMonitor.Collectors/PgIndexBloatCollector.cs +++ b/PerformanceMonitor.Collectors/PgIndexBloatCollector.cs @@ -92,6 +92,17 @@ private PgIndexBloatCollector() /// Indexes at or above this many bytes are recorded but NEVER measured — permanently, not this cycle. /// The read is proportional to index size, so this bounds what any single index can cost. /// + /// It is the DEADLINE that sets this figure, and that only became visible once the block rate + /// was measured (#3164). One index just under this ceiling is the largest amount of work a single + /// statement can be asked to do, so the ceiling has to fit the deadline on its own — at + /// , 1 GiB is 131,072 blocks and about 129 s, inside the 150 s + /// that leaves half of for everything else. + /// ThePerIndexCeiling_FitsTheDeadline_OnItsOwn asserts that directly rather than leaving it to + /// be inherited from the lockstep below, because the lockstep is exactly what a future decoupling + /// would remove. At the measured rate a 2 GiB ceiling is 259 s in one statement — past the whole + /// allowance on a single index — which is why this ceiling could not stay where it was whatever the + /// cycle budget did. + /// /// It moves in lockstep with , because the cycle budget may /// never sit below it: the band between the two would be indexes that are legitimate candidates, /// earn no "too large" reason, and yet exceed the whole cycle on their own first row every run. @@ -99,20 +110,24 @@ private PgIndexBloatCollector() /// stall the ROTATION CURSOR rather than merely mislabel one index, which is a strictly worse /// failure. See for the argument in full. /// - /// Over-ceiling indexes are a terminal state, and the row says so. On the first - /// production target 43 indexes sat above this ceiling, holding 311 GB — 68% of that instance's index - /// footprint, mean 7.4 GB, largest 27 GB. At the rate that target's own SUCCESS row measured, reading - /// that set is about 11 hours and its largest member alone is roughly an hour in one statement, so no - /// per-statement deadline this product could plausibly set reaches them: pgstatindex is the - /// wrong instrument for them rather than a mis-tuned one. Raising the ceiling does not help, and an - /// estimator is not available — pg_stats returns zero rows to a pg_monitor-only login - /// (see the type header). So their reason states PERMANENCE instead of implying a deferral, and points - /// at what does cover them: runs on the same target at the - /// same daily cadence with the same retention, needs no extension, records index_bytes and - /// table_bytes per index, and covers all 43 — an index growing while its table's row count does - /// not is itself a bloat signal, and it is already collected. + /// Over-ceiling indexes are a terminal state, and the row says so. Measured on the first + /// production target AT A 2 GiB CEILING, 43 indexes sat above it holding 311 GB — 68% of that + /// instance's index footprint, mean 7.4 GB, largest 27 GB. That census belongs to the ceiling it was + /// taken at: this ceiling is lower, so the terminal set is LARGER by however many indexes fall between + /// the two, and that count has not been measured — the store holding that target was not reachable for + /// #3164. What is measured is the direction and the mechanism, not a new count. At + /// the 311 GB set is about 11 hours of reading and its largest + /// member alone is roughly an hour in one statement, so no per-statement deadline this product could + /// plausibly set reaches them: pgstatindex is the wrong instrument for them rather than a + /// mis-tuned one. Raising the ceiling does not help, and an estimator is not available — + /// pg_stats returns zero rows to a pg_monitor-only login (see the type header). So their + /// reason states PERMANENCE instead of implying a deferral, and points at what does cover them: + /// runs on the same target at the same daily cadence with the + /// same retention, needs no extension, records index_bytes and table_bytes per index, and + /// covers every one of them whatever this ceiling is — an index growing while its table's row count + /// does not is itself a bloat signal, and it is already collected. /// - public const long MeasureCeilingBytes = 2L * 1024 * 1024 * 1024; + public const long MeasureCeilingBytes = 1L * 1024 * 1024 * 1024; /// /// PostgreSQL's block size. pgstatindex is charged per BLOCK rather than per byte, so this is @@ -125,8 +140,8 @@ private PgIndexBloatCollector() public const int BlockSizeBytes = 8192; /// - /// The block rate is sized against, and the reason the budget is - /// a small number rather than a large one. + /// The block rate and are both + /// sized against, and the reason each is a small number rather than a large one. /// /// Why a block rate and not a byte throughput. pgstatindex walks the index one /// block at a time through the buffer manager with no prefetch, so its cost is a count of @@ -134,12 +149,40 @@ private PgIndexBloatCollector() /// each miss is a round trip, so per-block LATENCY sets the rate and a sequential-throughput figure /// overstates it by orders of magnitude. /// - /// This value is an assumption, not a measurement — no SUCCESS row has ever supplied a duration - /// for this collector — so it is deliberately pessimistic, and - /// TheCycleBudget_FitsTheDeadline_AtThePessimisticBlockRate pins the budget, the deadline and - /// this rate together so that raising any one of them has to argue against the other two. + /// This is now a MEASUREMENT, and it came in at roughly HALF the figure that preceded it + /// (#3164). The figure here used to be an assumption of 2,000 blocks/s, named pessimistic and + /// carrying no measurement, because none existed. One does now: the first SUCCESS row this collector + /// has ever produced recorded collection_log.sql_duration_ms of 252,940 ms, which is the + /// instrument pre-registered for exactly this decision. That run + /// executed under a 2 GiB cycle budget, and the gate admits an index only when the running total + /// THROUGH that index is still within budget, so the bytes it read cannot have exceeded the budget — + /// which bounds the rate from above at 262,144 blocks / 252.94 s = 1,036 blocks/s using nothing + /// but that row's duration and the shipped gate. The rate derived from the bytes that run actually + /// measured is ~1,013 blocks/s, and it is the figure kept here. + /// + /// What this figure does NOT rest on. n = 1 — one run, one target, one night. Two numbers + /// elsewhere in this type read like corroboration and are not: ' "about + /// 11 hours" for the over-ceiling set and "roughly an hour" for its largest member are this same rate + /// restated at coarse precision, not independent derivations. So the only independent check on the + /// figure below is the 1,036 blocks/s upper bound above, which agrees on the order and on the + /// direction. The three-significant-figure value is kept rather than rounded because rounding it would + /// invent a margin, and the margin belongs in ONE place — see the next paragraph. + /// + /// The safety margin is the half-deadline allowance, and it must not be double-counted here. + /// TheCycleBudget_FitsTheDeadline_AtTheMeasuredBlockRate compares the budget against HALF of + /// , so a run whose rate comes in materially below this + /// figure still has the other half of the deadline to finish in. Shading this constant below the + /// measurement as well would spend that headroom twice and make neither figure mean anything. One + /// number, one meaning: this one is what was measured, and the allowance is what pays for being wrong + /// about it. + /// + /// What would justify moving it. A DISTRIBUTION rather than another single row — several + /// SUCCESS rows across cache states and instance load, because per-block latency on network-attached + /// storage is the rate-setter and a single sample of it is not a rate. A measurement LOWER than this + /// reds the deadline pins and brings the budget and the ceiling down again; that is the mechanism + /// working rather than a problem, and it is the same trigger that produced this change. /// - public const int PessimisticBlocksPerSecond = 2_000; + public const int MeasuredBlocksPerSecond = 1_013; /// /// How many bytes of index ONE ATTEMPT will measure in total, largest first. Independent of @@ -153,19 +196,25 @@ private PgIndexBloatCollector() /// The deadline is per statement, so that is the right granularity for the bound, but it is not a /// bound on a sweep. /// - /// Why 2 GB. Not from a throughput measurement — none exists for this collector, and a - /// deadline-cut run bounds only its own duration, never a rate. It is derived instead from the cost - /// model in : 2 GB is 262,144 blocks, which at 2,000 - /// blocks/s is about 131 seconds, inside half of - /// . The remaining headroom pays for the catalog scan, - /// connection setup, and the tail index admitted while the running total was still just under - /// budget. + /// Why 1 GiB, and why it used to be 2. Derived from the cost model in + /// : 1 GiB is 131,072 blocks, which at 1,013 blocks/s is about + /// 129 seconds, inside half of . The remaining headroom + /// pays for the catalog scan, connection setup, and being wrong about the rate. The figure was 2 GiB + /// while the rate was an ASSUMED 2,000 blocks/s; the same arithmetic at the measured 1,013 puts 2 GiB + /// at 259 seconds, past the whole allowance, so the budget came down rather than the allowance going + /// up (#3164). + /// + /// The measurement that decided it is the one this comment used to ask for. The line + /// here previously read "until a SUCCESS row exists, every value here is an argument rather than a + /// measurement, and the small end of the argument is the one that produces the row" — and that is + /// what happened: the small end produced a row, the row supplied a rate, and the rate argued the + /// budget DOWN. So the trigger fired as pre-registered rather than being overruled. /// - /// What would justify raising it. A SUCCESS row's own - /// collection_log.sql_duration_ms (#2997), which is a measured rate for this instance and - /// this index population rather than the assumed one above. Until such a row exists, every value - /// here is an argument rather than a measurement, and the small end of the argument is the one that - /// produces the row. + /// What would justify raising it now. Not another single SUCCESS row — a distribution of + /// them, for the reason given in : one sample of a + /// latency-bound rate is not a rate. Raising the command deadline is the other arithmetic route and + /// is deliberately not taken here; see for what it + /// would and would not cost. /// /// It must never drop BELOW , and that it equals it is a /// floor rather than a coincidence. A cycle budget under the per-index ceiling opens a band @@ -185,9 +234,22 @@ private PgIndexBloatCollector() /// sitting at the cursor is admitted by neither gate, so the cycle measures NOTHING, records the pass /// as complete, wraps to the largest index, and arrives back at the same band index — the collector /// stops measuring anything at all, on every index, permanently. The band used to mislabel one index; - /// it now stops the whole mechanism. Decoupling the two therefore needs an unconditional - /// first-admission rule and a deadline constraint restated as a SUM (ceiling + budget) rather - /// than an ordering, which is a separate change with its own arithmetic to argue. + /// it now stops the whole mechanism. + /// + /// Decoupling the two would not have helped here, and the arithmetic is what says so + /// (#3164). The route #3153 left open was an unconditional first-admission rule plus the deadline + /// constraint restated as a SUM (ceiling + budget) rather than an ordering, whose whole point + /// was to let the budget fall to fit a slower rate while the ceiling stayed at 2 GiB. At the measured + /// rate that goal is unreachable by any budget: 2 GiB is 262,144 blocks and about 259 s in ONE + /// statement, so the ceiling alone is already past the 150 s allowance before a budget is chosen, and + /// ceiling + budget <= allowance has no solution at all. So the binding constraint was never + /// the ordering between these two figures — it was the ceiling's own deadline cost, which the lockstep + /// had been hiding by making the two numbers equal. Once the ceiling comes down to fit the deadline, + /// setting the budget equal to it satisfies everything with no new mechanism, and decoupling would + /// only permit the one direction (budget < ceiling) that reopens the band. The transitive + /// coverage is also the reason ThePerIndexCeiling_FitsTheDeadline_OnItsOwn now exists: while + /// budget >= ceiling holds, a deadline pin on the budget covers the ceiling for free, so a + /// decoupling would silently remove the only assertion the ceiling's cost had. /// /// Accepted consequence. An index at or above the ceiling is reported at its size with /// the ceiling's own reason and is never measured — on a large target that includes the biggest and @@ -198,12 +260,33 @@ private PgIndexBloatCollector() /// /// What this bound does NOT buy. Coverage is not budget-limited; before #3153 it was /// memory-limited, and now it is CADENCE-limited. A complete pass over the first production target's - /// 2,457 measurable indexes is a fixed ~19.6M blocks however it is spread, so at one statement per day - /// a full pass takes months. Rotation converts "never" into "eventually"; how long "eventually" is, is - /// set by how much pgstatindex time per day is acceptable on a production instance and by - /// nothing else. That is a scheduling decision, not a number this constant can express. + /// 2,457 measurable indexes was a fixed ~19.6M blocks however it was spread, so at one statement per + /// day a full pass takes months. Rotation converts "never" into "eventually"; how long "eventually" + /// is, is set by how much pgstatindex time per day is acceptable on a production instance and + /// by nothing else. That is a scheduling decision, not a number this constant can express. + /// + /// What halving it cost, stated in the unit it is paid in (#3164). Pass length is + /// measurable blocks divided by this budget, so halving the budget at most DOUBLES the pass: ~75 + /// cycles became at most ~150, and at the 1,440-minute cadence in CollectorScheduleDefaults + /// that is at most ~150 days rather than ~75. "At most", because the same change lowered + /// , which removes the indexes between the old and new ceilings from + /// the measurable set entirely — a smaller numerator against the halved denominator, so the true pass + /// length lands somewhere below the doubling. How far below depends on how much of that target's + /// ~150 GB of measurable index sits between 1 and 2 GiB, and that has not been measured. The cadence + /// is deliberately NOT touched in the same change: lowering the budget lengthens the pass, so moving + /// both at once would conflate two effects and leave neither figure meaning anything. It is re-derived + /// from whatever this budget is, separately. + /// + /// Why slower coverage is the right thing to pay. A budget that overruns its deadline + /// produces NO ROW AT ALL — strictly worse than a smaller budget that completes, and it is also the + /// state that produces no sql_duration_ms to measure the next decision from. The one SUCCESS + /// row this collector has ever produced spent 84% of its deadline, so the margin being defended here + /// is real rather than theoretical. Before #3153 a smaller budget cost REACHABILITY — the same + /// largest indexes were re-selected every cycle and anything below the cut was never measured — and + /// rotation is what converted that cost into a rate. Paying in coverage rate is only an option at all + /// because rotation exists. /// - public const long CycleMeasureBudgetBytes = 2L * 1024 * 1024 * 1024; + public const long CycleMeasureBudgetBytes = 1L * 1024 * 1024 * 1024; /// /// Prefix of the per-database rotation cursor key in collector_state (V44, primary key @@ -575,7 +658,7 @@ LEFT JOIN measured AS m NULL::bigint AS cursor_bytes, NULL::bigint AS cursor_oid"; - private const string CeilingLiteral = "2147483648"; + private const string CeilingLiteral = "1073741824"; /* The upper bound on how many indexes one attempt will MEASURE, largest first: 200 indexes OR CycleMeasureBudgetBytes, whichever comes first. The byte figure is the one that binds on any @@ -584,7 +667,7 @@ LEFT JOIN measured AS m /* Kept in the C# type system as well as in the SQL literal so a reader has one authoritative figure and TheBudgetLiterals_AgreeWithTheirConstants can pin that the two agree. */ - private const string CycleByteBudgetLiteral = "2147483648"; + private const string CycleByteBudgetLiteral = "1073741824"; /// /// Five minutes, because even a bounded cycle of pages is real work and a slow single index @@ -602,6 +685,23 @@ figure and TheBudgetLiterals_AgreeWithTheirConstants can pin that the two agree. /// while it scans, so it gets no reprieve from that and the deadline does fire - which is why /// the failures arrive as Exception while reading from stream, the transport's own words /// for a read that ran out of time. + /// + /// Raising it is the other way to make the arithmetic work, and it is NOTHING to do with a + /// 120-second wall clock (#3164). That objection was checked and does not apply: 120 s is not a + /// product-wide budget but the value three SQL Server collectors chose for + /// , the opt-in per-item budget #2673 added, whose base default is + /// null and which query_store already sets to 600 s — longer than this deadline. This + /// collector does not override it, so no wall clock bounds it at all and this figure is its only + /// deadline. A per-collector budget above 300 s is therefore established rather than novel. + /// + /// It is still not the move, for reasons that are about cost rather than mechanism. The + /// figure that would fit a 2 GiB ceiling at the measured rate is ~518 s, so this would sit at over + /// eight minutes of pgstatindex in one statement against a production instance — and because + /// this is a backstop rather than a bound, it is also eight minutes that a MIS-BUDGETED run spends + /// before reporting nothing. It would be sized to make one n=1 measurement fit rather than derived + /// from anything, which is the move the budget's own pin exists to prevent. And it would leave + /// wrong, which is the actual defect. Raising it needs its own + /// argument about acceptable occupancy of a production instance, made on its own terms. /// public override int? CommandTimeoutSecondsOverride => 300; From bc99a4d6fdc10d2d560935229016a1dcbb593be0 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:33:20 -0400 Subject: [PATCH 2/4] Census the byte bounds from the type, so the ceiling's deadline cost does not depend on a pin naming it Mutating ThePerIndexCeiling_FitsTheDeadline_OnItsOwn to read CycleMeasureBudgetBytes instead of MeasureCeilingBytes is one token and leaves every pin green, because the two figures are equal - and equal is the value TheCycleBudget_IsNeverBelowThePerIndexCeiling documents as preferred. So the two pins are indistinguishable for as long as the collector is correct, and diverge only in the decoupled state one of them exists to catch. EveryDeclaredByteBound_FitsTheDeadline_CensusedFromTheType reads the bounds off the type's own public const longs instead, so the ceiling is covered whatever any single pin names. It asserts a non-empty census and requires both figures by name, because a reflection filter that matches nothing reports success for having looked. --- .../PgIndexBloatCollectorDefinitionTests.cs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs b/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs index e5dcc57867..d205af46c1 100644 --- a/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs +++ b/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs @@ -10,6 +10,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Reflection; using System.Text.RegularExpressions; using System.Threading; using Lite.Tests.Helpers; @@ -586,6 +587,68 @@ public void ThePerIndexCeiling_FitsTheDeadline_OnItsOwn() + "and say on what measurement"); } + /// + /// EVERY byte bound this collector declares fits the deadline, with the population of bounds CENSUSED + /// FROM THE TYPE rather than typed out here. + /// + /// Why the census, when two pins above already do this arithmetic. Because those two name + /// their subject inline, and a pin's subject is the one thing it cannot check about itself. Mutating + /// to read + /// CycleMeasureBudgetBytes instead of MeasureCeilingBytes — one token — leaves all of + /// this file green while the ceiling's deadline cost goes unasserted, because the two figures are + /// currently EQUAL. That is not a hypothetical: equal values are precisely what + /// is documented as preferring, so the two + /// pins are arithmetically indistinguishable for as long as this collector is correct, and become + /// distinguishable only in the decoupled state where one of them is supposed to fire. + /// + /// So the ceiling's coverage is made independent of any hand-typed subject: the bounds are read + /// off the type's own public constants, and each is required to fit the same allowance. A third byte + /// bound added later is covered without anyone remembering to add a pin, which is the case a + /// hand-written list gets wrong. + /// + /// It cannot pass vacuously. An empty census is asserted against — if these constants ever + /// stop being public const long (made static readonly, moved to a settings type, renamed + /// out of the filter) the reflection returns nothing, and a reflection-driven check that silently + /// matches nothing is the shape that reports success for having looked. The two figures this change is + /// about are additionally required BY NAME, so the census shrinking to one of them is red rather than + /// quietly narrower. + /// + [Fact] + public void EveryDeclaredByteBound_FitsTheDeadline_CensusedFromTheType() + { + var deadlineSeconds = PgIndexBloatCollector.Instance.CommandTimeoutSecondsOverride; + + Assert.NotNull(deadlineSeconds); + + var allowed = deadlineSeconds.Value / 2.0; + + var bounds = typeof(PgIndexBloatCollector) + .GetFields(BindingFlags.Public | BindingFlags.Static | BindingFlags.FlattenHierarchy) + .Where(f => f.IsLiteral && !f.IsInitOnly && f.FieldType == typeof(long)) + .ToDictionary(f => f.Name, f => (long)f.GetRawConstantValue()!, StringComparer.Ordinal); + + Assert.NotEmpty(bounds); + + foreach (var required in new[] { "MeasureCeilingBytes", "CycleMeasureBudgetBytes" }) + { + Assert.Contains(required, bounds.Keys); + } + + foreach (var (name, bytes) in bounds) + { + var blocks = bytes / PgIndexBloatCollector.BlockSizeBytes; + var seconds = blocks / (double)PgIndexBloatCollector.MeasuredBlocksPerSecond; + + Assert.True( + seconds <= allowed, + $"{name} is {bytes} bytes = {blocks} blocks, which at " + + $"{PgIndexBloatCollector.MeasuredBlocksPerSecond} blocks/s takes {seconds:F0}s — past " + + $"the {allowed:F0}s that leaves half of the {deadlineSeconds}s command deadline for " + + "everything else. Every byte bound here bounds work inside ONE statement under ONE " + + "deadline, so each has to fit it on its own"); + } + } + /* ---------------- rotation (#3153) ---------------- */ /// From 1de431bd999d7f7b61df4dfbcb2e595e0cc7da3d Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:37:58 -0400 Subject: [PATCH 3/4] Say what the block rate's denominator contains, since it is not a pure I/O rate sql_duration_ms on a per-database collector sums connect + execute + drain across databases, and the run this figure comes from swept two, one of which failed fast on a missing extension. So the figure is blocks over everything the statement spent, which makes the true per-block read rate faster than it - the safe direction, and the right quantity, because what the constant feeds is a comparison against a deadline that covers connect and drain too. --- PerformanceMonitor.Collectors/PgIndexBloatCollector.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/PerformanceMonitor.Collectors/PgIndexBloatCollector.cs b/PerformanceMonitor.Collectors/PgIndexBloatCollector.cs index 6bd6282b87..c1b255b2eb 100644 --- a/PerformanceMonitor.Collectors/PgIndexBloatCollector.cs +++ b/PerformanceMonitor.Collectors/PgIndexBloatCollector.cs @@ -160,6 +160,16 @@ private PgIndexBloatCollector() /// but that row's duration and the shipped gate. The rate derived from the bytes that run actually /// measured is ~1,013 blocks/s, and it is the figure kept here. /// + /// What the denominator actually contains, because the name says "blocks per second" and + /// this is not a pure I/O rate. sql_duration_ms on a collector + /// is the SUM across databases of connect + execute + drain, so the figure below is blocks divided by + /// everything the statement spent, not by pgstatindex time alone — and the run it comes from + /// swept two databases, one of which failed fast on a missing extension. Every one of those terms + /// inflates the denominator, so the true per-block read rate is FASTER than this. That is the right + /// direction and the right quantity: what this constant feeds is a comparison against a command + /// deadline, and the deadline covers connect and drain too. A pure I/O rate would understate the + /// budget's real cost by exactly the terms it left out. + /// /// What this figure does NOT rest on. n = 1 — one run, one target, one night. Two numbers /// elsewhere in this type read like corroboration and are not: ' "about /// 11 hours" for the over-ceiling set and "roughly an hour" for its largest member are this same rate From 79601f12a39d50cefe96a3b6914038abda3fcee1 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:42:17 -0400 Subject: [PATCH 4/4] Correct what the half-deadline reserve covers, and say why the census takes every long The deadline pin's doc said the reserve also paid for a tail index charged past a nearly-spent budget. The shipped gate is not that shape: measured_bytes_through_here is a window sum over ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW, so it includes the row being tested and the tail index is admitted only if it fits. Prefix sums are non-decreasing, so the admitted set is a prefix and the total read is bounded by the budget exactly - there is nothing to reserve against. The wrong version made the reserve look like it covered a 2x read, leaving the rate error uncovered, and the same property is what lets a SUCCESS row's duration bound the block rate from above. The census filter takes every long rather than every name ending in Bytes, because the suffix fails green - a bound named otherwise escapes it - while the broad filter fails noisy. --- .../PgIndexBloatCollectorDefinitionTests.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs b/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs index d205af46c1..59b87b529c 100644 --- a/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs +++ b/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs @@ -514,6 +514,20 @@ public void TheCycleBudget_IsNeverBelowThePerIndexCeiling() /// is now what was measured rather than a /// figure shaded downwards. Spending the margin in two places would leave neither meaning anything. /// + /// One thing this reserve does NOT cover, corrected rather than quietly rewritten (#3164). + /// This paragraph used to say the remainder also paid for "the tail index admitted while the running + /// total was still just under budget — that index is charged for itself, so the last admission can be + /// almost a whole index past the point where the budget was nearly spent." That describes a gate of the + /// form total BEFORE this row <= budget, and the shipped gate is not that one: + /// measured_bytes_through_here is a window sum over + /// ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW, so it INCLUDES the row being tested, and + /// measured_bytes_through_here <= budget admits the tail index only if it fits in the + /// headroom that is left. Prefix sums are non-decreasing, so the admitted set is a prefix and the total + /// read is bounded by the budget exactly — there is no overshoot to reserve against. Worth stating + /// because the wrong version made the reserve look like it was covering a 2x read, which would have + /// left the actual rate error uncovered; and because the same property is what lets a SUCCESS row's + /// duration bound the block rate from above at all. + /// /// The rate is a measurement now, and this pin is what made the trigger fire (#3164). The /// figure was an assumed 2,000 blocks/s; the first SUCCESS row this collector ever produced put it at /// ~1,013, and this assertion is what turned that into a required budget change rather than a note — @@ -612,6 +626,16 @@ public void ThePerIndexCeiling_FitsTheDeadline_OnItsOwn() /// matches nothing is the shape that reports success for having looked. The two figures this change is /// about are additionally required BY NAME, so the census shrinking to one of them is red rather than /// quietly narrower. + /// + /// The filter is every long constant, not every one whose name ends in + /// Bytes. The suffix would read more precisely and fails the wrong way: a byte bound added + /// under some other name escapes the census and is never checked, which is green and wrong. Taking + /// every long means an unrelated long constant added later gets asserted against the + /// deadline as well — noisy and wrong, which someone has to come and look at. Given the choice, fail + /// toward the label that costs attention rather than the one that costs coverage. The int + /// constants on this type (BlockSizeBytes, MeasuredBlocksPerSecond, + /// MaxSplicedCursors) are excluded by the type test, which is why the suffix is not needed to + /// keep BlockSizeBytes out of a census of byte BOUNDS. /// [Fact] public void EveryDeclaredByteBound_FitsTheDeadline_CensusedFromTheType()