dynamic-window: fix reusing window segments when client exits early#937
Conversation
Signed-off-by: László Várady <[email protected]>
MrAnno
force-pushed
the
dynamic-window-fc-disconnect-rebalance
branch
from
65987be to
17c6bb4
Compare
|
@mitzkia FYI |
Signed-off-by: László Várady <[email protected]>
MrAnno
force-pushed
the
dynamic-window-fc-disconnect-rebalance
branch
from
17c6bb4 to
1465476
Compare
|
Thanks for the fixes. |
bazsi
left a comment
bazsi
left a comment
There was a problem hiding this comment.
Approved. Great to see the additional notes/asserts on proper threading constraints.
Just one nit: maybe we could check if we still have an outstanding dynamic window request at log_source_free(), because we most probably returned the dynamic window when the connection closes (if the empty window allowed that).
With that the latency associated with main_loop_call() will not affect the destination, unless really necessary.
|
Thanks! I will check what I can do about this. Dynamic-pool-related stuffs are not thread safe, and there is a race between the destination thread (acks, free) and main loop actions (realloc, deinit) |
I would assume that a check against 0 should be safe. If it was 0 because of the early release at deinit time, it won't increase back again. But if it's too complex, don't bother. |
Signed-off-by: László Várady <[email protected]>
There is a mechanism that releases the dynamic window for dead sources, but it works on an "all-or-nothing" basis: the dynamic window is only released when the source refcount reaches zero (when all messages have been acked). This is insufficient, because dead sources can retain unused dynamic window capacity for a long time if there is at least one in-flight message in the system. Signed-off-by: László Várady <[email protected]>
Signed-off-by: László Várady <[email protected]>
Signed-off-by: László Várady <[email protected]>
Signed-off-by: László Várady <[email protected]>
Signed-off-by: László Várady <[email protected]>
MrAnno
force-pushed
the
dynamic-window-fc-disconnect-rebalance
branch
from
1465476 to
3d9db43
Compare
|
I added a refactor though, but I didn't add the 0-check unless you want me to make this atomic. |
it's fine this way, approving... |
3 participants
There is a mechanism that releases the dynamic window for dead sources,
but it works on an "all-or-nothing" basis: the dynamic window is only
released when the source refcount reaches zero (when all messages have
been acked).
This is insufficient, because dead sources can retain unused dynamic window
capacity for a long time if there is at least one in-flight message in
the system.