Coalesce poll events on queue eviction #33
Conversation
Refer: mathieucarbou/ESPAsyncWebServer#165 Let's try to coalesce two (or more) consecutive poll events into one this usually happens with poor implemented user-callbacks that are runs too long and makes poll events to stack in the queue if consecutive user callback for a same connection runs longer that poll time then it will fill the queue with events until it deadlocks. This is a workaround to mitigate such poor designs and won't let other events/connections to starve the task time. It won't be effective if user would run multiple simultaneous long running callbacks due to message interleaving. todo: implement some kind of fair dequeing or (better) simply punish user for a bad designed callbacks by resetting hog connections
For discussion: => mathieucarbou/ESPAsyncWebServer#165 (comment) |
vortigont
force-pushed
the
coalescedq
branch
from
825a2e9 to
045448a
Compare
|
I reworked it. Also added more safeguards on queue operation. This should make a big difference in robustness and congestion handling. |
- do not let `_remove_events_with_arg()` call to block on queue pertubation, otherwise it could deadlock
- in any case do not block on adding LWIP_TCP_POLL event to the queue, it does not make much sense anyway
due to poll events are repetitive
- `_get_async_event()` will discard LWIP_TCP_POLL events when q gets filled up
this will work in combination with throttling poll events when adding to the queue
Poor designed apps and multiple parallel connections could flood the queue with interleaved poll events
that can't be properly throttled or coalesced. So we can discard it in the eviction task after coalescing
It will work in this way:
- queue is up to 1/4 full - all events are entering the queue and serviced
- queue is from 1/4 and up to 3/4 full - new poll events are throttled on enqueue with linear probability
- queue is from 3/4 up to full top - all new poll events are ignored on enqueue and existing poll events
already in the queue are discarded on eviction with linear probability giving away priority for all other
events to be serviced. It is expected that on a new poll timer connection polls could reenter the queue
vortigont
force-pushed
the
coalescedq
branch
from
045448a to
7bfbb94
Compare
I think the WDT is mostly useful for the user ondata callback to make sure the user code runs fast and does not do some delay or long loop tasks. I think it has a value and should be activated by default. |
|
moving to draft until testing is done (see mathieucarbou/ESPAsyncWebServer#171) |
Coalesce poll events on queue eviction
Refer: mathieucarbou/ESPAsyncWebServer#165
Try to coalesce two (or more) consecutive poll events into one.
This usually happens with poor implemented user-callbacks that are runs too long and makes poll events to stack in the queue if consecutive user callback for a same connection runs longer that poll time then it will fill the queue with events until it deadlocks.
This is a workaround to mitigate such poor designs and won't let other events/connections to starve the task time.
It won't be effective if user would run multiple simultaneous long running callbacks due to message interleaving.
queue deadlock and congestion avoidance
do not let
_remove_events_with_arg()call to block on queue pertubation, otherwise it could deadlockin any case do not block on adding LWIP_TCP_POLL event to the queue, it does not make much sense anyway
due to poll events are repetitive
_get_async_event()will discard LWIP_TCP_POLL events when q gets filled upthis will work in combination with throttling poll events when adding to the queue
Poor designed apps and multiple parallel connections could flood the queue with interleaved poll events
that can't be properly throttled or coalesced. So we can discard it in the eviction task after coalescing
It will work in this way:
already in the queue are discarded on eviction with linear-increasing probability giving away priority for all other
events to be serviced. It is expected that on a new poll timer connection polls could reenter the queue