Skip to content

Conversation

@shibd
Copy link
Member

@shibd shibd commented Nov 12, 2024

Motivation

After #13344, the default value of maxPendingMessages and maxPendingMessagesAcrossPartitions change to 0.

When creating a producer with the partitioned topic, setting one of the above configurations doesn't take effect; the actual value is always 0.

Because here have a minimum judgment.

// MaxPendingMessagesAcrossPartitions doesn't support partial partition such as SinglePartition correctly
int maxPendingMessages = Math.min(conf.getMaxPendingMessages(),
conf.getMaxPendingMessagesAcrossPartitions() / numPartitions);
conf.setMaxPendingMessages(maxPendingMessages);

Modifications

  • When setting one of the values, use it directly. Only perform the min check when both values are not zero.

Verifying this change

  • Add PartitionedProducerImplTest.testMaxPendingQueueSize to cover this logic.
  • Add testProducerQueueFullBlockingWithPartitionedTopic test to cover blockQueueFullWithPartitiond case.

Documentation

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

Matching PR in forked repository

PR in forked repository:

@shibd shibd requested review from HQebupt and poorbarcode November 12, 2024 14:10
@shibd shibd self-assigned this Nov 12, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 12, 2024
@shibd shibd requested a review from Technoboy- November 13, 2024 01:39
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, good catch

@shibd shibd merged commit 0f934f2 into apache:master Nov 13, 2024
shibd added a commit that referenced this pull request Nov 13, 2024
shibd added a commit that referenced this pull request Nov 13, 2024
shibd added a commit that referenced this pull request Nov 13, 2024
@Technoboy- Technoboy- added this to the 4.1.0 milestone Nov 20, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 20, 2024
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants