Skip to content

release: keep stale rollups honest without losing fast path - #199

Merged
NikolayS merged 7 commits into
NikolayS:mainfrom
samo-agent:agent/w3-123
Jul 27, 2026
Merged

release: keep stale rollups honest without losing fast path#199
NikolayS merged 7 commits into
NikolayS:mainfrom
samo-agent:agent/w3-123

Conversation

@NikolayS

Copy link
Copy Markdown
Owner

Summary

Relates to #122. Supersedes and reworks #123; this deliberately does not close #122.

#123 correctly prevented a lagging rollup_1m from hiding newer raw load, but compared its watermark with the unclamped until. A standard dashboard boundary of date_trunc('minute', now()) + interval '1 minute' is unreachable by that watermark, forcing a healthy wide read onto raw data.

This rework:

  • delegates canonical fallback selection to _pick_source(since);
  • when that fallback is raw, requires the rollup watermark to reach least(until, date_trunc('minute', now()));
  • otherwise preserves the established picker result instead of duplicating B3's physical/logical raw-boundary logic.

Measured performance

Five runs per implementation on the same local PostgreSQL 17.9 fixture: 21,600 one-second raw rows over six hours, 40 active backends per row, 6,096 kB raw table, and 360 healthy minute-rollup rows. Each call used the rounded-up dashboard boundary.

Implementation / chosen source Runs (ms) Median
#123 (6123780), raw 1494.746, 1345.164, 1386.175, 1364.884, 1351.251 1364.884 ms
This branch (100e95c), rollup_1m 7.521, 2.193, 1.980, 1.961, 1.914 1.980 ms

The rejected #123 path was 689.3× slower on this reproduction. Both paths returned 360 covered buckets and 864000.00 backend-seconds; the discriminating difference was the selected source.

Exact regression coverage

Test 2.0: stale and rounded-up rollup source selection runs in an isolated BEGIN/ROLLBACK fixture and asserts:

  • stalled rollup with complete raw coverage: source = 'raw', buckets_with_data = 2, backend_seconds = 2.00;
  • healthy historical rollup: source = 'rollup_1m', buckets_with_data = 90, backend_seconds = 90.00;
  • healthy rollup with until rounded up one minute: source = 'rollup_1m', buckets_with_data = 120, backend_seconds = 120.00.

The literal final test block is RED on untouched #123 head 6123780:

BEGIN
ERROR:  healthy rollup must keep the fast path for a rounded-up until, got raw
CONTEXT:  PL/pgSQL function inline_code_block line 89 at ASSERT

It is GREEN on this rebased branch:

BEGIN
NOTICE:  stale/rounded-up rollup source selection PASSED
DO
ROLLBACK

Explicit non-goal

A stalled rollup for a window beginning before physical raw coverage can still return a silently truncated rollup answer. The post-rebase probe remains:

NOTICE:  older-than-raw stalled rollup: source=rollup_1m, buckets=1, backend_seconds=1.00

Completeness disclosure for that case is intentionally out of scope here and remains tracked by #122, which stays open.

Post-rebase validation

Rebased immediately before push onto origin/main 3af54a7e552ad3f29225a1f7278138de4570ac78, including #189, #182, #197, #180, and the later #186 documentation update.

  • Fresh install and idempotent installer reapply: passed on PostgreSQL 17.9.
  • Exact selector fixture: passed with the GREEN output above.
  • Full release: reject inverted reader windows and clarify diagnostics #180 until-only/inverted-window fixture: passed (B5 until-only reader windows PASSED, B5 inverted-window validation PASSED, and B5 #63 cross-horizon clamp PASSED).
  • git diff --check, workflow YAML parsing, and changed shell-block syntax: passed.
  • Independent final diff review after the last rebase: no actionable blockers.

Record the deliberate reader behavior change: wide windows fall back to raw when the canonical raw source is newer than the rollup watermark, while a dashboard end rounded into the next minute keeps a healthy rollup fast path. Keep the older-than-physical-raw completeness case explicitly open under NikolayS#122.
@NikolayS
NikolayS marked this pull request as ready for review July 27, 2026 23:40

@samo-agent samo-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Codex review standing in for REV — audit verdict: PASS on exact rebased head de169f4. Rebased by hand onto main 6d72583. Workflow conflict: retained #198s extracted query-attribution gate first, then #199s isolated stale/rounded-up selector gate. Installer comment conflicts: preserved #198s exact-query terminology while adding #199s watermark/fallback contract and explicit #122 non-goal. Product predicate is exact: last_rollup_1m_ts reaches least(until, date_trunc('minute', now())) only when canonical fallback is raw. The gate directly asserts source=raw for stale coverage and source=rollup_1m for both historical and rounded-up healthy windows; it has no timing assertion. Current main RED is "stale rollup must fall back to raw, got rollup_1m". Exact head GREEN is "stale/rounded-up rollup source selection PASSED". Focused PG17 #180 and #198 cross-checks pass; #182/#189/#194 bodies and #198s helper are unchanged. RELEASE_NOTES records the behavior and leaves older-than-physical-raw completeness under #122. Full PG14-19beta1 plus cron-off CI is green: https://github.com/NikolayS/pg_ash/actions/runs/30314863835. No merge-blocking finding.

@NikolayS
NikolayS merged commit 9efad9c into NikolayS:main Jul 27, 2026
8 checks passed
@samo-agent
samo-agent deleted the agent/w3-123 branch July 27, 2026 23:45
samo-agent added a commit to samo-agent/pg_ash that referenced this pull request Jul 27, 2026
Combining NikolayS#192 with NikolayS#198 and NikolayS#199 makes test.yml 513,005 bytes, above GitHub Actions' 512,000-byte workflow limit. Move NikolayS#199's exact 110-line transaction fixture to devel/tests and invoke it with the same psql connection and ON_ERROR_STOP behavior. The payload is byte-identical after removing YAML indentation; all nine assertions remain. The combined inventory is 848 inline plus 184 shared, 1,032 total.
NikolayS pushed a commit that referenced this pull request Jul 28, 2026
* build: add pg_cron release-gate image

Keep the Docker tag independent from the numeric package major so the same image definition supports PostgreSQL 14 through 19beta2.

Relates to #160

* test: share release-gate SQL assertions

Source cron, degraded-mode, and schema snapshots from shared SQL while preserving all 594 workflow assertions. Include attnum in schema snapshots so column-order divergence is visible.

Relates to #160

* feat: add Docker release-gate runner

Run every gate surface across bounded parallel PostgreSQL workers, reuse exact CI step bodies, emit validated TSV and human summaries, and guarantee ID-scoped container and volume teardown.

Relates to #160

* test: add behavioral feature coverage

* test: add features release-gate surface

* test: align feature contracts with privilege fixes

* test: expose rollup return contract mismatch

* fix: normalize ash.config upgrade ordinals

Rebuild the v1.5 upgrade table in canonical fresh-install order while preserving the singleton row, defaults, constraints, indexes, ownership, ACLs, and comments. Reject unsupported custom dependencies and catalog properties transactionally instead of dropping them.

Add an upgrade regression that seeds non-default values and custom catalog objects, verifies preservation, and proves migration reapplication keeps the replacement table OID.

* docs: clarify config normalization transaction

* test: adapt release gate to current main

* test: include current PG17 upgrade coverage

* test: distinguish rollup progress from rows

* test: isolate feature pgss state

The shared feature step runs after the full workflow has accumulated hundreds of pg_stat_statements entries. Dropping and recreating the extension does not clear the preloaded shared state, so the exact marker query can be missing even though the reader code is correct.

Reset pg_stat_statements immediately after recreating it. This preserves the exact ash.top()/ash.samples() enrichment assertions while making their fixture independent of prior workflow activity.

* test: reap cron-path workload

Tag H-CI-3's three synthetic pg_sleep clients, terminate only those tagged backends on every exit, and wait for their client processes. The prior step left all three sessions live for later workflow steps, so features_lifecycle.sql correctly observed four active sleepers instead of its single owned fixture. Keep the exact active_count = 1 and decoded-row assertions unchanged; restore the quiescent precondition they test.

* test: make report diagnostics PG19-safe

PostgreSQL 19 beta 2 resolves text || jsonb as JSON concatenation, so both B5 ASSERT message expressions raise on the JSON token before reporting the contract. Cast the coverage object explicitly to text. The asserted bounds and minutes-with-data predicates are unchanged; this is a diagnostic typing fix, not a behavior change. The old exact head fails on PG19beta2, while the canonical fresh-install surface passes with these casts.

* test: share stale-rollup selector gate

Combining #192 with #198 and #199 makes test.yml 513,005 bytes, above GitHub Actions' 512,000-byte workflow limit. Move #199's exact 110-line transaction fixture to devel/tests and invoke it with the same psql connection and ON_ERROR_STOP behavior. The payload is byte-identical after removing YAML indentation; all nine assertions remain. The combined inventory is 848 inline plus 184 shared, 1,032 total.

* fix: floor report bounds before epoch conversion

ash.ts_from_timestamptz() rounds fractional seconds while casting to int4. report() previously converted first and divided by 60 afterward, so an endpoint at :59.5 or later crossed into the next minute before the documented minute floor. Floor v_from/v_to in timestamp space first. Add a deterministic +59.9-second feature assertion: the old code shifts both bounds and reports three of four data-bearing minutes; the corrected code preserves the exact four-minute coverage. Record the public behavior change in RELEASE_NOTES.md. No assertion is weakened.

* test: include query-history helper in reader bundle

After #198, grant_reader() correctly discovers 44 non-admin reader functions rather than the pre-#198 count of 43. Update both the explicit-role and default pg_monitor feature contracts, and name _exact_query_uses_coarser(integer,integer,name) in a direct EXECUTE assertion so an unrelated 44th grant cannot mask a missing helper. The initial exact PG14/PG17 feature run failed 44-versus-43; all four feature-bearing surfaces now pass on both majors.

---------

Co-authored-by: samo-agent <280144521+samo-agent@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: 2.0 readers prefer a stale rollup over complete raw data

2 participants