[
https://issues.apache.org/jira/browse/ARTEMIS-4579?focusedWorklogId=901021&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-901021
]
ASF GitHub Bot logged work on ARTEMIS-4579:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 22/Jan/24 19:42
Start Date: 22/Jan/24 19:42
Worklog Time Spent: 10m
Work Description: clebertsuconic commented on code in PR #4752:
URL: https://github.com/apache/activemq-artemis/pull/4752#discussion_r1462332213
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScheduledDeliveryHandlerImpl.java:
##########
@@ -188,6 +194,34 @@ private void scheduleDelivery(final long deliveryTime) {
}
}
+ protected void notifyScheduledReferencesUpdated() {
+ oldestMessage = null;
+ }
+
+ @Override
+ public MessageReference peekFirstScheduledMessage() {
+ synchronized (scheduledReferences) {
+ if (scheduledReferences.isEmpty()) {
+ return null;
+ }
+ if (oldestMessage != null) {
+ return oldestMessage;
+ }
+ MessageReference result = null;
+ long oldestTimestamp = Long.MAX_VALUE;
+ for (RefScheduled scheduledReference : scheduledReferences) {
+ MessageReference ref = scheduledReference.getRef();
+ long refTimestamp = ref.getMessage().getTimestamp();
+ if (refTimestamp < oldestTimestamp) {
+ oldestTimestamp = refTimestamp;
+ result = ref;
+ }
+ }
+ oldestMessage = result;
+ return result;
+ }
+ }
Review Comment:
```suggestion
@Override
public MessageReference peekFirstScheduledMessage() {
synchronized (scheduledReferences) {
if (scheduledReferences.isEmpty()) {
return null;
}
RefScheduled refScheduled = scheduledReferences.first();
return refScheduled != null ? refScheduled.getRef() : null;
}
}
```
Why not just use scheduledReferences.first()?
I have been dealing with memory leaks in the past, especially the ones that
I tested with the leak-tests I wrote, and this would actually eventually leak
one message. Besides I think it's more efficient and less error prone to just
call getfirst().
in My suggestion I'm still checking for isEmpty(), which I'm not sure it's
needed.. something to verify.
Issue Time Tracking
-------------------
Worklog Id: (was: 901021)
Time Spent: 20m (was: 10m)
> Add the *FirstMessage* API for scheduled messages
> -------------------------------------------------
>
> Key: ARTEMIS-4579
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4579
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Components: API
> Affects Versions: 2.31.2
> Reporter: Jan Šmucr
> Priority: Major
> Fix For: 2.32.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Alerting on issues with messages not being received properly for a period of
> time is an uneasy task. We use the {{getFirstMessageAge()}} command to
> trigger alerts in Zabbix, and it works as long as there are no consumers.
> But this approach fails when there are consumers repeatedly failing to
> receive a message. That message is getting scheduled for redelivery over and
> over, and even though there still is an old message in the queue to be
> reported, it's no longer visible via {{getFirstMessage*()}} API.
> The goal here is to add a set of functions working with messages scheduled
> for delivery:
> {noformat}
> getFirstScheduledMessageAsJSON()
> getFirstScheduledMessageTimestamp()
> getFirstScheduledMessageAge()
> {noformat}
> It may be not the most effective approach but it's quite a convenient one,
> especially when monitoring a wide set of queues, each with its own set of
> alerts.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
