build(deps): Bump postgres 18 - #4538
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dc9b008. Configure here.
dc9b008 to
319ad8b
Compare
76fc105 to
924b369
Compare
|
This has been tested locally with different bases (pg14-bookworm, pg14-trixie) and it works fine. |
aldy505
left a comment
aldy505
left a comment
There was a problem hiding this comment.
This is already very good to me, but there is one thing to address.
| if [[ "$postgres_version" == "14" || -z "$postgres_version" ]]; then | ||
| needs_reindex=$($CONTAINER_ENGINE run --rm -v sentry-postgres:/db busybox sh -c 'if [ -f /db/PG_VERSION ] && [ ! -f /db/14-trixie-reindexed ]; then echo yes; fi') | ||
| if $CONTAINER_ENGINE volume inspect sentry-postgres-new >/dev/null 2>&1; then | ||
| echo "Found sentry-postgres-new from an interrupted PostgreSQL upgrade. Recover the database before removing this volume and rerunning install.sh." |
There was a problem hiding this comment.
Should we put the big warning sign here?
There was a problem hiding this comment.
Can you please explain what do you mean by "big warning sign"? This should not happen for normal users if they do not interrupt their ./install.sh or that does not break.
This is unrelated to reindex issues which users reported on different issues.
There was a problem hiding this comment.
Just a nitpick, to avoid any user complains that they didn't read the error message.
| echo "[1/2] PostgreSQL 14 Bookworm -> 14 Trixie: reindexing for the glibc change, this may take a while..." | ||
| $CONTAINER_ENGINE run --rm --user postgres --network none --shm-size=256m \ | ||
| -v sentry-postgres:/var/lib/postgresql/data \ | ||
| postgres:14.24-trixie bash -ec ' | ||
| trap "pg_ctl -m fast -w stop" EXIT | ||
| pg_ctl -w start | ||
| psql -U postgres -v ON_ERROR_STOP=1 -c "REINDEX DATABASE postgres;" | ||
| touch "$PGDATA/14-trixie-reindexed" | ||
| ' |
| echo "[2/2] PostgreSQL 14 Trixie -> 18 Trixie: upgrading..." | ||
| $CONTAINER_ENGINE run --rm \ | ||
| -e POSTGRES_INITDB_ARGS=--no-data-checksums \ | ||
| -v sentry-postgres:/var/lib/postgresql/14/data \ | ||
| -v sentry-postgres-new:/var/lib/postgresql/18/docker \ | ||
| tianon/postgres-upgrade:14-to-18 | ||
|
|
||
| echo "[2/2] pg_upgrade completed. Replacing the PostgreSQL 14 volume with PostgreSQL 18 data..." | ||
| $CONTAINER_ENGINE volume rm sentry-postgres | ||
| $CONTAINER_ENGINE volume create --name sentry-postgres | ||
| $CONTAINER_ENGINE run --rm -v sentry-postgres-new:/from -v sentry-postgres:/to alpine ash -ec \ | ||
| "mkdir -p /to/18/docker; cp -av /from/. /to/18/docker; echo 'host all all all trust' >> /to/18/docker/pg_hba.conf" | ||
| $CONTAINER_ENGINE volume rm sentry-postgres-new |
There was a problem hiding this comment.
I think we'd need a third step, from @kostirez1 on Discord:
FYI:
POSTGRES_INITDB_ARGS=--no-data-checksumsStarting with PG18 (26.10.0+), new Sentry installs will have data checksums enabled, because that's now the PostgreSQL default. Existing clusters (PG14) will keep running without them.
It's a minor point, but the next major upgrade (PG18 -> 19) has to account for this setting. pg_upgrade fails if the old and new clusters have different checksum settings.
To fix this, enable checksums with
pg_checksums --enableright after the upgrade, before PG18 takes traffic. It needs the cluster offline and rewrites every data file, so it fits best inside the upgrade's downtime window.
There was a problem hiding this comment.
Continuation and summary of my messages on Discord:
Enabling pg_checksums is something unrelated to this PR, we can decide whether to activate that or not, as that would need a complete read of all databases / tables / indexes, it could happen in another release or this release but another PR, either way I intend to keep this PR just for upgrading from 14 to 18.
Let's decide whether to enable pg_checksums or keep it disabled later.
3 participants
Follow up to #4504.
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.