Websocket pipe congestion may cause false-positive timeouts

[erictheswift]

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:

1. 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.
2. Pause timeouts while doing sync and notify remote peer about incoming sync to do the same (may be partially related to https://github.com/logux/logux/issues/100 )

Wdyt?

[ai]

I am right now busy on the work and Nano Stores. Will be able to review thee issue son next week.

[erictheswift]

Sure, nothing of it is urgent)

[ai]

How big is your message?

We can add something like logux/big-is-coming message (and send it automatically by client or server) which will disable timeout for the node for a few minutes.

[erictheswift]

How big is your message?

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-coming aka pause-pingpongs will 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 ws 100M 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 synced response 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 sync json length is less than some limit, e.g. 1MB'. That will allow sending big messages 1 at a time & idling send until synced response, 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]

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 synced response before sending next item/batch (instead of batching all of them together and sending without wait)?

It will slow down performance on small messages case.

We can think of changing the strategies.

[erictheswift]

It will slow down performance on small messages case.

Yeah, if we wait for every action/batch synced before 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. ws implementation throws Max 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]

I like the idea to track size and change mode on message size

[ai]

I like this idea of block size.

But I will not have time to implement it right now, so PR is welcome.