Skip to content

Fix flaky resource_queue_terminate, gp_relsizes_stats, ic and memoize tests - #2234

Open
chenjinbao1989 wants to merge 7 commits into
apache:mainfrom
chenjinbao1989:test_flaky2
Open

Fix flaky resource_queue_terminate, gp_relsizes_stats, ic and memoize tests#2234
chenjinbao1989 wants to merge 7 commits into
apache:mainfrom
chenjinbao1989:test_flaky2

Conversation

@chenjinbao1989

@chenjinbao1989 chenjinbao1989 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

resource_queue_terminate (isolation2):

  • pg_terminate_backend() only signals the target backend, and the queue slot held by the terminated session is released later during process exit (for holdable cursors in ProcKill() via AtExitCleanup_ResPortals()). The sanity check in scenarios 1 and 2 could therefore run before the cleanup and see rsqcountvalue = 1. Waiting for the pid to leave pg_stat_activity is not enough, since RemoveProcFromArray() runs before ProcKill(). Poll pg_resqueue_status until the queue count drops to 0 before the sanity checks instead.

gp_relsizes_stats:

  • The test verifies that relsizes_collect_stats_once() scans files without the per-file naptime by checking that it finishes within 5 seconds. The paused scan of the 10k partition files takes at least 10s, but the unpaused run also includes worker startup, populating segment_file_sizes and updating the size history, which can exceed 5s on slow CI hosts. LOCALTIMESTAMP(0) also rounds to whole seconds. Use clock_timestamp() and raise the threshold to 8s, which still stays below the 10s floor of the paused scan.

ic (regress and pax):

  • The QE error detection case injects one-shot exec_mpp_query_start faults on all segments without a session filter. Autovacuum is still enabled at this point of the schedule, and auto-analyze on the coordinator dispatches gp_acquire_sample_rows() to the segments, which can consume the faults first, so the test query occasionally hit no fault at all. Limit the faults to the current session.

memoize:

  • For the expr_key join, ORCA chooses between Index Scan and Index Only Scan based on relallvisible. The VACUUM ANALYZE before the query can only mark the pages all-visible when no concurrent transaction from another test in the parallel group holds back the xmin horizon, so the plan flipped between runs. Disable optimizer_enable_indexonlyscan for this query. The planner uses Seq Scans here and is not affected.

Fixes #ISSUE_Number

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions

my-ship-it
my-ship-it approved these changes Nov 4, 2026
View reviewed changes

@my-ship-it my-ship-it left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks

@fairyfar
fairyfar self-requested a review November 4, 2026 08:01
@fairyfar

fairyfar commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

LGTM

my-ship-it
my-ship-it approved these changes Nov 5, 2026
View reviewed changes

@my-ship-it my-ship-it left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

lss602726449
lss602726449 approved these changes Nov 6, 2026
View reviewed changes
Comment thread src/test/isolation2/sql/resource_queue_terminate.sql Resolved
leborchuk
leborchuk approved these changes Nov 6, 2026
View reviewed changes

@leborchuk leborchuk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@chenjinbao1989
chenjinbao1989 force-pushed the test_flaky2 branch 2 times, most recently from 5e8e863 to 681b6ea Compare November 9, 2026 16:36
chenjinbao1989 and others added 6 commits November 9, 2026 21:12
@chenjinbao1989
Fix flaky diskquota test_primary_failure test
ba79b17 
The test checked only once that the mirror of content 0 was in sync
before stopping the primary. After the large insert into seg0 the
mirror may still be catching up (mode 'n'). FTS does not promote a
mirror that is not in sync, so content 0 was left without a primary
and the rest of the test failed.

Wait for content 0 to be synchronized before stopping the primary.
@chenjinbao1989
resgroup_cpu_max_percent: report measured cpu usage on failure
37a71a8 
When verify_cpu_usage() fails, the diff only shows 'f'.  The samples
normally read 10.0-10.2 for a 10% cap, so a miss of the +/-3 tolerance
means something went systematically wrong, not sampling noise.  Emit a
warning with the average, the expected range and the raw samples when
the check fails, so the next failure shows which way it missed.  The
output of a passing run is unchanged.
@chenjinbao1989
Fix flaky vacuum_progress_column test
2fc38e0 
Apply the fix that vacuum_progress_row got earlier to the mirror-down
case of vacuum_progress_column.

FTS releases the syncrep wait as soon as it has marked seg1's mirror
down, but only bumps the FTS version at the end of the probe cycle.  If
the coordinator starts the post-cleanup phase before that, it sees the
old version and reuses the old vacuum workers, so vacuum_worker_changed
is never hit and the test waits ten minutes for it to trigger.

Hold the coordinator at vacuum_rel_finished_one_relation after the
compact phase, and release it only after a full FTS probe cycle.  Also
wait for the mirrors to be in sync first, since the worker only changes
if the compact phase commit blocks on syncrep.
@chenjinbao1989
vacuum_progress_column: update expected post-cleanup progress
814a541 
Since 0b2d681 the mirror-down case reliably gets a new vacuum
worker for the post-cleanup phase.  That worker starts from fresh
progress counters, so num_dead_tuples from the compact phase is still
zero as the test expects, but it now reports the work post-cleanup
itself did: two index vacuums and the truncated segfile blocks.  The
old expected output had all zeros, which only matched when the phase
did nothing.  vacuum_progress_row was updated the same way.
@chenjinbao1989 @claude
Fix flaky master_wal_switch test
faea41a 
pg_basebackup() returns as soon as the client is done, but the
walsender that served the backup may still be exiting.  The following
check of pg_stat_replication then sometimes saw an extra
"pg_basebackup | backup" row.  Wait until that walsender is gone before
checking.

Apply the same change to the isolation2 and singlenode_isolation2
copies of the test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chenjinbao1989 @claude
Fix flaky select_throttle test
e8ce7b6 
The throttled SELECT sometimes finished at once instead of waiting for
syncrep.  The test runs with autovacuum on, and an auto-vacuum or
auto-analyze that reads select_throttle first sets the hint bits, so
the SELECT generates no WAL and has nothing to be throttled on.
Disable autovacuum on both test tables.

Also wait for the wal_sender_loop fault to trigger before running the
throttled SELECT.  The suspend only takes effect the next time the
walsender goes round its loop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fairyfar

fairyfar commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

The multiple commits are all modifications to the CI test. It is recommended to rebase them into one commit and change the title of commit message to "CI: ...".

It is not recommended to use "Fix" in the title. Generally, "Fix" is used to fix bug for codes.

Just MY OPINION. Do others agree? @leborchuk @my-ship-it @lss602726449

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lss602726449 lss602726449 lss602726449 approved these changes
@leborchuk leborchuk leborchuk approved these changes
@my-ship-it my-ship-it my-ship-it approved these changes
+1 more reviewer
@fairyfar fairyfar fairyfar approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@chenjinbao1989 @fairyfar @lss602726449 @leborchuk @my-ship-it