Do not choose less busy connection if query is LWT

[sylwiaszunejko]

There is a code in gocql that selects another connection to a node if chosen connection is too busy (has less than half available stream ids).

We should not do this with LWT.

[sylwiaszunejko]

I thought it might be better to have Pick(Token, ExecutableQuery) *Conn interface instead of Pick(Token, string, string, bool) *Conn but I had issues with rewriting scylla_test.go with that version so I decided to first go with the simpler solution and ask about second opinion.

[Lorak-mmk]

Out of all the LWT fixes this one is the least urgent, imo we can do it properly. What are the problems that you faced?

[sylwiaszunejko]

Out of all the LWT fixes this one is the least urgent, imo we can do it properly. What are the problems that you faced?

I just wasn't sure how to mock ExecutableQuery for the test purpose, but I guess I could just implement something like MockQuery and implement all of the methods as empty (returning nil, "", false etc.) to have .Keyspace(), .Table() and IsLWT() for the purpose of Pick (e.g. here: https://github.com/scylladb/gocql/blob/master/scylla_test.go#L27)

[Lorak-mmk]

One thing to consider; is type ConnPicker interface something that the user may implement?
Because this PR changes it's definition, so if it is exposed to the user (I'm not sure how that works in Go) then this is a breaking change

[dkropachev]

There is a code in gocql that selects another connection to a node if chosen connection is too busy (has less than half available stream ids).

We should not do this with LWT.

@sylwiaszunejko , why we should not do that with LTW queries ? is there core issue on that or something that have details on reasoning ?

[dkropachev]

There is a code in gocql that selects another connection to a node if chosen connection is too busy (has less than half available stream ids).
We should not do this with LWT.

@sylwiaszunejko , why we should not do that with LTW queries ? is there core issue on that or something that have details on reasoning ?

I think I found answer, please correct me if I am wrong, but related issues are:
https://github.com/scylladb/scylla-enterprise/issues/4369
#201

[dkropachev]

Out of all the LWT fixes this one is the least urgent, imo we can do it properly. What are the problems that you faced?

I just wasn't sure how to mock ExecutableQuery for the test purpose, but I guess I could just implement something like MockQuery and implement all of the methods as empty (returning nil, "", false etc.) to have .Keyspace(), .Table() and IsLWT() for the purpose of Pick (e.g. here: https://github.com/scylladb/gocql/blob/master/scylla_test.go#L27)

I guess that in most cases you can just pass nil

[dkropachev]

One thing to consider; is type ConnPicker interface something that the user may implement? Because this PR changes it's definition, so if it is exposed to the user (I'm not sure how that works in Go) then this is a breaking change

Greate idea, but it would be better to address it in separate PR since not only connpicker is currently not a publicly availalbe, but also we will have to somehow provide API that would suggest to use certain conn picker in certain conditions, or, maybe have one connpicker to handle all the cases.

[dkropachev]

@sylwiaszunejko, I made attempt to make Pick(Token, ExecutableQuery), please take a look.

[Lorak-mmk]

One thing to consider; is type ConnPicker interface something that the user may implement? Because this PR changes it's definition, so if it is exposed to the user (I'm not sure how that works in Go) then this is a breaking change

Greate idea, but it would be better to address it in separate PR since not only connpicker is currently not a publicly availalbe, but also we will have to somehow provide API that would suggest to use certain conn picker in certain conditions, or, maybe have one connpicker to handle all the cases.

I wasnt suggesting to make it publicly available. If it isn't - great, we don't have to worry about breaking changes.

[sylwiaszunejko]

@dkropachev I used the code proposed by you just changed two lines to simplify