Skip to content

[improve][client] Support MAX_ACK_GROUP_SIZE configurable - #18107

Merged
RobertIndie merged 4 commits into
apache:masterfrom
lordcheng10:support_MAX_ACK_GROUP_SIZE_config
Oct 21, 2022
Merged

[improve][client] Support MAX_ACK_GROUP_SIZE configurable#18107
RobertIndie merged 4 commits into
apache:masterfrom
lordcheng10:support_MAX_ACK_GROUP_SIZE_config

Conversation

@lordcheng10

@lordcheng10 lordcheng10 commented Oct 19, 2022

Copy link
Copy Markdown
Contributor

Motivation

The current MAX_ACK_GROUP_SIZE is fixed at 1000, increase the configuration acknowledgementsGroupSize, support MAX_ACK_GROUP_SIZE configurable:

Modifications

1.Add configuration acknowledgementsGroupSize;

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: lordcheng10#33

@RobertIndie

Copy link
Copy Markdown
Member

Please add some tests to verify the PR.

@lordcheng10

Copy link
Copy Markdown
Contributor Author

Please add some tests to verify the PR.

OK , I will fix

@lordcheng10
lordcheng10 force-pushed the support_MAX_ACK_GROUP_SIZE_config branch from 6540bd4 to 094121f Compare October 19, 2022 12:07
@lordcheng10

Copy link
Copy Markdown
Contributor Author

Please add some tests to verify the PR.

Fixed.PTAL,thanks!@RobertIndie

@lordcheng10

Copy link
Copy Markdown
Contributor Author

@eolivelli @RobertIndie @AnonHxy @Jason918 @HQebupt PTAL,thanks!

Comment thread pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ConsumerBuilder.java Outdated
Comment thread pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ConsumerBuilder.java Outdated
Comment thread pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ConsumerBuilder.java Outdated
@lordcheng10
lordcheng10 requested review from AnonHxy and RobertIndie and removed request for AnonHxy and RobertIndie October 20, 2022 02:01
Comment thread pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java Outdated
Comment thread pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java Outdated
@lordcheng10
lordcheng10 force-pushed the support_MAX_ACK_GROUP_SIZE_config branch from ce5215a to 9cd92bc Compare October 20, 2022 03:54
@lordcheng10

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@lordcheng10

lordcheng10 commented Oct 20, 2022

Copy link
Copy Markdown
Contributor Author

CI all passed: lordcheng10#33

@lordcheng10

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@lordcheng10

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@codecov-commenter

codecov-commenter commented Oct 21, 2022

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.74%. Comparing base (6c65ca0) to head (a68537e).
⚠️ Report is 3531 commits behind head on master.

Files with missing lines Patch % Lines
...impl/PersistentAcknowledgmentsGroupingTracker.java 14.28% 4 Missing and 2 partials ⚠️
...apache/pulsar/client/impl/ConsumerBuilderImpl.java 0.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #18107       +/-   ##
=============================================
+ Coverage     34.91%   45.74%   +10.82%     
- Complexity     5707    17568    +11861     
=============================================
  Files           607     1574      +967     
  Lines         53396   128526    +75130     
  Branches       5712    14149     +8437     
=============================================
+ Hits          18644    58789    +40145     
- Misses        32119    63658    +31539     
- Partials       2633     6079     +3446     
Flag Coverage Δ
unittests 45.74% <25.00%> (+10.82%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ar/client/impl/conf/ConsumerConfigurationData.java 90.42% <100.00%> (ø)
...apache/pulsar/client/impl/ConsumerBuilderImpl.java 27.51% <0.00%> (ø)
...impl/PersistentAcknowledgmentsGroupingTracker.java 50.81% <14.28%> (ø)

... and 1120 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@RobertIndie
RobertIndie merged commit 092d3b3 into apache:master Oct 21, 2022
Technoboy- pushed a commit that referenced this pull request Feb 8, 2023
The current MAX_ACK_GROUP_SIZE is fixed at 1000, increase the configuration acknowledgementsGroupSize, support MAX_ACK_GROUP_SIZE configurable:
https://github.com/apache/pulsar/blob/afcdbf0e2b5fb905e1f82f0220436f8f9ec0c742/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PersistentAcknowledgmentsGroupingTracker.java#L63-L64

1.Add configuration acknowledgementsGroupSize;
@Technoboy- Technoboy- added this to the 3.0.0 milestone Feb 8, 2023
@Technoboy-

Copy link
Copy Markdown
Contributor

#18345 relies on this, so cherry-picked to branch-2.11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants