You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2018/07/30 06:45:29 UTC
[GitHub] eolivelli closed pull request #1560: ISSUE #1559: Fix for
moveLedgerIndexFile logic in relocateIndexFileAndFlushHeader
eolivelli closed pull request #1560: ISSUE #1559: Fix for moveLedgerIndexFile logic in relocateIndexFileAndFlushHeader
URL: https://github.com/apache/bookkeeper/pull/1560
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
index 5375002695..83cb88fcc3 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
@@ -495,7 +495,20 @@ int getOpenFileLimit() {
private void relocateIndexFileAndFlushHeader(long ledger, FileInfo fi) throws IOException {
File currentDir = getLedgerDirForLedger(fi);
if (ledgerDirsManager.isDirFull(currentDir)) {
- moveLedgerIndexFile(ledger, fi);
+ try {
+ moveLedgerIndexFile(ledger, fi);
+ } catch (NoWritableLedgerDirException nwe) {
+ /*
+ * if there is no other indexDir, which could accommodate new
+ * indexFile but the current indexDir has enough space
+ * (minUsableSizeForIndexFileCreation) for this flushHeader
+ * operation, then it is ok to proceed without moving
+ * LedgerIndexFile.
+ */
+ if (!ledgerDirsManager.isDirWritableForNewIndexFile(currentDir)) {
+ throw nwe;
+ }
+ }
}
fi.flushHeader();
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
index 00d332c7a7..cdaa66814b 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
@@ -316,6 +316,11 @@ File pickRandomWritableDirForNewIndexFile(File excludedDir) throws NoWritableLed
return pickRandomDir(writableDirsForNewIndexFile, excludedDir);
}
+ boolean isDirWritableForNewIndexFile(File indexDir) {
+ return (ledgerDirectories.contains(indexDir)
+ && (indexDir.getUsableSpace() > minUsableSizeForIndexFileCreation));
+ }
+
/**
* Return one dir from all dirs, regardless writable or not.
*/
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 447fcc3c1f..b3937ca2d5 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
@@ -37,6 +37,7 @@
import com.google.common.util.concurrent.UncheckedExecutionException;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
+
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
@@ -626,6 +627,99 @@ public void testCompactionPersistence() throws Exception {
}
}
+ @Test
+ public void testCompactionWhenLedgerDirsAreFull() throws Exception {
+ /*
+ * for this test scenario we are assuming that there will be only one
+ * bookie in the cluster
+ */
+ assertEquals("Numbers of Bookies in this cluster", 1, bsConfs.size());
+ ServerConfiguration serverConfig = bsConfs.get(0);
+ File ledgerDir = serverConfig.getLedgerDirs()[0];
+ assertEquals("Number of Ledgerdirs for this bookie", 1, serverConfig.getLedgerDirs().length);
+ assertTrue("indexdirs should be configured to null", null == serverConfig.getIndexDirs());
+ /*
+ * this test is for validating EntryLogCompactor, so make sure
+ * TransactionalCompaction is not enabled.
+ */
+ assertFalse("Bookies must be using EntryLogCompactor", baseConf.getUseTransactionalCompaction());
+ // prepare data
+ LedgerHandle[] lhs = prepareData(3, true);
+
+ for (LedgerHandle lh : lhs) {
+ lh.close();
+ }
+
+ bs.get(0).getBookie().getLedgerStorage().flush();
+ assertTrue(
+ "entry log file ([0,1,2].log should be available in ledgerDirectory: "
+ + serverConfig.getLedgerDirs()[0],
+ TestUtils.hasLogFiles(serverConfig.getLedgerDirs()[0], false, 0, 1, 2));
+
+ long usableSpace = ledgerDir.getUsableSpace();
+ long totalSpace = ledgerDir.getTotalSpace();
+
+ baseConf.setForceReadOnlyBookie(true);
+ baseConf.setIsForceGCAllowWhenNoSpace(true);
+ // disable minor compaction
+ baseConf.setMinorCompactionThreshold(0.0f);
+ baseConf.setGcWaitTime(60000);
+ baseConf.setMinorCompactionInterval(120000);
+ baseConf.setMajorCompactionInterval(240000);
+ baseConf.setMinUsableSizeForEntryLogCreation(1);
+ baseConf.setMinUsableSizeForIndexFileCreation(1);
+ baseConf.setDiskUsageThreshold((1.0f - ((float) usableSpace / (float) totalSpace)) * 0.9f);
+ baseConf.setDiskUsageWarnThreshold(0.0f);
+
+ /*
+ * because of the value set for diskUsageThreshold, when bookie is
+ * restarted it wouldn't find any writableledgerdir. But we have set
+ * very low values for minUsableSizeForEntryLogCreation and
+ * minUsableSizeForIndexFileCreation, so it should be able to create
+ * EntryLog file and Index file for doing compaction.
+ */
+
+ // restart bookies
+ restartBookies(baseConf);
+
+ assertFalse("There shouldn't be any writable ledgerDir",
+ bs.get(0).getBookie().getLedgerDirsManager().hasWritableLedgerDirs());
+
+ long lastMinorCompactionTime = getGCThread().lastMinorCompactionTime;
+ long lastMajorCompactionTime = getGCThread().lastMajorCompactionTime;
+ assertTrue(getGCThread().enableMajorCompaction);
+ assertFalse(getGCThread().enableMinorCompaction);
+
+ // remove ledger1 and ledger3
+ bkc.deleteLedger(lhs[0].getId());
+ bkc.deleteLedger(lhs[2].getId());
+ LOG.info("Finished deleting the ledgers contains most entries.");
+ getGCThread().enableForceGC();
+ getGCThread().triggerGC().get();
+
+ // after garbage collection, minor compaction should not be executed
+ assertTrue(getGCThread().lastMinorCompactionTime > lastMinorCompactionTime);
+ assertTrue(getGCThread().lastMajorCompactionTime > lastMajorCompactionTime);
+
+ /*
+ * GarbageCollection should have succeeded, so no previous entrylog
+ * should be available.
+ */
+
+ // entry logs ([0,1,2].log) should be compacted
+ for (File ledgerDirectory : tmpDirs) {
+ assertFalse("Found entry log file ([0,1,2].log that should have not been compacted in ledgerDirectory: "
+ + ledgerDirectory, TestUtils.hasLogFiles(ledgerDirectory, true, 0, 1, 2));
+ }
+
+ // even entry log files are removed, we still can access entries for
+ // ledger2
+ // since those entries has been compacted to new entry log
+ long ledgerId = lhs[1].getId();
+ long lastAddConfirmed = lhs[1].getLastAddConfirmed();
+ verifyLedger(ledgerId, 0, lastAddConfirmed);
+ }
+
@Test
public void testMajorCompactionAboveThreshold() throws Exception {
// prepare data
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services