You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by "hangc0276 (via GitHub)" <gi...@apache.org> on 2023/04/01 02:18:18 UTC

[GitHub] [bookkeeper] hangc0276 opened a new pull request, #3901: Fix garbage collection blocked by runtime exception

hangc0276 opened a new pull request, #3901:
URL: https://github.com/apache/bookkeeper/pull/3901

   ### Motivation
   When one ledger file is broken, `getEntryLogMetadata` will throw an IllegalArgumentException, which is a RuntimeException and won't be caught in the whole garbage collector execution path. The exception will be caught by the thread SafeRunnable and interrupt the garbage collector, leading to those deleted ledgers can't be recycled and the ledger disk usage up.
   
   ```
   2023-04-01T00:55:29,497+0000 [GarbageCollectorThread-11-1] INFO  org.apache.bookkeeper.bookie.GarbageCollectorThread - Garbage collector thread forced to perform GC before expiry of wait time.
   2023-04-01T00:55:29,498+0000 [GarbageCollectorThread-11-1] INFO  org.apache.bookkeeper.bookie.GarbageCollectorThread - Extracting entry log meta from entryLogId: 17
   2023-04-01T00:55:29,498+0000 [GarbageCollectorThread-11-1] INFO  org.apache.bookkeeper.bookie.EntryLogger - Failed to get ledgers map index from: 17.log : Cannot deserialize ledgers map from ledger 9062744587808030975
   2023-04-01T00:55:29,553+0000 [GarbageCollectorThread-11-1] ERROR org.apache.bookkeeper.common.util.SafeRunnable - Unexpected throwable caught
   java.lang.IllegalArgumentException: Negative position
   	at sun.nio.ch.FileChannelImpl.read(FileChannelImpl.java:785) ~[?:?]
   	at org.apache.bookkeeper.bookie.BufferedReadChannel.read(BufferedReadChannel.java:93) ~[org.apache.bookkeeper-bookkeeper-server-4.14.5.jar:4.14.5]
   	at org.apache.bookkeeper.bookie.BufferedReadChannel.read(BufferedReadChannel.java:65) ~[org.apache.bookkeeper-bookkeeper-server-4.14.5.jar:4.14.5]
   	at org.apache.bookkeeper.bookie.EntryLogger.readFromLogChannel(EntryLogger.java:418) ~[org.apache.bookkeeper-bookkeeper-server-4.14.5.jar:4.14.5]
   	at org.apache.bookkeeper.bookie.EntryLogger.scanEntryLog(EntryLogger.java:996) ~[org.apache.bookkeeper-bookkeeper-server-4.14.5.jar:4.14.5]
   	at org.apache.bookkeeper.bookie.EntryLogger.extractEntryLogMetadataByScanning(EntryLogger.java:1136) ~[org.apache.bookkeeper-bookkeeper-server-4.14.5.jar:4.14.5]
   	at org.apache.bookkeeper.bookie.EntryLogger.getEntryLogMetadata(EntryLogger.java:1045) ~[org.apache.bookkeeper-bookkeeper-server-4.14.5.jar:4.14.5]
   	at org.apache.bookkeeper.bookie.GarbageCollectorThread.extractMetaFromEntryLogs(GarbageCollectorThread.java:607) ~[org.apache.bookkeeper-bookkeeper-server-4.14.5.jar:4.14.5]
   	at org.apache.bookkeeper.bookie.GarbageCollectorThread.runWithFlags(GarbageCollectorThread.java:348) ~[org.apache.bookkeeper-bookkeeper-server-4.14.5.jar:4.14.5]
   	at org.apache.bookkeeper.bookie.GarbageCollectorThread.safeRun(GarbageCollectorThread.java:329) ~[org.apache.bookkeeper-bookkeeper-server-4.14.5.jar:4.14.5]
   	at org.apache.bookkeeper.common.util.SafeRunnable.run(SafeRunnable.java:36) ~[org.apache.bookkeeper-bookkeeper-common-4.14.5.jar:4.14.5]
   	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) ~[?:?]
   	at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305) ~[?:?]
   	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305) ~[?:?]
   	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) ~[?:?]
   	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) ~[?:?]
   	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) ~[io.netty-netty-common-4.1.77.Final.jar:4.1.77.Final]
   	at java.lang.Thread.run(Thread.java:829) ~[?:?]
   ```
   
   ### Changes
   Catch the RuntimeException when getting the metadata for each entry log file.
   


-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [bookkeeper] hangc0276 commented on pull request #3901: Fix garbage collection blocked by runtime exception

Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3901:
URL: https://github.com/apache/bookkeeper/pull/3901#issuecomment-1498367229

   @eolivelli Would you please take a look? Thanks.


-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [bookkeeper] dlg99 commented on a diff in pull request #3901: Fix garbage collection blocked by runtime exception

Posted by "dlg99 (via GitHub)" <gi...@apache.org>.
dlg99 commented on code in PR #3901:
URL: https://github.com/apache/bookkeeper/pull/3901#discussion_r1159295327


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java:
##########
@@ -759,7 +759,7 @@ protected void extractMetaFromEntryLogs() throws EntryLogMetadataMapException {
                 } else {
                     entryLogMetaMap.put(entryLogId, entryLogMeta);
                 }
-            } catch (IOException e) {
+            } catch (IOException | RuntimeException e) {

Review Comment:
   what if the RuntimeException is thrown for a different reason and can't be that easily skipped
   I think it makes more sense to make EntryLogger.getEntryLogMetadata throw the IOException in this case



-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [bookkeeper] hangc0276 commented on a diff in pull request #3901: Fix garbage collection blocked by runtime exception

Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on code in PR #3901:
URL: https://github.com/apache/bookkeeper/pull/3901#discussion_r1161760601


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java:
##########
@@ -759,7 +759,7 @@ protected void extractMetaFromEntryLogs() throws EntryLogMetadataMapException {
                 } else {
                     entryLogMetaMap.put(entryLogId, entryLogMeta);
                 }
-            } catch (IOException e) {
+            } catch (IOException | RuntimeException e) {

Review Comment:
   This try-catch code block reads each entry log file's metadata and checks if this entry log file can be deleted.  The RuntimeException will be thrown by the read entry log file's metadata logic. We should block the whole garbage collection thread if one entry log file's metadata read failed. We can skip this entry log file if it is broken.



-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [bookkeeper] hangc0276 merged pull request #3901: Fix garbage collection blocked by runtime exception

Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 merged PR #3901:
URL: https://github.com/apache/bookkeeper/pull/3901


-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [bookkeeper] xiang092689 commented on pull request #3901: Fix garbage collection blocked by runtime exception

Posted by "xiang092689 (via GitHub)" <gi...@apache.org>.
xiang092689 commented on PR #3901:
URL: https://github.com/apache/bookkeeper/pull/3901#issuecomment-1518991108

   Is the broken EntryLog file will be left in file system 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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [bookkeeper] hangc0276 commented on pull request #3901: Fix garbage collection blocked by runtime exception

Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3901:
URL: https://github.com/apache/bookkeeper/pull/3901#issuecomment-1534060014

   @dlg99 Do you have any concerns?


-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [bookkeeper] hangc0276 commented on pull request #3901: Fix garbage collection blocked by runtime exception

Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3901:
URL: https://github.com/apache/bookkeeper/pull/3901#issuecomment-1546782838

   > @dlg99 Do you have any concerns?
   
   @dlg99 If no objections, I will merge this PR.


-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [bookkeeper] hangc0276 commented on pull request #3901: Fix garbage collection blocked by runtime exception

Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3901:
URL: https://github.com/apache/bookkeeper/pull/3901#issuecomment-1547905389

   > > @dlg99 Do you have any concerns?
   > 
   > @dlg99 If no objections, I will merge this PR.
   
   Merge it.


-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [bookkeeper] eolivelli commented on a diff in pull request #3901: Fix garbage collection blocked by runtime exception

Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on code in PR #3901:
URL: https://github.com/apache/bookkeeper/pull/3901#discussion_r1155073886


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java:
##########
@@ -759,7 +759,7 @@ protected void extractMetaFromEntryLogs() throws EntryLogMetadataMapException {
                 } else {
                     entryLogMetaMap.put(entryLogId, entryLogMeta);
                 }
-            } catch (IOException e) {
+            } catch (Throwable e) {

Review Comment:
   What about RuntimeException | IOException ?



-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [bookkeeper] hangc0276 commented on pull request #3901: Fix garbage collection blocked by runtime exception

Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3901:
URL: https://github.com/apache/bookkeeper/pull/3901#issuecomment-1534059796

   > Is the broken EntryLog file will be left in file system forever?
   
   @xiang092689 Yes. It will be better than all the entry log files can't be garbage collected.


-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org