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/01/07 05:35:00 UTC
[GitHub] sijie closed pull request #957: ISSUE #954: dir_*_usage stats are always 0
sijie closed pull request #957: ISSUE #954: dir_*_usage stats are always 0
URL: https://github.com/apache/bookkeeper/pull/957
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/LedgerDirsManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
index 209328051..4bd28b2fb 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 LedgerDirsManager(ServerConfiguration conf, File[] dirs, DiskChecker disk
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 e53ddcef0..7a7f67277 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.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 @@
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 void setUp() throws Exception {
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 void testGetWritableDir() throws Exception {
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 void testLedgerDirsMonitorHandlingWithMultipleLedgerDirectories() throws
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 @@ private void setUsageAndThenVerify(File dir1, float dir1Usage, File dir2, float
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 b55ebad29..b71031ae3 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 void stop() {
private Map<String, Gauge<? extends Number>> gaugeMap = new ConcurrentHashMap<>();
@Override
- public StatsLogger getStatsLogger(String scope) {
+ public TestStatsLogger getStatsLogger(String scope) {
return new TestStatsLogger(scope);
}
----------------------------------------------------------------
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