[#2404] WIP: Network advisory clean-ups - #2405
Conversation
demandConsumerDispatched is a plain int incremented and reset from multiple transport/executor threads in ackAdvisory. The unsynchronized read-modify-write and unguarded reset let concurrent threads claim overlapping counter ranges in their advisory acks (observed: 800k dispatches acked as 1.16M) or lose counted dispatches outright, corrupting the remote advisory prefetch window. Widens ackAdvisory to package-private for the test.
Replace the plain-int demandConsumerDispatched with an AtomicInteger. The threshold check now claims the observed count via compareAndSet before acking, so concurrent advisory deliveries can neither lose increments nor ack overlapping counter ranges; a losing thread's increment stays counted for a later advisory to claim (ack lag bounded by one message).
… not aborted as slow The bridge's demand advisory consumer batched acks purely by advisoryAckPercentage, so a bridge holding a partial batch below the threshold never acked during quiet periods. New bridge config advisoryAckInterval (default 15000 ms, <= 0 disables) acknowledges pending advisory dispatches once the interval elapses: the
|
Responding to: #2404 (comment) Flushing outstanding acks probably makes sense anyways, but the motivation seems weird to me. (trying to work around stale connections) Anytime you do batching it usually makes sense to have a time limit to flush so you don't wait forever. In fact, I've had to configure bridges to lower the percentage for acking advisories for bridges that are not as busy just so we don't show pending acks for a long time in metrics. I am mostly just curious why you need to use this as a work around for bad connections? Does the inactivity monitor not work for your use case? That's the tool to detect bad connections and to cause resetting if there are no heart beats, so it seems strange to rely on this for that reason. I would think if the connection is bad the broker would stop getting heart beats. |
This hardens network consumers from appearing slow or hung due to outstanding ack's and being unintentionally closed due to slow consumer strategy.