Skip to content

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

Open
chenjinbao1989 wants to merge 5 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 5 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
WHERE query='DECLARE cs2 CURSOR WITH HOLD FOR SELECT 0;';

2<:
-- The terminated backend releases its queue slot during process exit, which

@lss602726449 lss602726449 Nov 6, 2026

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.

It seems that this case still failed

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
Fix flaky resource_queue_terminate, gp_relsizes_stats, ic and memoize…
774a68a 
… tests

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.
@chenjinbao1989
Fix flaky diskquota test_primary_failure test
4114e60 
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
09d1971 
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
a86dbfd 
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
5e8e863 
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.
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