You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by yo...@apache.org on 2022/04/20 13:43:21 UTC
[bookkeeper] branch master updated: Fix force GC doesn't work under forceAllowCompaction when disk is full (#3205)
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new f0ff353d0 Fix force GC doesn't work under forceAllowCompaction when disk is full (#3205)
f0ff353d0 is described below
commit f0ff353d02ec56715d671040717870ec5f1a7630
Author: Hang Chen <ch...@apache.org>
AuthorDate: Wed Apr 20 21:43:15 2022 +0800
Fix force GC doesn't work under forceAllowCompaction when disk is full (#3205)
## Motivation
When I set `forceAllowCompaction=true` and one ledger disk reaches max usage threshold and transfer bookie to readOnly mode, I expire some pulsar topics or delete some topics to free up disk space. I found that ledger compression cannot be triggered when using `curl -XPUT http://localhost:8000/api/v1/bookie/gc` command.
The root cause is that when one ledger disk reaches max usage threshold, it will suspend minor and major compaction
https://github.com/apache/bookkeeper/blob/f7579fd13d62ce630ea26638e73f5884da505ec8/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java#L1041-L1058
When we use `curl -XPUT http://localhost:8000/api/v1/bookie/gc` command to trigger compaction, it will be filtered by `suspendMajor` and `suspendMinor` flag.
https://github.com/apache/bookkeeper/blob/f7579fd13d62ce630ea26638e73f5884da505ec8/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java#L416-L444
It will lead to
- The bookie won't clean up deleted ledgers
- Ledger disk can't free up disk usage
- Bookie can't recover from readOnly state into Writeable state.
And then we can only trigger compaction by the following steps.
- Increase max disk usage threshold
- Restart the bookie
- Use command `curl -XPUT http://localhost:8000/api/v1/bookie/gc` to trigger compaction
### Changes
1. Don't take the `suspendMajor` and `suspendMinor` flag into consideration when setting `forceAllowCompaction=true` and triggered by force GC.
---
.../bookkeeper/bookie/GarbageCollectorThread.java | 9 +++
.../bookie/InterleavedLedgerStorage.java | 5 ++
.../apache/bookkeeper/bookie/LedgerStorage.java | 7 +++
.../bookkeeper/bookie/SortedLedgerStorage.java | 5 ++
.../bookie/storage/ldb/DbLedgerStorage.java | 5 ++
.../ldb/SingleDirectoryDbLedgerStorage.java | 5 ++
.../server/http/service/TriggerGCService.java | 13 ++++-
.../apache/bookkeeper/bookie/CompactionTest.java | 68 ++++++++++++++++++++++
.../bookkeeper/bookie/MockLedgerStorage.java | 5 ++
9 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
index 600d0f414..87a947731 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
@@ -294,6 +294,15 @@ public class GarbageCollectorThread extends SafeRunnable {
}
}
+ public void enableForceGC(Boolean forceMajor, Boolean forceMinor) {
+ if (forceGarbageCollection.compareAndSet(false, true)) {
+ LOG.info("Forced garbage collection triggered by thread: {}, forceMajor: {}, forceMinor: {}",
+ Thread.currentThread().getName(), forceMajor, forceMinor);
+ triggerGC(true, forceMajor == null ? suspendMajorCompaction.get() : !forceMajor,
+ forceMinor == null ? suspendMinorCompaction.get() : !forceMinor);
+ }
+ }
+
public void disableForceGC() {
if (forceGarbageCollection.compareAndSet(true, false)) {
LOG.info("{} disabled force garbage collection since bookie has enough space now.", Thread
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java
index 5d0b1ef37..8d346f55a 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java
@@ -268,6 +268,11 @@ public class InterleavedLedgerStorage implements CompactableLedgerStorage, Entry
gcThread.enableForceGC();
}
+ @Override
+ public void forceGC(Boolean forceMajor, Boolean forceMinor) {
+ gcThread.enableForceGC(forceMajor, forceMinor);
+ }
+
@Override
public boolean isInForceGC() {
return gcThread.isInForceGC();
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerStorage.java
index eab61af03..8f8f19a9b 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerStorage.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerStorage.java
@@ -230,6 +230,13 @@ public interface LedgerStorage {
return;
}
+ /**
+ * Force trigger Garbage Collection with forceMajor or forceMinor parameter.
+ */
+ default void forceGC(Boolean forceMajor, Boolean forceMinor) {
+ return;
+ }
+
/**
* Class for describing location of a generic inconsistency. Implementations should
* ensure that detail is populated with an exception which adequately describes the
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
index 305cdc0a7..428a12627 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
@@ -370,6 +370,11 @@ public class SortedLedgerStorage
interleavedLedgerStorage.forceGC();
}
+ @Override
+ public void forceGC(Boolean forceMajor, Boolean forceMinor) {
+ interleavedLedgerStorage.forceGC(forceMajor, forceMinor);
+ }
+
@Override
public List<DetectedInconsistency> localConsistencyCheck(Optional<RateLimiter> rateLimiter) throws IOException {
return interleavedLedgerStorage.localConsistencyCheck(rateLimiter);
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java
index 0a563a1c9..7d33e5eb5 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java
@@ -403,6 +403,11 @@ public class DbLedgerStorage implements LedgerStorage {
ledgerStorageList.stream().forEach(SingleDirectoryDbLedgerStorage::forceGC);
}
+ @Override
+ public void forceGC(Boolean forceMajor, Boolean forceMinor) {
+ ledgerStorageList.stream().forEach(s -> s.forceGC(forceMajor, forceMinor));
+ }
+
@Override
public boolean isInForceGC() {
return ledgerStorageList.stream().anyMatch(SingleDirectoryDbLedgerStorage::isInForceGC);
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java
index 2697abde4..753b88f7e 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java
@@ -249,6 +249,11 @@ public class SingleDirectoryDbLedgerStorage implements CompactableLedgerStorage
gcThread.enableForceGC();
}
+ @Override
+ public void forceGC(Boolean forceMajor, Boolean forceMinor) {
+ gcThread.enableForceGC(forceMajor, forceMinor);
+ }
+
@Override
public boolean isInForceGC() {
return gcThread.isInForceGC();
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/TriggerGCService.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/TriggerGCService.java
index ebf9ea9c3..9f01c5c1b 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/TriggerGCService.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/TriggerGCService.java
@@ -20,6 +20,8 @@ package org.apache.bookkeeper.server.http.service;
import static com.google.common.base.Preconditions.checkNotNull;
+import java.util.HashMap;
+import java.util.Map;
import org.apache.bookkeeper.common.util.JsonUtil;
import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.bookkeeper.http.HttpServer;
@@ -61,7 +63,16 @@ public class TriggerGCService implements HttpEndpointService {
HttpServiceResponse response = new HttpServiceResponse();
if (HttpServer.Method.PUT == request.getMethod()) {
- bookieServer.getBookie().getLedgerStorage().forceGC();
+ String requestBody = request.getBody();
+ if (null == requestBody) {
+ bookieServer.getBookie().getLedgerStorage().forceGC();
+ } else {
+ @SuppressWarnings("unchecked")
+ Map<String, Object> configMap = JsonUtil.fromJson(requestBody, HashMap.class);
+ Boolean forceMajor = (Boolean) configMap.getOrDefault("forceMajor", null);
+ Boolean forceMinor = (Boolean) configMap.getOrDefault("forceMinor", null);
+ bookieServer.getBookie().getLedgerStorage().forceGC(forceMajor, forceMinor);
+ }
String output = "Triggered GC on BookieServer: " + bookieServer.toString();
String jsonResponse = JsonUtil.toJson(output);
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
index 1f37cea9d..765d8256a 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
@@ -332,6 +332,74 @@ public abstract class CompactionTest extends BookKeeperClusterTestCase {
});
}
+ @Test
+ public void testForceGarbageCollectionWhenDiskIsFull() throws Exception {
+ testForceGarbageCollectionWhenDiskIsFull(true);
+ testForceGarbageCollectionWhenDiskIsFull(false);
+ }
+
+ public void testForceGarbageCollectionWhenDiskIsFull(boolean isForceCompactionAllowWhenDisableCompaction)
+ throws Exception {
+
+ restartBookies(conf -> {
+ if (isForceCompactionAllowWhenDisableCompaction) {
+ conf.setMinorCompactionInterval(0);
+ conf.setMajorCompactionInterval(0);
+ conf.setForceAllowCompaction(true);
+ conf.setMajorCompactionThreshold(0.5f);
+ conf.setMinorCompactionThreshold(0.2f);
+ } else {
+ conf.setMinorCompactionInterval(120000);
+ conf.setMajorCompactionInterval(240000);
+ }
+ return conf;
+ });
+
+ getGCThread().suspendMajorGC();
+ getGCThread().suspendMinorGC();
+ long majorCompactionCntBeforeGC = 0;
+ long minorCompactionCntBeforeGC = 0;
+ long majorCompactionCntAfterGC = 0;
+ long minorCompactionCntAfterGC = 0;
+
+ // disable forceMajor and forceMinor
+ majorCompactionCntBeforeGC = getGCThread().getGarbageCollectionStatus().getMajorCompactionCounter();
+ minorCompactionCntBeforeGC = getGCThread().getGarbageCollectionStatus().getMinorCompactionCounter();
+ getGCThread().triggerGC(true, true, true).get();
+ majorCompactionCntAfterGC = getGCThread().getGarbageCollectionStatus().getMajorCompactionCounter();
+ minorCompactionCntAfterGC = getGCThread().getGarbageCollectionStatus().getMinorCompactionCounter();
+ assertEquals(majorCompactionCntBeforeGC, majorCompactionCntAfterGC);
+ assertEquals(minorCompactionCntBeforeGC, minorCompactionCntAfterGC);
+
+ // enable forceMajor and forceMinor
+ majorCompactionCntBeforeGC = getGCThread().getGarbageCollectionStatus().getMajorCompactionCounter();
+ minorCompactionCntBeforeGC = getGCThread().getGarbageCollectionStatus().getMinorCompactionCounter();
+ getGCThread().triggerGC(true, false, false).get();
+ majorCompactionCntAfterGC = getGCThread().getGarbageCollectionStatus().getMajorCompactionCounter();
+ minorCompactionCntAfterGC = getGCThread().getGarbageCollectionStatus().getMinorCompactionCounter();
+ assertEquals(majorCompactionCntBeforeGC + 1, majorCompactionCntAfterGC);
+ assertEquals(minorCompactionCntBeforeGC, minorCompactionCntAfterGC);
+
+ // enable forceMajor and disable forceMinor
+ majorCompactionCntBeforeGC = getGCThread().getGarbageCollectionStatus().getMajorCompactionCounter();
+ minorCompactionCntBeforeGC = getGCThread().getGarbageCollectionStatus().getMinorCompactionCounter();
+ getGCThread().triggerGC(true, false, true).get();
+ majorCompactionCntAfterGC = getGCThread().getGarbageCollectionStatus().getMajorCompactionCounter();
+ minorCompactionCntAfterGC = getGCThread().getGarbageCollectionStatus().getMinorCompactionCounter();
+ assertEquals(majorCompactionCntBeforeGC + 1, majorCompactionCntAfterGC);
+ assertEquals(minorCompactionCntBeforeGC, minorCompactionCntAfterGC);
+
+ // disable forceMajor and enable forceMinor
+ majorCompactionCntBeforeGC = getGCThread().getGarbageCollectionStatus().getMajorCompactionCounter();
+ minorCompactionCntBeforeGC = getGCThread().getGarbageCollectionStatus().getMinorCompactionCounter();
+ getGCThread().triggerGC(true, true, false).get();
+ majorCompactionCntAfterGC = getGCThread().getGarbageCollectionStatus().getMajorCompactionCounter();
+ minorCompactionCntAfterGC = getGCThread().getGarbageCollectionStatus().getMinorCompactionCounter();
+ assertEquals(majorCompactionCntBeforeGC, majorCompactionCntAfterGC);
+ assertEquals(minorCompactionCntBeforeGC + 1, minorCompactionCntAfterGC);
+
+ }
+
@Test
public void testMinorCompaction() throws Exception {
// prepare data
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/MockLedgerStorage.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/MockLedgerStorage.java
index ea3ff0492..06703966f 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/MockLedgerStorage.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/MockLedgerStorage.java
@@ -270,6 +270,11 @@ public class MockLedgerStorage implements LedgerStorage {
LedgerStorage.super.forceGC();
}
+ @Override
+ public void forceGC(Boolean forceMajor, Boolean forceMinor) {
+ LedgerStorage.super.forceGC(forceMajor, forceMinor);
+ }
+
@Override
public List<DetectedInconsistency> localConsistencyCheck(Optional<RateLimiter> rateLimiter) throws IOException {
return LedgerStorage.super.localConsistencyCheck(rateLimiter);