[fix](regression) Avoid midnight race in auto recycle case by shuke987 · Pull Request #65435 · apache/doris

shuke987 · 2026-07-10T02:39:38Z

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.

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
- 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:
- No.
- Yes.
Does this need documentation?
- No.
- Yes.

Check List (For Reviewer who merge this PR)

Confirm the release note
Confirm test cases
Confirm document
Add branch pick label

hello-stephen · 2026-07-10T02:39:42Z

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:

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

shuke987 · 2026-07-10T02:39:47Z

run buildall

shuke987 · 2026-07-10T02:39:49Z

/review

github-actions

Automated review found one issue that should be fixed before merge.

Critical checkpoint conclusions:

Goal/test proof: the PR addresses the midnight race by removing the now()-based data, but it also removes coverage for the current-day partition boundary that the old assertion covered.
Scope: one regression-test file; the change is focused.
Concurrency/lifecycle/config/persistence/data-write protocol: no product code changes; the existing scheduler/config behavior was inspected for review scope.
Compatibility/parallel paths: no FE/BE protocol or persisted format changes.
Tests/results: git diff --check 68ec8d084990de36f4dc6835479fc9cec0283f45..HEAD passed. I did not run the regression suite because this runner has no initialized Doris output/thirdparty/protoc environment.

User focus: no additional user-provided focus.

Subagent conclusions: both tests-session-config and optimizer-rewrite reported the same current-day boundary coverage gap; it was merged into one inline comment. Convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the final ledger/comment set.

github-actions · 2026-07-10T02:52:22Z

+        insert into auto_recycle
+        select date_add('1900-01-01 00:00:00', interval number day) from numbers("number" = "5")
+        union all
+        select date_add('2900-01-01 00:00:00', interval number day) from numbers("number" = "3");


This removes the current-day boundary from the test. The old now() + [-5, 2] data covered five historical partitions plus the partition containing the scheduler's current time and two future partitions, but the replacement only has 1900 historical partitions and 2900 far-future partitions. In getDropPartitionOpForAutoPartition, the boundary being protected is upperBound.compareTo(currentTimeKey) < 0, so a current-day partition is different from a far-future one: its lower bound is already before currentTimeKey, and only its upper bound keeps it out of the drop set. With this rewrite, a regression that classifies the current-day partition as history could still pass while preserving the 2900 partitions. Please keep the fixed far-future rows, but add deterministic coverage for the partition containing the scheduler's current time as well.

shuke987 · 2026-07-10T03:11:00Z

/review

github-actions

Automated review completed for PR 65435 at head 845856c.

I found no additional inline issues to raise on the current patch.

Checkpoint conclusions:

Goal and coverage: the current head keeps the original now() + [-5, 2] data shape for the auto-partition recycle regression, so it still covers historical, current-day, and future partitions while moving the day-boundary wait to the only time-sensitive insert.
Scope: the net current change is focused on this regression test and does not change production FE/BE behavior.
Concurrency/lifecycle/config: the touched path depends on the FE dynamic partition scheduler interval. The existing mutable config callback updates the daemon interval, and the test keeps the existing 8 second waits around interval changes; no new production concurrency or lifecycle path is introduced.
Compatibility/persistence/protocol: not applicable; this is a regression-test-only change.
Parallel paths: the relevant recycle path is DynamicPartitionScheduler.getDropPartitionOpForAutoPartition, whose current/future boundary is based on upperBound.compareTo(currentTimeKey) < 0; the restored now() range continues to exercise that boundary.
Test result review: no .out update is needed for the changed section because it uses assertions rather than qt_* output.
Style/basic CI: the current diff adds no trailing whitespace and restores the final newline.

User focus: no additional user-provided review focus.

Subagent conclusions:

optimizer-rewrite: initial pass and final convergence both returned NO_NEW_VALUABLE_FINDINGS.
tests-session-config: initial pass and final convergence both returned NO_NEW_VALUABLE_FINDINGS.
The only existing inline thread was against the first patch version that used fixed 1900/2900 dates; the current head addressed it by restoring current-day coverage.

Validation limits: static review only. This GitHub Actions checkout is missing thirdparty/installed and thirdparty/installed/bin/protoc, so I did not run the Doris regression suite locally.

shuke987 · 2026-07-15T08:34:41Z

skip buildall

github-actions · 2026-07-15T08:41:00Z

PR approved by at least one committer and no changes requested.

dismiss

#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

) ### 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.

@shuke987

…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 <shuke@selectdb.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

fix(regression): avoid midnight race in auto recycle case

da31413

github-actions Bot previously requested changes Jul 10, 2026

View reviewed changes

fix(regression): preserve current partition coverage

845856c

github-actions Bot reviewed Jul 10, 2026

View reviewed changes

hello-stephen approved these changes Jul 15, 2026

View reviewed changes

github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jul 15, 2026

hello-stephen merged commit 77cf85a into apache:master Jul 15, 2026
35 checks passed

shuke987 mentioned this pull request Jul 15, 2026

[fix](regression) branch-4.1: avoid midnight race in auto recycle case #65646

Merged

3 of 16 tasks

morningman mentioned this pull request Nov 5, 2026

[test](regression) Stabilize test_auto_new_recycle and test_sql_digest_generation #70308

Merged

Navigation Menu

Repository navigation

[fix](regression) Avoid midnight race in auto recycle case - #65435

[fix](regression) Avoid midnight race in auto recycle case#65435
hello-stephen merged 2 commits into
apache:masterfrom
shuke987:fix-master-auto-new-recycle-midnight

shuke987 commented Jul 10, 2026 •

edited

Loading

Uh oh!

hello-stephen commented Jul 10, 2026

Uh oh!

shuke987 commented Jul 10, 2026

Uh oh!

shuke987 commented Jul 10, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Jul 10, 2026

Uh oh!

shuke987 commented Jul 10, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

shuke987 commented Jul 15, 2026

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

shuke987 commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

Release note

Check List (For Author)

Check List (For Reviewer who merge this PR)

Uh oh!

hello-stephen commented Jul 10, 2026

Uh oh!

shuke987 commented Jul 10, 2026

Uh oh!

shuke987 commented Jul 10, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

shuke987 commented Jul 10, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

shuke987 commented Jul 15, 2026

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shuke987 commented Jul 10, 2026 •

edited

Loading