Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions PerformanceMonitor.Collectors/PgBufferUsageCollector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,20 @@ HAVING count(*) >= 8
/// </summary>
public override bool AppliesTo(CollectorTargetInfo target) => true;

/// <summary>
/// Two minutes, because the COST here is the input scan and no LIMIT can reduce it.
///
/// <para><c>pg_buffercache</c> materialises one row per shared buffer and takes buffer partition locks
/// while it does — on a large instance that is millions of rows, and the aggregate needs all of them,
/// so the usual trick of capping output does nothing. The output IS bounded (grouped per relation,
/// with an eight-buffer floor); the read is not.</para>
///
/// <para>Without an override a slow scan ends as <c>Exception while reading from stream</c>, an
/// unclassified Npgsql failure — which is exactly how #2617 presented and why it went unnoticed for a
/// collector that had never once succeeded. A classified timeout says which collector was too slow.</para>
/// </summary>
public override int? CommandTimeoutSecondsOverride => 120;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No pinning test for this override. The sibling fix in #2618 (PgIndexBloatCollector) added a regression guard in Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs:

Assert.NotNull(PgIndexBloatCollector.Instance.CommandTimeoutSecondsOverride);
Assert.True(PgIndexBloatCollector.Instance.CommandTimeoutSecondsOverride >= 120);

PgBufferUsageCollectorDefinitionTests.cs has no equivalent assertion, so a future edit that drops or lowers this override (undoing the fix this PR exists to make) wouldn't be caught by the test suite — only by another production incident like #2617.


public override CollectorQuery BuildQuery(CollectorContext context) => new(QueryText);

public override IReadOnlyList<CollectorColumn> PayloadColumns { get; } = new[]
Expand Down
8 changes: 7 additions & 1 deletion PerformanceMonitor.Collectors/PgColumnStatsCollector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,13 @@ JOIN pg_catalog.pg_namespace AS n
WHERE s.schemaname NOT IN ('pg_catalog', 'information_schema')
AND c.relkind IN ('r', 'm', 'p')
AND c.relpages >= 128
ORDER BY c.relpages DESC, s.schemaname, s.tablename, s.attname";
ORDER BY c.relpages DESC, s.schemaname, s.tablename, s.attname
/* Bounded (#2617's lesson applied before it bites). This is a catalog read rather than a page scan, so
it is nowhere near as costly as pg_index_bloat was - but it was still unbounded, and row count here is
tables x columns: 361 tables over the size floor on one measured target, and a wide schema turns that
into five figures per database per DAY. The ORDER BY already puts the biggest tables first, so the cap
keeps exactly the columns a plan-shape question is asked about. */
LIMIT 5000";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same gap as PgBufferUsageCollector: PgColumnStatsCollectorDefinitionTests.cs has no assertion that this query is bounded (e.g. Assert.Matches(new Regex(@"LIMIT\s+\d+"), Sql())). Every other correctness property of this query is pinned by a dedicated test in that file — this LIMIT is the one this PR adds and it's the one left unpinned, so a later edit that drops it silently regresses back to the unbounded-read shape #2617/#2619 exist to fix.


public override string Name => "pg_column_stats";

Expand Down
Loading