Skip to content

[fix](regression) Wait for auto partition recycle - #65375

Merged
yiguolei merged 1 commit into
apache:branch-4.1from
shuke987:fix-branch41-auto-new-recycle-wait
Jul 10, 2026
Merged

yiguolei merged 1 commit into
apache:branch-4.1from
shuke987:fix-branch41-auto-new-recycle-wait

Conversation

@shuke987

@shuke987 shuke987 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix test_auto_new_recycle to wait for auto partition recycle by polling the expected partition count.
  • Replace fixed sleeps before retention-count assertions with an Awaitility-based helper.
  • Keep the original validation intent: auto partition retention should eventually keep only the latest configured historical partitions.

Testing

  • git diff --check
  • git diff --cached --check
  • Not run locally: no regression environment was allocated for this case.

Failure evidence:

  • x64 branch-4.1 NonConcurrent build 201293, occurrence 2000000189.
  • Recent same-pipeline history showed repeated line-129 failures mixed with passes, which points to async scheduler timing rather than a deterministic product semantic failure.

@shuke987
shuke987 requested a review from yiguolei as a code owner July 8, 2026 09:36
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@shuke987

shuke987 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@shuke987

shuke987 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Jul 10th, 2026 6:44 AM.
Workflow run: https://github.com/apache/doris/actions/runs/28932798326

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@shuke987

shuke987 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot 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.

Automated review completed for PR #65375.

Critical checkpoints:

  • Scope verified from GitHub metadata and patch: one modified file, regression-test/suites/partition_p0/auto_partition/test_auto_new_recycle.groovy, at head 0e74c0a68d3255acb2af8336246db9207881a8d8.
  • Existing inline review context was empty, so no duplicate inline issue was suppressed.
  • The repo-local .claude/skills/code-review/SKILL.md was not present in this checkout after hidden-path checks; I followed the review prompt and shared-ledger workflow.
  • Patch-scoped git diff --check against the fetched base passed. I did not run the regression suite because this runner does not provide the required Doris regression environment.

Result: requesting changes for one test stability regression introduced by the longer Awaitility polling window.

User focus: no additional user-provided review focus was supplied.

Subagent conclusions: optimizer-rewrite found no optimizer/rewrite issue. tests-session-config proposed TSC-001; I verified and accepted it as MAIN-001. Convergence round C1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same final ledger/comment set.


waitUntilSafeExecutionTime("NOT_CROSS_DAY_BOUNDARY", 20)

def waitPartitionCount = { int expectedCount ->

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.

The longer poll makes this final now()-based case much easier to race with midnight. The only guard is still waitUntilSafeExecutionTime("NOT_CROSS_DAY_BOUNDARY", 20), but this helper can now wait up to 1200 seconds, and this final call replaces a fixed 8-second sleep after inserting date_add(now(), interval number-5 day). The recycle code decides whether a partition is historical using the scheduler's current time, so if the rows are inserted shortly before midnight and this await observes the recycle after midnight, the partition for the insert day becomes historical and retention can leave 5 partitions instead of the asserted 6. Please either make this case use a fixed base date/time or move/resize the day-boundary guard so it covers the maximum wait through this final assertion.

@yiguolei
yiguolei merged commit 115b977 into apache:branch-4.1 Jul 10, 2026
32 of 34 checks passed
hello-stephen pushed a commit that referenced this pull request Jul 15, 2026
### What problem does this PR solve?

Related PR: #65375

Problem Summary:

`test_auto_new_recycle` uses `now()` to create five historical
partitions, one
current-day partition, and two future partitions, then asserts that
retention
leaves six partitions. The day-boundary guard previously ran near the
start of
the case and only reserved 20 seconds, while the time-sensitive
assertion ran
after multiple fixed sleeps. The case could therefore cross midnight
before
the final scheduler pass, changing the correct partition count from six
to
five.

This change moves the guard directly before the final time-sensitive
insert
and reserves 100 seconds for that section. It preserves coverage for the
current-day partition boundary and future partitions while preventing
the
final scheduler pass from crossing midnight.
yiguolei pushed a commit that referenced this pull request Jul 16, 2026
#65646)

### What problem does this PR solve?

Related PRs: #65435, #65375

Problem Summary:

`test_auto_new_recycle` uses `now()` to create historical, current-day,
and
future partitions, then waits for asynchronous partition recycling. The
day-boundary guard ran near the start of the case, so it did not protect
the
final time-sensitive assertion after the preceding Awaitility waits.

This branch-4.1 backport moves the guard directly before the final
`now()`
insert. The 1300-second protected window covers the existing 1200-second
Awaitility timeout plus the insert and assertion overhead. The guard
only
sleeps when this final section would otherwise start too close to
midnight.

Testing:

- `git diff --check`
- Not run locally because no Doris regression environment was allocated.
The
  PR build will run the regression coverage.

### Release note

None

### Check List (For Author)

- Test
    - [x] Regression test
    - [ ] Unit Test
    - [ ] Manual test (add detailed scripts or steps below)
    - [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
        - [ ] Previous test can cover this change.
        - [ ] No code files have been changed.
        - [ ] Other reason

- Behavior changed:
    - [x] No.
    - [ ] Yes.

- Does this need documentation?
    - [x] No.
    - [ ] Yes.

### Check List (For Reviewer who merge this PR)

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
wyxxxcat pushed a commit to wyxxxcat/doris that referenced this pull request Aug 17, 2026
)

### What problem does this PR solve?

Related PR: apache#65375

Problem Summary:

`test_auto_new_recycle` uses `now()` to create five historical
partitions, one
current-day partition, and two future partitions, then asserts that
retention
leaves six partitions. The day-boundary guard previously ran near the
start of
the case and only reserved 20 seconds, while the time-sensitive
assertion ran
after multiple fixed sleeps. The case could therefore cross midnight
before
the final scheduler pass, changing the correct partition count from six
to
five.

This change moves the guard directly before the final time-sensitive
insert
and reserves 100 seconds for that section. It preserves coverage for the
current-day partition boundary and future partitions while preventing
the
final scheduler pass from crossing midnight.
Gabriel39 pushed a commit that referenced this pull request Nov 6, 2026
…t_generation (#70308)

### Background

An auto-partition table with `partition.retention_count = N` keeps only
its N latest historical partitions: the FE's `DynamicPartitionScheduler`
drops older ones on every run, once per
`dynamic_partition_check_interval_seconds` (600 by default).
`partition_p0/auto_partition/test_auto_new_recycle` lowers that interval
to 1 s and checks the partition count after each load or ALTER.
`audit/test_sql_digest_generation` checks that the audit log gets an
`sql_digest` only for queries slower than the FE config
`sql_digest_generation_threshold_ms`, once with the threshold at 0 and
once at 100000. Both suites are in group `nonConcurrent`.

### What problem does this PR solve?

Issue Number: None
Related PR: #65375 and #65646 (the branch-4.1 recycle-suite fixes by
@shuke987, ported here), #65343 (the branch-4.1 digest-suite fix by
@shuke987, ported here), #65435 (master's earlier move of the recycle
suite's day-boundary guard)
PR Type: Test

Problem Summary:

Both suites rely on timing the product does not promise. The recycle
suite checks the partition count after a fixed `sleep(8000)` (5 s after
an ALTER), while the drop happens asynchronously at an unbounded time
after the config change; branch-4.1's NonConcurrent pipeline failed at
the first such check repeatedly (#65375). The digest suite's threshold-0
check needs the query's elapsed time to be above 0 ms. This PR gives
both suites their branch-4.1/4.2 form: the recycle suite polls the
partition count every second, for at most 1200 s, and widens its
day-boundary guard to cover the poll; the digest query adds `sleep(1)`.
Both files become identical to branch-4.2.

### Details

- **Failure:** On master's public NonConcurrent pipeline neither suite
has failed on its own since mid-July: of about 8,000 runs each, the
recycle suite failed 3 times and the digest suite 87 times, all in
builds whose cluster had died (most suites failing). The failures that
motivated the fixes are on branch-4.1's pipeline (#65375); #65343 made
the digest query deterministic there.
- **Cause:** `ADMIN SET FRONTEND CONFIG` sets the scheduler's new
interval at once, but the scheduler sleeps in 5-second slices and
notices the change only at the next slice. It then runs
`executeDynamicPartition` over every registered table before the drop.
So the drop lands 0 to 5 s plus one full pass after the config change,
and a pass longer than about 3 s fails the 8-second check. In the digest
suite, `AuditLogHelper` writes a digest only when `elapseMs >
sql_digest_generation_threshold_ms`, and nothing bounds the elapsed time
of `select *` on a two-row table from below (9 to 15 ms locally). With
`sleep(1)` on each row it takes about 2 s, still far below the second
check's 100000 ms.
- **Stability evidence:** The recycle checks now wait for the drop and
fail only when the count misses the target for 1200 s. The poll runs on
the suite thread, because the framework sets
`Awaitility.pollInSameThread()` (#68338), so it does not leak
connections. Shortening one of master's waits to 1 s reproduces the
branch-4.1 failure shape, and the ported suite passes; mutants show that
both suites still catch wrong results (see Testing).

### Testing

All runs used `./run-regression-test.sh --run -d
partition_p0/auto_partition -s test_auto_new_recycle` or `--run -d audit
-s test_sql_digest_generation` against a local single-FE, single-BE
cluster built from master.

| Suite | Version | Result |
|---|---|---|
| `test_auto_new_recycle` | master | passes |
| `test_auto_new_recycle` | this PR | passes; the polls reached the
target after 1, 2, 1, 3 and 3 s |
| `test_auto_new_recycle` | master, with the wait before the
10-partition check cut to 1 s | fails as #65375 describes: `expected:
<101> but was: <10>` (master's `assertEquals` has its arguments swapped,
so 101 partitions were still there) |
| `test_auto_new_recycle` | this PR, expecting 11 partitions, poll
capped at 20 s | fails as it should: `ConditionTimeoutException` |
| `test_sql_digest_generation` | master | passes |
| `test_sql_digest_generation` | this PR | passes; the query took 2,024
and 2,025 ms |
| `test_sql_digest_generation` | this PR, second threshold 1000 ms |
fails as it should: `Count of sql_digest changed` |

The PR NonConcurrent pipeline runs both suites; cloud_p0 skips
`nonConcurrent` suites.

### Notes for Reviewers

- The day-boundary guard before the final `now()`-based section goes
from 100 s to 1300 s, as in #65646, so that it covers the 1200 s poll
plus the insert and checks. It sleeps only when that section would
otherwise start within 1300 s of midnight.
- Four more suites fixed in the same branch-4.1 batch need nothing on
master: #65340 (master #68449 enables the publish debug point on all
FEs, superseding the open port #65556), #65929 (master #68329 fixed the
actual cause, a connection leak), #65885 (master #69723 removed the V1
scanner whose message it also accepted), and #67363 (ported as #67402).

### Release note

None

### Check List (For Author)

- Behavior changed: No
- Does this need documentation: No
- Pre-submission status: Ready

### AI Disclosure

- Tool: Claude Code (Claude Opus 5.5)
- Generated: the port (both suites taken from branch-4.2), the local
verification runs, and this description
- Human review: not stated
- Prompts: The author asked to continue moving branch-4.2's changes back
to master, where six branch-4.1 test fixes were recorded as one task.
After the check showed that master already covers four of them and has
not failed on the other two, the author chose to port those two exactly
as branch-4.2 has them and to verify them on a local cluster.

Co-authored-by: shuke <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants