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

[Java Client] Fix DLQ message can not send when enable poolMessages #13280

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wangjialing218
Copy link
Contributor

Motivation

Fixes #13269

Modifications

When message is put into possibleSendToDeadLetterTopicMessages, do not clear it in release() since it will be used when send to DLQ later.
And clear the message when it is removed from possibleSendToDeadLetterTopicMessages

Verifying this change

This change is already covered by existing tests, such as PulsarSinkE2ETest.testPulsarSinkDLQ()
just enable poolMessage for client.

Documentation

  • no-need-doc

    Only bug fix

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 14, 2021
@wangjialing218 wangjialing218 changed the title [PulsarClient] Fix DLQ message can not send when enable poolMessages [Java Client] Fix DLQ message can not send when enable poolMessages Dec 14, 2021
@@ -76,6 +76,7 @@
private BrokerEntryMetadata brokerEntryMetadata;

private boolean poolMessage;
private boolean dlqMessage;
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it is possible to not add this new field to MessageImpl.
A new field has a runtime weight.

if the problem is only about not "releasing" the message if it is send to the DLQ, what about simply "retaining" one more time before adding it to the possibleSendToDeadLetterTopicMessages list ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only retain byteBuf is not enough. We have to not call recycle() in release() if the message was added to possibleSendToDeadLetterTopicMessages list.

If message is acked by consumer as normal case, the message will not be added to possibleSendToDeadLetterTopicMessages list and should be clear in release().

So how could we known whether this message was added to possibleSendToDeadLetterTopicMessages list when user call release()? If there is a way then we could remove this field.

@wangjialing218 wangjialing218 force-pushed the branch-poolMessageAndDLQ branch from 0ea6abf to c4f7ee0 Compare December 14, 2021 09:24
@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@Technoboy- Technoboy- added this to the 3.2.0 milestone Jul 31, 2023
@Technoboy- Technoboy- modified the milestones: 3.2.0, 3.3.0 Dec 22, 2023
@coderzc coderzc modified the milestones: 3.3.0, 3.4.0 May 8, 2024
@lhotari lhotari modified the milestones: 4.0.0, 4.1.0 Oct 14, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
doc-not-needed Your PR changes do not impact docs lifecycle/stale Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PoolMessage and DLQ can not work together
5 participants