Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZooKeeper connection pooling #53

Merged
merged 6 commits into from
Oct 31, 2024
Merged

ZooKeeper connection pooling #53

merged 6 commits into from
Oct 31, 2024

Conversation

detro
Copy link
Contributor

@detro detro commented Oct 20, 2024

This makes the operator hold the connection to ZooKeeper in a pool of client.Client, indexed by their construction parameters.

The underlying ZooKeeper client is perfectly able to handle concurrent use, as well as automatic reconnection. So this approach ensures we keep around the minimum amount of connections necessary, and delegate to the underlying client the concurrency.

Additionally, this also stops the resource.ParallelTest provided by Terraform SDKv2, spawn 1 provider for each test execution. Instead, it shares the one instance.

This also solves the issue identified by @abarabash-sift in #49.

detro added 2 commits October 21, 2024 00:19
…eating too many connections

This is necessary because Terraform will invoke the Provider's `ConfigureContextFunc` once for each operation a resource requires.
Invoking `NewClient` in there was leading to spawning a connection each time.
The underlying client DOES SUPPORT concurrent use, so we create a _pool_ where each `Client` is identified by a key made of all it's construction parameters.
@detro detro changed the title Debug connections issue Debug connections issue: ZooKeeper connection pooling Oct 20, 2024
@detro detro force-pushed the detro/debug_connections_issue branch from cc071de to 376f219 Compare October 20, 2024 23:25
@detro
Copy link
Contributor Author

detro commented Oct 20, 2024

@abarabash-sift any chance you want to take a look and see what you think? I did a bit of test and I'm fairly sure this is good. But it's late at night and I'd appreciate a second pair of eyes on this one.

If you can't no worries.

@detro detro changed the title Debug connections issue: ZooKeeper connection pooling ZooKeeper connection pooling Oct 22, 2024
@detro detro force-pushed the detro/debug_connections_issue branch from b9f36ea to 0309b9c Compare October 22, 2024 22:41
@detro detro force-pushed the detro/debug_connections_issue branch 6 times, most recently from 399bf59 to cc3035a Compare October 22, 2024 23:18
@detro detro force-pushed the detro/debug_connections_issue branch 2 times, most recently from 9866451 to bb43814 Compare October 31, 2024 18:23
@detro detro force-pushed the detro/debug_connections_issue branch from bb43814 to 5821625 Compare October 31, 2024 18:25
@detro detro merged commit 3524afd into main Oct 31, 2024
67 checks passed
@detro detro deleted the detro/debug_connections_issue branch October 31, 2024 18:30
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.

1 participant