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

Send heartbeat OPTIONS message less frequent and enable keep alive #169

Merged
merged 2 commits into from
May 9, 2024

Conversation

sylwiaszunejko
Copy link
Collaborator

Now we are sending OPTIONS message on every connection every 5 seconds.

This PR changes that to 30 seconds (this is the number used in python-driver - https://github.com/scylladb/python-driver/blob/7cdc1ca25ffa2194a0772e46ccd89645f8514c1b/cassandra/cluster.py#L893) and enables the keepalive - with 15s by default (matches Golang's default).

Fixes: #162

@mykaul
Copy link

mykaul commented Apr 25, 2024

If we have TCP keepalive, why do we need the application level OPTIONS as well? (and vice versa of course).

@mykaul
Copy link

mykaul commented Apr 25, 2024

Should the fix be sent to https://github.com/gocql/gocql first?

@sylwiaszunejko
Copy link
Collaborator Author

Should the fix be sent to https://github.com/gocql/gocql first?

Not sure about that. It looks like the upstream is not actively maintained, and this change is not something that might make merging upstream difficult in the future.

@roydahan
Copy link
Collaborator

Should the fix be sent to https://github.com/gocql/gocql first?

Not sure about that. It looks like the upstream is not actively maintained, and this change is not something that might make merging upstream difficult in the future.

I agree.
Normally, such general changes are better to go first to the upstream and then get them during the upstream merge, but in the current situation of upstream it can wait there unnoticed for months.

@mykaul
Copy link

mykaul commented May 6, 2024

@sylwiaszunejko - what's the latest here?

@sylwiaszunejko
Copy link
Collaborator Author

@avelanarius could you review it?

@avelanarius avelanarius merged commit 46c0e71 into scylladb:master May 9, 2024
1 check passed
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.

Heartbeat (via OPTIONS frame) is sent every 5 seconds - too low
4 participants