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

[Bug] The javadoc of ConsumerBuilder.isAckReceiptEnabled is not consistent, it references ack timeout and redelivery which isn't related to ack receipt #23249

Open
2 of 3 tasks
lhotari opened this issue Sep 3, 2024 · 2 comments
Labels
type/bug The PR fixed a bug or issue reported a bug

Comments

@lhotari
Copy link
Member

lhotari commented Sep 3, 2024

Search before asking

  • I searched in the issues and found nothing similar.

Read release policy

  • I understand that unsupported versions don't get bug fixes. I will attempt to reproduce the issue on a supported version of Pulsar client and Pulsar broker.

Version

any released version, master branch

Minimal reproduce step

The javadoc of ConsumerBuilder.isAckReceiptEnabled is not consistent, it references ack timeout and redelivery which isn't related to ack receipt at all:

/**
* Acknowledgement returns receipt, but the message is not re-sent after getting receipt.
*
* Configure the acknowledgement timeout mechanism to redeliver the message if it is not acknowledged after
* ackTimeout, or to execute a timer task to check the acknowledgement timeout messages during every
* ackTimeoutTickTime period.
*
* @param isAckReceiptEnabled {@link Boolean} enables acknowledgement for receipt
* @return the consumer builder instance

What did you expect to see?

There should be a proper explanation of why ack receipt is useful and what considerations are needed when using it.

One reason to enable ack receipt feature is to ensure that acknowledgements reach the server. The current assumption is that there isn't any automatic acknowledgement retry logic, but this is to be confirmed when documenting the feature.
When using this feature, it's recommended to use acknowledgeAsync. Using acknowledge would cause performance issues for most applications since it introduces a round trip to the server and would prevent pipelining (having multiple messages in-flight). In some usecases that might be desirable.

What did you see instead?

Misleading description

Anything else?

No response

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@lhotari lhotari added the type/bug The PR fixed a bug or issue reported a bug label Sep 3, 2024
@lhotari lhotari changed the title [Bug] The javadoc of ConsumerBuilder.isAckReceiptEnabled is not consistent, it references ack timeout isn't related to ack receipt [Bug] The javadoc of ConsumerBuilder.isAckReceiptEnabled is not consistent, it references ack timeout which isn't related to ack receipt Sep 3, 2024
@lhotari lhotari changed the title [Bug] The javadoc of ConsumerBuilder.isAckReceiptEnabled is not consistent, it references ack timeout which isn't related to ack receipt [Bug] The javadoc of ConsumerBuilder.isAckReceiptEnabled is not consistent, it references ack timeout and redelivery which isn't related to ack receipt Sep 3, 2024
@lhotari
Copy link
Member Author

lhotari commented Sep 5, 2024

Opened an ack receipt related bug report #23261

@lhotari
Copy link
Member Author

lhotari commented Sep 7, 2024

There's a separate proposal "PIP-377: Automatic retry for failed acknowledgements", #23267 (rendered doc) . Discussion thread https://lists.apache.org/thread/7sg7hfv9dyxto36dr8kotghtksy1j0kr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Development

No branches or pull requests

1 participant