Repository navigation
CI: run the gpMgmt Behave suite - #2170
Draft
MisterRaindrop wants to merge 13 commits into
Draft
MisterRaindrop wants to merge 13 commits into
MisterRaindrop wants to merge 13 commits into
Conversation
Run the gpMgmt Behave suite on Rocky Linux 9, one job per management command so that a failure in one feature file does not mask or pollute the others. The workflow builds an RPM once and fans out to a per-command matrix, each entry creating its own demo cluster. It is kept separate from the main build/installcheck workflow so that Behave-specific matrix expansion and result parsing can evolve without disturbing the primary CI path. A run costs about 190 minutes across 21 jobs, so a pull request only triggers it when it touches gpAux, gpMgmt, src/bin, this workflow, or system_views.sql, where gp_stat_replication is defined. The push trigger is left unfiltered on purpose: src/backend is too broad to put in the filter, but the utilities sit on top of it, so running after merge is what keeps a regression there from going unnoticed. 'edited' is not among the pull_request types, since it fires on a title or body change. Six of the 23 suites under test/behave/mgmt_utils are not in the matrix. gpcheckperf, gprecoverseg_newhost and gpssh_exkeys are @concourse_cluster throughout, so against a single-host demo cluster the tag filter selects nothing and the job reports success without running a scenario; a green check that tests nothing is worse than no check. cross_subnet needs the coordinator and segments on different subnets. gpexpand and gppkg are the two that could plausibly be added later. The core dump check treats a SIGABRT core as expected only when gpstop logged that it sent the signal. gpstop escalates to SIGABRT when a postmaster will not shut down, which "gpstop succeeds when pg_ctl command fails" exercises deliberately by injecting a sleep into checkpoint, so that core belongs to a scenario that passed. Without that log line a SIGABRT core still fails the job. (cherry picked from commit ea42235)
The Behave suite had drifted from the one it was forked from: scenarios added upstream for gprecoverseg, gpmovemirrors, gpaddmirrors, gpexpand, gpcheckcat, gpstop and minirepro were missing, several existing ones had diverged, and a number of step definitions carried only one of the @given/@when/@then decorators Greenplum registers, so features using "And" could not match them and Behave reported the steps as undefined even though the implementation was right there. Bring the feature files and their step definitions back in line with github.com/greenplum-db/gpdb-archive, keeping the Cloudberry-specific adjustments where the two products genuinely differ. Differential recovery is the one place they differ outright: Cloudberry has no "gprecoverseg --differential", so those scenarios cannot pass here. Tag them @Differential and filter them out of the gprecoverseg matrix entry, splitting the Examples tables that mix differential with full and incremental so the other variants keep running. The scenarios stay in place, aligned with Greenplum, ready to be enabled with the feature. (cherry picked from commit 2b4b6d3)
Cloudberry does not implement every feature the Greenplum suite covers,
and some scenarios encode a cluster topology we do not run. Tag or drop
them so the matrix selects only what this product can actually satisfy.
Each exclusion and its reason:
Note that the @not_implemented tags added here are lifted again later in
this same series: the commits that follow port the missing behaviour from
Greenplum, and by the end of the branch nothing carries that tag. They are
added here so that this commit -- the one that puts Behave into CI -- lands
with a job that is green, rather than one that is red until the last commit
in the series.
* test/behave: filter the scenarios Cloudberry does not support
Four scenarios in the matrix test behaviour the product does not have.
They are kept in the tree, aligned with Greenplum, so they can be
enabled with the feature -- but tagged and filtered out, because a job
that ships permanently red teaches everyone to ignore it.
@not_implemented, for behaviour that was never built:
- "should drop existing slot on full recovery" and the full variant of
"should not try to drop slot if slot does not exist" expect
gprecoverseg to check and drop internal_wal_replication_slot during a
full recovery. Cloudberry instead runs pg_basebackup without
--create-slot and retries with it if anything fails, so it never
prints what these scenarios assert.
- "recovers segment for valid max-rate options" needs gprecoverseg
--max-rate, which Cloudberry does not have.
- the two "config file contains hostname" scenarios need gprecoverseg's
-i file to accept a leading hostname. Greenplum's _parseConfigFile()
takes 3, 4 or 5 parts and cross-checks hostname against address;
Cloudberry's takes 3, so the run stops at "expected 3 parts on failed
segment group, obtained 4".
@known_failure, a separate tag for behaviour that exists and is broken:
- "gpinitsystem should print FQDN in pg_hba.conf for standby when
HBA_HOSTNAMES=1" never gets a standby. CREATE_STANDBY_QD runs
gpinitstandby -s localhost --hba-hostnames and it gives up in under a
second against a coordinator that is up and answering queries,
leaving no standby data directory while gpinitsystem still exits 0.
* test/behave: un-skip the HBA_HOSTNAMES=1 standby scenario
The scenario was tagged @known_failure because gpinitsystem left it with
no standby and nothing but a warning. The cause is the bare -f fixed in
the previous commit: with -I, gpinitsystem never reaches the line that
sets CLUSTER_BOOT_MODE=DEMO -- that assignment sits inside the
'no input config' branch -- so the scenario took the PRODUCTION path and
gpinitstandby died on 'option requires 1 argument'.
Verified on a five-node cluster, on the scenario's own shape:
gpinitsystem -a -I <config> -s scdw -h <hostfile>
-> rc 0, 'Successfully completed standby coordinator initialization',
standby streaming in gp_stat_replication
@known_failure had exactly one user, so the tag and its matrix filter go
with it.
* test/behave: drop the imported gpstop scenarios that have no steps
The alignment commit brought seven gpstop.lock scenarios over from
Greenplum that cannot run: they need
the user runs gpstop -a, selects s and interrupt the process
verify if the gpstop.lock directory is present in coordinator_data_directory
and neither step is defined -- not in Cloudberry, and not in
gpdb-archive either, where the scenarios sit in gpstop.feature with no
implementation behind them. Greenplum tags them @concourse_cluster and
@demo_cluster both, so they are undefined in its own runs too.
Nothing caught this because the Behave matrix filters @concourse_cluster
out, and a scenario carrying that tag is excluded whatever else it
carries, so a dry run under the matrix's own tags reported zero
undefined steps. Running them on a five-node cluster reports 13
undefined steps and 11 failed scenarios.
They contribute nothing to the CI this series adds -- which does not run
@concourse_cluster at all -- and an imported scenario that can never
pass is the same trap as the differential-recovery and
recovery-config-hostname imports already dropped here. Remove them; the
two added gpstop scenarios that do have steps stay.
behave --dry-run on gpstop.feature now reports 0 undefined for every tag
combination, not just the matrix's.
* test/behave: drop the imported scenarios whose steps do not exist
Same finding as the gpstop.lock scenarios, in gprecoverseg and
gpmovemirrors: 51 undefined steps in scenarios the alignment brought
over, invisible to CI because they are @concourse_cluster and a scenario
carrying that tag is filtered out whatever else it carries.
Two kinds, and upstream decides which is which:
- Steps Greenplum defines and we simply did not port. Those are worth
having, and the previous commit took the four gpstart ones across.
- Steps that exist upstream but are not applicable. gpmovemirrors'
'a sample gpmovemirrors input file is created in "{config}"
configuration' is the clear case: it hardcodes Greenplum's Concourse
topology -- ports 21000/21001 and a fixed sdw1/sdw2/sdw3 mirror map,
with a comment saying so. This cluster uses 7000/7001 and its spread
layout differs at contents 2 and 3, so the code cannot be reused,
only rewritten.
- Steps upstream does not have at all: the pg_start_backup /
pg_stop_backup pair, 'pg_rewind is killed on mirror with content',
the recovery-type config-file steps, the SUSPEND_PG_REWIND pair.
Several belong to features Cloudberry does not implement -- the
recovery-type column is the 5-part -i config format already tagged
@not_implemented here.
Drop the scenarios that depend on either of the last two. behave
--dry-run now reports 0 undefined for gprecoverseg, gpmovemirrors,
gpstart, gpstop, gpstate, gpaddmirrors and gpactivatestandby under both
the CI matrix's tags and the @concourse_cluster tags.
* test/behave: tag the scenarios that need differential recovery
gprecoverseg --differential does not exist in Cloudberry: the option is
not declared, and the rsync machinery behind it (sync_pg_data,
sync_tablespaces, write_conf_files, isDifferentialSynchronization) was
never carried over either. The feature file already excludes such
scenarios with @Differential, and the CI job filters on it, but six
places were missed when the file was imported.
One of them runs in CI today. "differential recovery runs successfully"
lost its tags entirely in the import -- upstream has @demo_cluster and
@concourse_cluster on it -- so nothing filters it out and it calls
gprecoverseg -av --differential.
Three more only matter on a multi-node cluster, where the
@concourse_cluster scenarios run, and the last two are Examples rows
driving a Scenario Outline whose other rows are fine; those are split
into their own tagged block, which is how the pg_controldata and
replication-slot outlines in this file already handle it.
* test/behave: finish excluding the recovery options Cloudberry does not have
The earlier pass only looked for --differential behind 'the user runs',
and missed the asynchronous step form, so two scenarios still drove a
differential recovery. It also stopped at --differential: --replay-lag
and --disable-replay-lag are equally absent -- no options declared, and
no ALLOWED_REPLAY_LAG or rebalanceSegments.replay_lag behind them -- so
those two scenarios get @not_implemented, the way the --max-rate
scenario in this same file already does.
The last one is ours to pass rather than skip. recoveryinfo.py prints
Failed to {} the following segments. You must run gprecoverseg -F
for all incremental failures
which is right for a tree with no differential recovery to suggest, and
is what this repository's own unit test expects. The scenario was
asserting upstream's wording, which offers "--differential or" first.
Match the message the product actually emits.
* test/behave: restore @concourse_cluster on two scenarios that need a second host
Both sit under this file's "@concourse_cluster tests" banner and both
carry the tag upstream; it was lost on import, so they run against the
single-host demo cluster in CI, where they cannot pass.
gpmovemirrors "mirrors come up even if one pg_ctl_start fails" is the
one failure in the ic-behave-gpmovemirrors job:
Exception: Host cdw has both primary and mirror for content 0
at 'verify that mirror segments are in "spread" configuration'
which is simply what spread mirroring looks like when there is one host.
The other three scenarios in that section kept their tag.
gprecoverseg "behave test requires a cluster with at least 2 hosts" says
so in its name and asks for a mirror segment on a remote host.
The two gpstop scenarios that also lost tags are left alone: they touch
only the coordinator, so they do run on a demo cluster, and keeping them
untagged keeps them in CI.
* test/behave: skip the scenarios that need pg_basebackup-in-progress detection
Upstream's gprecoverseg asks gp_stat_replication which segments already
have a pg_basebackup running against them and leaves those out of the
recovery, announcing
Found pg_basebackup running for segments with contentIds [...],
skipping recovery of these segments
Cloudberry has none of it -- not get_segments_with_running_basebackup,
not the handling built around it in _convert_requests_to_triplets -- and
no part of the tree prints that message.
Seven scenarios assert it and cannot pass. On a five-node cluster they
also take the rest of their suite with them: gpmovemirrors.feature:246
failed and the five scenarios after it then failed on "all segments are
not currently running", which is how three real failures became six.
Tag them @not_implemented, as this file already does for --max-rate, and
say in the file what is missing so they can be turned back on with the
feature. Bringing that logic over is a change to gprecoverseg itself and
belongs in its own commit, not in the one that puts behave into CI.
gprecoverseg.feature:792 is deliberately left running: it only asserts
that the message is *not* printed, and the rest of it exercises a real
recovery with a suspended walsender.
(cherry picked from commit f196edc)
The alignment brought over scenarios whose step definitions were missing,
wrong for this product, or wrong for PostgreSQL 16. Repair them so the
suite tests what it claims to:
* test/behave: make gpcheckcat's test operator a prefix operator
add_operator.sql declares my_pk_schema.!# with LEFTARG only -- a postfix
operator, which PostgreSQL removed in 14:
ERROR: operator right argument type must be specified
DETAIL: Postfix operators are not supported.
psql without ON_ERROR_STOP still exits 0, so the scenario's 'psql should
return a return code of 0' passed and the failure only showed up two
steps later: with no pg_operator row to delete there is nothing for
missing_extraneous to find, and gpcheckcat returned 0 where the scenario
wants 3.
RIGHTARG gives a prefix operator, which is still a pg_operator entry
identified by a non-oid primary key -- which is what 'gpcheckcat should
report and repair extra entries with non-oid primary keys' is about.
* test/behave: snapshot the logs of a scenario that failed
Scenarios routinely begin by emptying gpAdminLogs, so by the time a
feature finishes the logs belonging to the scenario that failed have
been deleted several times over and the CI artifact says nothing about
it. Chasing an intermittent gprecoverseg failure through three CI runs
turned up exactly one surviving recovery log, from a scenario that had
passed.
On failure, copy gpAdminLogs, gp_segment_configuration and every
segment's postgresql.conf into build-logs/failed-scenarios/<line>.<name>/
while they still describe the failure. Wrapped so that a problem
collecting diagnostics can never turn one failure into a different
one.
* test/behave: give each segment its own copy of postgresql.conf to check
'check segment conf: postgresql.conf' rsyncs every segment's
postgresql.conf to a local path named after the segment's host. On a
single-host demo cluster all of them are 'cdw', so all eight land on one
file -- and rsync's quick check skips a transfer whose size and mtime
match the destination. Segments initialised by the same gpinitsystem run
share an mtime, and their port lines are the same length (port=7003 vs
port=7006), so the transfer is skipped and the check reads the previous
segment's file:
port value in postgresql.conf of cdw is incorrect.
Expected:'7006', given:'7003'
The snapshot this series now takes when a scenario fails shows every
segment holding its own correct port moments later, dbid6 included -- the
file was never wrong, the check was reading the wrong one. It looked
intermittent because it depends on two segments' files agreeing on size
and mtime.
Greenplum has the same step, but runs it on a multi-host concourse
cluster where the hostnames differ and the collision cannot happen.
Name the copy after the dbid as well, and say which dbid in the error.
* test/behave: port the gpstart steps the alignment missed
Running gpstart's @concourse_cluster scenarios on a real cluster reports
four undefined steps:
fts probing is disabled
the host for the {seg_type} on content {content} is made unreachable
and do not wait for failover
the role of the {seg_type} on content {content} should be "{role}"
Greenplum has all of them, so this is a gap in the port rather than
anything to invent: gpstart.py factors the body of the 'is made
unreachable' step into _make_unreachable(context, seg_type, content,
utility=False) and builds both variants on it, change_hostname takes the
same utility flag, and must_have_expected_role mirrors the existing
must_have_expected_status. 'fts probing is disabled' is in mgmt_utils.py
beside the walsender fault-injection steps.
Taken across as-is; the files now line up with upstream.
behave --dry-run on gpstart.feature and gpstop.feature reports 0
undefined for every tag combination.
* test/behave: teach the background-pid steps about the coordinator
gpstop.feature's lock-file scenario runs both of these against
"coordinator", and neither handles it, so hostname is never assigned:
UnboundLocalError: local variable 'hostname' referenced before
assignment
at 'we run a sample background script to generate a pid on
"coordinator" segment'
which is the one failure in the ic-behave-gpstop job. The scenario is
identical to upstream's; only the elif arm was lost on import.
The kill step lost its second command as well, the one that removes
/tmp/bgpid. That matters beyond tidiness: the step that starts the
script waits for /tmp/bgpid to appear and then reads it, so a file left
behind by an earlier scenario is picked up immediately and the test goes
on to kill a pid that is no longer running. Upstream also sleeps a second
before that wait for the same reason; restore both.
* test/behave: save the port and address of the remote primary again
'the information of a "primary" segment on a remote host is saved' lost
the last two of its six assignments, so context.remote_pair_primary_port
was left to whatever an earlier scenario happened to store and
context.remote_pair_primary_address was never assigned anywhere in the
tree at all -- while seven steps in this same file read it:
port, hostname, datadir, address = context.remote_pair_primary_port,
context.remote_pair_primary_host,
context.remote_pair_primary_datadir,
context.remote_pair_primary_address
Eight scenarios in gprecoverseg.feature start from that step, all of
them @concourse_cluster, which is why nothing noticed: they only run on
a multi-host cluster, and there they fail on AttributeError before
testing anything.
* test/behave: repair the demo-cluster command and wait out the failover
Two things in replication_slots_utils.py, both reached from CI jobs that
are green today.
create_cluster builds a shell command out of a continued string, and one
continuation had picked up a stray character:
export WITH_MIRRORS={with_mirrors} && \A
The backslash escapes the A instead of the newline, so the shell runs a
command called A, the && chain ends there, and demo_cluster.sh runs as a
separate statement on the next line. It happens to still build a cluster,
and the exit status checked afterwards is that last statement's, which is
why nobody noticed -- but nothing was actually holding the chain
together. replication_slots.feature drives this four times.
'a preferred primary has failed' stops the primary and returns
immediately, leaving the next step to race the failover. Upstream waits
for transactions to unblock first; the helper is already imported here.
gpaddmirrors.feature and replication_slots.feature both start scenarios
with that step.
* test/behave: always put back a hostname the test made unreachable
These scenarios point a segment at invalid_host and rely on their last
step to put the real name back. When anything in between fails, that step
never runs and the catalog keeps invalid_host, which is not a failure the
next scenario can survive: while a primary's address does not resolve the
coordinator will not accept an ordinary connection at all, so every
scenario after it dies in before_scenario.
That is what happened to the gpstart suite on a five-node cluster --
21 scenarios failed, but only two of them for a reason of their own; the
other 19 never got a connection.
The old name is already saved a couple of lines up, so register a cleanup
with it. behave runs cleanups whether the scenario passed or failed, and
utility mode still connects while the address is unresolvable, which is
how the name was changed to begin with. The same pattern is used for the
temp directories in gpconfig and the crashed mirror in
replication_slots_utils.
* test/behave: give gpstate's recovery scenarios a lock with a live pid
These two scenarios fake an in-progress recovery: they write a sample
recovery_progress.file and create a gprecoverseg.lock directory. That was
enough while gpstate only checked whether the directory existed, and it
stops being enough now that it reads the pid inside and asks whether that
process is alive -- both scenarios started failing with "All segments are
running normally".
Upstream sets them up the way this change needs: start a background
script, build the lock directory around its pid, and kill it at the end.
The steps for that already exist here and now handle "coordinator", so
use them, and drop the pid-less variant -- it is Cloudberry-only, nothing
else calls it, and leaving it would invite someone to build a lock
directory that gpstate quite deliberately ignores.
* test/behave: match the way pg_rewind reports failure on PostgreSQL 16
The step that checks a failed incremental recovery looked for
pg_rewind: fatal
which is what PostgreSQL 12 printed. The fatal level was removed in 13:
this tree's src/common/logging.c emits only "error: ", "warning: " and
"detail: ", and pg_rewind never calls pg_log_fatal, so the string can
never appear and the scenario could not pass.
"error" alone is not the answer either. A failed pg_rewind now writes two
lines -- "error: postgres single-user mode in target cluster failed" and
then "detail: Command was: ..." -- while gprecoverseg streams whichever
line is last in the progress file when it next polls, which is nearly
always the detail. Observed on a five-node cluster: seven of the polls
for the failing dbid showed the detail line and none showed the error.
The assertion is a regex, so accept either. It still distinguishes a
failure from the ordinary progress lines ("syncing target data
directory", "49550/1202320 kB (4%) copied") that pg_rewind also emits.
* test/behave: stop counting recovery logs per failing segment
recovery_fail_check gained an assertion upstream does not make:
And gpAdminLogs directory has "{logfile_name}" files on respective
hosts only for content {content_ids}
with content_ids being only the segments whose recovery was expected to
fail. That step counts the log files on each host holding one of those
mirrors and insists the count matches exactly, so it is wrong whenever
another segment on the same host was recovered the same way in the same
run.
gprecoverseg.feature:1042 does exactly that -- content 1 full inplace
(succeeds), content 2 full to a bad directory (fails) -- and on a
single-host cluster both write a pg_basebackup log to the same
gpAdminLogs:
expected 1 pg_basebackup* files ... on host cdw, but found 2
The scenarios that do want this check call the step themselves with the
complete list of contents recovered that way, which is the only way the
count can be right: gprecoverseg.feature:784 asks for content 0,1 and
gpmovemirrors.feature:518 for content 0,1,2,3,4,5.
(cherry picked from commit 70fd128)
The core check accepts a SIGABRT core only when gpstop said it sent the signal, and looked for that line under build-logs/details/. gpstop writes it to its own log in /home/gpadmin/gpAdminLogs, and behave does not copy a passing step's output into the make log, so the line was never found: "gpstop succeeds when pg_ctl command fails" passed and its expected core still failed ic-behave-gpstop.
This was referenced 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.
…USTED_SHELL demo_cluster.sh writes TRUSTED_SHELL="./lalshell" into the generated cluster config when it is run as ./demo_cluster.sh, so the scenarios that run gpinitsystem -c ../gpAux/gpdemo/clusterConfigFile from gpMgmt fail with "./lalshell: No such file or directory". 767f126 in apache#1639 fixes that; tag them needs_fix until it lands.
With an absolute TRUSTED_SHELL the scenario gets as far as checking the mirrors' postgresql.conf, which still carries the primary's port until the gpinitsystem mirror-port fix lands.
2 tasks done
1bfc082 replaced pipes with shlex throughout gpMgmt for Python 3.13, and the alignment brought this fallback in with pipes.quote() and no import, so it raised NameError the first time gpstop was refused by pg_hba.conf. Every later scenario then started from a running cluster it could not stop.
2 tasks done
apache#1889 has landed, so the scenario it was tagged for can run.
2 tasks done
…fixes Eighteen gprecoverseg scenarios and three gpmovemirrors scenarios were tagged @not_implemented, on the grounds that Cloudberry lacks what they test. apache#1639 implements all of it, so they are tagged @needs_fix instead, each with the apache#1639 commit that makes it pass, the way the other scenarios waiting on a fix are: 7056e02 skip segments that already have a recovery running (7) 0aa2666 a hostname in the -i recovery config file (9) 8377856 drop a stale replication slot (2) 993b99d --max-rate (1) ad3c9d5 the rebalance replay-lag check (2) The comment on the feature-level tag now names only the fixes the whole feature waits on; the three Greenplum ports above are tagged scenario by scenario. The two gpaddmirrors scenarios whose fix was not pinned down are pinned to a3bfe47. Without it both fail on port value in postgresql.conf of cdw dbid 6 is incorrect because a recovered mirror keeps the port of the primary it was copied from, and with it both pass.
Three changes to gpcheckcat.feature, the half of apache#1639's gpcheckcat commit (0bfc1d7) that edits the suite rather than gpcheckcat: - The two ao_lastrownums scenarios go. That check reads pg_attribute_encoding.lastrownums, which Cloudberry's catalog does not have, so the gpcheckcat fixes do not bring it over and the scenarios can never pass. - "validate session GUC passed with -x is set" is tagged @not_implemented instead of @needs_fix. Its first half needs a postmaster started with gp_role=utility to refuse non-utility connections, a check Greenplum makes in InitPostgres() and Cloudberry dropped; no gpcheckcat change makes it pass. The gpcheckcat job now skips @not_implemented, as the gprecoverseg job already does. - The primary-key scenario deletes the operator through psql, as upstream does, rather than in utility mode on the first primary. Both pass; this keeps the scenario aligned with upstream. The remaining eight @needs_fix tags in the file still point at 0bfc1d7. With the tags removed, all eight fail without it and pass with it, including "report and repair owner errors on appendonly tables": its repair script alters the table's visimap relation, which the server refuses as a system catalog, until the gpcheckcat fix maps the visimap to its table.
MisterRaindrop
pushed a commit
to MisterRaindrop/cloudberry
that referenced
this pull request
Oct 24, 2026
* gpcheckcat: add the mix_distribution_policy check
Back-port from Greenplum a check Cloudberry was missing.
mix_distribution_policy reports tables whose distribution policy mixes
legacy and non-legacy hash opclasses, and cross-checks the result
against the gp_use_legacy_hashops GUC so the operator is told which of
the two states is inconsistent.
Add the SQL fixtures the corresponding Behave scenarios load.
Greenplum's unit tests for the check mock a psycopg2 cursor, but this
gpcheckcat reaches the database through PyGreSQL (conn.query() and
getresult()) and never opens a cursor, so three of them failed and the
rest passed without exercising the code. They are rewritten against
conn.query(), with tests added for a mixed policy, a clean one, and a
GUC that is read successfully.
Greenplum's companion ao_lastrownums check is deliberately not brought
over: it reads pg_attribute_encoding.lastrownums, a column Cloudberry's
catalog does not have, so the query errors out on every run and
gpcheckcat reports failure on a freshly created database.
* gpcheckcat: give pg_shdescription its TableMainColumn entry
TableMainColumn maps a catalog to the column that identifies the object
an entry belongs to. Greenplum has a row for pg_description and one for
pg_shdescription; Cloudberry has only the first. Without it,
processMissingDuplicateEntryResult() falls through to getOidFromPK(),
which looks the entry up by
SELECT oid FROM pg_shdescription WHERE ...
and pg_shdescription has no oid column, so missing_extraneous died with
Execution error: ERROR: column "oid" does not exist
LINE 4: SELECT oid FROM pg_shdescription
* gpcheckcat: attribute an AO table's visimap to the table in the owner check
checkOwners() compares each relation's owner across segments, and
folds an append-optimized table's auxiliary relations into the table
they belong to, so a mismatch is reported, and repaired, once, on the
table. It knew the segment relation, the block directory and the block
directory's index, but not the visimap relation or its index. Those
were reported as relations of their own, and the repair script tried
ALTER TABLE "pg_aoseg"."pg_aovisimap_17032" OWNER TO "gpadmin";
which the server refuses:
ERROR: permission denied: "pg_aovisimap_17032" is a system catalog
so "gpcheckcat -R owner" on an append-optimized table whose owner
differs on a segment produced a repair that could not run. Match the
visimap and its index, and the block directory's index by the table it
indexes. The repair is then the ALTER TABLE on the table itself, which
carries its auxiliary relations with it.
The feature-file half of this change -- dropping the two ao_lastrownums
scenarios and tagging the -x session GUC scenario, which Cloudberry no
longer supports -- edits the Behave suite as apache#2170 aligns it, so it goes
with that PR.
Split out of apache#1639 (originally 0bfc1d7).
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?
Runs the gpMgmt Behave suite in CI. This is the test-only part of #1639, split out as discussed there; the fixes from #1639 follow as separate PRs, listed below.
These four are cherry-picked unchanged from #1639 and keep @tuhaihe as their author. One commit is new:
build-logs/details/, while gpstop writes it to/home/gpadmin/gpAdminLogs. That is what failedic-behave-gpstopin CI: add gpMgmt Behave test #1639; the core was the one "gpstop succeeds when pg_ctl command fails" is meant to produce (see the analysis there).The workflow runs only when
gpAux/,gpMgmt/,src/bin/,system_views.sqlor the workflow itself changes, as agreed with @leborchuk.The rest skip, or align, the scenarios that cannot pass on Cloudberry as it is today:
Scenarios waiting on a fix
A scenario that fails until a fix lands is tagged
@needs_fix, every job skips that tag, and a comment beside each tag names the fix. Whichever lands second, this PR or the fix, removes the tags the fix makes pass. The fixes:gprecoverseg.featureis skipped as a whole until it lands.pg_dump --function-oids(minirepro)Scenarios that test something Cloudberry does not have, and that no open PR adds, are tagged
@not_implemented. A comment says why.Type of Change
Checklist
Please squash when merging.