Bound the cycle, not just the index, in pg_index_bloat (#2617)

CHANGELOG.md

@@ -68,6 +68,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Lite's portable ZIP is self-contained, which HALVED it** ([#2501]) - `Publish Lite` is now `-r win-x64 --self-contained` in both `build.yml` and `nightly.yml`, so neither Lite artifact has a .NET prerequisite any more and the failure [#2489] documented stops existing: a tester who unzips onto a stock Windows Server no longer meets the .NET host's bare `You must install .NET to run this application` before a line of our code runs. **The size went the opposite way from what bundling a runtime suggests.** The old publish was RID-agnostic, so it copied every platform its packages ship - **537 MB of `runtimes\` on a 565 MB tree** (osx 130, linux-x64 116, linux-arm64 70, win-arm64 56, then win-x86, musl, loongarch64 and riscv64), of which only the **52 MB `win-x64`** folder could ever load on Windows. `DuckDB.NET.Bindings.Full` is most of it, SkiaSharp and SqlClient behind it. Dropping ~485 MB of unloadable native payload beats the cost of bundling .NET, WPF and ASP.NET Core by roughly two to one: measured on one commit and one SDK, **565 MB tree / 212.7 MB zipped becomes 277 MB / 114.2 MB**. It matters most for the **nightly** ZIP, which is the UAT download and is not offered as a `Setup.exe` at all. **A RID-specific publish needed two more files than the flag.** `Lite/packages.lock.json` had only a `net10.0-windows7.0` target, and a RID restore adds `net10.0-windows7.0/win-x64` to it - after which the `dotnet restore --locked-mode` that BOTH workflows run before the publish fails `NU1004: the project's runtime identifiers have changed`, because locked mode compares the PROJECT's RID set (empty) against the lock file's (win-x64). Reproduced locally; that is a red CI run on every PR, not the future `--no-restore` trap it was filed as. The fix is `<RuntimeIdentifiers>win-x64</RuntimeIdentifiers>` in `PerformanceMonitorLite.csproj`, so the project itself asks for that graph and one committed lock file satisfies the RID-less locked-mode restore and the RID publish alike; `RuntimeIdentifiers` (plural) sets no RID on the build, so a plain `dotnet build` stays RID-agnostic and `Lite.Tests` is untouched. **SignPath needed nothing** - the `Lite` artifact-configuration slug already receives both shapes today, and the signed re-zip reads `signed/Lite/*`, inheriting whatever shape `publish/Lite` has. Auto-update is unaffected; the ZIP is not a Velopack channel. `LiteRuntimePrerequisiteDocsTests` went red on the flag alone (3 of its 7 facts) and was rewritten to state every claim BOTH ways round: [#2499]'s version asserted only that the docs DID name the runtimes, so two of its facts stayed green while the prose went stale. It now also derives the lock file's RID coverage from the `-r` flags in the workflows, and every new assertion was proven red with its fix reverted.
 
 ### Fixed
+- **`pg_index_bloat` had never returned a single row** ([#2617]) - found by dogfooding v99 on a live Aurora target, where the collector failed every cycle with `Exception while reading from stream` and `rows_ever = 0` for its entire life. **The wrong dimension was bounded.** `pgstatindex` reads every page of the index it is pointed at, and the 20 GB `MeasureCeilingBytes` bounds one INDEX while nothing bounded the STATEMENT: measured on that target, **1,517 indexes totalling 461 GB** in a single query on the default command timeout, which never finished and dropped the connection mid-read. A cycle work budget now measures the **200 largest** indexes per run - a count rather than a byte cap, because the cost is per page and index sizes are wildly uneven, so a byte cap would measure three indexes on one server and four hundred on another with no way for an operator to predict which. Everything past the budget is still **returned** carrying `skipped_reason`, never dropped: an index missing from the result reads as one that does not exist, while an index present with a stated reason cannot be mistaken for healthy. A 300-second `CommandTimeoutSecondsOverride` means a slow single index now yields a CLASSIFIED timeout instead of an unclassified stream failure. The local rig that verified #2561 had two indexes on one table, so the question of total work never arose there - which is the class of defect only a real instance shows.
 - **The README documented an upgrade script that no released build contains** ([#2593]) - reported by somebody upgrading 3.3 to 3.5 who could not find the procedure written down anywhere, and they were right: `upgrade-darling.ps1` landed on 2026-08-22, three days AFTER 3.5.0 was tagged, so neither their install nor the 3.5.0 zip contains it - while the README on the default branch has been describing it as the supported path to anyone reading the repo. The upgrade section now says plainly that the script does not exist in 3.5.0 or earlier, and a new section gives the manual procedure it automates. That procedure also states the thing the reporter had to infer: **nothing needing preservation lives in the install directory** - the managed store, the DPAPI credential blobs and the logs are all under ProgramData, so only `darling.json` has to travel, and renaming the install folder is a BINARY rollback rather than a data one. Their own plan turned out to be better than ours on one axis and it is written down as such: extracting into a clean directory cannot accumulate files a newer build stopped shipping, which is exactly what `-RemoveStaleFiles` exists to clean off an overlay-upgraded tree (#2529).
 - **Tag management had no discoverable entry point in the Darling Viewer, and none at all on a viewer with no servers** ([#2595]) - reported, and correct on both counts. The only door was the group-header context menu, which means right-clicking a row labelled **Untagged** - a place nobody looks for tag management. And that header only renders when at least one server is untagged (`FleetView` emits it under `untagged.Count > 0`), so on a freshly registered viewer with no servers there was no right-click target at all and tags were unreachable entirely. Lite has carried a visible **Manage Tags** button in the same sidebar footer all along, so this was a parity gap rather than a missing feature; the viewer now has the same button, next to Manage Servers. The reporter's second observation - that clicking "Assign Tags" does nothing - is WPF behaving normally: that item is a submenu parent, so it opens on hover rather than doing anything on click, and with no tags defined its submenu holds only "No tags yet" and a nested "Manage Tags..." that was the sole remaining door. `ManageTags_Click` already carried its own read-only backstop (#2008), so the new button needed no gating of its own. Pinned by a test that locates the handler INSIDE the sidebar footer and checks it is a `Button` - asserting mere presence would have passed against the broken build, because the handler was wired, just only inside a `ContextMenu`. The pin strips XML comments before scanning, because the new button carries a comment that names the handler and an unstripped scan finds the explanation instead of the wiring.
 - **The plan-capture remedy recommended the one setting measured to be catastrophic** ([#2565]) - the `capture_threshold` facet told anyone sitting at `log_min_duration = -1` to "set it to 0 to capture every statement, or a millisecond threshold". Then #2565 measured what 0 costs: on PostgreSQL 17 under pgbench with 8 clients, capturing every statement cost **31 percent of throughput** and wrote **772 MB of server log in 20 seconds**, while the same instrumentation at a 10ms threshold cost nothing measurable and at 100ms measured slightly above baseline. On Aurora that log is bounded by `rds.log_retention_period` (3 days on this fleet) and read back through the RDS API, so at that rate plans age out before anything can collect them. The remedy now says **set a millisecond threshold, not 0**, and explains why in one sentence rather than leaving it as taste. A server already sitting at 0 gets its own arm rather than the generic everything-is-fine sentence - it is still `is_satisfied = true`, because capture genuinely is working, but the detail names the cost and says to move off it. Both arms now also note that the threshold is a **dynamic** parameter on Aurora/RDS and applies without a reboot, unlike the library itself - measured across all 51 cluster parameter groups, where all 14 `auto_explain.*` parameters are dynamic. One more thing came out of checking how to compare against 0 safely: `current_setting` renders this GUC in the largest unit that divides evenly, so -1 and 0 come back bare, 250 comes back `250ms`, **1000 comes back `1s`** and 60000 comes back `1min`. Anything downstream that read this as a number would break on a perfectly ordinary one-second threshold, which is why `observed` is stored as the server's own text and never cast. Verified against a live server in all four states.

Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs

@@ -135,4 +135,53 @@ public void InvalidIndexes_AreExcluded()
         Assert.Contains("x.indisvalid", sql, StringComparison.Ordinal);
         Assert.Contains("x.indisready", sql, StringComparison.Ordinal);
     }
+
+    /// <summary>
+    /// The CYCLE has a work budget, not just each index (#2617).
+    ///
+    /// <para><b>This is the assertion that would have caught a collector which never returned a row.</b>
+    /// <c>pgstatindex</c> reads every page it is pointed at, and the 20 GB ceiling bounds one index while
+    /// nothing bounded the statement. Measured on a live Aurora target: 1,517 indexes totalling 461 GB in a
+    /// single statement, which never finished and dropped the connection mid-read — <c>rows_ever = 0</c> for
+    /// the collector's entire life. The local rig had two indexes, so the question never arose there.</para>
+    /// </summary>
+    [Fact]
+    public void TheCycleHasAWorkBudget_NotJustAPerIndexCeiling()
+    {
+        var sql = Sql();
+
+        /* Ranked by size so the measured ones are where bloat is worth reclaiming. */
+        Assert.Contains("row_number() OVER (ORDER BY k.index_bytes DESC", sql, StringComparison.Ordinal);
+
+        /* And the budget gates the LATERAL, which is the thing that costs pages. Gating only the
+           skipped_reason would label rows correctly while still reading every index. */
+        Assert.Matches(new Regex(@"LEFT JOIN LATERAL[\s\S]*?size_rank\s*<=\s*\d+"), sql);
+    }
+
+    /// <summary>
+    /// Over-budget indexes are RETURNED with a reason, never dropped. An index missing from the result
+    /// reads as one that does not exist; an index present with a stated reason cannot be mistaken for
+    /// healthy — the same argument that put <c>skipped_reason</c> on the size ceiling.
+    /// </summary>
+    [Fact]
+    public void OverBudgetIndexesAreReturnedWithAReason()
+    {
+        var sql = Sql();
+
+        Assert.Contains("not measured this cycle (work budget)", sql, StringComparison.Ordinal);
+
+        /* No WHERE that would remove them from the result set. */
+        Assert.DoesNotMatch(new Regex(@"WHERE[^;]*size_rank\s*<="), sql);
+    }
+
+    /// <summary>
+    /// A command-timeout override, so a slow single index yields a CLASSIFIED timeout rather than the
+    /// unclassified <c>Exception while reading from stream</c> that #2617 actually surfaced as.
+    /// </summary>
+    [Fact]
+    public void ItOverridesTheCommandTimeout()
+    {
+        Assert.NotNull(PgIndexBloatCollector.Instance.CommandTimeoutSecondsOverride);
+        Assert.True(PgIndexBloatCollector.Instance.CommandTimeoutSecondsOverride >= 120);
+    }
 }

PerformanceMonitor.Collectors/PgIndexBloatCollector.cs

@@ -127,6 +127,21 @@ AND   x.indisvalid
     AND   x.indisready
     AND   n.nspname NOT IN ('pg_catalog', 'information_schema', 'pg_toast')
     OFFSET 0
+),
+/* THE WORK BUDGET (#2617). The per-index ceiling below bounds one index; this bounds the CYCLE.
+   pgstatindex reads every page it is pointed at, so without this the statement reads the whole
+   instance: measured on a live Aurora target, 1,517 indexes totalling 461 GB in a single
+   statement, which never finished and dropped the connection mid-read. The collector had
+   returned zero rows for its entire life.
+
+   Ranked by size and measured largest-first, because bloat that matters is concentrated in big
+   indexes - a small index at 40% density is worth kilobytes. Everything past the budget is still
+   RETURNED, with a reason, so the read never mistakes unmeasured for healthy. */
+ranked AS (
+    SELECT
+        k.*,
+        row_number() OVER (ORDER BY k.index_bytes DESC, k.index_name) AS size_rank
+    FROM candidates AS k
 )
 SELECT
     current_database()::text            AS database_name,
@@ -145,14 +160,35 @@ OFFSET 0
         WHEN k.index_bytes >= " + CeilingLiteral + @"
             THEN 'index is larger than the measurement ceiling; pgstatindex reads every page, so it is '
                  || 'recorded but not measured'
+        WHEN k.size_rank > " + BudgetLiteral + @"
+            THEN 'not measured this cycle (work budget): pgstatindex reads every page, so only the '
+                 || 'largest ' || " + BudgetLiteral + @" || ' indexes are measured per run. This one is '
+                 || 'recorded at its size so it is never mistaken for healthy.'
     END::text                           AS skipped_reason
-FROM candidates AS k
+FROM ranked AS k
 LEFT JOIN LATERAL public.pgstatindex(k.index_oid::regclass) AS s
-  ON k.index_bytes < " + CeilingLiteral + @"
+  ON  k.index_bytes < " + CeilingLiteral + @"
+  AND k.size_rank  <= " + BudgetLiteral + @"
 ORDER BY k.index_bytes DESC";
 
     private const string CeilingLiteral = "21474836480";
 
+    /* How many indexes one cycle will actually MEASURE, largest first. 200 rather than a byte budget
+       because the cost is per PAGE and the sizes are wildly uneven: a byte cap would measure three
+       large indexes on one server and four hundred small ones on another, and neither operator could
+       predict what they were getting. A count is legible, and the ORDER BY means the 200 measured are
+       always the ones where bloat is worth reclaiming. */
+    private const string BudgetLiteral = "200";
+
+    /// <summary>
+    /// Five minutes, because even 200 indexes of pages is real work and a slow single index should
+    /// yield a CLASSIFIED timeout rather than a dropped connection. #2617 surfaced as
+    /// <c>Exception while reading from stream</c> - an unclassified Npgsql failure - precisely because
+    /// there was no budget and no override; index_object_stats took the same override for the same
+    /// reason (#1135).
+    /// </summary>
+    public override int? CommandTimeoutSecondsOverride => 300;
+
     public override string Name => "pg_index_bloat";
 
     public override string TargetTable => "pg_index_bloat";