search

scylladb / gocql

Package gocql implements a fast and robust ScyllaDB client for the Go programming language.
https://docs.scylladb.com/stable/using-scylla/drivers/cql-drivers/scylla-go-driver.html
BSD 3-Clause "New" or "Revised" License
169 stars 47 forks source link

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

Closed sylwiaszunejko closed 2 months ago

sylwiaszunejko commented 2 months ago

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 commented 2 months ago

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

mykaul commented 2 months ago

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

sylwiaszunejko commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@sylwiaszunejko - what's the latest here?

sylwiaszunejko commented 2 months ago

@avelanarius could you review it?