This is an automated email from the ASF dual-hosted git repository.
sijie 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 1a8b35d ISSUE #954: dir_*_usage stats are always 0
1a8b35d is described below
commit 1a8b35d38737f6d043995a756da10ea111268b4a
Author: Pasha Kuznetsov <[email protected]>
AuthorDate: Sat Jan 6 21:34:46 2018 -0800
ISSUE #954: dir_*_usage stats are always 0
due to mismatch between the way they are initialized in `LedgerDirsManager`
and
the way the `diskUsages` is populated subsequently in `LedgerDirsMonitor`
Decisions potentially worth some attention:
- stat names are kept `dir_{dir}_usage` while the actual dirs being checked
have been changed to `{dir}/current`;
- the stats aren't being updated if the bookie is in a readonly mode --
uncovered while testing, not sure if it's worth addressing at all, kept the fix
minimalistic for now.
(bug W-4594204)
Author: Pasha Kuznetsov <[email protected]>
Reviewers: Enrico Olivelli <[email protected]>, Jia Zhai <None>, Sijie
Guo <[email protected]>
This closes #957 from pasha-kuznetsov/dir-usage-stats, closes #954
---
.../bookkeeper/bookie/LedgerDirsManager.java | 4 +--
.../bookkeeper/bookie/TestLedgerDirsManager.java | 36 ++++++++++++++++++++--
.../apache/bookkeeper/test/TestStatsProvider.java | 2 +-
3 files changed, 36 insertions(+), 6 deletions(-)
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 2093280..4bd28b2 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
@@ -72,9 +72,9 @@ public class LedgerDirsManager {
this.forceGCAllowWhenNoSpace = conf.getIsForceGCAllowWhenNoSpace();
this.entryLogSize = conf.getEntryLogSizeLimit();
this.minUsableSizeForIndexFileCreation =
conf.getMinUsableSizeForIndexFileCreation();
- for (File dir : dirs) {
+ for (File dir : ledgerDirectories) {
diskUsages.put(dir, 0f);
- String statName = "dir_" + dir.getPath().replace('/', '_') +
"_usage";
+ String statName = "dir_" + dir.getParent().replace('/', '_') +
"_usage";
final File targetDir = dir;
statsLogger.registerGauge(statName, new Gauge<Number>() {
@Override
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 e53ddce..7a7f672 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
@@ -20,8 +20,12 @@
*/
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;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -36,6 +40,8 @@ import
org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener;
import
org.apache.bookkeeper.bookie.LedgerDirsManager.NoWritableLedgerDirException;
import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.bookkeeper.conf.TestBKConfiguration;
+import org.apache.bookkeeper.stats.Gauge;
+import org.apache.bookkeeper.test.TestStatsProvider;
import org.apache.bookkeeper.util.DiskChecker;
import org.apache.bookkeeper.util.IOUtils;
import org.apache.commons.io.FileUtils;
@@ -56,6 +62,8 @@ public class TestLedgerDirsManager {
LedgerDirsManager dirsManager;
LedgerDirsMonitor ledgerMonitor;
MockDiskChecker mockDiskChecker;
+ private TestStatsProvider statsProvider;
+ private TestStatsProvider.TestStatsLogger statsLogger;
int diskCheckInterval = 1000;
float threshold = 0.5f;
float warnThreshold = 0.5f;
@@ -81,8 +89,10 @@ public class TestLedgerDirsManager {
conf.setIsForceGCAllowWhenNoSpace(true);
mockDiskChecker = new MockDiskChecker(threshold, warnThreshold);
+ statsProvider = new TestStatsProvider();
+ statsLogger = statsProvider.getStatsLogger("test");
dirsManager = new LedgerDirsManager(conf, conf.getLedgerDirs(),
- new DiskChecker(conf.getDiskUsageThreshold(),
conf.getDiskUsageWarnThreshold()));
+ new DiskChecker(conf.getDiskUsageThreshold(),
conf.getDiskUsageWarnThreshold()), statsLogger);
ledgerMonitor = new LedgerDirsMonitor(conf,
mockDiskChecker, dirsManager);
ledgerMonitor.init();
@@ -103,7 +113,7 @@ public class TestLedgerDirsManager {
List<File> writeDirs = dirsManager.getWritableLedgerDirs();
assertTrue("Must have a writable ledgerDir", writeDirs.size() > 0);
} catch (NoWritableLedgerDirException nwlde) {
- fail("We should have a writeble ledgerDir");
+ fail("We should have a writable ledgerDir");
}
}
@@ -251,7 +261,8 @@ public class TestLedgerDirsManager {
mockDiskChecker = new MockDiskChecker(nospace, warnThreshold);
dirsManager = new LedgerDirsManager(conf, conf.getLedgerDirs(),
- new DiskChecker(conf.getDiskUsageThreshold(),
conf.getDiskUsageWarnThreshold()));
+ new DiskChecker(conf.getDiskUsageThreshold(),
conf.getDiskUsageWarnThreshold()),
+ statsLogger);
ledgerMonitor = new LedgerDirsMonitor(conf, mockDiskChecker,
dirsManager);
usageMap = new HashMap<File, Float>();
usageMap.put(curDir1, 0.1f);
@@ -320,13 +331,32 @@ public class TestLedgerDirsManager {
usageMap.put(dir2, dir2Usage);
mockDiskChecker.setUsageMap(usageMap);
Thread.sleep((diskCheckInterval * 2) + 100);
+
+ 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));
}
}
+ private Gauge<? extends Number> getGauge(String path) {
+ String gaugeName = String.format("test.dir_%s_usage",
path.replace('/', '_'));
+ return statsProvider.getGauge(gaugeName);
+ }
+
private class MockDiskChecker extends DiskChecker {
private volatile float used;
diff --git
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/TestStatsProvider.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/TestStatsProvider.java
index b55ebad..b71031a 100644
---
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/TestStatsProvider.java
+++
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/TestStatsProvider.java
@@ -192,7 +192,7 @@ public class TestStatsProvider implements StatsProvider {
private Map<String, Gauge<? extends Number>> gaugeMap = new
ConcurrentHashMap<>();
@Override
- public StatsLogger getStatsLogger(String scope) {
+ public TestStatsLogger getStatsLogger(String scope) {
return new TestStatsLogger(scope);
}
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
