Skip to content

Conversation

@liangyepianzhou
Copy link
Contributor

@liangyepianzhou liangyepianzhou commented Apr 9, 2025

PIP: #23526

Motivation

Currently, the ProducerBuilder API lacks the ability to directly configure the compressMinMsgBodySize property during producer initialization. Users are forced to modify the internal ProducerConfigurationData post-creation, which:

  1. Violates encapsulation by exposing internal configuration fields.
  2. Introduces inconsistency with other compression settings (e.g., compressionType) that are already configurable via the builder.
  3. Creates usability hurdles, requiring workarounds like:
    Producer<byte[]> producer = client.newProducer().topic("test").create();  
    producer.conf.setCompressMinMsgBodySize(1024); // Insecure and error-prone  

This change aligns the builder API with standard compression configuration patterns and improves developer ergonomics.

Modifications

  1. API Enhancement:
    • Added compressMinMsgBodySize(int) method to the ProducerBuilder interface.
    • Propagate the configured value to ProducerConfigurationData during producer initialization.
  2. Behavior Preservation:
    • Default value remains unchanged if the method is not invoked.
    • Fully backward-compatible with existing code.
  3. Testing & Documentation:
    • Added unit tests to validate compression threshold behavior via the new builder method.
    • Updated Javadoc and configuration examples to reflect the new API.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

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

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository:

@liangyepianzhou liangyepianzhou added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Apr 9, 2025
@liangyepianzhou liangyepianzhou changed the title Add setCompressMinMsgBodySize method to ProducerBuilder for compression configuration flexibility [fix][client]Add setCompressMinMsgBodySize method to ProducerBuilder for compression configuration flexibility Apr 9, 2025
@liangyepianzhou liangyepianzhou added this to the 4.1.0 milestone Apr 9, 2025
Copy link
Member

@StevenLuMT StevenLuMT left a comment

Choose a reason for hiding this comment

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

LGTM

@apache apache deleted a comment from github-actions bot Apr 9, 2025
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Apr 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.23%. Comparing base (bbc6224) to head (3798359).
Report is 1017 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24164      +/-   ##
============================================
+ Coverage     73.57%   74.23%   +0.65%     
+ Complexity    32624    32055     -569     
============================================
  Files          1877     1864      -13     
  Lines        139502   144478    +4976     
  Branches      15299    16484    +1185     
============================================
+ Hits         102638   107251    +4613     
+ Misses        28908    28758     -150     
- Partials       7956     8469     +513     
Flag Coverage Δ
inttests 26.71% <0.00%> (+2.12%) ⬆️
systests 23.17% <0.00%> (-1.16%) ⬇️
unittests 73.74% <100.00%> (+0.90%) ⬆️

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

Files with missing lines Coverage Δ
...apache/pulsar/client/impl/ProducerBuilderImpl.java 83.07% <100.00%> (+0.50%) ⬆️

... and 1067 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.

@liangyepianzhou liangyepianzhou merged commit 240d03b into apache:master Apr 9, 2025
52 checks passed
@liangyepianzhou liangyepianzhou deleted the add_setCompressMinMsgBodySize_in_Producer_builder branch April 9, 2025 12:53
poorbarcode pushed a commit to poorbarcode/pulsar that referenced this pull request Apr 15, 2025
…for compression configuration flexibility (apache#24164)

PIP: apache#23526

#### Motivation  
Currently, the `ProducerBuilder` API lacks the ability to directly configure the `compressMinMsgBodySize` property during producer initialization. Users are forced to modify the internal `ProducerConfigurationData` post-creation, which:  
1. **Violates encapsulation** by exposing internal configuration fields.  
2. **Introduces inconsistency** with other compression settings (e.g., `compressionType`) that are already configurable via the builder.  
3. **Creates usability hurdles**, requiring workarounds like:  
   ```java  
   Producer<byte[]> producer = client.newProducer().topic("test").create();  
   producer.conf.setCompressMinMsgBodySize(1024); // Insecure and error-prone  
   ```  
This change aligns the builder API with standard compression configuration patterns and improves developer ergonomics.  

#### Modifications  
1. **API Enhancement**:  
   - Added `compressMinMsgBodySize(int)` method to the `ProducerBuilder` interface.  
   - Propagate the configured value to `ProducerConfigurationData` during producer initialization.  
2. **Behavior Preservation**:  
   - Default value remains unchanged if the method is not invoked.  
   - Fully backward-compatible with existing code.  
3. **Testing & Documentation**:  
   - Added unit tests to validate compression threshold behavior via the new builder method.  
   - Updated Javadoc and configuration examples to reflect the new API.
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
…for compression configuration flexibility (apache#24164)

PIP: apache#23526

#### Motivation  
Currently, the `ProducerBuilder` API lacks the ability to directly configure the `compressMinMsgBodySize` property during producer initialization. Users are forced to modify the internal `ProducerConfigurationData` post-creation, which:  
1. **Violates encapsulation** by exposing internal configuration fields.  
2. **Introduces inconsistency** with other compression settings (e.g., `compressionType`) that are already configurable via the builder.  
3. **Creates usability hurdles**, requiring workarounds like:  
   ```java  
   Producer<byte[]> producer = client.newProducer().topic("test").create();  
   producer.conf.setCompressMinMsgBodySize(1024); // Insecure and error-prone  
   ```  
This change aligns the builder API with standard compression configuration patterns and improves developer ergonomics.  

#### Modifications  
1. **API Enhancement**:  
   - Added `compressMinMsgBodySize(int)` method to the `ProducerBuilder` interface.  
   - Propagate the configured value to `ProducerConfigurationData` during producer initialization.  
2. **Behavior Preservation**:  
   - Default value remains unchanged if the method is not invoked.  
   - Fully backward-compatible with existing code.  
3. **Testing & Documentation**:  
   - Added unit tests to validate compression threshold behavior via the new builder method.  
   - Updated Javadoc and configuration examples to reflect the new API.
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 ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants