Skip to content

[fix](test) Reuse the suite connection when polling lower-case tables - #68329

Open
Gabriel39 wants to merge 1 commit into
apache:masterfrom
Gabriel39:fix/lower-case-wait-connection
Open

Gabriel39 wants to merge 1 commit into
apache:masterfrom
Gabriel39:fix/lower-case-wait-connection

Conversation

@Gabriel39

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Related PR: #68101

test_lower_case_meta_with_lower_table_conf_show_and_select can exhaust the test user's connection quota while waiting for tables. Each independent Awaitility wait creates a polling thread whose JDBC connection is stored in SuiteContext.threadLocalConn; suite-thread cleanup cannot close those connections. In build 1053466, the polling thread repeatedly received Reach limit of connections before 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)

  • Test
    • Manual test (details below)
    • Regression test on a running Doris cluster
  • Behavior changed:
    • No production behavior change; test polling reuses the suite connection.
  • Does this need documentation?
    • No.

Validation:

  • Compiled the complete Groovy case with Groovy 3.0.8.
  • Evaluated the actual case wait closure and the framework's actual awaitUntil implementation 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 --check passed.
  • Full external regression remains to be validated in CI; the local ownership check does not establish end-to-end cluster stability.

### 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.
@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?

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

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.

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.

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.

2 participants