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


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java:
##########
@@ -67,7 +70,19 @@ public EntryLocationIndex(ServerConfiguration conf, 
KeyValueStorageFactory stora
 
     @Override
     public void close() throws IOException {
+        log.info("Closing EntryLocationIndex");
+        while (!compacting.compareAndSet(false, true)) {
+            // Wait till the locationsDb stops compacting
+            log.info("Waiting the locationsDb stops compacting");
+            try {
+                Thread.sleep(1000);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+                throw new IOException(e);
+            }
+        }
         locationsDb.close();

Review Comment:
   We need to test this case:
   - One thread triggered `locationsDb.compact()`, and the RocksDB compact is 
working in progress. Then the bookie triggered a shutdown. My concern is 
whether this case will make the RocksDB index corrupt.



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java:
##########
@@ -297,6 +299,12 @@ public void close() throws IOException {
         try {
             closedLock.writeLock().lock();
             closed = true;
+            db.cancelAllBackgroundWork(true);

Review Comment:
   locationsDb.compact(); is not a background action, and cancel here doesn't 
work.



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java:
##########
@@ -67,7 +70,19 @@ public EntryLocationIndex(ServerConfiguration conf, 
KeyValueStorageFactory stora
 
     @Override
     public void close() throws IOException {
+        log.info("Closing EntryLocationIndex");
+        while (!compacting.compareAndSet(false, true)) {

Review Comment:
   We should not wait without any timeout; the Bookie pod shutdown will be 
blocked forever.



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