hangc0276 commented on code in PR #3329:
URL: https://github.com/apache/bookkeeper/pull/3329#discussion_r929466044


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java:
##########
@@ -245,7 +246,8 @@ void readLog() {
                     }
                     bb.clear();
                     mark.readLogMark(bb);
-                    if (curMark.compare(mark) < 0) {
+                    // get the minimum mark position from all the ledger 
directories to ensure no data loss.
+                    if (curMark.compare(mark) > 0) {

Review Comment:
   It will introduce complex logic for this comparison.
   1. If the ledger directory expands or shrinks, the map logic of the ledgerId 
to the ledger directory (`logMark`) also changed. It may be located on the 
wrong `logMark` file, and will lead to skipping the unflushed entries.
   2. There are many kinds of storage implementation, such as dbLedgerStorage, 
SortedLedgerStorage, and InterleavedLedgerStorage, we should get the ledgerId 
related storage instance to check the `logMark` position for each storage 
implementation. This operation will introduce complex logic.
   3. For the comparison, we can only save the write ledger throughput. We also 
need to read the data from the journal log file out.
   
   Based on the above reason, I prefer to replay all entries in the journal log 
file based on the min `logMark` position.



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