[
https://issues.apache.org/jira/browse/AMQ-9813?focusedWorklogId=993490&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-993490
]
ASF GitHub Bot logged work on AMQ-9813:
---------------------------------------
Author: ASF GitHub Bot
Created on: 26/Nov/25 17:01
Start Date: 26/Nov/25 17:01
Worklog Time Spent: 10m
Work Description: cshannon commented on code in PR #1551:
URL: https://github.com/apache/activemq/pull/1551#discussion_r2565749074
##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/FilePendingMessageCursor.java:
##########
@@ -220,7 +232,7 @@ public synchronized boolean
tryAddMessageLast(MessageReference node, long maxWai
}
if (!hasSpace()) {
if (isDiskListEmpty()) {
- expireOldMessages();
+ expiredMessages.addAll(expireOldMessages());
Review Comment:
Same comment as in addMessageFirst
##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/FilePendingMessageCursor.java:
##########
@@ -263,16 +284,16 @@ public synchronized void addMessageFirst(MessageReference
node) {
memoryList.addMessageFirst(node);
node.incrementReferenceCount();
setCacheEnabled(true);
- return;
+ return expiredMessages;
}
}
if (!hasSpace()) {
if (isDiskListEmpty()) {
- expireOldMessages();
+ expiredMessages = expireOldMessages();
Review Comment:
So I just realized that this is actually could be issue because the
expireOldMessages() method is going to decrement the reference on each message,
but the messages get copied to that temporary list. The problem is that now
that we are hanging onto the list reference (to process later outside of the
lock) we are more likely to blow memory because the hasSpace() method will
think there is free space but those messages are still held in the heap until
later. We are only adding one message but if that message happens to be very
large it could be an issue.
Issue Time Tracking
-------------------
Worklog Id: (was: 993490)
Time Spent: 20m (was: 10m)
> Incorrect QueueSize if Non-Persistent messages with TTL is used
> ---------------------------------------------------------------
>
> Key: AMQ-9813
> URL: https://issues.apache.org/jira/browse/AMQ-9813
> Project: ActiveMQ
> Issue Type: Bug
> Components: Broker
> Affects Versions: 6.2.0, 5.19.1
> Reporter: Radek Kraus
> Assignee: Christopher L. Shannon
> Priority: Major
> Fix For: 6.1.9, 5.19.2, 6.2.1
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Sometimes, I detect a problem with incorrect QueueSize, when non-persitent
> messages with TTL is used.
> *UseCase/Symptoms*
> * produce/consume non-persistent messages with defined TTL (expiration)
> * producerFlowControl=false
> * consumers "are slow", it means that some messages are expired (broker side
> or client side)
> * QueueSize sometimes reports non-zero value even when all messages are
> consumed/expired
> * Broker does not "push" next messages to consumers
> ** JMX method {{browseAsTable()}} does not provide messages
> ** JMX method {{purge()}} does not remove messages (QueueSize is still
> non-zero, nothing is deleted)
> * It seems like messages are expired, but QueueSize still reports non-zero
> values
> * after Broker restart QueueSize is reset to zero (expected for
> non-persitent messages)
> *Analyse*
> * I guess, that problem was introduced by AMQ-5785 (by
> [8b23e07|https://github.com/apache/activemq/commit/8b23e07], where deadlock
> was solved)
> * there is a valid AMQ-5785 comment without next reaction
> * what happens in
> [8b23e07|https://github.com/apache/activemq/commit/8b23e07] commit (nearly
> the same what is written in [AMQ-5785
> comment|https://issues.apache.org/jira/browse/AMQ-5785?focusedCommentId=15607861&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15607861]):
> ** contract of {{expireOldMessages()}} method was changed to don't invoke
> {{discardExpiredMessage(MessageReference)}} (which is finally responsible to
> "process expired message")
> ** the invocation of {{discardExpiredMessage(MessageReference)}} was moved
> out of synchronized section (deadlock prevention), see
> {{onUsageChanged(Usage, int, int)}} method
> ** but the *problem is that {{expireOldMessages()}} method is invoked from
> next methods* ({{{}tryAddMessageLast(MessageReference, long){}}},
> {{{}addMessageFirst(MessageReference){}}}), where
> *{{discardExpiredMessage(MessageReference)}} method is no longer invoked*
> (after change), *what it is problem from my point of view*
> * IMHO "message expiration process" can be invoked:
> ** from {{onUsageChanged(Usage, int, int)}} method ({{{}UsageListener{}}}),
> asynchronously triggered by Usage.setPercentUsage(int) (thread pool executor)
> ** or "directly" from {{tryAddMessageLast(MessageReference, long)}} (in case
> when {{OrderedPendingList}} is used)
> ** it depends on situations (timings), which method is invoked "at first"
> ** when {{tryAddMessageLast(MessageReference, long)}} method is invoked,
> then expired messages are only removed from {{{}memoryList{}}}, but
> {{discardExpiredMessage(MessageReference)}} is never invoked (what it is the
> problem)
> *PR* (https://github.com/apache/activemq/pull/1551)
> * I added "missing" invocation of
> {{discardExpiredMessage(MessageReference)}} method into
> {{{}tryAddMessageLast(MessageReference, long){}}},
> {{addMessageFirst(MessageReference)}} methods
> * I tried to use same "postponed" strategy (outside of synchronization) like
> was already done in original commit (see {{{}onUsageChanged(Usage, int,
> int){}}}) method
> * I tried to write JUnit test, but it is complicated, because it depends on
> timings (from my expiriences TTL must be very small to simulate the problem,
> see {{{}FullDestinationMemoryMessageExpirationTest{}}})
> Could you please check/analyze my findings?
> In each case (IMHO)
> [8b23e07|https://github.com/apache/activemq/commit/8b23e07] breaks previous
> contract of {{expireOldMessages()}} methods, which can caused that expired
> messages are not "processed", when expiration is triggered by
> {{{}tryAddMessageLast(MessageReference, long){}}},
> {{addMessageFirst(MessageReference)}} methods.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
