Conversation
danolivo
added
enhancement
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow and a large bash harness that builds a 4-node PostgreSQL (REL_15–18) + Spock mesh, runs installcheck with replication active, and performs authoritative spock.sync_event propagation and subscription-enabled verification across all nodes. ChangesMulti-PostgreSQL Mesh Installcheck and Verification
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Up to standards ✅🟢 Issues
|
Builds PG REL_15/16/17/18_STABLE + Spock against each in parallel,
applies patches/<ver>/*.diff, wires four single-node clusters into a
12-subscription full mesh (exception_behaviour='discard', auto-DDL on),
stresses it with `make installcheck-parallel` on n1 (PG15), then
verifies the mesh survived.
Success requires both:
- sub_enabled = true on every subscription, and
- spock.sync_event() round-trips from each provider to every other
node within 10s -- the authoritative end-to-end check. The
status='replicating' snapshot is also polled but diagnostic only,
because apply workers can flicker 'down' through their
restart_delay window right after installcheck.
Layout (all under multi-pg-installcheck/ in the repo, gitignored):
src/pgNN, bin/pgNN, spock-build/pgNN, pgdata/nN, sock/, pid/
log/<node>.log per-builder summary
log/<node>-<phase>.log raw output of each command
log/main.log orchestrator events
log/installcheck.log make installcheck output
log/sync-event-check.log per-edge wait_for_sync_event output
Bash 3.2 compatible (macOS /bin/bash). Re-runs reuse existing PG and
Spock binaries by default; pass --force to rebuild. Terminal output
is one OK/FAILED line per phase plus the final RESULT, with the raw
psql state of every node printed at the end.
Companion workflow .github/workflows/installcheck-multi-pg.yml runs
on push and workflow_dispatch; needs only bison + flex on top of
ubuntu-latest.
![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
🧹 Nitpick comments (1)
.github/workflows/installcheck-multi-pg.yml (1)
50-50: 🏗️ Heavy liftConsider pinning actions to commit SHAs for supply-chain security.
Currently, the workflow references
actions/checkout@v4andactions/upload-artifact@v4using mutable tags. Pinning to full commit SHAs (e.g.,actions/checkout@<sha>) prevents tag-moving attacks and improves supply-chain security by ensuring the exact action code is reviewed and locked.Example pinning (verify current SHAs before applying):
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1Note: This requires looking up and maintaining SHA references, which adds maintenance overhead when updating action versions.
Also applies to: 67-67
🤖 Prompt for 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. In @.github/workflows/installcheck-multi-pg.yml at line 50, The workflow uses mutable action tags (actions/checkout@v4 and actions/upload-artifact@v4); replace those with the corresponding immutable commit SHAs (e.g., actions/checkout@<commit-sha> and actions/upload-artifact@<commit-sha>) by looking up the current, verified commit SHAs for the desired versions and updating the `uses:` entries for the steps that reference `actions/checkout@v4` and `actions/upload-artifact@v4` so the workflow pins to exact commits.
🤖 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 @.github/workflows/installcheck-multi-pg.yml:
- Around line 49-50: Update the GitHub Actions checkout step named "Checkout
spock" (the actions/checkout@v4 usage) to include persist-credentials: false to
avoid persisting the GITHUB_TOKEN to disk; locate the step with name "Checkout
spock" and add a persist-credentials: false key under that step (properly
indented) so the checkout action explicitly disables credential persistence.
In `@tests/run-multi-pg-installcheck.sh`:
- Around line 204-218: on_err currently logs failure and kills builders but does
not stop any started Postgres nodes; update on_err to invoke stop_all_nodes (or
the existing node-teardown function used in main) unless the run was started
with the "--keep" option, i.e. check the same keep-flag variable used elsewhere
in the script and call stop_all_nodes || true before exit; retain
dump_logs_on_failure and kill_outstanding_builders behavior and make the
stop_all_nodes call idempotent/safe so it can run from the ERR trap without
breaking command-substitution contexts.
- Around line 338-342: When a partial patch application leaves a checkout in a
modified state, reruns hit already-applied patches because _do_patch_pg() only
writes .spock-patches-applied at the end; update the logic so that before
reapplying patches (in patch_pg() or at the start of _do_patch_pg()) you detect
the absence of the .spock-patches-applied marker and then either reset the
existing checkout (e.g., git reset --hard && git clean -fdx) or force a fresh
clone via clone_pg(); ensure the reset/clean happens whenever the marker is not
present but the repository directory exists so partial applies are wiped before
attempting the patch sequence again.
- Around line 232-260: Validate BASE_DIR (after resolving with BASE_DIR="$(cd
"${BASE_DIR}" && pwd)") before running mkdir/rm: ensure it's non-empty and not a
dangerous root like "/" or other top-level system roots (at minimum reject ""
and "/"), or require a harness-owned sentinel file (e.g. check for
"${BASE_DIR}/.spock-harness") before performing rm -rf on LOG_DIR and PID_DIR;
if validation fails, print an error and exit non‑zero. Update the logic around
BASE_DIR, SOCK_DIR, LOG_DIR, PID_DIR, mkdir -p and rm -rf to enforce this guard.
---
Nitpick comments:
In @.github/workflows/installcheck-multi-pg.yml:
- Line 50: The workflow uses mutable action tags (actions/checkout@v4 and
actions/upload-artifact@v4); replace those with the corresponding immutable
commit SHAs (e.g., actions/checkout@<commit-sha> and
actions/upload-artifact@<commit-sha>) by looking up the current, verified commit
SHAs for the desired versions and updating the `uses:` entries for the steps
that reference `actions/checkout@v4` and `actions/upload-artifact@v4` so the
workflow pins to exact commits.
🪄 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: CHILL
Plan: Pro
Run ID: cca87c39-c1bb-4f8e-ab01-1367688b0037
📒 Files selected for processing (3)
.github/workflows/installcheck-multi-pg.yml.gitignoretests/run-multi-pg-installcheck.sh
Comment on lines
+49
to
+50
| - name: Checkout spock | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
Set persist-credentials: false to prevent credential leakage.
The checkout action should explicitly set persist-credentials: false to ensure the GitHub token is not persisted to disk after checkout completes. While the artifacts being uploaded are only logs and test output (not the git checkout itself), this is a security best practice to prevent accidental credential exposure.
🔒 Proposed fix
- name: Checkout spock
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 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.
Suggested change
| - name: Checkout spock | |
| uses: actions/checkout@v4 | |
| - name: Checkout spock | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 49-50: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 50-50: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for 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.
In @.github/workflows/installcheck-multi-pg.yml around lines 49 - 50, Update the
GitHub Actions checkout step named "Checkout spock" (the actions/checkout@v4
usage) to include persist-credentials: false to avoid persisting the
GITHUB_TOKEN to disk; locate the step with name "Checkout spock" and add a
persist-credentials: false key under that step (properly indented) so the
checkout action explicitly disables credential persistence.
Comment on lines
+204
to
+218
| trap 'on_err $? $LINENO' ERR | ||
|
|
||
| on_err() { | ||
| local rc=$1 line=$2 | ||
| log "Aborted: exit ${rc} at line ${line}" | ||
| dump_logs_on_failure || true | ||
| # Deliberately NO stop_all_nodes here. It used to live in this | ||
| # trap, but combined with set -E it would fire from inside command | ||
| # substitutions and silently kill every cluster on a single | ||
| # transient psql hiccup. Teardown lives in main()'s normal flow | ||
| # (where it has the correct context); kill_outstanding_builders is | ||
| # still safe to call here because it operates on PIDs we know are | ||
| # ours. | ||
| kill_outstanding_builders || true | ||
| exit "${rc}" |
There was a problem hiding this comment.
Always tear down started clusters on failure unless --keep is set.
Right now the only stop_all_nodes() call is on the success path in main(). Any error after some postmasters have started—including the already-hit wait_for_builders || fail "one or more builders failed" path—exits without cleanup, so a failed run can leave sockets/processes behind even though the script documents “servers will be stopped on script exit” by default.
Also applies to: 1012-1016, 1082-1086
🤖 Prompt for 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.
In `@tests/run-multi-pg-installcheck.sh` around lines 204 - 218, on_err currently
logs failure and kills builders but does not stop any started Postgres nodes;
update on_err to invoke stop_all_nodes (or the existing node-teardown function
used in main) unless the run was started with the "--keep" option, i.e. check
the same keep-flag variable used elsewhere in the script and call stop_all_nodes
|| true before exit; retain dump_logs_on_failure and kill_outstanding_builders
behavior and make the stop_all_nodes call idempotent/safe so it can run from the
ERR trap without breaking command-substitution contexts.
Comment on lines
+232
to
+260
| while [ "$#" -gt 0 ]; do | ||
| case "$1" in | ||
| --base-dir) BASE_DIR="$2"; shift 2 ;; | ||
| --keep) KEEP_RUNNING=1; shift ;; | ||
| --force) FORCE_REBUILD=1; shift ;; | ||
| --jobs) JOBS_TOTAL="$2"; shift 2 ;; | ||
| -h|--help) usage; exit 0 ;; | ||
| *) fail "unknown argument: $1" 4 ;; | ||
| esac | ||
| done | ||
|
|
||
| mkdir -p "${BASE_DIR}/src" \ | ||
| "${BASE_DIR}/bin" \ | ||
| "${BASE_DIR}/spock-build" \ | ||
| "${BASE_DIR}/pgdata" \ | ||
| "${BASE_DIR}/log" \ | ||
| "${BASE_DIR}/sock" \ | ||
| "${BASE_DIR}/pid" | ||
| BASE_DIR="$(cd "${BASE_DIR}" && pwd)" | ||
| SOCK_DIR="${BASE_DIR}/sock" | ||
| LOG_DIR="${BASE_DIR}/log" | ||
| PID_DIR="${BASE_DIR}/pid" | ||
|
|
||
| # Fresh log and pid directories per run: stale lines from a previous | ||
| # run would otherwise commingle with new output and make diagnosis | ||
| # painful. Deliberately scoped to log/ and pid/ -- src/, bin/, | ||
| # spock-build/, and pgdata/ are preserved so reuse-on-rerun still works. | ||
| rm -rf "${LOG_DIR}" "${PID_DIR}" | ||
| mkdir -p "${LOG_DIR}" "${PID_DIR}" |
There was a problem hiding this comment.
Guard --base-dir before the script starts deleting under it.
BASE_DIR is accepted verbatim and then used for mkdir -p plus rm -rf "${LOG_DIR}" "${PID_DIR}". A typo like --base-dir /var or --base-dir / turns this harness into a host-filesystem cleanup tool. Please reject unsafe roots (at minimum empty, /, and other top-level system paths) or require a harness-owned sentinel file before removing anything.
🤖 Prompt for 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.
In `@tests/run-multi-pg-installcheck.sh` around lines 232 - 260, Validate BASE_DIR
(after resolving with BASE_DIR="$(cd "${BASE_DIR}" && pwd)") before running
mkdir/rm: ensure it's non-empty and not a dangerous root like "/" or other
top-level system roots (at minimum reject "" and "/"), or require a
harness-owned sentinel file (e.g. check for "${BASE_DIR}/.spock-harness") before
performing rm -rf on LOG_DIR and PID_DIR; if validation fails, print an error
and exit non‑zero. Update the logic around BASE_DIR, SOCK_DIR, LOG_DIR, PID_DIR,
mkdir -p and rm -rf to enforce this guard.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
tests/run-multi-pg-installcheck.sh (3)
243-260:⚠️ Potential issue | 🟠 MajorValidate
--base-dirbefore deleting under it.This path is accepted verbatim and then used for
rm -rf "${LOG_DIR}" "${PID_DIR}". A typo like--base-dir /or another top-level system path turns the harness into a host-filesystem cleanup step.🤖 Prompt for 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. In `@tests/run-multi-pg-installcheck.sh` around lines 243 - 260, The script currently accepts --base-dir verbatim and then runs rm -rf "${LOG_DIR}" "${PID_DIR}" which can wipe critical paths; before deleting, validate and canonicalize BASE_DIR (the variable set by --base-dir) and assert it is non-empty, not "/" (or other top-level paths), and that LOG_DIR and PID_DIR are actual subdirectories of that canonicalized BASE_DIR; if the checks fail, abort with an error. Use the existing BASE_DIR, LOG_DIR, PID_DIR variables (and the canonicalization step BASE_DIR="$(cd "${BASE_DIR}" && pwd)") to perform these checks and refuse to run rm -rf unless the safe-guard conditions pass.
206-218:⚠️ Potential issue | 🟠 MajorDefault cleanup still does not run on failure.
on_err()only kills builders, and the explicitwait_for_builders || fail .../wait_for_all_ready || fail ...paths inmain()bypass the ERR trap entirely. A failed run can still leave started postmasters and sockets behind even when--keepwas not set.Also applies to: 1012-1016
🤖 Prompt for 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. In `@tests/run-multi-pg-installcheck.sh` around lines 206 - 218, The failure paths bypass the ERR trap so started postmasters/sockets can be left running; update cleanup so failures always perform default teardown: modify on_err() to call stop_all_nodes and the normal teardown (or the same function main() uses for cleanup) in addition to kill_outstanding_builders and dump_logs_on_failure, honoring the --keep flag if set, and change the explicit failure paths in main() (the points that call wait_for_builders || fail... and wait_for_all_ready || fail...) to invoke on_err with the proper exit code (or call the shared teardown function) instead of exiting directly so cleanup runs consistently.
363-381:⚠️ Potential issue | 🟠 MajorA partial patch failure still leaves reruns stuck on a dirty checkout.
The marker is only written after the entire patch loop succeeds. If one patch fails after earlier ones applied, the next run reuses that modified tree and re-enters
git applyon already-applied patches until the checkout is manually reset.Also applies to: 390-394
🤖 Prompt for 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. In `@tests/run-multi-pg-installcheck.sh` around lines 363 - 381, The loop in _do_patch_pg applies patches but always writes the .spock-patches-applied marker even if some git apply invocations fail; modify _do_patch_pg so that each git apply is checked and on any failure the function immediately prints an error, returns non-zero (or exits), and does not write the marker, and only touch "${src}/.spock-patches-applied" after the loop completes successfully (i.e., after all git apply calls returned success and any==1); reference the _do_patch_pg function, the git apply invocation and the .spock-patches-applied marker when making this change.
🤖 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 @.github/workflows/installcheck-multi-pg.yml:
- Around line 52-53: The workflow uses mutable action tags; replace the tagged
usages of the actions with the provided immutable commit SHAs so the steps
"uses: actions/checkout@v4" and "uses: actions/upload-artifact@v4" are updated
to "uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5" and "uses:
actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02" respectively
to pin the actions to immutable SHAs.
In `@tests/run-multi-pg-installcheck.sh`:
- Around line 338-342: The --force flag is currently not forcing a fresh
source+patch because clone_pg() reuses existing checkouts and patch_pg() skips
when a marker exists; update both to honor FORCE by having clone_pg() delete or
reclone the ${src} directory (or run a fresh git clone) when FORCE/--force is
set so it cannot reuse a stale checkout, and have patch_pg() remove or ignore
the existing marker (the patch-applied sentinel used there) when FORCE is set so
patches are re-applied; modify the logic around clone_pg() and patch_pg() to
check the FORCE variable and remove the existing source tree and marker before
proceeding.
---
Duplicate comments:
In `@tests/run-multi-pg-installcheck.sh`:
- Around line 243-260: The script currently accepts --base-dir verbatim and then
runs rm -rf "${LOG_DIR}" "${PID_DIR}" which can wipe critical paths; before
deleting, validate and canonicalize BASE_DIR (the variable set by --base-dir)
and assert it is non-empty, not "/" (or other top-level paths), and that LOG_DIR
and PID_DIR are actual subdirectories of that canonicalized BASE_DIR; if the
checks fail, abort with an error. Use the existing BASE_DIR, LOG_DIR, PID_DIR
variables (and the canonicalization step BASE_DIR="$(cd "${BASE_DIR}" && pwd)")
to perform these checks and refuse to run rm -rf unless the safe-guard
conditions pass.
- Around line 206-218: The failure paths bypass the ERR trap so started
postmasters/sockets can be left running; update cleanup so failures always
perform default teardown: modify on_err() to call stop_all_nodes and the normal
teardown (or the same function main() uses for cleanup) in addition to
kill_outstanding_builders and dump_logs_on_failure, honoring the --keep flag if
set, and change the explicit failure paths in main() (the points that call
wait_for_builders || fail... and wait_for_all_ready || fail...) to invoke on_err
with the proper exit code (or call the shared teardown function) instead of
exiting directly so cleanup runs consistently.
- Around line 363-381: The loop in _do_patch_pg applies patches but always
writes the .spock-patches-applied marker even if some git apply invocations
fail; modify _do_patch_pg so that each git apply is checked and on any failure
the function immediately prints an error, returns non-zero (or exits), and does
not write the marker, and only touch "${src}/.spock-patches-applied" after the
loop completes successfully (i.e., after all git apply calls returned success
and any==1); reference the _do_patch_pg function, the git apply invocation and
the .spock-patches-applied marker when making this change.
🪄 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: CHILL
Plan: Pro
Run ID: 36b9fb51-6a70-4c1d-bc1a-50ab582519ee
📒 Files selected for processing (3)
.github/workflows/installcheck-multi-pg.yml.gitignoretests/run-multi-pg-installcheck.sh
✅ Files skipped from review due to trivial changes (1)
- .gitignore
Comment on lines
+52
to
+53
| - name: Checkout spock | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for repo in actions/checkout actions/upload-artifact; do
type="$(gh api "repos/${repo}/git/ref/tags/v4" --jq '.object.type')"
sha="$(gh api "repos/${repo}/git/ref/tags/v4" --jq '.object.sha')"
if [ "$type" = tag ]; then
sha="$(gh api "repos/${repo}/git/tags/${sha}" --jq '.object.sha')"
fi
printf '%s@v4 -> %s\n' "$repo" "$sha"
doneRepository: pgEdge/spock
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/installcheck-multi-pg.yml"
echo "== File excerpt around lines 45-85 =="
nl -ba "$FILE" | sed -n '45,85p'
echo
echo "== All uses of `@v4` in that workflow =="
rg -n "uses:\s*.*`@v4`" "$FILE" || true
echo
echo "== All uses of actions/checkout@v4 and actions/upload-artifact@v4 =="
rg -n "uses:\s*actions/(checkout|upload-artifact)`@v4`" "$FILE" || trueRepository: pgEdge/spock
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/installcheck-multi-pg.yml"
echo "== File excerpt around lines 45-85 =="
awk 'NR>=45 && NR<=85 {printf "%5d %s\n", NR, $0}' "$FILE"
echo
echo "== All uses of `@v4` in that workflow =="
rg -n "uses:\s*.*`@v4`" "$FILE" || true
echo
echo "== All uses of actions/checkout@v4 and actions/upload-artifact@v4 =="
rg -n "uses:\s*actions/(checkout|upload-artifact)`@v4`" "$FILE" || trueRepository: pgEdge/spock
Length of output: 1850
Pin GitHub Actions versions to immutable SHAs.
This workflow uses mutable tags:
- Line 53:
actions/checkout@v4→actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 - Line 73:
actions/upload-artifact@v4→actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 52-53: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 53-53: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for 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.
In @.github/workflows/installcheck-multi-pg.yml around lines 52 - 53, The
workflow uses mutable action tags; replace the tagged usages of the actions with
the provided immutable commit SHAs so the steps "uses: actions/checkout@v4" and
"uses: actions/upload-artifact@v4" are updated to "uses:
actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5" and "uses:
actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02" respectively
to pin the actions to immutable SHAs.
Comment on lines
+338
to
+342
| if [ -d "${src}/.git" ] \ | ||
| && [ -f "${src}/src/test/regress/parallel_schedule" ]; then | ||
| log "${node}: [pg-clone] PG${ver} source already present, skipping" | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
--force does not actually force a fresh source+patch state.
clone_pg() still reuses an existing checkout, and patch_pg() still skips when the marker exists, so --force only rebuilds binaries from whatever source tree was already on disk. That contradicts the CLI contract and can silently test stale PostgreSQL or stale patches.
🛠️ Proposed fix
clone_pg() {
local ver="$1"
local node; node="$(ver_to_node "${ver}")"
local branch="REL_${ver}_STABLE"
local src; src="$(src_for "${ver}")"
+ if [ "${FORCE_REBUILD}" -ne 0 ]; then
+ rm -rf "${src}"
+ fi
+
if [ -d "${src}/.git" ] \
&& [ -f "${src}/src/test/regress/parallel_schedule" ]; then
log "${node}: [pg-clone] PG${ver} source already present, skipping"
return 0
fi
@@
patch_pg() {
local ver="$1"
local node; node="$(ver_to_node "${ver}")"
local src; src="$(src_for "${ver}")"
local patch_dir="${SPOCK_SRC}/patches/${ver}"
- if [ -f "${src}/.spock-patches-applied" ]; then
+ if [ "${FORCE_REBUILD}" -eq 0 ] && [ -f "${src}/.spock-patches-applied" ]; then
log "${node}: [pg-patch] patches already applied (marker present), skipping"
return 0
fi
run_phase "${node}" pg-patch _do_patch_pg "${ver}" "${src}" "${patch_dir}"
}Also applies to: 390-394
🤖 Prompt for 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.
In `@tests/run-multi-pg-installcheck.sh` around lines 338 - 342, The --force flag
is currently not forcing a fresh source+patch because clone_pg() reuses existing
checkouts and patch_pg() skips when a marker exists; update both to honor FORCE
by having clone_pg() delete or reclone the ${src} directory (or run a fresh git
clone) when FORCE/--force is set so it cannot reuse a stale checkout, and have
patch_pg() remove or ignore the existing marker (the patch-applied sentinel used
there) when FORCE is set so patches are re-applied; modify the logic around
clone_pg() and patch_pg() to check the FORCE variable and remove the existing
source tree and marker before proceeding.
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
1 participant
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.
Builds PG REL_15/16/17/18_STABLE + Spock against each in parallel, applies patches//*.diff, wires four single-node clusters into a 12-subscription full mesh (exception_behaviour='discard', auto-DDL on), stresses it with
make installcheck-parallelon n1 (PG15), then verifies the mesh survived.Success requires both:
Layout (all under multi-pg-installcheck/ in the repo, gitignored):
src/pgNN, bin/pgNN, spock-build/pgNN, pgdata/nN, sock/, pid/
log/.log per-builder summary
log/-.log raw output of each command
log/main.log orchestrator events
log/installcheck.log make installcheck output
log/sync-event-check.log per-edge wait_for_sync_event output
Bash 3.2 compatible (macOS /bin/bash). Re-runs reuse existing PG and Spock binaries by default; pass --force to rebuild. Terminal output is one OK/FAILED line per phase plus the final RESULT, with the raw psql state of every node printed at the end.
Companion workflow .github/workflows/installcheck-multi-pg.yml runs on push and workflow_dispatch; needs only bison + flex on top of ubuntu-latest.