[jira] [Comment Edited] (ARTEMIS-6222) Slow consumer detection false positive after idle periods

["Benjamin Holewienko (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/ARTEMIS-6222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18126791#comment-18126791 ] 

Benjamin Holewienko edited comment on ARTEMIS-6222 at 10/13/26 4:08 PM:
------------------------------------------------------------------------

sure! bholewienko works as the name and [benjamin.holewienko@cwan.com|mailto:benjamin.holewienko@cwan.com] works as the email. Thanks for putting in the fix! 


was (Author: JIRAUSER314529):
sure! bholewienko works as the name and [benjamin.holewienko@cwan.com|mailto:benjamin.holewienko@cwan.com] works as the email. Thanks for the fix! 

> Slow consumer detection false positive after idle periods
> ---------------------------------------------------------
>
>                 Key: ARTEMIS-6222
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-6222
>             Project: Artemis
>          Issue Type: Bug
>          Components: Broker
>    Affects Versions: 2.19.1, 2.58.0
>            Reporter: Benjamin Holewienko
>            Priority: Minor
>              Labels: pull-request-available
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> Related to ARTEMIS-1011 ("Slow consumer detection - producer msg/s rate for queue should take into account messages which are already in queue"), fixed in 2019 (commit ef2ec7b). That fix addressed a false _negative_: a queue-level guard in {{QueueImpl.SlowConsumerReaperRunnable.run()}} that could permanently skip inspecting consumers if a producer stopped sending after leaving an undrained backlog. The fix added a {{queueMessages < queueThreshold}} check alongside the existing {{queueRate < queueThreshold}} check, so a queue sitting on a real backlog gets inspected even with zero new arrivals.
> This ticket is the false-_positive_ counterpart, in a different method that 1011 didn't touch.
> h3. The bug
> {{ServerConsumerImpl#getRate()}} computes:
> {code:java}
> public float getRate() {
>    float timeSlice = (System.currentTimeMillis() - consumerRateCheckTime.getAndSet(System.currentTimeMillis())) / 1000.0f;
>    long acks = metrics.getMessagesAcknowledged();
>    if (timeSlice == 0) { messageConsumedSnapshot.getAndSet(acks); return 0.0f; }
>    return BigDecimal.valueOf((acks - messageConsumedSnapshot.getAndSet(acks)) / timeSlice)...;
> }
> {code}
> This is a since-last-invocation snapshot with no cap on {{timeSlice}}. It is only ever invoked from inside {{SlowConsumerReaperRunnable.run()}}'s per-consumer loop, which itself only runs when the queue-level guard ({{queueRate < queueThreshold && queueMessages < queueThreshold}}) does *not* skip the cycle. On a queue that is legitimately idle for several consecutive check periods — no messages, no consumers doing anything wrong — that guard correctly skips every cycle, which means {{ServerConsumerImpl#getRate()}} is never called during the idle stretch, and its internal clock ({{consumerRateCheckTime}}, {{messageConsumedSnapshot}}) goes stale.
> The first time a burst arrives and pushes {{queueMessages}} above the threshold, the guard finally lets the per-consumer loop run — but {{getRate()}}'s {{timeSlice}} now spans the _entire_ preceding idle period, not just the current check period. A consumer that drains the burst essentially instantly can still compute a rate far below the configured threshold, purely because the denominator is inflated by idle time it was never unfair to sit through in the first place. The longer the queue was quiet beforehand, the larger a burst has to be just to survive — with no upper bound.
> h3. Reproduction (verified against current {{main}})
> Added {{SlowConsumerTest#testSlowConsumerSparedAfterIdleThenBurst}}: connect a consumer to an empty queue, let it sit idle across 5 check periods, then send {{3 * threshold + 1}} messages and drain all but one of them immediately. Asserts the consumer survives ({{receiveImmediate()}} returns the last message rather than throwing {{ActiveMQObjectClosedException}}).
> * *Against unpatched {{QueueImpl.java}}:* fails — {{AMQ219017: Consumer is closed}}. The consumer is killed despite having done nothing slow.
> * *Against the patched version below:* passes.
> * *Full {{SlowConsumerTest}} class against the patch:* 17/17 pass (16 pre-existing tests + the new one) — no regressions.
> h3. Proposed fix
> Minimal, contained to the existing "skip" branch — does not change behavior when the guard lets the cycle proceed, and does not add the {{getMessageCount()}} locking cost Justin Bertram flagged during the 1011 discussion, since that call already happens unconditionally at the top of {{run()}} today. The only new unconditional call is {{ServerConsumerImpl#getRate()}} itself, which is a cheap, non-locking {{AtomicLong}} operation:
> {code:java}
> if (queueRate < queueThreshold && queueMessages < queueThreshold) {
>    // Still touch each consumer's rate so ServerConsumerImpl#getRate()'s internal clock never
>    // accumulates dilution across skipped reaper cycles -- otherwise a consumer that sat idle
>    // (fairly, per this same guard) for several check periods and then drains a fresh burst well
>    // within a single period can still be measured against the entire preceding idle stretch and
>    // incorrectly flagged as slow.
>    for (ConsumerHolder consumerHolder : consumers) {
>       Consumer consumer = consumerHolder.consumer();
>       if (consumer instanceof ServerConsumerImpl serverConsumer) {
>          serverConsumer.getRate();
>       }
>    }
>    logger.debug("Insufficient messages received on queue \"{}\" to satisfy slow-consumer-threshold. Skipping inspection of consumer.", queueConfiguration.getName());
>    return;
> }
> {code}
> h3. Affected versions
> Confirmed present on current {{main}} (2.57.0-SNAPSHOT at time of writing) and in 2.19.1.1. The underlying method has been present in this form since before the 1011 fix landed (2019), and 1011 never touched it, so this likely affects every release since.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@artemis.apache.org
For additional commands, e-mail: issues-help@artemis.apache.org