fix: Add PostgreSQL collation refresh script for glibc updates#1190
Conversation
Fixes Epic-Breakfast-Productions#1138 When PostgreSQL minor patches are applied through container updates, the glibc library may also be updated. This can cause collation version mismatches that break queries. This commit adds an initialization script that automatically refreshes collation versions for all non-template databases when the container restarts. The script: - Only runs on existing databases (not new installations) - Checks which databases need collation refresh - Runs ALTER DATABASE REFRESH COLLATION VERSION on affected databases - Includes proper error handling and logging The script is mounted in the Docker Compose PostgreSQL service at /docker-entrypoint-initdb.d/ Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAvast: Docker Compose now mounts Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Host as Docker Host
participant Compose as Compose Postgres Service
participant Entrypoint as /usr/local/bin/custom-entrypoint.sh
participant InitDir as /docker-entrypoint-initdb.d
participant DockerEP as /usr/local/bin/docker-entrypoint.sh
participant Postgres as Postgres Server
Host->>Compose: start container
Compose->>Entrypoint: run custom entrypoint
Entrypoint->>InitDir: list executable *.sh files
InitDir-->>Entrypoint: script(s) (e.g., 01-refresh-collations.sh)
Entrypoint->>Entrypoint: execute script(s)
Entrypoint->>Postgres: (scripts connect as postgres user) run ALTER DATABASE ... REFRESH COLLATION VERSION
Entrypoint->>DockerEP: exec official docker-entrypoint.sh "$@"
DockerEP->>Postgres: start postgres process
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6dd89f09-0fdf-45fb-80f9-8c4e81abd789
📒 Files selected for processing (2)
deployment/Compose/compose/compose.yamldeployment/Compose/compose/setup/init-db/01-refresh-collations.sh
📜 Review details
🔇 Additional comments (1)
deployment/Compose/compose/setup/init-db/01-refresh-collations.sh (1)
1-7: Ahoy, mate! The hull looks seaworthy on first inspection.Aye, checkin' for
PG_VERSIONbefore we set sail be a sound approach—she'll only run on an existing vessel, not when we're first launchin' from the shipyard. The shebang andset -ebe proper rigging for this here script.
- Fix docker-entrypoint-initdb.d limitation: Replace mounting init scripts in entrypoint-initdb.d (which only runs on fresh init) with a custom entrypoint wrapper that runs on every container start - Fix SQL syntax: pg_collation_actual_version() requires an OID argument, not a COLLATE clause. Use pg_collation_actual_version(oid) and add IS NOT NULL check for collversion - Improve error logging: Replace suppressed stderr with visible warning messages when collation check fails - Add proper shell quoting: Quote database name variable in psql commands and ALTER DATABASE statement to prevent injection attacks and handle special characters - Remove unnecessary su postgres calls: Script runs as postgres user through the entrypoint, so su is redundant Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a46397ca-15ce-4bdd-9a62-918125bca76a
📒 Files selected for processing (3)
deployment/Compose/compose/compose.yamldeployment/Compose/compose/setup/docker-entrypoint.shdeployment/Compose/compose/setup/init-db/01-refresh-collations.sh
✅ Files skipped from review due to trivial changes (1)
- deployment/Compose/compose/setup/init-db/01-refresh-collations.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- deployment/Compose/compose/compose.yaml
📜 Review details
🔇 Additional comments (1)
deployment/Compose/compose/setup/docker-entrypoint.sh (1)
19-19: ⛵ Good helm on Line 19:exec ... "$@"correctly preserves the original command/args.
| if [ -f /var/lib/postgresql/data/PG_VERSION ]; then | ||
| echo "Running custom initialization tasks..." | ||
| for script in /docker-entrypoint-initdb.d/*; do | ||
| if [ -f "$script" ] && [ -x "$script" ] && [[ "$script" == *.sh ]]; then | ||
| echo "Running $script" | ||
| "$script" | ||
| fi | ||
| done | ||
| fi |
There was a problem hiding this comment.
🚨 Line 8-16 runs DB maintenance before Postgres is up, so psql-based init scripts can fail hard.
The wrapper executes scripts before Line 19 delegates to the official entrypoint. But deployment/Compose/compose/setup/init-db/01-refresh-collations.sh runs psql; without a running server, that path can fail under set -e and break startup.
Safer direction
-# Run our custom initialization scripts
-if [ -f /var/lib/postgresql/data/PG_VERSION ]; then
- echo "Running custom initialization tasks..."
- for script in /docker-entrypoint-initdb.d/*; do
- if [ -f "$script" ] && [ -x "$script" ] && [[ "$script" == *.sh ]]; then
- echo "Running $script"
- "$script"
- fi
- done
-fi
-
-# Invoke the original postgres entrypoint
-exec /usr/local/bin/docker-entrypoint.sh "$@"
+# NOTE: DB-dependent scripts must run only after postgres is accepting connections.
+# Consider moving collation refresh to:
+# 1) a post-start one-shot job (after healthcheck), or
+# 2) logic that explicitly starts/stops a temporary server before running scripts.
+exec /usr/local/bin/docker-entrypoint.sh "$@"| if [ -f /var/lib/postgresql/data/PG_VERSION ]; then | ||
| echo "Running custom initialization tasks..." | ||
| for script in /docker-entrypoint-initdb.d/*; do | ||
| if [ -f "$script" ] && [ -x "$script" ] && [[ "$script" == *.sh ]]; then |
There was a problem hiding this comment.
⚓ Line 11 blocks the refresh script from ever running in this Compose setup.
Ye guard uses -x, but deployment/Compose/compose/compose.yaml Line 43 bind-mounts host files as-is, and deployment/Compose/compose/setup/init-db/01-refresh-collations.sh is non-executable in the provided context. Result: the collation fix is silently skipped.
Proposed patch
- if [ -f "$script" ] && [ -x "$script" ] && [[ "$script" == *.sh ]]; then
+ if [ -f "$script" ] && [[ "$script" == *.sh ]]; then
echo "Running $script"
- "$script"
+ bash "$script"
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -f "$script" ] && [ -x "$script" ] && [[ "$script" == *.sh ]]; then | |
| if [ -f "$script" ] && [[ "$script" == *.sh ]]; then | |
| echo "Running $script" | |
| bash "$script" | |
| fi |
|
@SanchitPanchwatikar I apologize for the delay in looking at this, had some workflows already in the works. I hope to get to this this weekend. I will say that the original ticket was meant for the single node deployment as opposed to the compose, but this will still be quite useful and drop-in capable in that setting too, so it is very much appreciated. |
|
@SanchitPanchwatikar apologies again, time makes fools of us all. This is at the top of my list this week. |
|
@all-contributors please add @SanchitPanchwatikar for code |
|
I've put up a pull request to add @SanchitPanchwatikar! 🎉 |
|
@SanchitPanchwatikar Happen to have tested this? Trying to find a specific conbination of postgres versions but they all seem to have been updated with compatible glibc versions |
3 participants
Fixes #1138
When PostgreSQL minor patches are applied through container updates, the glibc library may also be updated. This can cause collation version mismatches that break queries. This commit adds an initialization script that automatically refreshes collation versions for all non-template databases when the container restarts.
The script:
The script is mounted in the Docker Compose PostgreSQL service at /docker-entrypoint-initdb.d/
Checklist:
Summary by CodeRabbit