diff --git a/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs b/Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs index a9374fa18c..59b87b529c 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; @@ -478,8 +479,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 +510,169 @@ 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. + /// + /// 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 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"); + } + + /// + /// 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. + /// + /// 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() + { + 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) ---------------- */ /// diff --git a/PerformanceMonitor.Collectors/PgIndexBloatCollector.cs b/PerformanceMonitor.Collectors/PgIndexBloatCollector.cs index 0ccb5447eb..c1b255b2eb 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,50 @@ 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 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 + /// 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 +206,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 +244,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 +270,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 +668,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 +677,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 +695,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;