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

[PIP 131] Resolve produce chunk messages failed when topic level maxMessageSize is set #13599

Merged
merged 2 commits into from
Jan 18, 2022

Conversation

Jason918
Copy link
Contributor

@Jason918 Jason918 commented Jan 4, 2022

Fixes #13544

Motivation

See #13544

Modifications

Add isChunked in PublishContext. Skip the maxMessageSize check if it's chunked

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#testMaxMessageSizeWithChunking

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

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 4, 2022
@Jason918
Copy link
Contributor Author

Jason918 commented Jan 4, 2022

/pulsarbot run-failure-checks

1 similar comment
@Jason918
Copy link
Contributor Author

Jason918 commented Jan 5, 2022

/pulsarbot run-failure-checks

Copy link
Member

@RobertIndie RobertIndie left a comment

Choose a reason for hiding this comment

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

Overall looks good to me.

@@ -105,6 +105,10 @@ default void setProperty(String propertyName, Object value) {
default Object getProperty(String propertyName) {
return null;
}

default boolean isChunked() {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to add the override annotation in the implementation class like MessagePublishContext? I think it is good to have.

@Jason918
Copy link
Contributor Author

Jason918 commented Jan 5, 2022

/pulsarbot run-failure-checks

2 similar comments
@Jason918
Copy link
Contributor Author

Jason918 commented Jan 5, 2022

/pulsarbot run-failure-checks

@Jason918
Copy link
Contributor Author

Jason918 commented Jan 6, 2022

/pulsarbot run-failure-checks

@Jason918
Copy link
Contributor Author

Jason918 commented Jan 7, 2022

/pulsarbot run-failure-checks

2 similar comments
@Jason918
Copy link
Contributor Author

Jason918 commented Jan 7, 2022

/pulsarbot run-failure-checks

@Jason918
Copy link
Contributor Author

Jason918 commented Jan 7, 2022

/pulsarbot run-failure-checks

@Jason918
Copy link
Contributor Author

@codelipenghui @315157973 @hangc0276 PTAL

@codelipenghui codelipenghui added this to the 2.10.0 milestone Jan 18, 2022
@codelipenghui codelipenghui merged commit c221905 into apache:master Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PIP 131] Resolve produce chunk messages failed when topic level maxMessageSize is set
3 participants