Handle Q.WATCH updates in a single event loop for Websockets
Closes #1153
In PR #1090 Q.WATCH updates were handled by processQwatchUpdates run in a goroutine. Based on this review comment, this PR moves this handling to a single event loop. It
- adds
subscriptionChanandsubscribedClientstoWebsocketServer - runs
listenForSubscriptionsandprocessQwatchUpdatesas part ofWebsocketServer.Run() - shutsdown above two goroutines through both
shutdownChan(called by abort command) andctx.Donechannel
This PR also
- adds
ReadResponsefunction insetup.goto read Q.WATCH updates - adds two more integration tests
TestQWatchWithMultipleClientsandTestQWatchWithMultipleClientsAndQueries - changes key names of all tests in
qwatch_test.goto be unique so that there is no interference with other tests. Since cancelling subscription (Q.UNWATCH) is not supported yet, any following tests that changes the key also triggers Q.WATCH updates. This is a known issue and will be fixed as part of #1154
This PR also fixes following bugs:
- Data races accessing
conn.WriteMessage. Fix: added a mutex - Any error in processing Q.WATCH updates resulted in returning
processQwatchUpdatesgoroutine. Fix: It skips update when it can't process it and logs error. RunWebsocketServerinsetup.gowas relying on both cancelling parent context to shutdownqueryManagergoroutine and abort command to shutdownshardManager. Fix: Made it consistent - everythings is gracefully shutdown by abort command
@lucifercr07 - Please review.
Have we done any benchmarking for the Q.Watch and do we want to check if we are not regressing anything with the above change?
Hey @psrvere could you please resolve the conflicts?
@JyotinderSingh - Resolved. Please review.
Closing this as QWATCH has been deprioritised.