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/01/09 00:58:24 UTC
[bookkeeper] branch branch-4.6 updated: ISSUE #954: dir_*_usage
stats are always 0
This is an automated email from the ASF dual-hosted git repository.
sijie pushed a commit to branch branch-4.6
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/branch-4.6 by this push:
new 1c4181b ISSUE #954: dir_*_usage stats are always 0
1c4181b is described below
commit 1c4181bcf798dbaa8d3694139b48475a9e215b17
Author: Pasha Kuznetsov <pk...@salesforce.com>
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 <pk...@salesforce.com>
Reviewers: Enrico Olivelli <eo...@gmail.com>, Jia Zhai <None>, Sijie Guo <si...@apache.org>
This closes #957 from pasha-kuznetsov/dir-usage-stats, closes #954
---
.../bookkeeper/bookie/LedgerDirsManager.java | 4 +--
.../bookkeeper/bookie/TestLedgerDirsManager.java | 39 +++++++++++++++++++---
.../apache/bookkeeper/test/TestStatsProvider.java | 2 +-
3 files changed, 37 insertions(+), 8 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 d47ecbb..ce00393 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,7 +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.NullStatsLogger;
+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;
@@ -54,6 +59,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;
@@ -79,9 +86,11 @@ 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()));
- ledgerMonitor = new LedgerDirsMonitor(conf,
+ new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold()), statsLogger);
+ ledgerMonitor = new LedgerDirsMonitor(conf,
mockDiskChecker, dirsManager);
ledgerMonitor.init();
}
@@ -101,7 +110,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");
}
}
@@ -249,7 +258,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);
@@ -318,13 +328,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
['"commits@bookkeeper.apache.org" <co...@bookkeeper.apache.org>'].