iconara / cql-rb

Cassandra CQL 3 binary protocol driver for Ruby
106 stars 31 forks source link

Tweaks to reduce time holding locks #128

Closed grddev closed 8 years ago

grddev commented 8 years ago

See individual commits for details.

iconara commented 8 years ago

What's the worst case scenario visibility-wise with this change? Can we be sure that all threads will eventually see new connections, and that no threads will use closed connections? (and if they do see closed connections that that will not be a problem)

grddev commented 8 years ago

What's the worst case scenario visibility-wise with this change?

Assuming you are talking specifically about the connection-manager change, I think the main difference lies in the each-method, where you previously could not call any methods from the connection manager from the callback, as that would result in a thread-error, whereas the new code allows retrieving a possibly inconsistent state from this callback. Obviously, connected, snapshot and random connection can return stale results now, but nothing prevented the results from becoming stale once the method returned before, so I'm not sure that distinction matters. Given that we still protect the instance variable access with the lock, I think the updated list should be visible to all other threads within reasonable time, and thus the only thing preventing new connections from appearing ought to be holding on to the lock forever, which was certainly possible before (with each), but shouldn't be possible now.