Skip to content

Fix flaky TestReadOnly.testConnectionStateNewClient in ZooKeeper 3.9.3 - #1272

Merged
kezhuw merged 1 commit into
apache:masterfrom
kezhuw:fix-flaky-TestReadOnly.testConnectionStateNewClient
Jul 4, 2025
Merged

kezhuw merged 1 commit into
apache:masterfrom
kezhuw:fix-flaky-TestReadOnly.testConnectionStateNewClient

Conversation

@kezhuw

@kezhuw kezhuw commented Jul 3, 2025

Copy link
Copy Markdown
Member

ZOOKEEPER-4508 reports SessionExpired after exhausting session timeout
in session establishment instead of endless retries. It is easy to get
in this now with small session timeout.

Besides above, sessionTimeoutMs should bigger than connectionTimeoutMs,
while default Timing::session() and Timing::connection() are 2s and
10s respectively now.

// from CuratorZookeeperClient

if (sessionTimeoutMs < connectionTimeoutMs) {
    log.warn(
            "session timeout [{}] is less than connection timeout [{}]", sessionTimeoutMs, connectionTimeoutMs);
}

ZOOKEEPER-4508 reports `SessionExpired` after exhausting session timeout
in session establishment instead of endless retries. It is easy to get
in this now with small session timeout.

Besides above, `sessionTimeoutMs` should bigger than `connectionTimeoutMs`,
while default `Timing::session()` and `Timing::connection()` are `2s` and
`10s` respectively now.

```
// from CuratorZookeeperClient

if (sessionTimeoutMs < connectionTimeoutMs) {
    log.warn(
            "session timeout [{}] is less than connection timeout [{}]", sessionTimeoutMs, connectionTimeoutMs);
}
```
@kezhuw
kezhuw requested review from Randgalt, eolivelli and tisonkun July 3, 2025 15:21
@kezhuw

kezhuw commented Jul 3, 2025

Copy link
Copy Markdown
Member Author

See also #1252 (review).

@kezhuw
kezhuw merged commit 7123060 into apache:master Jul 4, 2025
@kezhuw
kezhuw deleted the fix-flaky-TestReadOnly.testConnectionStateNewClient branch August 18, 2025 06:44
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