codelipenghui commented on a change in pull request #8347:
URL: https://github.com/apache/pulsar/pull/8347#discussion_r511477297
##########
File path: pulsar-common/src/main/proto/PulsarMarkers.proto
##########
@@ -80,10 +80,12 @@ message ClusterMessageId {
message MessageIdData {
required uint64 ledger_id = 1;
required uint64 entry_id = 2;
+ optional int32 partition = 3 [default = -1];
Review comment:
Why we need partition information in the marker? A marker can only
belong to a single partition right?
##########
File path: pulsar-common/src/main/proto/PulsarMarkers.proto
##########
@@ -80,10 +80,12 @@ message ClusterMessageId {
message MessageIdData {
required uint64 ledger_id = 1;
required uint64 entry_id = 2;
+ optional int32 partition = 3 [default = -1];
}
/// --- Transaction marker ---
message TxnCommitMarker {
- required MessageIdData message_id = 1;
+ optional MessageIdData message_id = 1;
+ repeated MessageIdData messageIdList = 2;
Review comment:
I think we can keep `repeated MessageIdData message_ids` here. Since we
don't need to consider the compatibility guarantee for the first transaction
release.
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/PersistentTransactionBufferProvider.java
##########
@@ -34,6 +34,7 @@
/**
* Persistent transaction buffer provider.
*/
+@Deprecated
Review comment:
It's some of the old codes are not useful, we can delete them directly.
we don't want to tell users there some deprecated components in the first
transaction release.
Please check all.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]