Skip to content

KAFKA-14249: Make TLS idle expiry test deterministic - #22943

Open
lh0156 wants to merge 3 commits into
apache:trunkfrom
lh0156:agent/KAFKA-14249-deterministic-idle-expiry
Open

KAFKA-14249: Make TLS idle expiry test deterministic#22943
lh0156 wants to merge 3 commits into
apache:trunkfrom
lh0156:agent/KAFKA-14249-deterministic-idle-expiry

Conversation

@lh0156

@lh0156 lh0156 commented Jul 26, 2026

Copy link
Copy Markdown

Summary

  • Wait for the channel to be fully ready before testing idle connection expiry.
  • Remove the TLS 1.3-specific flaky-test override so the shared test runs normally for TLS 1.3.

Motivation

Fixes KAFKA-14249.

The idle-expiry test only waited for the TCP connection to reach CONNECTED. With TLS 1.3, the first subsequent poll can still process handshake I/O and refresh the selector's last-active timestamp. The test then misses the expiry deadline and was marked flaky and excluded from normal test execution.

Waiting for READY makes the test precondition match the behavior being tested: no handshake work remains before the idle period starts. This lets the TLS 1.3 variant run as a regular regression test.

Validation

  • Ran the TLS 1.3 selector suite with testCloseOldestConnection included; it passed.
  • Ran SelectorTest, Tls12SelectorTest, and Tls13SelectorTest; all passed.
  • Ran ./gradlew :clients:test --no-build-cache --console=plain successfully.
  • Ran git diff --check successfully.

Reviewers: Gaurav Narula gaurav_narula2@apple.com

@github-actions github-actions Bot added triage PRs from the community tests Test fixes (including flaky tests) clients small Small PRs labels Jul 26, 2026

@gaurav-narula gaurav-narula 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.

Thanks for this. I've a question about post handshake messages in TLS 1.3

@github-actions github-actions Bot removed the triage PRs from the community label Jul 28, 2026
@smjn
smjn requested a review from Copilot July 31, 2026 04:40

Copilot AI 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.

Pull request overview

This PR makes the TLS idle connection expiry test deterministic across TLS versions by ensuring the connection is fully handshake-complete (READY) before the idle timer window begins, and re-enables the TLS 1.3 variant as a normal (non-flaky) regression test.

Changes:

  • In SelectorTest#testCloseOldestConnection, wait for the channel to become READY (not just TCP CONNECTED) before advancing time and asserting idle expiry.
  • Remove the TLS 1.3-only @Flaky override so testCloseOldestConnection runs normally under TLS 1.3.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
clients/src/test/java/org/apache/kafka/common/network/Tls13SelectorTest.java Removes the TLS 1.3 @Flaky override so the shared idle-expiry test executes normally.
clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java Adds a waitForChannelReady precondition to make idle-expiry timing deterministic after TLS handshakes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lh0156

lh0156 commented Aug 2, 2026

Copy link
Copy Markdown
Author

Rebased this PR onto the latest trunk and added Kafka's required Generated-by: OpenAI Codex (GPT-5) trailer to both follow-up commits.

The previous Java 25 full-suite failure was unrelated to this change: SelectorTest.testCloseOldestConnection, Tls12SelectorTest.testCloseOldestConnection, and Tls13SelectorTest.testCloseOldestConnection all passed in that run. After the rebase, the focused tests and clients static checks also pass:

  • SelectorTest.testCloseOldestConnection
  • Tls12SelectorTest.testCloseOldestConnection
  • Tls13SelectorTest.testCloseOldestConnection
  • clients Checkstyle and SpotBugs

lh0156 added 2 commits August 2, 2026 22:24
Generated-by: OpenAI Codex (GPT-5)
@lh0156
lh0156 force-pushed the agent/KAFKA-14249-deterministic-idle-expiry branch from b3fafa7 to db5fa2a Compare August 2, 2026 13:26
@lh0156

lh0156 commented Aug 2, 2026

Copy link
Copy Markdown
Author

Follow-up on the TLS 1.3 post-handshake question:

I rechecked the selector ordering. During pollSelectionKeys, the idle-expiry timestamp is refreshed before the channel read/processing; maybeCloseOldestConnection runs after the selected keys have been processed. Therefore, if a pending TLS 1.3 post-handshake message makes the key readable, it refreshes the connection before the expiry check. The test also waits for TCP connectivity before protocol readiness, so the idle interval starts only after the initial connection has been processed.

The targeted selector suites and full :clients:test passed. No production change is needed for this test scenario. @gaurav-narula, please re-review when convenient.

Exercise an application round trip before advancing the idle timer so TLS 1.3 post-handshake messages cannot race with the expiry assertion.

Generated-by: OpenAI Codex (GPT-5)

Signed-off-by: Yunseop Eom <62834176+lh0156@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-approved clients small Small PRs tests Test fixes (including flaky tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants