Skip to content

Commit 2d23e3e

Browse files
committed
fix(postgresql): address review correctness gaps
1 parent cd939dd commit 2d23e3e

22 files changed

Lines changed: 539 additions & 61 deletions

addons/postgresql/config/pg12-config-constraint.cue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@
271271
extra_float_digits?: int & >=-15 & <=3
272272

273273
// Forces use of parallel query facilities.
274-
force_parallel_mode?: #PgBool
274+
force_parallel_mode?: string & =~"(?i)^(off|on|regress)$"
275275

276276
// Sets the FROM-list size beyond which subqueries are not collapsed.
277277
from_collapse_limit?: int & >=1 & <=2147483647

addons/postgresql/config/pg12-config.tpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ enable_sort = 'True'
106106
enable_tidscan = 'True'
107107
escape_string_warning = 'True'
108108
extra_float_digits = '1'
109-
force_parallel_mode = '0'
109+
force_parallel_mode = 'off'
110110
from_collapse_limit = '8'
111111
# fsync=off
112112
# full_page_writes=off

addons/postgresql/config/pg14-config-constraint.cue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@
295295
extra_float_digits?: int & >=-15 & <=3
296296

297297
// Forces use of parallel query facilities.
298-
force_parallel_mode?: #PgBool
298+
force_parallel_mode?: string & =~"(?i)^(off|on|regress)$"
299299

300300
// Sets the FROM-list size beyond which subqueries are not collapsed.
301301
from_collapse_limit?: int & >=1 & <=2147483647

addons/postgresql/config/pg14-config.tpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ enable_sort = 'True'
115115
enable_tidscan = 'True'
116116
escape_string_warning = 'True'
117117
extra_float_digits = '1'
118-
force_parallel_mode = '0'
118+
force_parallel_mode = 'off'
119119
from_collapse_limit = '8'
120120
# fsync=off # patroni for Extreme Performance
121121
# full_page_writes=off # patroni for Extreme Performance

addons/postgresql/config/pg15-config-constraint.cue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@
295295
extra_float_digits?: int & >=-15 & <=3
296296

297297
// Forces use of parallel query facilities.
298-
force_parallel_mode?: #PgBool
298+
force_parallel_mode?: string & =~"(?i)^(off|on|regress)$"
299299

300300
// Sets the FROM-list size beyond which subqueries are not collapsed.
301301
from_collapse_limit?: int & >=1 & <=2147483647

addons/postgresql/config/pg15-config.tpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ enable_sort = 'True'
115115
enable_tidscan = 'True'
116116
escape_string_warning = 'True'
117117
extra_float_digits = '1'
118-
force_parallel_mode = '0'
118+
force_parallel_mode = 'off'
119119
from_collapse_limit = '8'
120120
# fsync=off # patroni for Extreme Performance
121121
# full_page_writes=off # patroni for Extreme Performance

addons/postgresql/dataprotection/pgdumpall-restore.sh

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,15 @@ if ! datasafed pull -d zstd-fastest "${DP_BACKUP_NAME}.sql.zst" - \
3333
exit 1
3434
fi
3535
cat "${errlog}" >&2
36-
if grep "^ERROR:" "${errlog}" | grep -vE "already exists" | grep -q .; then
36+
if ! awk '
37+
/^ERROR:/ {
38+
if ($0 !~ /^ERROR:[[:space:]]+role "(postgres|kbadmin|kbdataprotection|kbprobe|kbmonitoring|kbreplicator)" already exists[[:space:]]*$/ &&
39+
$0 !~ /^ERROR:[[:space:]]+database "postgres" already exists[[:space:]]*$/) {
40+
invalid = 1
41+
}
42+
}
43+
END { exit invalid }
44+
' "${errlog}"; then
3745
echo "ERROR: pgdumpall restore hit non-conflict SQL errors (see above); data may be partially restored" >&2
3846
exit 1
3947
fi

addons/postgresql/dataprotection/postgresql-fetch-wal-log.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ function fetch-wal-log(){
1515
exit_fetch_wal=0 && mkdir -p $wal_destination_dir
1616
for dir_name in $(datasafed list /) ; do
1717
if [[ $exit_fetch_wal -eq 1 ]]; then
18-
exit 0
18+
break
1919
fi
2020

2121
# check if the latest_wal_log after the start_wal_log
@@ -54,4 +54,4 @@ function fetch-wal-log(){
5454
fi
5555
done
5656
done
57-
}
57+
}

addons/postgresql/dataprotection/wal-g-archive-restore.sh

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#!/bin/bash
22
set -e
3+
set -o pipefail
34
backup_base_path="$(dirname $DP_BACKUP_BASE_PATH)/wal-g/"
45

56
function config_wal_g_for_fetch_wal_log() {
@@ -86,19 +87,42 @@ if [[ "${IS_REPLICA}" == "true" ]]; then
8687
exit 1
8788
fi
8889
89-
# Primary restore: Restore from successful backup (.old directory exists)
90-
if [[ -d "${DATA_DIR}.old" ]]; then
91-
echo "Restoring data from ${DATA_DIR}.old..."
92-
mkdir -p "${DATA_DIR}"
93-
mv -f ${DATA_DIR}.old/* ${DATA_DIR}/
94-
# Durability barrier BEFORE removing the signal: the signal is the retry
95-
# marker for this hook, so it must be the FINAL commit action. If sync
96-
# (or the mv) fails, the signal survives and the bootstrap re-runs the
97-
# hook instead of losing the restore marker with a half-durable DATA_DIR.
90+
HANDOFF_MARKER=".kb_walg_handoff"
91+
92+
function finalize_handoff() {
93+
local source_dir="${DATA_DIR}.old"
94+
local marker_path="${DATA_DIR}/${HANDOFF_MARKER}"
95+
96+
if [[ -d "${source_dir}" ]]; then
97+
if [[ -f "${marker_path}" ]]; then
98+
if [[ "$(cat "${marker_path}")" != "normal" ]] \
99+
|| [[ -n "$(find "${source_dir}" -mindepth 1 -maxdepth 1 -print -quit)" ]]; then
100+
echo "ERROR: WAL-G archive restore handoff state is inconsistent" >&2
101+
exit 1
102+
fi
103+
rmdir "${source_dir}"
104+
else
105+
if [[ -d "${DATA_DIR}" ]] \
106+
&& [[ -n "$(find "${DATA_DIR}" -mindepth 1 -maxdepth 1 -print -quit)" ]]; then
107+
echo "ERROR: PostgreSQL data directory is not empty before WAL-G archive handoff" >&2
108+
exit 1
109+
fi
110+
printf 'normal\n' > "${source_dir}/${HANDOFF_MARKER}"
111+
rm -rf "${DATA_DIR}"
112+
mv "${source_dir}" "${DATA_DIR}"
113+
fi
114+
elif [[ ! -f "${marker_path}" ]] || [[ "$(cat "${marker_path}")" != "normal" ]]; then
115+
echo "ERROR: WAL-G archive restore handoff source is missing" >&2
116+
exit 1
117+
fi
118+
98119
sync
99-
rm -f ${RESTORE_SCRIPT_DIR}/kb_restore.signal
100-
echo "Data restore completed successfully"
101-
fi
120+
rm -f "${RESTORE_SCRIPT_DIR}/kb_restore.signal"
121+
}
122+
123+
echo "Restoring data from ${DATA_DIR}.old..."
124+
finalize_handoff
125+
echo "Data restore completed successfully"
102126
EOF
103127

104128
# Replace variables in the generated script

addons/postgresql/dataprotection/wal-g-backup.sh

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@ export DATASAFED_BACKEND_BASE_PATH=${backup_base_path}
99
export WALG_DELTA_MAX_STEPS=0
1010
# 20Gi for bundle file
1111
export WALG_TAR_SIZE_THRESHOLD=21474836480
12-
# The cmpd pins Service serviceName=postgresql with roleSelector=primary; the
13-
# suffix must be that fixed serviceName, not the user-chosen component name.
14-
# pg_switch_wal()/archive checks below must run on the primary.
15-
PSQL="psql -h ${CLUSTER_COMPONENT_NAME}-postgresql -U ${DP_DB_USER} -d postgres"
12+
# Use the ActionSet-provided target endpoint instead of reconstructing a
13+
# controller-owned Service name.
14+
PSQL="psql -h ${DP_DB_HOST} -U ${DP_DB_USER} -p ${DP_DB_PORT} -d postgres"
1615

1716
# if the script exits with a non-zero exit code, touch a file to indicate that the backup failed,
1817
# the sync progress container will check this file and exit if it exists
@@ -90,11 +89,10 @@ fi
9089

9190
# 1. do full backup
9291
writeSentinelInBaseBackupPath "${backup_base_path}" "wal-g-backup-repo.path"
93-
PGHOST=${DP_DB_HOST} PGUSER=${DP_DB_USER} PGPORT=5432 wal-g backup-push ${DATA_DIR} 2>&1 | tee result.txt
92+
PGHOST=${DP_DB_HOST} PGUSER=${DP_DB_USER} PGPORT=${DP_DB_PORT} wal-g backup-push ${DATA_DIR} 2>&1 | tee result.txt
9493

95-
set +e
9694
echo "switch wal log"
97-
PSQL="psql -h ${CLUSTER_COMPONENT_NAME}-postgresql -U ${DP_DB_USER} -d postgres"
95+
PSQL="psql -h ${DP_DB_HOST} -U ${DP_DB_USER} -p ${DP_DB_PORT} -d postgres"
9896
${PSQL} -c "select pg_switch_wal();"
9997

10098
# 2. get backup name of the wal-g
@@ -117,4 +115,4 @@ START_TIME=$(echo $result_json | jq -r ".StartTime")
117115
TOTAL_SIZE=$(echo $result_json | jq -r ".CompressedSize")
118116

119117
# 5. update backup status
120-
echo "{\"totalSize\":\"$TOTAL_SIZE\",\"extras\":[{\"wal-g-backup-name\":\"${backupName}\"}],\"timeRange\":{\"start\":\"${START_TIME}\",\"end\":\"${STOP_TIME}\"}}" >"${DP_BACKUP_INFO_FILE}"
118+
echo "{\"totalSize\":\"$TOTAL_SIZE\",\"extras\":[{\"wal-g-backup-name\":\"${backupName}\"}],\"timeRange\":{\"start\":\"${START_TIME}\",\"end\":\"${STOP_TIME}\"}}" >"${DP_BACKUP_INFO_FILE}"

0 commit comments

Comments
 (0)