Skip to content

Conversation

@lhotari
Copy link
Member

@lhotari lhotari commented Apr 10, 2025

Motivation

After #23988, MockZooKeeper is correctly single threaded and an injected delay will block the execution of this code in ZKSessionWatcher:

try {
zkClientState = future.get(tickTimeMillis, TimeUnit.MILLISECONDS);
} catch (TimeoutException e) {
// Consider zk disconnection if zk operation takes more than TICK_TIME
zkClientState = Watcher.Event.KeeperState.Disconnected;
}
checkState(zkClientState);

This will trigger a ConnectionLost session event. That's why a delay should be kept under 2000 ms.
Currently MockZooKeeper's session timeout is hard coded to 30000ms and the check interval is 1/15 of this, therefore 2000ms.

When there's a test that blocks the ZooKeeper thread for more than 2000ms, it will result in a ConnectionLost event.
This PR makes the session timeout configurable and it's possible to increase the session timeout for tests where it's necessary to use a longer session timeout to avoid ConnectionLost session events.

Modifications

  • refactor the code in PulsarTestContext to postpone creation of the instances after the configuration including the configured session timeout is available
  • also configure other MetadataStoreConfig settings and pass them from the ServiceConfiguration instance

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Copy link
Member

@nodece nodece left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Apr 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.26%. Comparing base (bbc6224) to head (bf0ba41).
Report is 1021 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24171      +/-   ##
============================================
+ Coverage     73.57%   74.26%   +0.69%     
+ Complexity    32624    32074     -550     
============================================
  Files          1877     1864      -13     
  Lines        139502   144517    +5015     
  Branches      15299    16492    +1193     
============================================
+ Hits         102638   107327    +4689     
+ Misses        28908    28727     -181     
- Partials       7956     8463     +507     
Flag Coverage Δ
inttests 26.78% <ø> (+2.20%) ⬆️
systests 23.18% <ø> (-1.14%) ⬇️
unittests 73.76% <ø> (+0.91%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1069 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lhotari lhotari merged commit 4c85b47 into apache:master Apr 11, 2025
52 checks passed
poorbarcode pushed a commit to poorbarcode/pulsar that referenced this pull request Apr 15, 2025
lhotari added a commit that referenced this pull request Apr 17, 2025
…TestZKServer in PulsarTestContext (#24171)

(cherry picked from commit 4c85b47)
lhotari added a commit that referenced this pull request Apr 17, 2025
…TestZKServer in PulsarTestContext (#24171)

(cherry picked from commit 4c85b47)
lhotari added a commit that referenced this pull request Apr 17, 2025
…TestZKServer in PulsarTestContext (#24171)

(cherry picked from commit 4c85b47)
nodece pushed a commit to nodece/pulsar that referenced this pull request Apr 22, 2025
…TestZKServer in PulsarTestContext (apache#24171)

(cherry picked from commit 4c85b47)
ganesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 24, 2025
…TestZKServer in PulsarTestContext (apache#24171)

(cherry picked from commit 4c85b47)
(cherry picked from commit de4f06b)
ganesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 24, 2025
…TestZKServer in PulsarTestContext (apache#24171)

(cherry picked from commit 4c85b47)
(cherry picked from commit de4f06b)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 27, 2025
…TestZKServer in PulsarTestContext (apache#24171)

(cherry picked from commit 4c85b47)
(cherry picked from commit de4f06b)
ganesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 30, 2025
…TestZKServer in PulsarTestContext (apache#24171)

(cherry picked from commit 4c85b47)
(cherry picked from commit de4f06b)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request May 2, 2025
…TestZKServer in PulsarTestContext (apache#24171)

(cherry picked from commit 4c85b47)
(cherry picked from commit de4f06b)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request May 2, 2025
…TestZKServer in PulsarTestContext (apache#24171)

(cherry picked from commit 4c85b47)
(cherry picked from commit eb7ba99)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 6, 2025
…TestZKServer in PulsarTestContext (apache#24171)

(cherry picked from commit 4c85b47)
(cherry picked from commit eb7ba99)
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants