Fix PostgreSQL out-of-space in integration tests#2792
Conversation
WalkthroughThe PostgreSQL startup script now exports a dedicated WAL directory, recreates and owns it during fresh cluster initialization, passes it to Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/tasks/test-rake-task.sh`:
- Around line 185-186: Remove the ALTER SYSTEM wal_level change from the
PostgreSQL setup sequence around the psql invocation, along with its related
reload attempt such as pg_reload_conf(). Preserve the existing max_wal_size and
full_page_writes configuration, since those settings remain the intended
disk-space safeguards.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b298a063-963a-4d41-b172-a32cb78ee093
📒 Files selected for processing (1)
ci/tasks/test-rake-task.sh
6abe459 to
2b3b943
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/tasks/test-rake-task.sh`:
- Around line 185-190: Update the PostgreSQL setup around the ALTER SYSTEM
commands so PGDATA and WAL no longer rely on the same 512MB tmpfs; move
PostgreSQL/WAL to separate storage, or reduce WAL generation before the workload
begins. Do not rely on max_wal_size as a hard storage cap, and preserve the
existing database initialization behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 466fcd4b-fc20-4902-82d8-3a505f7a0ff1
📒 Files selected for processing (1)
ci/tasks/test-rake-task.sh
ragaskar
left a comment
There was a problem hiding this comment.
i'm assuming the prior "minimal" directive was just dropped (since it wouldn't really work w/o restart) and this just depends on max_wal_size to effect the changes?
lgtm -- sounds like this setting doesn't need a restart, so let's see what happens
PR #2770 moved the PostgreSQL data directory to a 512MB tmpfs to speed up integration tests, and PR #2773 optimized teardown with TRUNCATE. Under high concurrency (the bosh-agent pipeline), WAL written to pg_wal inside PGDATA exhausts the 512MB tmpfs, crashing the server with "No space left on device". Rather than trying to cap WAL growth in place with max_wal_size (a soft limit Postgres overshoots during checkpoints, so it can't guarantee the tmpfs won't fill), this relocates the WAL directory off the tmpfs entirely via `initdb -X /tmp/postgres_wal`. Because fsync and synchronous_commit are already off, WAL writes stay in the page cache and flush lazily, so moving them to the container's disk-backed overlay costs almost nothing while freeing the full 512MB tmpfs for table data. full_page_writes is kept off to further reduce WAL volume. ai-assisted=yes [TNZ-88995]
2b3b943 to
34a9f67
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/tasks/test-rake-task.sh`:
- Around line 189-190: Update the PostgreSQL configuration command in the test
setup to also apply the WAL size limits, setting max_wal_size to 128MB and
min_wal_size to 32MB alongside full_page_writes = off. Keep the existing run_as
postgres and psql invocation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 57d3b8eb-dfea-4455-87ab-27471efd5a71
📒 Files selected for processing (1)
ci/tasks/test-rake-task.sh
What is this change about?
Fixes a
No space left on deviceerror during high-concurrency PostgreSQL integration tests (notably thebosh-agentpipeline).The problem: #2770 moved the PostgreSQL data dir to a 512MB
tmpfs(withfsync=off,synchronous_commit=off) for performance parity with MySQL, and #2773 switched teardown toTRUNCATE. But WAL is still written topg_walinsidePGDATA, so under high concurrency it fills the 512MBtmpfsand Postgres hard-crashes. This passed inboshonly becausePARALLEL_TEST_MULTIPLY_PROCESSESis throttled to0.6; it failed consistently inbosh-agentwhere concurrency is higher.The fix: relocate the WAL directory off the tmpfs entirely via
initdb -X /tmp/postgres_wal, so WAL no longer competes with table data for the 512MB budget.An earlier revision of this PR instead tried to cap WAL in place with
max_wal_size = 128MB/min_wal_size = 32MB. That was replaced becausemax_wal_sizeis a soft limit — Postgres overshoots it during checkpoints under exactly the write pressure that triggers the failure, so it can't guarantee the tmpfs won't fill. Relocation removes WAL from the tmpfs budget deterministically. Becausefsyncandsynchronous_commitare already off, WAL writes stay in the OS page cache and flush lazily, so moving them to the container's disk-backed overlay (/tmp, which is per-container ephemeral storage in Concourse — not the 512MB tmpfs and not shared across runs) costs effectively nothing.full_page_writes = offis retained to further reduce WAL volume.Please provide contextual information.
What tests have you run against this PR?
bash -n ci/tasks/test-rake-task.shandshellcheck ci/tasks/test-rake-task.shboth pass clean.test-rake-taskPostgreSQL jobs — the definitive check is thebosh-agentpostgres run that was failing with ENOSPC.How should this change be described in bosh release notes?
N/A — CI/test-infrastructure only; no operator-facing impact.
Does this PR introduce a breaking change?
No.
Tag your pair, your PM, and/or team!
(reviewer suggested by CodeRabbit:
@aramprice)AI Review Feedback
wal_level = minimal(requires restart, conflicts with defaultmax_wal_senders) — removed in an earlier commit; thread resolved.max_wal_sizewon't hard-cap the tmpfs — addressed by relocating the WAL off the tmpfs entirely (exactly what the comment recommended); thread resolved.