Repository navigation
pg_basebackup: actually send the EXCLUDE options to the server - #2172
Open
MisterRaindrop wants to merge 1 commit into
Open
MisterRaindrop wants to merge 1 commit into
MisterRaindrop wants to merge 1 commit into
Conversation
build_exclude_list() assembled the EXCLUDE clauses into a buffer of its own, and BaseBackup() then freed that buffer without ever putting it in the BASE_BACKUP command -- so -E/--exclude and --exclude-from were silently ignored and every excluded path was copied anyway. Worse than the wasted copying: gpinitstandby passes -E ./promote, and 'promote' is the name of PostgreSQL's promotion signal file. Copying the coordinator's promote directory into the standby left a 'promote' entry there, CheckPromoteSignal()'s stat() succeeds on a directory, and the brand new standby coordinator promoted itself the moment it reached a consistent state -- picking a new timeline instead of streaming, so pg_stat_replication on the coordinator stayed empty. Append the options to the command buffer with AppendStringCommandOption() instead. The server has always understood a repeated EXCLUDE (parse_basebackup_options collects them into one hash table); only the client half was missing. Exercised by gpinitstandby.feature:33 'gpinitstandby exclude dirs'. Split out of apache#1639 (originally b432ad8).
MisterRaindrop
added a commit
to MisterRaindrop/cloudberry
that referenced
this pull request
Oct 23, 2026
The suite comes from apache#1639, where it ran with that PR's fixes. Here it runs without them, and these scenarios fail until they land. Tag them @needs_fix and skip that tag in every job; each fix removes the tag from the scenarios it makes pass, which is also how it shows they do. A comment beside each tag names the fix. analyzedb 1 0977ac5 (identifiers and pg.escape_string) gpactivatestandby 1 apache#1889 (forced activation after a clean shutdown) gpaddmirrors 2 mirror postgresql.conf check; not pinned down yet, among b971d48, a3bfe47 and 8377856 gpcheckcat 11 0bfc1d7 gpinitstandby 2 apache#2172 (pg_basebackup EXCLUDE); without it the new standby promotes itself gpinitsystem 19 b971d48 gpmovemirrors 4 a3bfe47, 51955ac, 7056e02 (one of them a Scenario Outline with four examples) gpstop 2 45a1155, 51955ac (the gpstop lock) minirepro 3 3fccb75 (tables a view reads), 51955ac, apache#2175 (pg_dump --function-oids) The commit hashes are those in apache#1639. gprecoverseg is tagged as a whole: without its fixes a scenario hangs and the job reaches its 120 minute timeout, so the scenarios that do fail cannot be told apart from the ones never reached. It comes back as the gprecoverseg fixes land.
Alena0704
self-requested a review
October 24, 2026 17:51
2 of 6 tasks
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
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.
What does this PR do?
pg_basebackup -E/--excludeand--exclude-fromhave been silently ignored:build_exclude_list()built the EXCLUDE clauses into a buffer of its own, andBaseBackup()freed it without ever putting it into theBASE_BACKUPcommand, so every excluded path was copied anyway. The options are now appended to the command withAppendStringCommandOption(). The server side already handled them:EXCLUDEscans as an identifier inside the generic option list, andparse_basebackup_options()collects every occurrence into one hash table.The visible damage is in
gpinitstandby, which passes-E ./promoteamong others. Apromotedirectory in the coordinator's data directory was copied to the new standby,CheckPromoteSignal()'sstat()succeeds on a directory, and the standby promoted itself as soon as it reached a consistent state, instead of streaming.Split out of #1639 (originally b432ad8, by @tuhaihe).
Test Plan
Demo cluster (Rocky 8 build image, 3 primaries + 3 mirrors), with
log/testfile,db_dumps/testfileandpromote/testfilecreated in the coordinator's data directory, as Behave's "gpinitstandby exclude dirs" does:mainpg_basebackup -E ./promote -E ./db_dumps -E ./logdb_dumps/andpromote/copiedgpinitstandby -a -s cdw -S ... -P 7011received promote request,selected new timeline ID: 2; gpinitstandby times out after 10 min with "failed to start standby", rc 2gp_walreceiver streamingand the standby is still in recoverysrc/bin/pg_basebackup/t/010_pg_basebackup.plcovers--exclude-from, but it cannot show this: it fails 24 subtests and bails out onmainwell before reaching that case, identically with and without this change (mostly--target-gp-dbid is required). CI does not run it, which is how both problems went unnoticed. That is worth its own fix; this PR leaves it alone. Coverage for this change will come from "gpinitstandby exclude dirs" once the Behave suite (#2170) is in.Type of Change
Checklist