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

allow empty payload for nonbatch message #236

Merged
merged 1 commit into from
May 6, 2020

Conversation

zzzming
Copy link
Contributor

@zzzming zzzming commented Apr 27, 2020

Motivation

This PR will fix a problem preventing consumer receiving empty payload with properties in non-batch mode; and empty payload without key and properties in batch mode. Error messages are prompted currently as such:

ERRO[0004] Discarding corrupted message                  msgID="ledgerId:92658 entryId:1458 partition:-1 " name=hwmiw subscription=consumer topic="<pulsar topic>" validationError=BatchDeSerializeError
ERRO[0004] handle message Id: ledgerId:92658 entryId:1458 partition:-1   consumerID=1 local_addr="192.168.1.111:33444" remote_addr="<pulsar URL>"

In addition, error will be printed out from MessageReceived instead of masking the real error reported.

Modifications

The buffer capacity is validated instead of solely replying on ReadableBytes to determine EOF is reached.

Verifying this change

Tested against batch and nonbatch mode to consume empty payload with and without properties.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: ( no)
  • The default values of configurations: (no)
  • The wire protocol: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

print out error message from MessageReceived
Copy link
Member

@wolfstudy wolfstudy left a comment

Choose a reason for hiding this comment

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

Cool, Lgtm +1

@wolfstudy wolfstudy merged commit e7f1673 into apache:master May 6, 2020
@wolfstudy wolfstudy added this to the 0.1.1 milestone Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants