Repository navigation
[fix](test) Reuse the suite connection when polling lower-case tables - #68329
Merged
morningman merged 1 commit intoSep 21, 2026
Merged
Conversation
### What problem does this PR solve? Related PR: apache#68101 Separate Awaitility polling threads create thread-local JDBC connections that suite-thread cleanup cannot close, eventually exhausting the user's connection quota. Use the existing same-thread awaitUntil helper and explain the connection ownership invariant next to the wait. ### Release note None ### Check List (For Author) - Test: Manual red/green ownership probe with the actual Groovy wait closure and framework helper; full-case Groovy compilation; git diff --check. - Behavior changed: No production behavior change. - Does this need documentation: No.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Static review conclusion: no blocking issue found at head 5fb3da8.
Critical checkpoint conclusions:
- Goal and proof: The change replaces default Awaitility worker-thread polling with the existing same-thread awaitUntil helper. All 48 wait_table_sync calls now reuse the suite executor thread JDBC connection, addressing the reported per-wait ThreadLocal connection leak. The existing suite remains the end-to-end functional coverage and no expected output changes.
- Scope and clarity: The patch is focused: one helper call, one ownership comment, and removal of three unused imports.
- Concurrency: ScriptContext runs the suite body in one suite-executor task; awaitUntil polls on that caller; sql obtains the SuiteContext ThreadLocal connection on the same thread. No shared mutable state, lock, lock-order, or deadlock concern is introduced.
- Lifecycle: SuiteContext start wraps the suite task in closeable cleanup, so closeThreadLocal removes and closes the reused JDBC connection on normal completion and propagated timeout or failure. Statement and result resources remain scoped by JdbcUtils.
- Configuration and compatibility: No configuration, FE-BE variable, protocol, function symbol, storage format, rolling-upgrade, cloud/shared-nothing, or production behavior changes. No parallel product path needs a matching change.
- Conditions and errors: The ten-second at-most setting, one-second interval, predicate, and caught SQL-exception behavior are preserved. Same-thread polling cannot preempt one blocked JDBC call, which is the established framework-helper tradeoff and is not a distinct regression for this suite, which also performs direct JDBC calls.
- Tests and results: No result file changes are needed because query expectations are unchanged. The author reports full-case Groovy compilation, a manual red/green ownership probe, and git diff check; I did not run builds or tests because this review environment prohibits them. Full external-regression execution therefore remains CI validation, not independently established here.
- Observability: Existing suite failure and exception output remain sufficient for this test-only lifecycle correction; no metric or production log change is warranted.
- Persistence, transactions, and data writes: No product persistence, EditLog, transaction, visibility, atomicity, or data-write implementation is changed.
- Performance: Poll frequency is unchanged while redundant polling threads and per-wait JDBC connections are eliminated.
- Additional focus: No user-specific focus was provided. The complete PR was reviewed normally.
Review completion: The required normal full-review and separate risk-focused pass both returned NO_NEW_VALUABLE_FINDINGS. Every initial risk was independently resolved with code evidence, the sole changed file received a final sweep, and the live duplicate fence contained no existing inline thread. No inline comments are proposed.
morningman
approved these changes
Sep 21, 2026
Contributor
|
PR approved by anyone and no changes requested. |
github-actions
Bot
added
reviewed
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
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 <shuke@selectdb.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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 problem does this PR solve?
Related PR: #68101
test_lower_case_meta_with_lower_table_conf_show_and_selectcan exhaust the test user's connection quota while waiting for tables. Each independent Awaitility wait creates a polling thread whose JDBC connection is stored inSuiteContext.threadLocalConn; suite-thread cleanup cannot close those connections. In build 1053466, the polling thread repeatedly receivedReach limit of connectionsbefore the case reported a 10-second metadata wait timeout.Use the existing
awaitUntil(10, 1)helper, which polls on the suite thread and reuses its JDBC connection. Add a comment explaining the connection ownership requirement and remove the unused imports. This preserves the polling condition, interval and configured wait budget without increasing connection limits.Release note
None
Check List (For Author)
Validation:
awaitUntilimplementation with Awaitility 4.2.1, using thread-local stand-ins for JDBC connections. Three waits before the fix created three resources on three polling threads, none reachable by suite-thread cleanup; the same check after the fix created one resource on the suite thread and cleaned it up. The ownership assertion failed before the fix and passed after it.git diff --checkpassed.