Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ARTEMIS-4588 30 second pause for large msgs + federation #4815

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

jbertram
Copy link
Contributor

@jbertram jbertram commented Feb 7, 2024

Large message support was added to
o.a.a.a.c.s.f.FederatedQueueConsumerImpl#onMessage via cf85d35 for ARTEMIS-3308. The problem with that change is that when onMessage returns o.a.a.a.c.c.i.ClientConsumerImpl#callOnMessage will eventually call o.a.a.a.c.c.i.ClientLargeMessageImpl#discardBody which eventually ends up in o.a.a.a.c.c.i.LargeMessageControllerImpl#popPacket waiting 30 seconds (i.e. the default readTimeout) for more packets to arrive (which never do). This happens because the FederatedQueueConsumer short-cuts the "normal" process by using LargeMessageControllerImpl#take.

This commit fixes that by tracking the number of bytes "taken" and then looking at that value later when discarding the body effectively skipping the 30 second wait.

Large message support was added to
o.a.a.a.c.s.f.FederatedQueueConsumerImpl#onMessage via cf85d35 for
ARTEMIS-3308. The problem with that change is that when onMessage
returns o.a.a.a.c.c.i.ClientConsumerImpl#callOnMessage will eventually
call o.a.a.a.c.c.i.ClientLargeMessageImpl#discardBody which eventually
ends up in o.a.a.a.c.c.i.LargeMessageControllerImpl#popPacket waiting 30
seconds (i.e. the default readTimeout) for more packets to arrive (which
never do). This happens because the FederatedQueueConsumer short-cuts
the "normal" process by using LargeMessageControllerImpl#take.

This commit fixes that by tracking the number of bytes "taken" and then
looking at that value later when discarding the body effectively
skipping the 30 second wait.
@jbertram jbertram requested a review from gtully February 9, 2024 15:54
Copy link
Contributor

@gtully gtully left a comment

Choose a reason for hiding this comment

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

Looks sensible. I recall a comment that server side large message apis may be a better match but never got to investigate. I wonder if those would be relevant here? From what I recall I reused that take pattern from an example!

@jbertram jbertram merged commit 77d9f10 into apache:main Feb 14, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants