Skip to content

feat(activity): xmin horizon and parallel worker grouping - #149

Merged
lesovsky merged 49 commits into
developfrom
feature/activity-xmin-horizon
Jul 26, 2026
Merged

feat(activity): xmin horizon and parallel worker grouping#149
lesovsky merged 49 commits into
developfrom
feature/activity-xmin-horizon

Conversation

@lesovsky

Copy link
Copy Markdown
Owner

Closes #148.

What

Three columns on the activity screen behind a PG 13+ query branch:

Column Source Answers
leader coalesce(leader_pid, pid) which parallel group this backend belongs to
backend_xid backend_xid::text has this transaction written anything
horizon_xacts age(backend_xmin) how far back it holds the xmin horizon

Together they answer, without leaving pgcenter for psql, the question the screen could not
answer before: which session is holding the xmin horizon and stopping vacuum, how long has it
been there, and is killing it cheap?

The branch is an early return above the existing selector switch, so PgStatActivityDefault
keeps naming that switch's default: case and PG 10–12 is untouched.

Two decisions worth reading

leader is derived, not the raw leader_pid. The original scope said to expose leader_pid
so workers sort next to their leader. That does not work: leader_pid is NULL for the leader
itself. Verified on a live 4-worker scan — the leader would have sorted into the NULL block
together with unrelated backends, away from its own workers. The derived column gives the leader
and all its workers one value.

backend_xid stays as a raw column even though only its presence matters. For an
idle-in-transaction session the query column shows the last statement, so a transaction that
ran INSERT then SELECT looks read-only. Reproduced: a session showed pg_sleep in query
while backend_xid = 784. That is the kill-safety question, and nothing else on the screen
answers it.

Two fixes that ride along

Neither is opportunistic — the feature makes both reachable.

Sorting of sparse columns (internal/stat/postgres.go). The comparator was chosen from a
single cell and blanks parsed as 0, so a blank was indistinguishable from a genuine zero and led
an ascending sort. That breaks this feature's primary story — ranking sessions by horizon — on a
column that is blank for most rows. The comparator is now chosen from the first non-empty cell and
blanks order last in both directions and all three modes. This is shared machinery: replslots
sorted by its default key is the most visible beneficiary, and its SQL already declared
DESC NULLS LAST, so the change brings Go into agreement with the query.

Tech debt [021] (report/report.go) — and it was three stale states, not one: the alignment
flag, the header-repeat counter, and the resolved sort-column index. The last one matters most
here because this feature inserts columns mid-layout, so a latched index silently sorts a report
by a different column. The zero-width guard in the truncation path is restored to match its twin
in top.

Review found two regressions, both fixed

Two reviewers independently reproduced them against develop:

  • restoring the seed sort key put an out-of-range index into sort() — now bounds-checked inside
    sort itself, which also closes the pre-existing routes and the top caller (debt [025]);
  • the zero-width guard converted a panic into a hangpgcenter report in a pipe became an
    immortal process. doReport now drains on every error path (debt [026]).

A third finding was sharper: the feature's headline invariant had no automated cover. Mutants
replacing coalesce(leader_pid, pid) with a raw leader_pid, or wrapping backend_xid in a
coalesce(..., '0'), passed the entire suite. Both are now killed by value-level assertions in the
live query test.

Verification

  • make test 70.3%, make lint 0 issues, gosec clean
  • Live column names and order asserted against PG 14–19 — 6/6 pass, replacing a test that only
    checked the query did not error
  • Existing report goldens pass unchanged — but they are a regression check, not coverage of the
    sort change: every activity golden sorts by pid, which is never blank
  • Manual acceptance on live clusters, recorded in the feature's QA report

Stated limits: the PG 12→13 branch boundary cannot be verified live (the test image has PG
14–19 only) and rests on a table test pinning it from both sides; the load measurement reached 96
sessions, not the few hundred intended, because the fixture caps max_connections at 100.

Cost to be aware of

query moves 38 characters right for everyone, including users who never look at the horizon. On
a narrow terminal nothing changes qualitatively — query was already past the edge, which is what
horizontal scroll exists for. On a wide terminal it may now need one scroll step. Accepted
deliberately.

🤖 Generated with Claude Code

Alexey Lesovsky and others added 30 commits July 25, 2026 10:44
Adds docs/roadmap-0.12.0.md covering eight features (012-019) plus the
already-merged 009/010/011 that ship with this release.

Organising principle: features are grouped by the code area they touch,
not by theme. A theme-based draft ("PG 19" vs "troubleshooting") was tried
first and rejected — it split single code areas across two features and
produced six places where the same view, hotkey group or test fixture
would have been entered twice (WAL area, replication area, tables area,
contention hotkey group, standby fixture, and colorization vs the activity
column set).

Notable planning decisions recorded in the document:
- PG 19 does not break pgcenter (version selectors use >=, queries list
  columns explicitly, BUFFERPIN is not hardcoded), so [012] is tested
  catch-up rather than an emergency fix; the CI beta-package probe is the
  day-one risk.
- record/report is decided per view by design risk, not by blanket rule,
  with the measured cost of the [008] retrofit as the input.
- Column-set changes precede colorization; colorization precedes every new
  screen. Color rules key off column names, not indices.
- Pause gates the render only, never the collector.
- xmin_age is expressed in transactions only; time is read from xact_age.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Interview (6 batches, 21 questions), code research and the PG 19 catalog
check for feature [012] of the 0.12.0 roadmap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…imeter, NULL criterion, replay edge case

Quality validator: approved (0 critical, 4 major) — fixed the undefined
"all screens" perimeter, added acceptance criteria for blank-NULL and for
non-empty columns, resolved the "no change on PG 14-18" contradiction
against the describe-text superset.

Adequacy validator: changes_required (1 critical) — checked the PG 19
incompatibility list: no removed columns in stats views, the single rename
touches a view pgcenter does not query yet. Reconciled the "all screens
render" criterion with same-release breakage deferral, stated the GA-drift
consequence, dropped a tech-spec-level detail, added the forward-replay
limitation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…progress views to backlog

Quality validator: changes_required (1 critical) — the "wait event types are
never hardcoded" premise was false; `wait_event_type = 'Lock'` is hardcoded in
internal/query/common.go:63,74 and top/signal.go:77. Reworded to name the
exception and tie it to the acceptance criterion that guards it. Minors fixed:
cross-references, the 80-column figure, the forward-replay limitation moved to
Ограничения, [013]-[015] spelled out.

Adequacy validator: approved (0 critical) — new major: PG 19 ships two progress
views the roadmap predates, pg_stat_progress_repack and
pg_stat_progress_data_checksums. Recorded as out of scope with a REPACK
backwards-compatibility check added to the verification pass, and both added to
the roadmap backlog as 0.13.0 candidates. Also corrected the pgdg-testing
rationale: explicit package names cannot pull PG 20, but the beta channel would
persist across rebuilds including the GA re-verification one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…CK criterion

Quality validator: changes_required (1 critical) — the corrected pgdg-testing
rationale had been applied to Ограничения but not to the matching risk, leaving
two mutually exclusive statements; the risk now says what actually happens (PG 19
keeps arriving from the beta channel on every rebuild) instead of the false "pulls
PG 20". Minors: REPACK backwards compatibility got its own acceptance criterion,
deferred-item destinations unified, the "later features" reference spelled out.

One validator conflict resolved in favour of adequacy (substance over form): the
uniform selector arity stays out of the user-spec as a tech-spec level detail.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…width diff guard

Template validator: changes_required (1 critical) — tasks 8 and 9 sat in one
wave despite a mandatory ordering; split into waves 5 and 6, moved the probe
into a wave of its own so no Go code runs beside it, enumerated task 4's file
list instead of a glob, and trimmed rationale out of task descriptions.

Security audit: approved (1 major) — the beta apt channel needs pinning, since
apt picks the highest version across sources and would otherwise replace the
PG 14-18 packages. Added as decision 5a. Side finding adopted as decision 5b:
the diff loop indexes the previous snapshot by the current snapshot's column
count, which this feature makes reachable by turning three view widths
version-dependent; guard plus test folded into task 6.

Architecture: approved, 0 critical. Completeness: pass — added the breakage
routing risk row and the deferred-items list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ftened, describe order

Skeptic: approved (0 critical, 62 claims checked). Decision 7 described the
wrong mechanism — the skip is inside the loop, but the tests lack a per-version
subtest wrapper so it fires on the parent; the consequence was right and the fix
is the opposite of what was implied. Call-site and loop counts replaced with
statements that do not depend on a number. AC 1 no longer contradicts the three
progress branches. Task 5 now pins describe row order to decision 1, since the
only document naming an insertion point is the superseded research pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…, restore task 6 scope

Completeness validator: fail (1 critical) — decision 5b was scope creep resting
on a false premise. Verified in report/report.go:250: when the archive's
recorded version changes, the previous snapshot is replaced and the sample
skipped, so no diff pair spans a width change; bgwriter and pg_stat_io have had
version-dependent column counts since PG 14/17/18, so mixed-width archives
predate this feature; and neither top nor record reconfigures mid-session. The
user-spec explicitly defers this defect and decision 7 declines the analogous
one, so fixing it here contradicted both.

The finding is recorded as a deferred item instead. Task 6 returns to
report-only scope and internal/stat/postgres_test.go returns to task 4.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e it proves

Template validator round 2: approved, 0 critical. Substantive fixes:
internal/view/view_test.go moves from task 4 to task 3, so the PG 19 rows that
prove the Configure wiring land in the same task and wave as the wiring itself.
Task 9 now runs the full Go suite against the published image, closing the gap
where PG 19 subtests were first exercised in Final QA; task 1 states that its
locally built cluster is what earlier tasks verify against, which is why the
gap was CI-only. Task 8's wave label corrected to name its real dependency.

Editorial: rationale trimmed from task 1, alternatives added to decisions 8
and 9, concrete golden paths in task 6, files fields on task 10, and the three
selector names settled in Architecture instead of left to the implementer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Architecture round 2 findings 1 and 3 were already closed by dropping decision
5b; finding 2 was new and correct — the backward-compatibility claim was
imprecise for an archive that changes version mid-stream, where the diff is safe
but the render is not, because column widths are computed once and never
recomputed. Wording corrected and the case recorded as a deferred item.

All five validators clear of critical findings across three rounds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…aseline

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…k, coverage gaps

Template validators: approved for both batches, 0 critical. Reality checkers:
approved for tasks 01-09, changes_required for task 10 with one critical.

The critical belonged to task 01, not 10: the environment script writes a
pg_hba.conf whose host line uses the `all` database keyword, which does not
match physical replication, so pg_basebackup cannot run and the backup_type
column — one of the three this feature adds — would be unverifiable. Caught
before the image is built, which is the only cheap moment.

Also fixed: the pin acceptance criterion tested version equality, which a
routine stable-channel minor release breaks legitimately — now an origin and
priority check; databases_test.go's single-version loop was wrongly excluded
though it is the only place the sessions query executes at all; task 05 gained
a row-order test where "nothing can catch this" was only half true; task 07
took ownership of the deployment doc's image tag, which no task owned; task 08's
image check was not executable as written; task 10's dependencies and bootstrap
were incomplete.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…verted a bad line fix

Both template validators and the reality re-check: approved, 0 critical, 0 major
outstanding. Fixes applied:

The image's build layer deletes the apt package indices, which broke two things
written as if they were present: task 01's pin check (apt-cache policy prints no
repository, suite or priority without indices, so a clean run would read as "the
pin failed") and task 10's tmux install. Both now say what to do about it.

Reverted my own bad edit: a round-1 finding claimed the workflow container lines
were at 8 and 10; grep says 9 and 11, and the original task text was right.

Traceability restored in the tech-spec for four things the fix agents added to
tasks but nobody recorded upstream: the pg_hba replication entry, the deployment
document's image tag ownership, the describe row-order test, and the reviewer-set
exemption for the two test-only tasks. Added an owner for a published-tag
divergence, which previously fell between three tasks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…line

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…re paused

Task 01's probe ran before any code, as designed, and did not get as far as
building the image: postgresql-19 exists in none of the PGDG channels. Checked
jammy-pgdg-testing, noble-pgdg-testing, noble-pgdg and jammy-pgdg-snapshot; the
package list ends at postgresql-18 in all of them, while the suites themselves
resolve correctly (postgresql-common shows both channels with priorities 500 and
100), so the absence is real rather than a misconfiguration.

This is rung 3 of the user-spec's ladder, not rung 2: the failure is not
base-image specific, so migrating to ubuntu:24.04 cannot help. PostgreSQL 19
itself is on schedule (beta 2 shipped 2026-07-16); only the apt packaging is
missing.

Feature paused in full per the approved decision. No branch, no code, no image
build. All planning artifacts remain valid; resume at task 01 with the one-minute
probe recorded in the decisions log.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Retracts the previous commit's conclusion. The earlier probe declared the beta
channel as `jammy-pgdg-testing main` and found nothing; the PGDG repository
publishes each major version in its own component, so the correct source line is
`jammy-pgdg-testing 19`. With it, postgresql-19 19~beta2-1.pgdg22.04+1 and
postgresql-plperl-19 are available, the cluster starts on 21919, plperlu loads,
and the three new columns are present in the live catalog. No base-image
migration is needed — rung 1 of the ladder.

Two corrections follow. Installation needs an explicit target release, or it
fails on libpq5: the stable channel outranks the beta one for the shared client
library, which necessarily moves to the beta version. And decision 5a's apt pin
was a no-op: the beta suite already ships NotAutomatic at priority 100, so the
real guarantee is restricting the source to the `19` component, not the pin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The beta channel is declared with the major-version component and installed from
with an explicit target release — both are load-bearing and neither is obvious;
the decisions log records why. Package origins verified after the build: PG 14-18
and postgresql-common still resolve from the stable channel, only PG 19 and the
shared libpq5 come from the beta one.

The environment script gains PG 19 in its six version loops and, independently of
PG 19, replication entries in the generated pg_hba.conf: the `all` database
keyword does not match physical replication connections, so pg_basebackup could
not run at all and the basebackup progress screen was unverifiable. Confirmed by
taking an actual base backup from the PG 19 cluster.

Probe outcome: rung 1 of the ladder — the base image stays ubuntu:22.04.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sions (task 02)

The test connection helper no longer falls back to the oldest cluster for a
version it has no port for. That fallback made a forgotten map entry invisible:
the red step of the new test connected successfully to PostgreSQL 14 while
asking for 200000, which is exactly how a "PG 19 verified" suite could have
proved nothing. The helper's doc comment already promised this error.

Note what the change does and does not buy: a missing entry now makes the
affected subtests skip rather than silently exercise the wrong server. Skipping
is honest but still green, so the stopped-cluster check in the QA task remains
the thing that proves the new version is actually reached.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ens (task 03)

started_by and mode on vacuum, started_by on analyze, backup_type on basebackup,
placed before the state column where they read as row attributes. That placement
shifts the diffed pairs, so each selector carries the layout: 15 cols {12,13},
13 cols {0,0}, 12 cols {10,10}. Below PG 19 every layout is byte-identical to
today, asserted explicitly at 140000 alongside the new 190000 block.

The execution tests now call the selector instead of the bare constant and
assert the column count the server actually returns. Without that they would
have run the PG 18 query under a "190000" subtest name and proved nothing —
verified the other way round too: all three PG 19 subtests report PASS, not
SKIP, against the live cluster.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Version lists across 21 files, eight per-version assertion tables, and a derived
row in the recording filter test. The derivation matters: at PG 19 nothing is
filtered — the highest MinRequiredVersion in the registry is PostgresV16 and no
statements view is dropped when a pgss schema is supplied — so all 27 views
survive. The red step used the copied PG14 row and failed with expected 3,
actual 0, which is exactly the mistake the copy would have baked in.

Two deviations from the plan, both on merit. The single-version sessions loop in
databases_test.go was in the do-not-touch list, but it is a live-connection loop
and the only place that query executes at all, so leaving it would give it zero
PG 19 coverage. The io_test string-inspection loops were also excluded, but
adding 190000 there asserts the NULL guards on the selector's PG 19 branch
output, which is worth having.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rows follow the order the queries emit, with a trailing note naming the version
— the convention the bgwriter and IO descriptions already use for version-varying
column sets.

The existing describe test compares by identity, so it is blind to a row landing
in the wrong slot; a row-order test now covers that. It asserts presence before
position, because strings.Index returns -1 for a missing marker and -1 is less
than anything, which would let an ordering-only check pass on a row that is not
there. Confirmed red on all three descriptions before the rows were added.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Supported versions, the test image contents, port map and tag, the three new
progress selectors with their layout shifts, and the test-connection helper's
new refusal of unmapped versions. The image tag is owned here — task 8 changes
no files and task 9 excludes documentation, so it would otherwise stay stale.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ask 09)

Both land together, after the image was published: CI runs the environment
script from the copy baked into the image but the e2e script from the checkout,
and the latter runs under strict error handling with no way to skip a missing
cluster. Verified against the published image — full suite green and the e2e
record/report cycle passes on 21919.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ask 06)

Two synthetic archives, one recorded on PG 18 and one on PG 19, prove the layout
comes from the version stored in the archive rather than the running server.

The sabotage step is what makes the test worth having: forcing the pre-19 diff
interval onto the PG 19 branch turns the pg19 case red while pg18 stays green.
Without that, a stale interval would not crash — it would land on the percentage
columns, whose values parse as numbers, and print plausible nonsense.

Fixture values are chosen so the asserted delta is not a substring of any printed
value; the model test this copies has that flaw (500 inside 1500). The three
existing progress goldens are untouched, as expected: their archive carries a
pre-19 version and takes the pre-19 branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ures catalog, ADR log, tech debt, metrics, archive

Four ADRs recorded: the beta apt channel scoped by component rather than pinned
(the pin was a no-op), the test connection refusing unmapped versions, the
mid-layout column placement with version-aware diff intervals, and the pg_hba
replication entries.

Five debt items opened, [017]-[021]: the beta channel to remove at GA,
delay_time left out, nine tests that skip wholesale on one missing version, and
two pre-existing mid-archive issues surfaced by review but deliberately not
fixed here.

Spec reconciliation found divergences worth recording: the probe hit a failure
mode the ladder did not anticipate, decision 5a was rewritten after the probe
contradicted it, and two manual QA items were left incomplete.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds three columns to the existing activity screen: leader (parallel group
grouping), backend_xid (did the transaction write) and horizon_xacts (how many
transactions back the session holds the xmin horizon). Closes issue #148.

Scope differs from roadmap [013] in four recorded ways: backend_xmin dropped as
redundant, raw leader_pid replaced by a derived column (raw leader_pid is NULL
for the leader itself, so it never achieved the stated goal), tech debt [021]
folded in, and the sort-mode detection bug fixed — the latter two are reachable
only because of this feature.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Alexey Lesovsky and others added 19 commits July 25, 2026 21:32
…ndings

Quality validator (critical): the spec claimed column order is covered by no test
while also requiring a describe-order test. The risk has two channels; split them.

Adequacy validator (major): blank cells parsed to 0 would surface at the top on
ascending sort and mix with a genuine zero, breaking the spec's own "blank is not
zero" rule — blanks now sort last in both directions. Also recorded that the sort
fix touches the shared engine rather than this screen alone, so the replay
measurement predates it.

Corrected a factual error: checkpointer/walwriter never reach the activity screen
at all, since the existing filter compares transaction and query start times which
are unset for them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ive server

The upgraded query test now asserts column names and their order, not just the
count. Same cost, and it closes the channel of the column-order risk that two
earlier rounds had accepted as untestable: the order inside the SQL itself is now
checked against what a live PG 14-19 returns. Residual risk is PG 12/13, which
have no cluster in the test image.

Interview synced with the validation outcomes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Interview closed after 8 question rounds and two validation rounds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six tasks in three waves. Two structural fixes ride with the feature: sort-mode
detection (empty cells must not collide with a genuine zero) and tech debt [021]
(stale widths on a mid-archive version change, which panics rather than
mis-renders).

Implementation-level research measured the query against a live PG 16: the
17-column layout, the leader/backend_xid/horizon_xacts semantics on a parallel
writing transaction, and the fact that age(backend_xmin) returns integer so only
backend_xid needs a cast.

User-spec corrected in two places: the query shift is 38 characters rather than
32 (the inter-column gap was omitted), and the synthetic-archive criterion is now
unconditional since the archive proved cheap to build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Valid

Nullness is available and survives the archive round-trip, so keying the sort on
Valid looked more precise. It would be wrong: every render path prints .String
alone, so a SQL NULL and a genuine empty string are indistinguishable on screen,
and ordering them differently would split visually identical blanks between the
top and the bottom with no visible reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Skeptic: approved, 54 claims checked, zero mirages. Its corrections applied —
the sort defect is deterministic rather than random (empty first cell falls to
the string comparator; non-empty numeric first cell makes blanks parse as zero),
replication.go is only a partial cast precedent, and the Ncols claim needed
widening to name readMeta as the second reader.

Security: the privileges caveat existed in Risks but was missing from the task
and criteria that enumerate "three caveats", so an implementer closing them
literally would drop it — now a named fourth. Two further findings accepted: a
replayed version change leaves a third stale state (the resolved sort index,
latched on the first sample and able to index past a narrower row), and the
render bounds guard is restored — the earlier rejection was wrong, since its
twin in top gained that guard after a real crash and leaving report without it
keeps two copies of one routine at different levels of hard-won knowledge.

Completeness: unchanged goldens were being offered as evidence the sort change
is safe. They are not — every activity golden sorts by pid, which is never
blank, and the one sparse key is sorted in the single direction where old and
new agree. Reframed as a regression check, with coverage moved to a case where
a blank meets a sparse default sort key. Also adds Configure-level assertions
for the new boundary.

Architecture: the string half of the sort rule is now examined on its own terms
rather than carried along — for application_name an empty string is a legitimate
value, so sorting it last is a presentation choice, not a correctness fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mantics

The constant rename from round 1 is withdrawn. Its justification was a project
convention that does not exist: wal.go keeps Default as the newest branch,
progress_vacuum.go (feature 012, the freshest precedent) does the opposite, and
bgwriter.go and io.go have no Default at all. An early return above the existing
switch achieves what the rename was for — PgStatActivityDefault keeps naming the
switch's default: case — while touching one file instead of five.

Guard semantics were specified two incompatible ways; its twin in top returns an
error rather than printing an empty cell, so that is what the spec now says.

The sort-index reset is re-argued: the panic path needs a narrowing layout, which
neither real boundary produces. The reachable harm is quieter and worse — columns
inserted mid-layout mean a latched index denotes a different column afterwards,
so a report silently sorts by something the operator did not ask for.

Also: the replslots replay case, named as the only real coverage of the sort
change, belonged to no task and would have been lost at decomposition.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Final round-2 finding closed: the spec claimed the string half of the sort change
was "checked by hand in Task 6", but no task or criterion contained that check.
Claiming coverage that does not exist is the exact defect this spec caught twice
in earlier rounds; the step is now named in Task 6, the verification table and the
acceptance criteria.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three corrections to the tech-spec came out of writing the tasks:

PostgresV13 does not need adding — it already exists alongside the full
V10-V19 range. That claim entered in round 2, after the mirage check had run.

The replslots replay case named as the sole real coverage of the sort change
would not have diverged: a lone blank under descending sort lands last under
both old and new behaviour. It needs a genuine "0" beside the blank, or the
blank in the first row so the old code falls into string mode.

A fourth state survives a replayed version change: when the -o column is absent
from the new layout, getColumnIndex fails, the latch stays down, and OrderKey
keeps the index resolved against the old layout. Clearing the latch is not
enough — the view's seed OrderKey/OrderDesc must be restored.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PostgresV13 already exists alongside the full V10-V19 range; the claim that it
needs adding entered in round 2, after the mirage check had run.

The replslots replay case named as the sole coverage of the sort change would
not have diverged — a lone blank under descending sort lands last under both old
and new behaviour. It needs a genuine "0" beside the blank, or the blank first so
the old code falls into string mode.

A fourth state survives a replayed version change: when the -o column is absent
from the new layout the latch stays down and OrderKey keeps the old index, so the
view's seed OrderKey/OrderDesc must be restored rather than the latch re-armed.

Decision 8 now names which block of the dead help.go is stale — the replication
one. Left ambiguous, it let a task file attach the fact to the activity block,
where xact_age* legitimately means transaction duration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…struction

The user-spec still asked for three caveats in two places while its own Risk 2
demands the fourth (a blank cell may mean missing privileges rather than no
horizon). Synchronised with the tech-spec.

The tech-spec's own guidance on constructing the sort-coverage test was itself
insufficient: putting the blank first only diverges if the remaining values also
order differently lexicographically than numerically, which "2048" before "1024"
does not. Spelled out both failing constructions and the preconditions for the
working one, and required that any such test be shown to fail against the unfixed
comparator rather than merely to pass against the fixed one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sible criterion

Task 05 attached the dead help.go's stale naming to the activity block. It lives
in the replication block: help.go:59 says xact_age* where the live describe.go:75
says horizon_xacts. In the activity block xact_age* legitimately means the
transaction's duration. Since that fact was headed for the permanent debt
register, an implementer following the criteria literally would have written a
falsehood into it.

Task 02's replay case was built on values whose lexicographic and numeric order
coincide, so the test would have passed against the unfixed comparator while the
criteria demanded it go red — and those values are exactly the ones in the
neighbouring existing test, so a copy-paste landed straight in the trap. Every
test in that task must now be shown failing first.

Task 06 required a run with no SKIP, which cannot happen: the version loop covers
9.5 through 19 and no cluster below 14 exists. Replaced with the expected split,
plus a warning against "fixing" it by narrowing the version list.

Task 04 claimed pgcenter top reads the column descriptions; nothing in top/ does.
Task 03 now fixes the implementation order — guard first, then the resets — since
without it the red step is a panic that kills the test binary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 04 had gone stale in the opposite direction: it told the implementer to go
fix a "three caveats" wording in the user-spec that had already been corrected to
four, and built a paragraph on that missing premise. Same class of defect as the
top/ claim removed in round 1 — an instruction resting on a fact that is no
longer true.

The tech-spec still attributed the internal/stat panics to debt [019] while the
task files, correctly, no longer did. Source of truth realigned with the tasks
rather than the other way round.

Also: task 04 frontmatter comments stripped, its file-scope criterion narrowed to
the code diff, and task 05's verify quoted so a future '#' cannot silently
truncate it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both validators returned approved. Remaining minors folded in: task 02 now says
which of its anchors go red only on ascending sort and which need the two
fixture preconditions, so "demonstrate the red step" is actionable rather than
aspirational; its panic attribution now names the test-body dereference as the
first failure and DB.Close as the repanic. Task 04's user-spec line references
re-synced after the caveat-count fix shifted them. Tasks 04 and 05 no longer
assert a file count that their own decisions-log entry would violate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three columns on the activity screen behind a PG 13+ query branch: leader
(coalesce of leader_pid and pid, so a parallel group collapses under one value),
backend_xid (has this transaction written), and horizon_xacts (how far back it
holds the xmin horizon). The branch is an early return above the existing switch,
so PgStatActivityDefault keeps naming that switch's default case.

The query test now compares the full ordered column-name list against a live
server instead of asserting the query merely runs; all six PG 14-19 subtests pass
rather than skip. The table test pins the branch boundary from both sides, which
is the only guard there is — no PG 12 or 13 cluster exists to catch a 140000
written for 130000.

Sorting of sparse columns is fixed in the shared engine: the comparator is chosen
from the first non-empty cell, and empty cells order last in both directions and
all three modes. Previously a blank parsed as 0 and therefore led an ascending
sort indistinguishably from a genuine zero — which would have broken this
feature's primary story, ranking sessions by horizon. Each of the seven tests was
demonstrated failing against the unfixed comparator first.

Tech debt [021] closed, and it was three stale states rather than one: the
alignment flag, the header-repeat counter, and the resolved sort-column index —
the last one silently sorting a report by a different column once positions
shift. The zero-width guard in the report truncation path is restored to match
its twin in top, and lands before the resets so the red step reads as a test
failure instead of a panic that kills the binary.

make test 70.1%, make lint 0 issues, gosec clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…did not fix

report -d -A now documents the three columns, notes they need PG 13+, and carries
four caveats — each pinned by its own assertion so a future edit cannot quietly
drop one. The fourth is the operationally heaviest: a blank cell may mean the
viewer lacks privileges rather than that the session holds nothing, and this is
the screen where a DBA decides whether to terminate a backend.

Tech debt: [021] moves to Resolved with the resolution read off the code rather
than off the spec. [020]'s justification rested on a guarantee this feature
disproved, and its two failure shapes were reproduced during task 03's security
audit — recorded as measured rather than inferred, severity left at Low for
consistency with [009], which covered a comparable malformed-archive class.

Five new entries: the dead and stale help.go (stale in its replication block, not
its activity one), the same horizon_xacts name computed two ways on two screens,
the port map promising clusters the image lacks, the unguarded sort key, and the
error paths that now hang where they used to panic — the last one a direct
consequence of the guard this feature added, stated plainly rather than left for
someone to discover.

Project knowledge updated: the activity selector no longer branches only at 9.6
and 10.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All gates pass on live PG 14-19. Two results are worth keeping:

The leader's raw leader_pid is NULL on a live parallel scan, which confirms the
roadmap's original scope would not have met its own goal — the leader would have
sorted into the NULL block away from its own workers. The derived column groups
leader and all four workers under one value.

A session that had run an INSERT and then pg_sleep showed backend_xid = 784 while
its query column read pg_sleep. That is the kill-safety case the column was kept
for, reproduced rather than argued.

Three limits are stated rather than glossed: the PG 12/13 boundary cannot be
verified live and rests on the table test alone; replslots ascending sort is
covered by the replay test rather than by hand; and the load measurement reached
96 sessions, not the few hundred the tech-spec asked for, because the fixture
caps max_connections at 100.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e headline invariant

Two independent reviewers reproduced the same two defects against develop, so both
are regressions this branch introduced rather than pre-existing exposure.

Restoring the view's seed sort key put an index into sort() that no longer belongs
to the layout in hand: a replayed archive whose -o column is absent from the newer
layout panicked with index out of range. The key is now bounds-checked inside
sort() rather than at the call site, which closes the pre-existing routes and the
top/ caller in the same three lines — debt [025] is resolved rather than merely
made no worse.

The zero-width guard turned a panic into a hang: processData returned an error and
left readTar blocked on two unbuffered channels, so `pgcenter report` in a pipe
became an immortal process. Telling detail — the test harness already drained
those channels with a comment explaining why, while production did not. The drain
now lives in doReport and covers every error path, not only the newest, so debt
[026] is resolved.

Test review found the sharper problem: the feature's own headline invariant had no
automated cover at all. Replacing coalesce(leader_pid, pid) with a raw leader_pid,
or wrapping backend_xid in a coalesce(...,'0'), left every test green while
breaking the parallel-grouping story and the "blank, never 0" rule respectively.
Both mutants were verified killed by the value-level assertions now in the live
query test. A feature built around not claiming absent coverage had left exactly
that gap at its centre.

Also: two describe tests were anchored to prose that any copy-edit would break,
and two assignments in the reset block had neither a consumer nor a test.

make test 70.3%, make lint 0 issues, gosec clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…amily

Both were registered as deferred on the reasoning that this feature added no new
failure class. Code review reproduced new routes into each against develop, which
retired that reasoning, so both were fixed and now move to Resolved with the
evidence that closed them.

[020] is the last of the family still open. Its note now says where the fix
belongs — validate(), the one place that sees a result before any consumer — and
records the review's finding that align.SetAlign is a third unsafe consumer, so
guarding diff alone would leave the class open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lesovsky
lesovsky merged commit 9baa70d into develop Jul 26, 2026
1 check failed
@lesovsky
lesovsky deleted the feature/activity-xmin-horizon branch July 26, 2026 14:57
lesovsky pushed a commit that referenced this pull request Jul 26, 2026
…h commit

The local develop was ahead of origin/develop when the branch was cut, so PR #149
carried 42 commits instead of three and merged feature 012's PostgreSQL 19 work
under a title naming only this feature. Nothing was lost; the history is what is
wrong, and rewriting a pushed shared branch to fix attribution is the worse trade.
Release notes for 0.12.0 will need to say so explicitly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.

1 participant