Speed up integration-postgres specs#2770
Merged
Merged
Conversation
WalkthroughThe PostgreSQL setup defines data and log paths, ensures 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce integration-postgres runtime by aligning its filesystem and durability settings with the existing integration-mysql optimizations (tmpfs-backed data dir and reduced write-ahead logging overhead) in the CI task runner.
Changes:
- Mount PostgreSQL’s data root (
/tmp/postgres) on a 512MB tmpfs beforeinitdbso database I/O stays in memory. - Append
fsync = offandsynchronous_commit = offtopostgresql.confto reduce WAL flush costs during tests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aramprice
force-pushed
the
speedup-postgresql
branch
from
e7faf9b to
ff868dd
Compare
aramprice
force-pushed
the
speedup-postgresql
branch
from
ff868dd to
811393d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 94-101: Update the mount validation around POSTGRES_ROOT to verify
it is a tmpfs with the required 512 MB capacity, not merely any mounted
filesystem. If the existing mount does not match, fail closed or replace it
before continuing; preserve the current setup path for an unmounted directory.
- Line 142: Update the database setup command in the postgres task to remove the
unsupported --if-not-exists option; query pg_database for the uaa database
first, and invoke createdb only when that query confirms it is missing,
preserving idempotent behavior.
- Around line 140-142: Update the PostgreSQL startup flow around pg_ctl and
createdb to wait until the server accepts connections before running createdb.
Add a bounded pg_isready check or short retry loop using the existing DB_HOST
context, while preserving the current startup behavior and failing clearly if
readiness is not reached.
🪄 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: 85f5be65-15d4-4345-9a5f-a06e73b3dfff
📒 Files selected for processing (1)
ci/tasks/test-rake-task.sh
aramprice
force-pushed
the
speedup-postgresql
branch
from
811393d to
0d2918a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 99-103: Update the tmpfs setup around the _fstype check to also
validate the mounted POSTGRES_ROOT capacity is 512 MB. Remount it with the
existing 512M option when the filesystem type or capacity differs, and fail
closed if the capacity cannot be validated or the remount fails.
- Around line 128-129: Update the PostgreSQL setup flow in
ci/tasks/test-rake-task.sh so the fsync and synchronous_commit tuning settings
are written whenever the cluster is prepared, not only inside the
first-initialization path guarded by PG_VERSION. Keep the writes idempotent and
ensure reused clusters receive the settings before startup, or reload/restart
PostgreSQL if the configuration is changed after startup.
🪄 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: f9b889f3-37ce-45e4-9546-a39a62a88ada
📒 Files selected for processing (1)
ci/tasks/test-rake-task.sh
aramprice
force-pushed
the
speedup-postgresql
branch
from
0d2918a to
905d3fb
Compare
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 94-108: Update the remount logic in the ${POSTGRES_ROOT} tmpfs
validation block to fail immediately with a diagnostic if unmounting the
existing mount or mounting the 512M tmpfs fails. Remove the unconditional
suppression around umount, check the mount command’s exit status, and ensure
subsequent chown/processing only runs after a successful remount.
- Around line 167-171: Replace the raw sudo invocation in the pg_database check
with the existing run_as helper, passing postgres and the full psql command
arguments. Preserve the idempotent database existence check, its output piping
to grep, and the fallback run_as createdb invocation while restoring the
helper’s standard logging.
🪄 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: 5470a5aa-8a9e-429e-ba54-8dced59f61a8
📒 Files selected for processing (1)
ci/tasks/test-rake-task.sh
aramprice
force-pushed
the
speedup-postgresql
branch
from
a7d2b04 to
0ed4a89
Compare
aramprice
force-pushed
the
speedup-postgresql
branch
from
0ed4a89 to
74b817f
Compare
aramprice
force-pushed
the
speedup-postgresql
branch
from
74b817f to
a8e6dea
Compare
aramprice
force-pushed
the
speedup-postgresql
branch
from
a8e6dea to
d9a2797
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 157-159: Replace the single pg_isready invocation in the
PostgreSQL readiness check with a bounded retry loop that repeatedly runs
pg_isready until it succeeds. Preserve the existing postgres user, DB_HOST, and
DB_PORT arguments, add a short delay between attempts, and fail explicitly after
the retry limit is exhausted.
🪄 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: 773333b0-ba6e-4a8b-81ef-db4f715471be
📒 Files selected for processing (1)
ci/tasks/test-rake-task.sh
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
… disabled
PostgreSQL integration tests were taking 3-5h vs MySQL's 2-3h despite running
identical test suites. The root cause is that MySQL's data directory was
explicitly mounted on a 512MB tmpfs (in-memory RAM disk) with binary logging
disabled, while PostgreSQL used a plain on-disk path (/tmp/postgres) with
default fsync and synchronous_commit settings.
Fix both asymmetries for PostgreSQL:
- Mount a 512MB tmpfs on ${POSTGRES_ROOT} before initdb, mirroring the MySQL
setup. All database I/O (schema creation, migrations, per-test drop/create
cycles) now runs entirely in memory.
- Disable fsync and synchronous_commit, eliminating WAL flush overhead per
transaction. This mirrors MySQL's skip-log-bin setting.
Initialization guard uses ${PGDATA}/PG_VERSION (written only by a successful
initdb) rather than directory existence; a partial previous run cannot skip
re-initialization.
The tmpfs mount check runs unconditionally before the PG_VERSION guard and
requires all three conditions to accept an existing mount: (1) POSTGRES_ROOT
is itself a mountpoint — not merely on a parent tmpfs (e.g. if /tmp itself is
a 512M tmpfs, findmnt would report the parent's type/size without this check,
falsely passing), (2) the filesystem type is tmpfs, and (3) the mounted size
is 512M. The SIZE field is compared as bytes via findmnt --bytes to avoid
locale/version-dependent human-formatted output (e.g. "512M" vs "512.0M")
which could cause a false remount on every run. _fstype and _fssize are
declared local.
When remounting is needed, the server stop is structured as an explicit if
block so that pg_ctl stop failures surface rather than being masked; a masked
failure would cause the subsequent umount to fail with EBUSY. The existing
mount is removed without || true (fail closed on umount) and the new 512M
tmpfs is mounted.
The password file for initdb is explicitly chowned to postgres before initdb
runs so that restrictive umask values (e.g. 077) cannot prevent the postgres
user from reading the file.
run_as now correctly quotes "${@}" instead of ${@}, which previously caused
arguments containing spaces (such as -c "ALTER SYSTEM ...") to be word-split
by the shell. The # shellcheck disable=SC2068 annotation is also removed.
pg_ctl start, the readiness check, and createdb all run on every invocation:
- PGLOGS is created unconditionally before startup in case the cluster was
preserved but the container was partially reset.
- A bounded retry loop (up to 30s) waits for the server to accept connections
before any SQL is run. Raw sudo is used in the loop body to avoid printing
"Running ... as postgres" on every retry iteration.
- Performance settings are applied via separate ALTER SYSTEM calls (one per
statement) after startup. A compound -c "stmt1; stmt2" is not used because
psql's simple query protocol wraps multi-statement strings in a single
implicit transaction, and ALTER SYSTEM cannot run inside a transaction block.
ALTER SYSTEM writes idempotently to postgresql.auto.conf (not postgresql.conf)
and ensures fsync=off and synchronous_commit=off are active for both fresh
and reused clusters. Both have sighup context so pg_reload_conf() applies
them without a restart.
- createdb lacks --if-not-exists; pg_database is queried with command
substitution (not a pipe) so that psql connection failures abort the script
rather than being silently treated as "database does not exist". Raw sudo is
used to prevent run_as's stdout echo from polluting the captured query result.
- All pg_isready, psql, and createdb calls use ${DB_HOST} and ${DB_PORT}
consistently.
Expected result: integration-postgres build time drops from ~4h to ~2h30m,
reaching parity with integration-mysql.
aramprice
force-pushed
the
speedup-postgresql
branch
from
d9a2797 to
ace1ffd
Compare
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
ragaskar
approved these changes
Jul 14, 2026
ragaskar
left a comment
ragaskar
left a comment
Contributor
There was a problem hiding this comment.
LGTM -- I assume any unexpected issues that might fall out from the config changes could be addressed w/ future commits.
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Speed up integration-postgres by running PostgreSQL on tmpfs with WAL disabled
PostgreSQL integration tests were taking 3-5h vs MySQL's 2-3h despite running identical test suites. The root cause is that MySQL's data directory was explicitly mounted on a 512MB tmpfs (in-memory RAM disk) with binary logging disabled, while PostgreSQL used a plain on-disk path (/tmp/postgres) with default fsync and synchronous_commit settings.
Fix both asymmetries for PostgreSQL:
fsync = offandsynchronous_commit = offto postgresql.conf, eliminating WAL flush overhead per transaction. This mirrors MySQL'sskip-log-binsetting.Expected result: integration-postgres build time drops from ~4h to ~2h30m, reaching parity with integration-mysql.
Integration run:
duration | 1h 36mCompare with:
duration | 3h 41m