Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Only retrieve one batch of messages at a time when responding to a large ResendRequest #643

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

philipwhiuk
Copy link
Contributor

Fixes #639

@chrjohn
Copy link
Member

chrjohn commented Jul 5, 2024

@philipwhiuk do you happen to have the time to create a unit test for this? So we can ensure that normal resend processing works as expected? Probably a test where we resend more than 1000 messages and check that every message is resent. Something along that lines?
Thank you :)
Edit: call me paranoid but I'm always cautious when introducing new loops with seqnum+1... ;)

@chrjohn chrjohn changed the title Only retrieve one batch of messages at a time when responding to a large resend request Only retrieve one batch of messages at a time when responding to a large ResendRequest Feb 27, 2025
try {
// QFJ-626
msg = parseMessage(message);
if (msg.getException() != null) {
Copy link
Member

Choose a reason for hiding this comment

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

@philipwhiuk I am not sure about this. This will not resend messages which were formerly (without this PR) sent. I mean we could discuss if it is sensible but I feel that this should be a separate change. WDYT?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

During a ResendRequest the request for messages from storage allows for unbounded memory usage
2 participants