You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/12/14 01:45:58 UTC
[bookkeeper] branch branch-4.7 updated: Issue #1884: (@W-5697664@)
dir_*_usage stats are reported as 0 ...
This is an automated email from the ASF dual-hosted git repository.
sijie pushed a commit to branch branch-4.7
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/branch-4.7 by this push:
new f307a36 Issue #1884: (@W-5697664@) dir_*_usage stats are reported as 0 ...
f307a36 is described below
commit f307a36acf9d2acf5c3ff315e1b3bfd7873e3cce
Author: Pasha Kuznetsov <pa...@tydbits.com>
AuthorDate: Thu Dec 13 17:45:11 2018 -0800
Issue #1884: (@W-5697664@) dir_*_usage stats are reported as 0 ...
for read-only bookies after a restart
### Motivation
Fixing the Issue #1884:
When a read-only bookie is restarted it keeps reporting `dir_*_usage`
stats as `0` until it becomes writable again.
This is caused by the `LedgerDirsMonitor.check` only updating
`diskUsages` if there are any writable dirs, or if the total usage
goes below the low water mark, otherwise relying on previously filled
values which are `0` after a bookie is restarted.
### Changes
* Change the `LedgerDirsMonitor.check` to update `diskUsages`
even when there are no writable dirs.
* Add new `testLedgerDirsMonitorStartReadOnly` testing this scenario.
* Simplify previous tests checking read-only since `diskUsages`
are now updated regardless if a bookie is in read-only mode.
jvrao reddycharan
Reviewers: Sijie Guo <si...@apache.org>, Charan Reddy Guttapalem <re...@gmail.com>
This closes #1885 from pasha-kuznetsov/issue-1884-dir-usage-ro-restart, closes #1884
(cherry picked from commit 1b2138fd4ecdbc4dfec00709b51e1367dfd49654)
Signed-off-by: Sijie Guo <si...@apache.org>
---
.../bookkeeper/bookie/LedgerDirsMonitor.java | 65 +++++++++---------
.../bookkeeper/bookie/TestLedgerDirsManager.java | 76 ++++++++++++++++------
2 files changed, 90 insertions(+), 51 deletions(-)
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 4ef02fa..cf2fdfe 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
@@ -121,43 +121,46 @@ class LedgerDirsMonitor {
}
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 b8555ca..9d166a4 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 @@ public class TestLedgerDirsManager {
@Test
public void testLedgerDirsMonitorHandlingLowWaterMark() throws Exception {
-
ledgerMonitor.shutdown();
final float warn = 0.90f;
@@ -370,13 +367,13 @@ public class TestLedgerDirsManager {
// 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 class TestLedgerDirsManager {
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 @@ public class TestLedgerDirsManager {
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) {