liangyepianzhou commented on code in PR #21419:
URL: https://github.com/apache/pulsar/pull/21419#discussion_r1368670905
##########
pip/pip-311.md:
##########
@@ -0,0 +1,73 @@
+# Title: Add a request ID parameter to the `newMultiTransactionMessageAck`
method signature
+
+# Background knowledge
+**newMultiTransactionMessageAck**: A public API in `Commands.java` that is
used to assemble a `CommandAck` to individually ack a list of message IDs with
a transaction ID.
+
+
+# Motivation
+In Pulsar, all operations are encapsulated as individual commands.
+For instance, the action of sending a message by ClientCnx is a `Send`
command, and the broker has a corresponding `handleSend` method to handle the
request of sending messages.
+Subsequently, the broker sends a successful command, and ClientCnx will have a
corresponding 'HandleSuccess' method to process the returned successful command.
+ClientCnx locally caches each request's ID and future. Upon receiving a
response from the broker, it operates on the corresponding future based on the
request ID.
+For requests that do not receive a response within a given timeout, their
futures will be completed with a Timeout exception and removed from the local
cache map.
+This requires that all command requests waiting for a response should include
the request ID, so that the response returned by the broker can carry the
corresponding request ID.
+This way, ClientCnx can clearly identify which request has been processed and
completed.
+
+But the public API `newMultiTransactionMessageAck` in the `Commands.java`
creates the ack commands without request ID and hopes to wait for the response.
+This causes all the requests created by `newMultiTransactionMessageAck` to be
timed out.
Review Comment:
So there are two problems:
1. The transactional ack for chunk messages should use
`newMultiTransactionMessageAck `.
2. The `newMultiTransactionMessageAck` does not pass the request ID, and
the transaction ack op needs to wait for a response.
This proposal resolves the problem 2.
In addition, even if `newMultiTransactionMessageAck` is not used
temporarily, we should ensure its correctness.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
