Skip to content

feat(#1797): add message delivery tracking to QueueMessageReference - #1796

Merged
jbonofre merged 2 commits into
apache:mainfrom
jeanouii:feat/message-delivery-tracking
Jul 9, 2026
Merged

feat(#1797): add message delivery tracking to QueueMessageReference#1796
jbonofre merged 2 commits into
apache:mainfrom
jeanouii:feat/message-delivery-tracking

Conversation

@jeanouii

Copy link
Copy Markdown
Contributor

Add isDelivered()/setDelivered() to QueueMessageReference interface with implementations in IndirectMessageReference (boolean field) and NullMessageReference (no-op). PrefetchSubscription.acknowledge() now marks messages as delivered=true when processing DeliveredAck, enabling downstream consumers to distinguish delivered-but-unacked messages.

Add isDelivered()/setDelivered() to QueueMessageReference interface
with implementations in IndirectMessageReference (boolean field) and
NullMessageReference (no-op). PrefetchSubscription.acknowledge() now
marks messages as delivered=true when processing DeliveredAck, enabling
downstream consumers to distinguish delivered-but-unacked messages.
@jeanouii jeanouii changed the title [#RODO] feat(#TODO): add message delivery tracking to QueueMessageReference feat(#1797): add message delivery tracking to QueueMessageReference Mar 18, 2026
}

int index = 0;
for (Iterator<MessageReference> iter = dispatched.iterator(); iter.hasNext(); index++) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this loop overrides what has been done in the previous loop (line 275). Maybe we should merge the logic in one loop ?

@cshannon

Copy link
Copy Markdown
Contributor

What caused you to make this change, did you see bugs show up or specific issues with tests? There are no test updates here to demonstrate the issue and fix. I'm not saying the change is incorrect (i haven't reviewed it) but i am wondering what triggered the change here.

@jbonofre

Copy link
Copy Markdown
Member

@cshannon I think it's coming for a flaky test (or needed by a test). @jeanouii am I right ?

@cshannon

Copy link
Copy Markdown
Contributor

@cshannon I think it's coming for a flaky test (or needed by a test). @jeanouii am I right ?

Yeah that would make sense, if there is an existing test already that is flaky that this fixes then that demonstrates the issue and fix but it would be helpful to know which one

@jeanouii

Copy link
Copy Markdown
Contributor Author

@cshannon @jbonofre Not a specific test, but I asked myself many times the question when debugging flaky tests. And I realized even in the Web Console we don't have information on hand.
I don't recall about a very specific test to be honest.
If you guys think we don't need it, I'm ok with it, I'll put it back locally when I need to debug again in the future

@cshannon

Copy link
Copy Markdown
Contributor

If you guys think we don't need it, I'm ok with it, I'll put it back locally when I need to debug again in the future

I just need time to look at it more, this week has been pretty busy so I'll try to get to the various backlogs of PR reviews next week.

@jeanouii jeanouii self-assigned this Apr 8, 2026
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions Bot added the Stale label Jul 8, 2026
@jbonofre

jbonofre commented Jul 9, 2026

Copy link
Copy Markdown
Member

This PR looks good to me but I would like @cshannon's opinion.

@jbonofre
jbonofre requested a review from cshannon July 9, 2026 14:17
@jbonofre
jbonofre merged commit ad9a30d into apache:main Jul 9, 2026
10 checks passed
@cshannon

Copy link
Copy Markdown
Contributor

this closes #1797

@cshannon cshannon linked an issue Jul 16, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Track message delivery state in QueueMessageReference

3 participants