Skip to content

Conversation

@BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Jan 30, 2022

Motivation

#13627 supports chunking messages for C++ client. However, the consumer configuration AutoOldestChunkedMessageOnQueueFull has a typo that it doesn't contain the Ack. See the corresponding Java configuration:

ConsumerBuilder<T> autoAckOldestChunkedMessageOnQueueFull(boolean autoAckOldestChunkedMessageOnQueueFull);

In addition, the default value of maxPendingChunkedMessages is 10 in Java client implementation but 100 in JavaDocs. The C++ client followed the JavaDocs and set the default value of maxPendingChunkedMessages 100. But it's better to keep the consistency with Java client's implementation instead of the wrong JavaDocs.

Since #13627 is not included in any release now, we should include this fix before 2.10.0 release.

Modifications

  • Change setAutoOldestChunkedMessageOnQueueFull to setAutoAckOldestChunkedMessageOnQueueFull.
  • Change isAutoOldestChunkedMessageOnQueueFull to isAutoAckOldestChunkedMessageOnQueueFull.
  • Change the default value of maxPendingChunkedMessage to 10.

Verifying this change

  • Make sure that the change passes the CI checks.

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

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): (yes / no)
  • The public API: (yes)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

@BewareMyPower BewareMyPower added component/c++ type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use doc-not-needed Your PR changes do not impact docs labels Jan 30, 2022
@BewareMyPower BewareMyPower added this to the 2.10.0 milestone Jan 30, 2022
@BewareMyPower BewareMyPower self-assigned this Jan 30, 2022
@BewareMyPower BewareMyPower changed the title [C++] Change AutoOldest to AutoAckOldest in consumer configuration [C++] Fix the consumer configuration inconsistency with Java client Jan 30, 2022
@merlimat merlimat merged commit fe32736 into apache:master Feb 1, 2022
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
…pache#14070)

* [C++] Change AutoOldest to AutoAckOldest in consumer configuration

* Change the default value of MaxPendingChunkedMessages to 10
@BewareMyPower BewareMyPower deleted the bewaremypower/cpp-fix-api-typo branch May 5, 2022 08:18
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 type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants