chore: increase topics reconnect delay to 500ms #1431
Conversation
| @@ -260,7 +260,7 @@ export abstract class AbstractPubsubClient<TGrpcError> | |||
| // transparently restart the stream instead of propagating an error. | |||
| if (shouldReconnectSubscription) { | |||
| options.restartedDueToError = true; | |||
| const reconnectDelayMillis = 100; | |||
| const reconnectDelayMillis = 5000; | |||
There was a problem hiding this comment.
I think let's be careful not to conflate 2 things here.
Thing 1: what is a good value to use here for UX. It might be higher than 100 but it's probably less than 5000.
Thing 2: regardless of connect/disconnect status, you will only see a discontinuity if you miss messages. The server buffers the last 5 seconds worth of messages, so if you reconnect in less than 5s, you won't get one. But I think that for your demo purposes if you leave the connection blocked for more than 5s, you will get a discontinuity even if this reconnect attempt is happening every 100ms?
There was a problem hiding this comment.
ah yeah you're right, I forgot about the 5s buffer 🙃
do you think it would still be worthwhile to increase the delay from 100 to maybe 1000?
otherwise I can just close this pr
There was a problem hiding this comment.
i think we should start a thread about that in slack. i can seed it.
anitarua
changed the title
Previously, discontinuities were never shown. Increases the delay for attempting to reconnect to 5 seconds to match the go sdk, which did show discontinuities after resubscribing.I think increasing the delay gives the server a little more breathing room to figure out that a reconnect had occurred and to send a discontinuity message to indicate the stream may have been interrupted.Just increases delay for resubscribing to 500ms.
Discontinuities will be seen for network interruptions longer than the server message buffer (currently 5s).