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

[Issue 12958] Use payload size for isExceedMaximumMessageSize #13147

Closed

Conversation

Jason918
Copy link
Contributor

@Jason918 Jason918 commented Dec 6, 2021

Fixes #12958

Motivation

See #12958

Modifications

Add Commands.getPayloadSize for get payload size from headerAndPayload ByteBuf.
Use this payload size for isExceedMaximumMessageSize

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • org.apache.pulsar.broker.admin.TopicPoliciesTest#testTopicMaxMessageSizeThreshHold

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

If yes was chosen, please highlight the changes

  • 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)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • no-need-doc
    Bug fix

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 6, 2021
@Jason918 Jason918 force-pushed the fix-12958-topic-max-messaage-size branch from d19e3a9 to 948fefd Compare December 6, 2021 11:51
Copy link
Contributor

@gaoran10 gaoran10 left a comment

Choose a reason for hiding this comment

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

It seems that you want to compute the real message payload size, but if the message is a batch message, even we skip the batch message metadata, the rest part will also include many single message metadata.

@Jason918
Copy link
Contributor Author

Jason918 commented Dec 7, 2021

It seems that you want to compute the real message payload size, but if the message is a batch message, even we skip the batch message metadata, the rest part will also include many single message metadata.

Yes, this fix is for non-batch messages.
For batching message, we still need to check the size at client side.

@Jason918
Copy link
Contributor Author

This will be fixed by PIP-132 in #13591
Close this issue.

@Jason918 Jason918 closed this Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Producer fails to send message size equals to maxMessageSize in topic policy setting.
2 participants