Repository navigation
Websocket pipe congestion may cause false-positive timeouts #113
Description
Observations:
When any peer (client or server) doing sendSync with payload/network bandwidth unable to fit options.timeout timeframe — this peer's side timeout occurs.
Ofc handling can be improved — e.g. we can stop timeouts and only rely on network error/close when in progress of big sync.
But then the other side timeout may occur also, because this peer couldn't respond on pingMessage with sendPong — message is queued and can also be not delivered on time.
This could be ok as known limitation with advices like "adjust your timeouts accordingly" or "reduce message sizes", but
the issue can easily occur when network is temporally faulty — queue of messages generated while network is offline/broken is stacked up and then sent as one sendSync payload.
This sometimes can stack up until server-side ws implementation exceeds its 100M message limit —
https://github.com/websockets/ws/blob/0b235e0f9b650b1bdcbdb974cbeaaaa6a0797855/lib/websocket-server.js#L60
then client can never recover and reconnects, server-side WS was closed occurs.
Suggestions:
- Send sync one message at a time or have some limit on how much size per single sync is allowed to group messages in single sync.
- Pause timeouts while doing sync and notify remote peer about incoming sync to do the same (may be partially related to Avoid timeout error on long processing on client/server #100 )
Wdyt?
Activity
ai commentedon Aug 17, 2023
I am right now busy on the work and Nano Stores. Will be able to review thee issue son next week.
erictheswift commentedon Aug 17, 2023
Sure, nothing of it is urgent)
ai commentedon Aug 22, 2023
How big is your message?
We can add something like
logux/big-is-comingmessage (and send it automatically by client or server) which will disable timeout for the node for a few minutes.erictheswift commentedon Dec 19, 2023
Depends on user data. Mostly below 10meg (hard limit for a single message), but cases occur when big ones stack together and then there's reconnect and there's quite a big pile clogging the queue that is sent in one message.
logux/big-is-comingakapause-pingpongswill work for some cases (note that this also must be done for sending side — not only pong can be stuck, but pings are stuck and wait for huge sync queue to be send),however as I said if there's long queue of unsent messages, sending them in 1 batch may be dangerous (e.g. exceeding mentioned
ws100M single-message limit).Also disabling ping-pongs/timeouts will make system less prone to bad networks conditions — otherwise why would we need them in the first place? :)
I'm not deep into logux machinery rn — do you see technical limitations that limit us sending 'sync' with one-message-at-a-time and waiting for
syncedresponse before sending next item/batch (instead of batching all of them together and sending without wait)?If this is flexible, then some smartass strategy can be applied, like 'merge messages in 1 batch until
syncjson length is less than some limit, e.g. 1MB'. That will allow sending big messages 1 at a time & idling send untilsyncedresponse, allowing ping/pongs to pass in between.Ofc that will increase avg processing latency of big messages by the roundtrip+parsing duration, however that's still less than infinity in faulty cases I described above
ai commentedon Dec 19, 2023
It will slow down performance on small messages case.
We can think of changing the strategies.
erictheswift commentedon Dec 19, 2023
Yeah, if we wait for every action/batch
syncedbefore starting sending new one, it's for sure added latency.What if we soft-limit outcoming buffer length? (e.g. 1MB by default)
When total length of sync-in-progress messages (not yet
synced) becomes above configurable threshold, we wait until incoming ACKs make it lower before sending new batches.Combined with soft-limiting sync batch length (let's say half of outcoming buffer length), it should work reliably in intensive data communication, as well as preserve same performance for small messages case.
I guess this way we implement kind of tcp back pressure strategy on application level 😄
Also I think sync batches, if can be measured precisely, should have hard limits — they should never exceed server-side 'ws' message size limit (100mb). By not respecting this limit, following sequence may happen:
[1. connect handshake]
2. send 100+mb in a single message
3. 100mb and a bit of message reaches the server
4.
wsimplementation throwsMax payload size exceeded[WS_ERR_UNSUPPORTED_MESSAGE_LENGTH]5. Connection is closed, not sure if by logux or ws (seems the latter), close code 1009 should reaching client side.
6. logux/client reconnects
7. goto p.1
I'd expect it throw immediately / probably undo the big action above limits or at least make sure 1009 is reaching client and reports this case as fatal afterwards without repeating.
ai commentedon Dec 19, 2023
I like the idea to track size and change mode on message size
ai commentedon Dec 19, 2023
I like this idea of block size.
But I will not have time to implement it right now, so PR is welcome.