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/12/14 01:45:13 UTC
[GitHub] sijie closed pull request #1885: Issue #1884: (@W-5697664@)
dir_*_usage stats are reported as 0 ...
sijie closed pull request #1885: Issue #1884: (@W-5697664@) dir_*_usage stats are reported as 0 ...
URL: https://github.com/apache/bookkeeper/pull/1885
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/LedgerDirsMonitor.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsMonitor.java
index fedebb7422..d003366932 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsMonitor.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsMonitor.java
@@ -122,43 +122,46 @@ private void check() {
}
List<File> fullfilledDirs = new ArrayList<File>(ldm.getFullFilledLedgerDirs());
- boolean hasWritableLedgerDirs = ldm.hasWritableLedgerDirs();
- float totalDiskUsage = 0;
+ boolean makeWritable = ldm.hasWritableLedgerDirs();
- // When bookie is in READONLY mode .i.e there are no writableLedgerDirs:
- // - Check if the total disk usage is below DiskLowWaterMarkUsageThreshold.
- // - If So, walk through the entire list of fullfilledDirs and add them back to writableLedgerDirs list if
- // their usage is < conf.getDiskUsageThreshold.
+ // When bookie is in READONLY mode, i.e there are no writableLedgerDirs:
+ // - Update fullfilledDirs disk usage.
+ // - If the total disk usage is below DiskLowWaterMarkUsageThreshold
+ // add fullfilledDirs back to writableLedgerDirs list if their usage is < conf.getDiskUsageThreshold.
try {
- if (hasWritableLedgerDirs
- || (totalDiskUsage = diskChecker.getTotalDiskUsage(ldm.getAllLedgerDirs())) < conf
- .getDiskLowWaterMarkUsageThreshold()) {
- // Check all full-filled disk space usage
- for (File dir : fullfilledDirs) {
- try {
- diskUsages.put(dir, diskChecker.checkDir(dir));
+ if (!makeWritable) {
+ float totalDiskUsage = diskChecker.getTotalDiskUsage(ldm.getAllLedgerDirs());
+ if (totalDiskUsage < conf.getDiskLowWaterMarkUsageThreshold()) {
+ makeWritable = true;
+ } else {
+ LOG.debug(
+ "Current TotalDiskUsage: {} is greater than LWMThreshold: {}."
+ + " So not adding any filledDir to WritableDirsList",
+ totalDiskUsage, conf.getDiskLowWaterMarkUsageThreshold());
+ }
+ }
+ // Update all full-filled disk space usage
+ for (File dir : fullfilledDirs) {
+ try {
+ diskUsages.put(dir, diskChecker.checkDir(dir));
+ if (makeWritable) {
ldm.addToWritableDirs(dir, true);
- } catch (DiskErrorException e) {
- // Notify disk failure to all the listeners
- for (LedgerDirsListener listener : ldm.getListeners()) {
- listener.diskFailed(dir);
- }
- } catch (DiskWarnThresholdException e) {
- diskUsages.put(dir, e.getUsage());
- // the full-filled dir become writable but still
- // above
- // warn threshold
+ }
+ } catch (DiskErrorException e) {
+ // Notify disk failure to all the listeners
+ for (LedgerDirsListener listener : ldm.getListeners()) {
+ listener.diskFailed(dir);
+ }
+ } catch (DiskWarnThresholdException e) {
+ diskUsages.put(dir, e.getUsage());
+ // the full-filled dir become writable but still above the warn threshold
+ if (makeWritable) {
ldm.addToWritableDirs(dir, false);
- } catch (DiskOutOfSpaceException e) {
- // the full-filled dir is still full-filled
- diskUsages.put(dir, e.getUsage());
}
+ } catch (DiskOutOfSpaceException e) {
+ // the full-filled dir is still full-filled
+ diskUsages.put(dir, e.getUsage());
}
- } else {
- LOG.debug(
- "Current TotalDiskUsage: {} is greater than LWMThreshold: {}."
- + " So not adding any filledDir to WritableDirsList",
- totalDiskUsage, conf.getDiskLowWaterMarkUsageThreshold());
}
} catch (IOException ioe) {
LOG.error("Got IOException while monitoring Dirs", ioe);
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestLedgerDirsManager.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestLedgerDirsManager.java
index b8555ca999..9d166a4ebf 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestLedgerDirsManager.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestLedgerDirsManager.java
@@ -21,8 +21,6 @@
package org.apache.bookkeeper.bookie;
import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.lessThan;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
@@ -243,7 +241,6 @@ private void testLedgerDirsMonitorDuringTransition(boolean highPriorityWritesAll
@Test
public void testLedgerDirsMonitorHandlingLowWaterMark() throws Exception {
-
ledgerMonitor.shutdown();
final float warn = 0.90f;
@@ -370,13 +367,13 @@ public void testLedgerDirsMonitorHandlingWithMultipleLedgerDirectories() throws
// should goto readwrite
setUsageAndThenVerify(curDir1, lwm - 0.17f, curDir2, nospace + 0.03f, mockDiskChecker, mockLedgerDirsListener,
false);
- assertTrue("Only one LedgerDir should be writable", dirsManager.getWritableLedgerDirs().size() == 1);
+ assertEquals("Only one LedgerDir should be writable", 1, dirsManager.getWritableLedgerDirs().size());
// bring both the dirs below lwm
// should still be readwrite
setUsageAndThenVerify(curDir1, lwm - 0.03f, curDir2, lwm - 0.02f, mockDiskChecker, mockLedgerDirsListener,
false);
- assertTrue("Both the LedgerDirs should be writable", dirsManager.getWritableLedgerDirs().size() == 2);
+ assertEquals("Both the LedgerDirs should be writable", 2, dirsManager.getWritableLedgerDirs().size());
// bring both the dirs above lwm but < threshold
// should still be readwrite
@@ -384,6 +381,52 @@ public void testLedgerDirsMonitorHandlingWithMultipleLedgerDirectories() throws
false);
}
+ @Test
+ public void testLedgerDirsMonitorStartReadOnly() throws Exception {
+ ledgerMonitor.shutdown();
+
+ final float nospace = 0.90f;
+ final float lwm = 0.80f;
+
+ File tmpDir1 = createTempDir("bkTest", ".dir");
+ File curDir1 = Bookie.getCurrentDirectory(tmpDir1);
+ Bookie.checkDirectoryStructure(curDir1);
+
+ File tmpDir2 = createTempDir("bkTest", ".dir");
+ File curDir2 = Bookie.getCurrentDirectory(tmpDir2);
+ Bookie.checkDirectoryStructure(curDir2);
+
+ conf.setDiskUsageThreshold(nospace);
+ conf.setDiskLowWaterMarkUsageThreshold(lwm);
+ conf.setDiskUsageWarnThreshold(nospace);
+ conf.setLedgerDirNames(new String[] { tmpDir1.toString(), tmpDir2.toString() });
+
+ // Both disks are out of space at the start.
+ HashMap<File, Float> usageMap = new HashMap<>();
+ usageMap.put(curDir1, nospace + 0.05f);
+ usageMap.put(curDir2, nospace + 0.05f);
+
+ mockDiskChecker = new MockDiskChecker(nospace, warnThreshold);
+ mockDiskChecker.setUsageMap(usageMap);
+ dirsManager = new LedgerDirsManager(conf, conf.getLedgerDirs(),
+ new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold()),
+ statsLogger);
+
+ ledgerMonitor = new LedgerDirsMonitor(conf, mockDiskChecker, dirsManager);
+ try {
+ ledgerMonitor.init();
+ fail("NoWritableLedgerDirException expected");
+ } catch (NoWritableLedgerDirException exception) {
+ // ok
+ }
+ final MockLedgerDirsListener mockLedgerDirsListener = new MockLedgerDirsListener();
+ dirsManager.addLedgerDirsListener(mockLedgerDirsListener);
+ ledgerMonitor.start();
+
+ Thread.sleep((diskCheckInterval * 2) + 100);
+ verifyUsage(curDir1, nospace + 0.05f, curDir2, nospace + 0.05f, mockLedgerDirsListener, true);
+ }
+
private void setUsageAndThenVerify(File dir1, float dir1Usage, File dir2, float dir2Usage,
MockDiskChecker mockDiskChecker, MockLedgerDirsListener mockLedgerDirsListener, boolean verifyReadOnly)
throws InterruptedException {
@@ -391,26 +434,19 @@ private void setUsageAndThenVerify(File dir1, float dir1Usage, File dir2, float
usageMap.put(dir1, dir1Usage);
usageMap.put(dir2, dir2Usage);
mockDiskChecker.setUsageMap(usageMap);
+ verifyUsage(dir1, dir1Usage, dir2, dir2Usage, mockLedgerDirsListener, verifyReadOnly);
+ }
+
+ private void verifyUsage(File dir1, float dir1Usage, File dir2, float dir2Usage,
+ MockLedgerDirsListener mockLedgerDirsListener, boolean verifyReadOnly) {
executorController.advance(Duration.ofMillis(diskCheckInterval));
float sample1 = getGauge(dir1.getParent()).getSample().floatValue();
float sample2 = getGauge(dir2.getParent()).getSample().floatValue();
- if (verifyReadOnly) {
- assertTrue(mockLedgerDirsListener.readOnly);
-
- // LedgerDirsMonitor stops updating diskUsages when the bookie is in the readonly mode,
- // so the stats will reflect an older value at the time when the bookie became readonly
- assertThat(sample1, greaterThan(90f));
- assertThat(sample1, lessThan(100f));
- assertThat(sample2, greaterThan(90f));
- assertThat(sample2, lessThan(100f));
- } else {
- assertFalse(mockLedgerDirsListener.readOnly);
-
- assertThat(sample1, equalTo(dir1Usage * 100f));
- assertThat(sample2, equalTo(dir2Usage * 100f));
- }
+ assertEquals(mockLedgerDirsListener.readOnly, verifyReadOnly);
+ assertThat(sample1, equalTo(dir1Usage * 100f));
+ assertThat(sample2, equalTo(dir2Usage * 100f));
}
private Gauge<? extends Number> getGauge(String path) {
----------------------------------------------------------------
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