Skip to content

Commit 9f4d559

Browse files
srixnsarupr
authored andcommitted
fix: cleanup stale postgres postmaster.pid (#35171)
## Description issue of Postgres not coming up with error` Previous Postgres was not shutdown cleanly. Please start and stop Postgres 14 properly with 'supervisorctl' only. ` https://www.notion.so/appsmith/Closed-Beta-Customer-issues-45a274a9eb8e4762a72cbff74cd3bad5?pvs=4#ecca04d205414f25a289884cebdc0f9b Fixes https://app.zenhub.com/workspaces/workflows-pod-652fff131a95920b9bf2bc7e/issues/zh/226_ ## Automation /ok-to-test tags="@tag.Sanity" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/10107093349> > Commit: 357bf5d > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10107093349&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Fri, 26 Jul 2024 07:15:19 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Enhanced PostgreSQL upgrade process with improved error handling and robust management of old server instances. - **Bug Fixes** - Reinstituted logic for checking and managing the `postmaster.pid` file, ensuring proper startup and shutdown of old PostgreSQL servers. - **Refactor** - Improved formatting and readability of shell scripts without altering their functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 4916b81 commit 9f4d559

File tree

2 files changed

+37
-17
lines changed

2 files changed

+37
-17
lines changed

deploy/docker/fs/opt/appsmith/entrypoint.sh

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -437,10 +437,19 @@ init_postgres() {
437437

438438
}
439439

440-
safe_init_postgres(){
441-
runEmbeddedPostgres=1
442-
# fail safe to prevent entrypoint from exiting, and prevent postgres from starting
443-
init_postgres || runEmbeddedPostgres=0
440+
safe_init_postgres() {
441+
runEmbeddedPostgres=1
442+
# fail safe to prevent entrypoint from exiting, and prevent postgres from starting
443+
# when runEmbeddedPostgres=0 , postgres conf file for supervisord will not be copied
444+
# so postgres will not be started by supervisor. Explicit message helps us to know upgrade script failed.
445+
446+
if init_postgres; then
447+
tlog "init_postgres succeeded."
448+
else
449+
local exit_status=$?
450+
tlog "init_postgres failed with exit status $exit_status."
451+
runEmbeddedPostgres=0
452+
fi
444453
}
445454

446455
setup_caddy() {

deploy/docker/fs/opt/appsmith/pg-upgrade.sh

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,6 @@ if [[ -z "$old_version" ]]; then
3737
exit
3838
fi
3939

40-
if [[ -f "$pg_data_dir/postmaster.pid" ]]; then
41-
tlog "Previous Postgres was not shutdown cleanly. Please start and stop Postgres $old_version properly with 'supervisorctl' only." >&2
42-
exit 1
43-
fi
44-
4540
top_available_version="$(postgres --version | grep -o '[[:digit:]]\+' | head -1)"
4641

4742
declare -a to_uninstall
@@ -55,14 +50,30 @@ if [[ "$old_version" == 13 && "$top_available_version" > "$old_version" ]]; then
5550
to_uninstall+=("postgresql-$old_version")
5651
fi
5752

58-
new_version="$((old_version + 1))"
59-
new_data_dir="$pg_data_dir-$new_version"
53+
if [[ -f "$pg_data_dir/postmaster.pid" ]]; then
54+
# Start old PostgreSQL using pg_ctl
55+
tlog "Stale postmaster.pid found. Starting old PostgreSQL $old_version using pg_ctl to cleanup."
56+
su postgres -c "$postgres_path/$old_version/bin/pg_ctl start -D '$pg_data_dir' "
57+
58+
# Wait for old PostgreSQL to be ready
59+
until su postgres -c "$postgres_path/$old_version/bin/pg_isready"; do
60+
tlog "Waiting for PostgreSQL $old_version to start..."
61+
sleep 1
62+
done
63+
64+
# Shut down PostgreSQL gracefully using pg_ctl
65+
su postgres -c "$postgres_path/$old_version/bin/pg_ctl stop -D '$pg_data_dir' -m smart"
66+
tlog "PostgreSQL $old_version has been shut down."
67+
fi
68+
69+
new_version="$((old_version + 1))"
70+
new_data_dir="$pg_data_dir-$new_version"
6071

61-
# `pg_upgrade` writes log to current folder. So change to a temp folder first.
62-
rm -rf "$TMP/pg_upgrade" "$new_data_dir"
63-
mkdir -p "$TMP/pg_upgrade" "$new_data_dir"
64-
chown -R postgres "$TMP/pg_upgrade" "$new_data_dir"
65-
cd "$TMP/pg_upgrade"
72+
# `pg_upgrade` writes log to current folder. So change to a temp folder first.
73+
rm -rf "$TMP/pg_upgrade" "$new_data_dir"
74+
mkdir -p "$TMP/pg_upgrade" "$new_data_dir"
75+
chown -R postgres "$TMP/pg_upgrade" "$new_data_dir"
76+
cd "$TMP/pg_upgrade"
6677

6778
# Required by the temporary Postgres server started by `pg_upgrade`.
6879
chown postgres /etc/ssl/private/ssl-cert-snakeoil.key
@@ -88,7 +99,7 @@ if [[ "$old_version" == 13 && "$top_available_version" > "$old_version" ]]; then
8899
fi
89100

90101
if [[ -n "${#to_uninstall[@]}" ]]; then
91-
apt-get purge "${to_uninstall[@]}"
102+
DEBIAN_FRONTEND=noninteractive apt-get purge --yes "${to_uninstall[@]}"
92103
apt-get clean
93104
fi
94105

0 commit comments

Comments
 (0)