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
181 stars 57 forks source link

fix(dealer): make net.Dialer not to bind to reuse ephimeral ports #181

Closed dkropachev closed 4 months ago

dkropachev commented 4 months ago

net.Dialer makes golang runtime run bind if LocalAddr is not nil, even if address is ":0". https://github.com/golang/go/blob/master/src/net/sock_posix.go#L109-L117

Calling bind, due to it's nature, marks obtained port as non-reusable, which limits number of possible connections/sessions.

It is inevitable for share-aware connections, since we need to pick the port, but for none-shard-aware connections it must be avoided.

Closes #179

roydahan commented 4 months ago

@sylwiaszunejko do we need another maintainer reviewing it? If so, please get someone to review and merge.

Also, please make sure we have tests to detect such an issue (looks like we don't).

sylwiaszunejko commented 4 months ago

@avelanarius Could you look at it to make sure it LGTM?

sylwiaszunejko commented 4 months ago

@dkropachev I was wondering how to write a test for this, but I am not sure. I was trying to use the issue reproducer, but without fmt.Println("Routine:", routine, "Try:", try) the issue stopped reproducing and with it or with analogues logging statement there was a error like that: testing.go:1465: race detected during execution of test. Do you have any ideas how to minimize reproducer or how to test this change some other way?

dkropachev commented 4 months ago

@dkropachev I was wondering how to write a test for this, but I am not sure. I was trying to use the issue reproducer, but without fmt.Println("Routine:", routine, "Try:", try) the issue stopped reproducing and with it or with analogues logging statement there was a error like that: testing.go:1465: race detected during execution of test. Do you have any ideas how to minimize reproducer or how to test this change some other way?

There is no good way of doing it. We could have an interface with something like :

DialContext(ctx context.Context, network, addr string) (net.Conn, error)
DialContextWithPort(ctx context.Context, network, addr string, sport uint16) (net.Conn, error)

And then mock it on test, but realy, this test won't acheve anything. To me better just not test it at all

sylwiaszunejko commented 4 months ago

@roydahan FYI