python-driver
python-driver copied to clipboard
scylladb
add connect_timeout inside `set_keyspace_blocking`
set_keyspace_blocking is called from places holding a lock, and in case that the connection is closed from the server side, it might hang forever.
using the connect_timeout on it to make sure it
won't hang forever.
Ref: https://github.com/scylladb/python-driver/issues/261
@Lorak-mmk I need you review on this one, at least it's pass the integration test.
And I think it's relatively harmless change
It would be good to have a tests that executes the timeout path - but if it's too hard to write then I think we can skip it.
It would be good to have a tests that executes the timeout path - but if it's too hard to write then I think we can skip it.
we might be able to copy the dtest casing it, but it wasn't 100% of the time...
It would be good to have a tests that executes the timeout path - but if it's too hard to write then I think we can skip it.
it might be a bit tricky, and we are still trying to find the root cause of that issue, so once fixed, that test won't be covering it anymore.
and simulated the situation which we don't know it's exact root case, it kind of futile.
This is actually a backwards-incompatible change, the behavior of the function is different and there might be users depending on current behavior.
I didn't look at the code, but wouldn't it be possible to solve this issue in wait_for_response? Intuitively, I think it should throw an exception when connection is closed.
This is actually a backwards-incompatible change, the behavior of the function is different and there might be users depending on current behavior. I didn't look at the code, but wouldn't it be possible to solve this issue in
wait_for_response? Intuitively, I think it should throw an exception when connection is closed.
That's what one would expect, but some when it works with TLS behaves differently and no part identifies the connection is closed, when TLS is the lower version it doesn't hang there.
We can have higher number, but having queries with no timeout at all seems like a recipe for trouble, especially when there's a lock involved
