[ https://issues.apache.org/jira/browse/ARTEMIS-1011?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15893884#comment-15893884 ]
Miroslav Novak commented on ARTEMIS-1011: ----------------------------------------- > I'm not clear why you need the AtomicLong messageCountSnapshot. Couldn't you > just invoke getMessageCount()? Only got inspired by {{messagesAddedSnapshot.getAndSet(locaMessageAdded))}}. It just makes code nicer because of the getAndSet() method. Otherwise I don't see reason to use AtomicLong for messageCountSnapshot and messagesAddedSnapshot. > Invoking getMessageCount() will lock the queue and therefore negatively > impact performance for high-throughput use-cases. We may want to add some > kind of optimization to getRate() to avoid that call if at all possible or > perhaps avoiding the call to getRate() unless absolutely necessary. Good point. Just a quick idea. Producer msg/s rate could be calculated just from messageAdded as it was until now. If it does not meet the condition in if statement in SlowConsumerReaperRunnable.run() line 3135 {{} else if (queueRate < (threshold * consumersSet.size())) {}} then we would calculate producer msg/s again from messageAdded and messageCount as suggested in description. This should avoid calling getMessageCount() when there is a high load. I assume here that high load means that there are lots of added messages. > This changes the semantics of the method such that it no longer returns the > rate of message production on the queue between invocations so it would > probably be good to rename the method to something more accurate. Yes :-) Test scenario could be like: * Start server with slow consumer policy set to KILL, stresshold 10 msg/s, check period 5 seconds * Send 100 messages to queue, wait 5 seconds * Start slow consumer - consumes messages in rate 1 msg/s Pass Criteria: Consumer was disconnected. This test will fail now. > Slow consumer detection - producer msg/s rate for queue should take into > account messages which are already in queue > -------------------------------------------------------------------------------------------------------------------- > > Key: ARTEMIS-1011 > URL: https://issues.apache.org/jira/browse/ARTEMIS-1011 > Project: ActiveMQ Artemis > Issue Type: Bug > Components: Broker > Affects Versions: 1.5.3 > Reporter: Miroslav Novak > Fix For: 2.0.0, 1.5.4 > > > There is still a problem how producer msg/s rate is calculated in > {{QueueImpl.getRate()}} for slow consumer detection. It calculates only > messages added during the last slow consumer check period. As this is used to > figure out, in which msg/s rate the queue could serve the consumer then it > should also take into account messages which are already in queue at the > start of queueRateCheckTime period. > Current implementation is problem for cases when messages are sent to queue > in bursts, for example producer sends 1000s messages in a few seconds and > then stops and will do that again in 1 hour. QueueImpl.getRate() method > returns 0 msg/s for slow consumer check period set to for example 5 min and > slow consumer detection will be skipped. > I tried to fix it by following change to QueueImpl.getRate() method and seems > to be ok, wdyt? > {code} > private final AtomicLong messageCountSnapshot = new AtomicLong(0); > public float getRate() { > long locaMessageAdded = getMessagesAdded(); > float timeSlice = ((System.currentTimeMillis() - > queueRateCheckTime.getAndSet(System.currentTimeMillis())) / 1000.0f); > if (timeSlice == 0) { > messagesAddedSnapshot.getAndSet(locaMessageAdded); > return 0.0f; > } > return BigDecimal.valueOf(((locaMessageAdded - > messagesAddedSnapshot.getAndSet(locaMessageAdded)) + > messageCountSnapshot.getAndSet(getMessageCount())) / timeSlice).setScale(2, > BigDecimal.ROUND_UP).floatValue(); > } > {code} -- This message was sent by Atlassian JIRA (v6.3.15#6346)