dao-jun commented on code in PR #22707:
URL: https://github.com/apache/pulsar/pull/22707#discussion_r1624839739


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/InMemTransactionBuffer.java:
##########
@@ -307,6 +307,7 @@ public CompletableFuture<Void> commitTxn(TxnID txnID, long 
lowWaterMark) {
                 txnBuffer.commitAt(committedAtLedgerId, committedAtEntryId);
                 addTxnToTxnIdex(txnID, committedAtLedgerId);
             }
+            updateLastDispatchablePosition(null);

Review Comment:
   I introduced a filed `lastDispatchablePosition` to store the last position 
which is valid to dispatch, in `PersistentTopic`. And before return 
`lastDispatchablePosition`, check if `lastDispatchablePosition` is null, 
otherwise, search the position in BK.
   About why set `lastDispatchablePosition` to null when commit txn, it is 
because of it's hard to determine which position to set, so, set to null to 
make it fallback to search the position from BK.
   
   > Because transaction committed will not change the previously recorded 
stable consumable position into a
   > non-consumable position.
   
   I don't agree with this point, after commit a txn, the 
`lastDispatchablePosition` may have a chance to move forward, right? But, we 
don't know which position to set, and, even though we record the last entry 
position of the txn, what if non-txn messages published? 
   Say, `txn1` contains `1, 2, 3` messages, and we record all the positions. 
Before commit `txn1`, there is a new non-txn message `4` published, when commit 
`txn1`, what value can we set to `lastDispatchablePosition`? So I set 
`lastDispatchablePosition` to null, we can get the `lastDispatchablePosition` 
by searching BK.
   
   My  point is, the situations here is very complex, right? After commit a 
txn, the `lastDispatchablePosition` can be moved forward, or not. A topic 
contains `txn messages`, `non-txn messages`, `Marker`; `txn messages` contains 
`abort txn`, `ongoing txn`, `commit txn`. 
   Set `lastDispatchablePosition` to null after commit/abort txn is the easiest 
way to make it consistent, 



-- 
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]

Reply via email to