Skip to content

fix: bound each wal-push so a stalled upload cannot stop WAL archiving - #227

Merged
vishr merged 2 commits into
mainfrom
fix/bound-wal-push
Oct 25, 2026
Merged

vishr merged 2 commits into
mainfrom
fix/bound-wal-push

Conversation

@vishr

@vishr vishr commented Oct 25, 2026

Copy link
Copy Markdown
Member

What this changes

Two commits.

1. Bound each wal-push (part of #226). The archive command is now timeout -k 10 120 /opt/onebox/backup/onebox-wal-g wal-push %p. PostgreSQL waits on archive_command with no limit of its own. On goal, two uploads hung for 6 to 7 minutes each, pg_wal grew, and failed_count stayed 0. A stalled push is now killed after 120 s (SIGKILL 10 s after SIGTERM), exits nonzero, and PostgreSQL retries the segment.

2. Upgrade dependencies. Go modules (compose-go v2.16.2, golang.org/x/*, indirect modules) and site packages (Astro 7.4.2, Starlight 0.42.7, @astrojs/check 0.9.12, sharp 0.35.5). Astro 7.4 no longer installs @astrojs/markdown-remark, so it is now a direct dependency. Starlight 0.42 removed the tagline option, which produced no output here. TypeScript stays on 6 because @astrojs/check does not accept 7.

Refs #226. Archive-lag reporting in ob backup status and ob doctor, and the v1alpha1 snapshot problem, are still open.

Why this is correct

  • A retry after a timeout is safe. In wal-g v3.0.8, checkWALOverwrite (internal/databases/postgres/wal_push_handler.go) decrypts and decompresses the stored copy, compares it with the local segment using bytes.Equal, and skips it when equal. An interrupted upload leaves no object, because S3 PUT is atomic and an unfinished multipart upload is not visible, so the retry uploads it again. This comes from reading the source; no timeout has been forced against real object storage yet.
  • timeout -k 10 1 sleep 5 exits 124 in debian:trixie-slim, the base of the Postgres image.
  • TestWalgArchiveCommandBoundsEachPush pins the command. The status check still finds WalgBinary in it, so existing hosts are not told to re-run ob backup enable.
  • For the upgrade, only one frozen contract verdict moved: compose-go words the immich published_ports rejection differently, and the case still fails to load.

Effect on the safety envelope

The archive_command passed to PostgreSQL changes. The next deploy of a backup-enabled service will probably recreate the Postgres container, which restarts the database once. No capability moves between "the schema accepts it" and "the binary does it".

Checklist

  • just check passes locally.
  • Tests cover the new behaviour, including the failure paths.
  • Generated documentation is current (just check verifies this).
  • I have accepted the CLA, or will when the bot asks on my first pull request.

vishr added 2 commits October 24, 2026 17:18
PostgreSQL waits on archive_command with no limit of its own. On goal, two
wal-g uploads to object storage hung for 6 to 7 minutes each: archiving
stopped, pg_wal grew, and failed_count stayed 0, so nothing reported it.

The archive command now runs under `timeout -k 10 120`. A stalled push is
killed, exits nonzero, and PostgreSQL retries the segment. A retry of a
segment that did land is safe: with WALG_PREVENT_WAL_OVERWRITE, wal-g
v3.0.8 decrypts and decompresses the stored copy, compares it with the
local segment, and skips it when equal.

Refs #226
Go: compose-go v2.16.2, golang.org/x/{crypto,term,sync,sys,text} and the
remaining indirect modules to their latest releases.

compose-go words one rejection differently: the immich corpus case still
fails to load, now as "services.immich-server.published_ports false
schema" instead of "additional properties 'published_ports' not allowed".
The frozen verdict is re-recorded for that one wording change.

Site: Astro 7.4.2, Starlight 0.42.7, @astrojs/check 0.9.12, sharp 0.35.5.
Astro 7.4 no longer installs @astrojs/markdown-remark, which the rehype
table-wrap plugin needs, so it is now a direct dependency. Starlight 0.42
removed the `tagline` option; it produced no output here because the
landing page supplies its own hero tagline. npm audit fix cleared the four
transitive advisories.

TypeScript stays on 6: @astrojs/check 0.9.12 does not accept TypeScript 7.
@vishr

vishr commented Oct 25, 2026

Copy link
Copy Markdown
Member Author

Verification

Stall test (real). postgres:18 + wal-g v3.0.8 (same release asset and checksum as the image) + MinIO, with the exact production archive_command and wal-g settings (encryption, WALG_PREVENT_WAL_OVERWRITE=true).

  • MinIO paused mid-upload: the push was killed after 123 s, PostgreSQL logged archive command failed with exit code 124, and failed_count went from 0 to 1. Without the bound this push hangs until the connection resumes.
  • MinIO resumed: the backlog drained straight away.
  • Re-pushing an already-archived segment: already archived with equal content, skipping, exit 0.
  • Same segment name with one byte changed: already archived, contents differ, unable to overwrite, exit 1. So the overwrite guard compares decrypted content, and a retry after a kill is safe.

Backlog test. One wal-push also uploads up to 31 more .ready segments in the background and waits for them before it exits (WALG_TOTAL_BG_UPLOADED_LIMIT defaults to 32), so 120 s covers a batch, not one 16 MB segment. About 22 incompressible segments (around 350 MB) were drained with the upload direction throttled to 16 Mbit/s:

wal-g batch limit drain time timeouts
default (32) 183 s 0
8 184 s 1

The longest default run took about 112 s. A smaller batch did not help, so this PR does not change wal-g's batch setting. Each segment wal-g finishes is recorded, so even a killed run keeps its progress: the run with a timeout still archived every segment. On a link much slower than 16 Mbit/s, a full batch can hit the limit during a healthy backlog. That costs a retry and PostgreSQL's 60 s pause after three failures in a row. It does not lose data. Archive-lag reporting (the rest of #226) is what would show it.

Review. An independent review found nothing blocking. Its notes: a SIGKILL after the 10 s grace exits 137, which restarts PostgreSQL's archiver instead of just retrying, and the segment is still retried. The batch behavior is covered above. The Postgres container restarts on the next deploy, as stated in the description.

Site. Built and checked in a browser: the landing page and the error-code reference page with its tables render correctly. One cell in that table cuts off "retention" at the right edge. onebox.run shows the same on main today, so the upgrade did not cause it.

@vishr
vishr merged commit ca86b09 into main Oct 25, 2026
6 checks passed
@vishr
vishr deleted the fix/bound-wal-push branch October 25, 2026 02:36
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