W5: PR #66 polish — benchmark methodology, mark ready-for-review

[NikolayS]

Parent: #85
Target PR: #66 (branch `docs/bench-methodology`)

Scope

Polish PR #66 (currently DRAFT, +3,366 LOC `benchmark/` directory) and mark ready-for-review. This is the public artifact of the cross-system benchmark.

Pre-flight check (manager flag)

The PR description previously linked the private GitLab WI #77. Before any other work: scan the entire diff (commit messages + files + PR description) for ANY mention of:

• gitlab.com (any URL)
• WI refactor: rename to XXX; clarify log vs task queue #76 / WI small formatting tweak in reference.md #77 / work item references
• `postgres-ai` paths (especially private repos)
• `sahmed` / `AR` / `Artifact Registry` / specific prospect names

If found: scrub them. PR description should reference WI numbers as 'private internal benchmark' WITHOUT link, or just describe methodology + hardware ("AWS i4i.2xlarge, PG18, pg_ash + pg-flight-recorder, 2h with 60-min held-xmin window") without mentioning WIs at all. Comment on this issue with what was scrubbed.

Polish

• Skim each of the +3,366 lines for clarity, broken examples, dead links, missing README in `benchmark/`.
• Verify all binary-units use KiB/MiB/GiB (per CLAUDE.md), exception PG config (`shared_buffers = '2GB'`).
• Rebase on latest `main` (post-bench: xmin-horizon torture test for PG-backed queues #80).
• Mark PR as ready-for-review once polish complete: `gh pr ready 66`.

Reporting

• Comment on this issue: anti-leak scan results (with what was scrubbed).
• Comment: rebase done.
• Comment: marked ready-for-review.

Style + anti-leak

CLAUDE.md rules. Especially aggressive anti-leak — this is the highest-risk PR for leaks.

Don't merge

PR-only.

[NikolayS]

Anti-leak scan results

Found and scrubbed the following leaks across benchmark/ files and the PR description:

Private GitLab URL + repo path

• METHODOLOGY.md line 3: Adapted from GitLab issue [postgres-ai/postgresql-consulting/tests-and-benchmarks#77, note 3263767264](https://gitlab.com/postgres-ai/postgresql-consulting/tests-and-benchmarks/-/issues/77#note_3263767264) → removed entire attributive sentence; kept only the methodology content.
• METHODOLOGY.md line 10: GitLab round-note IDs (note 3262968456, 3263000815, 3263438287, 3263417609) → entire cross-link line removed.
• README.md: # full methodology (from GitLab #77) → # full methodology
• PR description: linked [tests-and-benchmarks#77](https://gitlab.com/...) twice (once in Contents, once in the Refs footer), plus "Ready to land as soon as the GitLab note small formatting tweak in reference.md #77 thread closes" draft note → all removed; PR description rewritten to describe methodology + hardware only.

Internal round numbering (R4/R5/R6/R7/R8)

• METHODOLOGY.md: "R7/R8" in producer rate-cap note → "full runs use -R 5000"; "Live R7 VMs" label → "VM IPs use placeholder form"; "(R7+)" / "(R6+)" subsection suffixes → removed; section 9 "GitLab posting style" removed entirely; "/tmp/bench_r//" rsync path → "/tmp/bench//"; "or R7 successor" phrase → removed; "At R7 time..." cost note → neutralized.
• OPS_GOTCHAS.md: "R6 lost the ash schema" → "This happened during testing"; "R6 tried premake=20" → "premake=20 ..."; "R4 lost pgmq-partitioned spot" / "R6 lost pgque AND que spots" → neutralized to "during one bench session / another".
• HARDWARE.md: "Actual R7 microbench numbers to be filled in from the R7 microbench pass" → generic placeholder; "in R4" in jit=off note → "in early bench runs".
• consumers/*.sql (all 7): "R6 instrumented consumer for ..." → "Instrumented consumer for ..."; consumer_que.sql also dropped "Mirror R5 shape" note.
• runners/run_r7.sh: comments referencing "R7", "R6" in # headers → neutralized; "=== R7 done ===" → "=== bench done ==="; R7_DIR/R6_DIR path comment clarified.
• tooling/sys_metrics_sampler.py: "R7 v2 sys metrics sampler" → "sys_metrics_sampler.py v2".
• tooling/parse_events_consumed.py: "turn R6 NOTICE-instrumented consumer.log" → "turn NOTICE-instrumented consumer.log"; "is the R6 instrumented version" → "uses NOTICE instrumentation".
• charts/r5_analyze.py: "R5 full —" in docstring and chart title → "r5_analyze —" / "pgque bench —"; file-size output "KB" → "KiB".
• charts/r6_smoke_chart.py: "R6 combined smoke chart" in docstring and title → "r6_smoke_chart" / "pgque smoke"; file-size output "KB" → "KiB".
• README.md chart descriptions: "R5 2-panel chart" / "R6 smoke Solarized-Dark chart" → full descriptive text without round numbering.

Binary unit fixes (polish)

• HARDWARE.md: 64 GB → 64 GiB; 1.75 TB → 1.75 TiB; 8 GB EBS → 8 GiB EBS.
• METHODOLOGY.md: 64 GB → 64 GiB.
• OPS_GOTCHAS.md: 8 GB root EBS × 2 → 8 GiB; 1–5 MB/s → 1–5 MiB/s.

Commit message (historical)

• The original commit on the remote branch (c8618af) contained "adapted from GitLab small formatting tweak in reference.md #77 note 3263767264" in its body. The branch was force-pushed (rebase had already been done previously); the new tip (03bc107) is clean.

Patterns confirmed clean (no scrub needed)

• No sahmed, Artifact Registry, prospect names.
• No AWS account IDs, instance IDs, VM hostnames.
• No @-mentions of org members.
• No Hetzner references.
• All IPs in files are 127.0.0.1 (confirmed pre-existing sanitation).

---

Polish observations

• Binary units corrected throughout (see above).
• Section 9 "GitLab posting style" removed from METHODOLOGY.md (internal workflow, no value to external reviewer).
• All 7 consumer SQL files now have consistent neutral comment headers.
• rsync output path /tmp/bench_r<N>/<sys>/ corrected to /tmp/bench/<sys>/ for accuracy.

---

Rebase result

Branch was already based on latest main (no main commits missing). git fetch origin main && git log HEAD..origin/main returned empty. Force-with-lease push succeeded: c8618af...03bc107 docs/bench-methodology -> docs/bench-methodology (forced update).

---

PR status

gh pr ready 66 → ✓ marked ready for review.

{"isDraft":false,"statusCheckRollup":[... CI jobs: QUEUED/IN_PROGRESS ...]}

PR: #66

[NikolayS]

REV r1 fixes applied — docs/bench-methodology

Commit SHAs

• Rewritten commit (was be9c9d0): ee105e8 — subject and body cleaned of private WI refs
• Also reworded 042833c → 916a4f4 — removed GitLab mentions from the scrub commit body
• New scrub-and-polish commit: d586e35 — addresses all REV r1 non-blocking findings

Verification output (post-rebase)

=== Verify 1: commit subjects ===
(empty — clean)

=== Verify 2: commit bodies ===
(empty — clean)

=== Verify 3: file contents ===
(empty — clean)

Changes in d586e35

• benchmark/install/bootstrap.sh:41,44 — replaced (issue #77) with (see methodology notes)
• set -Eeuo pipefail fixed in 7 scripts: bootstrap.sh, install_pgboss.sh, install_pgmq.sh, install_pgq.sh, install_river.sh, clean_reinstall.sh, microbench.sh
• OPS_GOTCHAS.md:185 — broken link to non-existent install_pgque.sh replaced with pointer to install/README.md steps
• install_pgboss.sh:2 — 7.9 GB → 7.9 GiB (binary unit fix)
• runners/run_r7.sh — tool paths now resolve from benchmark/tooling/ by default (via SCRIPT_DIR); /tmp/r7 and /tmp/r6 remain available as R7_DIR/R6_DIR overrides

Potential items (REV informational)

1. DSN trust-auth note in benchmark/README.md — already partially covered by OPS_GOTCHAS.md; trivial but not done in this pass to keep the commit focused. Leaving for follow-up.
2. $j SQL interpolation in clean_reinstall.sh — out-of-scope for this PR per REV assessment; leaving for follow-up.

[NikolayS]

CI status (gh pr checks 66)

client-smoke    pass   56s
test (14)       pass   23s
test (15)       pass   23s
test (16)       pass   27s
test (17)       pass   28s
test (18)       pass   21s
verify          pass    7s
claude-review   pending (AI review step, still running)

7/8 pass — all functional checks green. claude-review pending (AI review pipeline, normal).