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

Add support for tcp keepalive #100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

irevoire
Copy link

@irevoire irevoire commented Oct 25, 2023

Hey, we have a lot of issues with this crate disconnecting from the network and making the whole process crash, basically.
It's strange because we didn't have any issues at the time when we were using https://docs.rs/zookeeper-client/latest/zookeeper_client/
Do you know what could be the culprit?
We're running with the exact same zookeeper version.

I thought adding support for tcp-keepalive could maybe help, but I doubt it.
Fix #92

Also, since we establishes the connection straight away in the new and don't use a builder or something, I had to introduce a new parameter in the new function which is a breaking change 🥺

@bonifaido
Copy link
Owner

Hi @irevoire, thank you for the work in this PR, huge help!

TBH I have no idea what could be the issue, I'm not familiar with zookeeper_client. But I'm curious why have you moved away from it to this crate?

@irevoire
Copy link
Author

But I'm curious why have you moved away from it to this crate?

We wanted a sync client and that's the only one available I think 🤔

@bonifaido
Copy link
Owner

No problems with the break, I think we can bump another "major" minor version here. Or creating another constructor with keepalive or more settings (like a config struct, I'm not sure how Rusty is this)?

@irevoire
Copy link
Author

irevoire commented Nov 2, 2023

No problems with the break, I think we can bump another "major" minor version here. Or creating another constructor with keepalive or more settings (like a config struct, I'm not sure how Rusty is this)?

I think the most common thing is to have a builder, but tbh, I won't have the time to do it

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.

please let zookeeper io socket set tcp keepalive
2 participants