[GitHub] [pulsar] codelipenghui commented on a change in pull request #8347: [Transaction] TransactionBuffer Refactor

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org