scylladb / scylla-go-driver

Experimental, high performance Scylla Driver, University of Warsaw students' project
Apache License 2.0
151 stars 7 forks source link

transport: conn, abort OpenShardConn faster #286

Closed Kulezi closed 2 years ago

Kulezi commented 2 years ago

Currently OpenShardConn retries every time it encounters an error, regardless of whether it is context related or not, bloating the test log in case of TestContextsIntegration.

This PR changes the behavior of OpenShardConn, so it checks if context is done before trying to open a new connection, leaving immediately if it's done with appropriate error message.

Relates to #281, solving the context case, another PR will be made resolving the case of whether source port was busy or the error was something else.

Kulezi commented 2 years ago

I think that something different should be done. In general, we should return every error, unless it's either EADDRINUSE or EPERM. For other errors it doesn't really make sense to retry - for example, in case of a timeout it does not make sense because a different port is not better than any other.

Something like that is already done in gocql, see here: https://github.com/scylladb/gocql/blob/e3e27db4440c6a1f0029322f2bb0e266589d9f0f/scylla.go#L631-L633

I included EPERM because it turns out that on Windows some programs with administrator privileges can reserve ports for themselves and Windows returns a WinApi equivalent of EPERM if you try to use it; we had an issue like that in the rust driver: scylladb/scylla-rust-driver#205

Implemented that as part of #295 bugfix PR. Closing this one its favor.