dlg99 commented on code in PR #3940:
URL: https://github.com/apache/bookkeeper/pull/3940#discussion_r1181742865


##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java:
##########
@@ -224,6 +224,7 @@ public void testBookieCompaction() throws Exception {
         entry3.writeLong(3); // entry id
         entry3.writeBytes("entry-3".getBytes());
         storage.addEntry(entry3);
+        storage.flush();

Review Comment:
   do these changes in test reflect actual behavior in production code path?



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -943,9 +943,6 @@ public Iterable<Long> getActiveLedgersInRange(long 
firstLedgerId, long lastLedge
 
     @Override
     public void updateEntriesLocations(Iterable<EntryLocation> locations) 
throws IOException {
-        // Trigger a flush to have all the entries being compacted in the db 
storage
-        flush();
-
         entryLocationIndex.updateLocations(locations);

Review Comment:
   How this change affects data consistency in case of e.g. node crash after 
updateLocations()?
   As I understand, bookie can end up with index persisted and pointing to the 
data that hasn't been persisted. 



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