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/11/25 05:29:16 UTC
[bookkeeper] branch master updated: Remove MathUtils.now()
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 a32e29d Remove MathUtils.now()
a32e29d is described below
commit a32e29d66162b76a8dd6f58d3eacb7b62dd07ba2
Author: Sijie Guo <gu...@gmail.com>
AuthorDate: Sat Nov 24 21:29:12 2018 -0800
Remove MathUtils.now()
Descriptions of the changes in this PR:
*Motivation*
`MathUtils.now()` using System.nanoTime doesn't provide accurate time. It is used
for measuring pupose. However since `nanoTime` can have numeric overflow, so when
it is used for converting into milliseconds, it is misleading and error-prone. A
lot of places using `MathUtis.now` can all replaced with `System.currentTimeMillis()`.
It leads to unknown behavior on compactions. examples is shown in the following figure.
<img width="1157" alt="wechatimg180" src="https://user-images.githubusercontent.com/1217863/48965961-d860b600-ef7c-11e8-9394-1d51cef5c68c.png">
example: this graph shows the major_compaction_count. major compaction is supposed to run every day. but in the graph, there is no major compaction occuring in 4 days.
*Changes*
Remove `MathUtils.now()` and replace it with `System.currentTimeMillis()`.
Reviewers: Jia Zhai <None>, Enrico Olivelli <eo...@gmail.com>
This closes #1837 from sijie/remove_mathutils_now
---
.../java/org/apache/bookkeeper/common/util/MathUtils.java | 15 ---------------
.../org/apache/bookkeeper/common/util/TestMathUtils.java | 8 --------
.../main/java/org/apache/bookkeeper/bookie/Bookie.java | 8 ++++----
.../apache/bookkeeper/bookie/GarbageCollectorThread.java | 8 ++++----
.../bookkeeper/bookie/ScanAndCompareGarbageCollector.java | 7 +++----
.../java/org/apache/bookkeeper/bookie/SyncThread.java | 7 +++----
.../apache/bookkeeper/net/StabilizeNetworkTopology.java | 7 +++----
.../java/org/apache/bookkeeper/util/LocalBookKeeper.java | 4 ++--
.../src/main/java/org/apache/bookkeeper/util/Shell.java | 4 ++--
.../java/org/apache/bookkeeper/bookie/CompactionTest.java | 7 +++----
10 files changed, 24 insertions(+), 51 deletions(-)
diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/MathUtils.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/MathUtils.java
index 94999a4..43d5d77 100644
--- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/MathUtils.java
+++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/MathUtils.java
@@ -42,21 +42,6 @@ public class MathUtils {
/**
* Current time from some arbitrary time base in the past, counting in
- * milliseconds, and not affected by settimeofday or similar system clock
- * changes. This is appropriate to use when computing how much longer to
- * wait for an interval to expire.
- *
- * <p>NOTE: only use it for measuring.
- * http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/System.html#nanoTime%28%29
- *
- * @return current time in milliseconds.
- */
- public static long now() {
- return System.nanoTime() / NANOSECONDS_PER_MILLISECOND;
- }
-
- /**
- * Current time from some arbitrary time base in the past, counting in
* nanoseconds, and not affected by settimeofday or similar system clock
* changes. This is appropriate to use when computing how much longer to
* wait for an interval to expire.
diff --git a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestMathUtils.java b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestMathUtils.java
index 3bd58c2..fc2318b 100644
--- a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestMathUtils.java
+++ b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/util/TestMathUtils.java
@@ -19,13 +19,11 @@
package org.apache.bookkeeper.common.util;
import static org.apache.bookkeeper.common.util.MathUtils.findNextPositivePowerOfTwo;
-import static org.apache.bookkeeper.common.util.MathUtils.now;
import static org.apache.bookkeeper.common.util.MathUtils.nowInNano;
import static org.apache.bookkeeper.common.util.MathUtils.signSafeMod;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
-import java.util.concurrent.TimeUnit;
import org.junit.Test;
/**
@@ -47,12 +45,6 @@ public class TestMathUtils {
}
@Test
- public void testNow() {
- long nowInMillis = now();
- assertTrue(TimeUnit.NANOSECONDS.toMillis(System.nanoTime()) >= nowInMillis);
- }
-
- @Test
public void testNowInNanos() {
long nowInNanos = nowInNano();
assertTrue(System.nanoTime() >= nowInNanos);
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
index cd0b2f7..f91bf28 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
@@ -733,7 +733,7 @@ public class Bookie extends BookieCriticalThread {
}
void readJournal() throws IOException, BookieException {
- long startTs = MathUtils.now();
+ long startTs = System.currentTimeMillis();
JournalScanner scanner = new JournalScanner() {
@Override
public void process(int journalVersion, long offset, ByteBuffer recBuff) throws IOException {
@@ -823,7 +823,7 @@ public class Bookie extends BookieCriticalThread {
for (Journal journal : journals) {
journal.replay(scanner);
}
- long elapsedTs = MathUtils.now() - startTs;
+ long elapsedTs = System.currentTimeMillis() - startTs;
LOG.info("Finished replaying journal in {} ms.", elapsedTs);
}
@@ -1492,7 +1492,7 @@ public class Bookie extends BookieCriticalThread {
Bookie b = new Bookie(new ServerConfiguration());
b.start();
CounterCallback cb = new CounterCallback();
- long start = MathUtils.now();
+ long start = System.currentTimeMillis();
for (int i = 0; i < 100000; i++) {
ByteBuf buff = Unpooled.buffer(1024);
buff.writeLong(1);
@@ -1501,7 +1501,7 @@ public class Bookie extends BookieCriticalThread {
b.addEntry(buff, false /* ackBeforeSync */, cb, null, new byte[0]);
}
cb.waitZero();
- long end = MathUtils.now();
+ long end = System.currentTimeMillis();
System.out.println("Took " + (end - start) + "ms");
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
index 4b1c834..ad752de 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
@@ -255,7 +255,7 @@ public class GarbageCollectorThread extends SafeRunnable {
LOG.info("Major Compaction : enabled=" + enableMajorCompaction + ", threshold="
+ majorCompactionThreshold + ", interval=" + majorCompactionInterval);
- lastMinorCompactionTime = lastMajorCompactionTime = MathUtils.now();
+ lastMinorCompactionTime = lastMajorCompactionTime = System.currentTimeMillis();
}
public void enableForceGC() {
@@ -364,13 +364,13 @@ public class GarbageCollectorThread extends SafeRunnable {
LOG.info("Disk full, suspend minor compaction to slow down filling disk.");
}
- long curTime = MathUtils.now();
+ long curTime = System.currentTimeMillis();
if (enableMajorCompaction && (!suspendMajor)
&& (force || curTime - lastMajorCompactionTime > majorCompactionInterval)) {
// enter major compaction
LOG.info("Enter major compaction, suspendMajor {}", suspendMajor);
doCompactEntryLogs(majorCompactionThreshold);
- lastMajorCompactionTime = MathUtils.now();
+ lastMajorCompactionTime = System.currentTimeMillis();
// and also move minor compaction time
lastMinorCompactionTime = lastMajorCompactionTime;
majorCompactionCounter.inc();
@@ -379,7 +379,7 @@ public class GarbageCollectorThread extends SafeRunnable {
// enter minor compaction
LOG.info("Enter minor compaction, suspendMinor {}", suspendMinor);
doCompactEntryLogs(minorCompactionThreshold);
- lastMinorCompactionTime = MathUtils.now();
+ lastMinorCompactionTime = System.currentTimeMillis();
minorCompactionCounter.inc();
}
this.gcThreadRuntime.registerSuccessfulEvent(
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
index d4c8bc2..6af5d15 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
@@ -45,7 +45,6 @@ import org.apache.bookkeeper.net.BookieSocketAddress;
import org.apache.bookkeeper.stats.Counter;
import org.apache.bookkeeper.stats.Gauge;
import org.apache.bookkeeper.stats.StatsLogger;
-import org.apache.bookkeeper.util.MathUtils;
import org.apache.bookkeeper.util.ZkUtils;
import org.apache.bookkeeper.zookeeper.ZooKeeperClient;
import org.apache.zookeeper.KeeperException;
@@ -96,7 +95,7 @@ public class ScanAndCompareGarbageCollector implements GarbageCollector{
this.conf = conf;
this.selfBookieAddress = Bookie.getBookieAddress(conf);
this.gcOverReplicatedLedgerIntervalMillis = conf.getGcOverreplicatedLedgerWaitTimeMillis();
- this.lastOverReplicatedLedgerGcTimeMillis = MathUtils.now();
+ this.lastOverReplicatedLedgerGcTimeMillis = System.currentTimeMillis();
if (gcOverReplicatedLedgerIntervalMillis > 0) {
this.enableGcOverReplicatedLedger = true;
}
@@ -137,7 +136,7 @@ public class ScanAndCompareGarbageCollector implements GarbageCollector{
Long.MAX_VALUE));
this.activeLedgerCounter = bkActiveLedgers.size();
- long curTime = MathUtils.now();
+ long curTime = System.currentTimeMillis();
boolean checkOverreplicatedLedgers = (enableGcOverReplicatedLedger && curTime
- lastOverReplicatedLedgerGcTimeMillis > gcOverReplicatedLedgerIntervalMillis);
if (checkOverreplicatedLedgers) {
@@ -150,7 +149,7 @@ public class ScanAndCompareGarbageCollector implements GarbageCollector{
} else {
LOG.info("Removed over-replicated ledgers: {}", overReplicatedLedgers);
}
- lastOverReplicatedLedgerGcTimeMillis = MathUtils.now();
+ lastOverReplicatedLedgerGcTimeMillis = System.currentTimeMillis();
}
// Iterate over all the ledger on the metadata store
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java
index a7c3a7a..00288dc 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java
@@ -36,7 +36,6 @@ import org.apache.bookkeeper.bookie.CheckpointSource.Checkpoint;
import org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener;
import org.apache.bookkeeper.bookie.LedgerDirsManager.NoWritableLedgerDirException;
import org.apache.bookkeeper.conf.ServerConfiguration;
-import org.apache.bookkeeper.util.MathUtils;
/**
* SyncThread is a background thread which help checkpointing ledger storage
@@ -205,11 +204,11 @@ class SyncThread implements Checkpointer {
requestFlush();
executor.shutdown();
- long start = MathUtils.now();
+ long start = System.currentTimeMillis();
while (!executor.awaitTermination(5, TimeUnit.MINUTES)) {
- long now = MathUtils.now();
+ long now = System.currentTimeMillis();
log.info("SyncThread taking a long time to shutdown. Has taken {}"
- + " seconds so far", now - start);
+ + " milliseconds so far", now - start);
}
}
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/StabilizeNetworkTopology.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/StabilizeNetworkTopology.java
index 0bac3bf..5c244f2 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/StabilizeNetworkTopology.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/StabilizeNetworkTopology.java
@@ -26,7 +26,6 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.TimeUnit;
-import org.apache.bookkeeper.util.MathUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -42,7 +41,7 @@ public class StabilizeNetworkTopology implements NetworkTopology {
boolean tentativeToRemove;
NodeStatus() {
- this.lastPresentTime = MathUtils.now();
+ this.lastPresentTime = System.currentTimeMillis();
}
synchronized boolean isTentativeToRemove() {
@@ -52,7 +51,7 @@ public class StabilizeNetworkTopology implements NetworkTopology {
synchronized NodeStatus updateStatus(boolean tentativeToRemove) {
this.tentativeToRemove = tentativeToRemove;
if (!this.tentativeToRemove) {
- this.lastPresentTime = MathUtils.now();
+ this.lastPresentTime = System.currentTimeMillis();
}
return this;
}
@@ -88,7 +87,7 @@ public class StabilizeNetworkTopology implements NetworkTopology {
// no status of this node, remove this node from topology
impl.remove(node);
} else if (status.isTentativeToRemove()) {
- long millisSinceLastSeen = MathUtils.now() - status.getLastPresentTime();
+ long millisSinceLastSeen = System.currentTimeMillis() - status.getLastPresentTime();
if (millisSinceLastSeen >= stabilizePeriodMillis) {
logger.info("Node {} (seen @ {}) becomes stale for {} ms, remove it from the topology.",
node, status.getLastPresentTime(), millisSinceLastSeen);
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/LocalBookKeeper.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/LocalBookKeeper.java
index 9555474..e431e8e 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/LocalBookKeeper.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/LocalBookKeeper.java
@@ -492,7 +492,7 @@ public class LocalBookKeeper {
}
public static boolean waitForServerUp(String hp, long timeout) {
- long start = MathUtils.now();
+ long start = System.currentTimeMillis();
String split[] = hp.split(":");
String host = split[0];
int port = Integer.parseInt(split[1]);
@@ -524,7 +524,7 @@ public class LocalBookKeeper {
LOG.info("server " + hp + " not up " + e);
}
- if (MathUtils.now() > start + timeout) {
+ if (System.currentTimeMillis() > start + timeout) {
break;
}
try {
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/Shell.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/Shell.java
index 9e37614..60c7648 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/Shell.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/Shell.java
@@ -93,7 +93,7 @@ public abstract class Shell {
* Check to see if a command needs to be executed and execute if needed.
*/
protected void run() throws IOException {
- if (lastTime + interval > MathUtils.now()) {
+ if (lastTime + interval > System.currentTimeMillis()) {
return;
}
exitCode = 0; // reset for next run
@@ -199,7 +199,7 @@ public abstract class Shell {
LOG.warn("Error while closing the error stream", ioe);
}
process.destroy();
- lastTime = MathUtils.now();
+ lastTime = System.currentTimeMillis();
}
}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
index e0c9e2f..e35c309 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
@@ -70,7 +70,6 @@ import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
import org.apache.bookkeeper.test.TestStatsProvider;
import org.apache.bookkeeper.util.DiskChecker;
import org.apache.bookkeeper.util.HardLink;
-import org.apache.bookkeeper.util.MathUtils;
import org.apache.bookkeeper.util.TestUtils;
import org.apache.bookkeeper.versioning.Version;
import org.apache.bookkeeper.versioning.Versioned;
@@ -268,7 +267,7 @@ public abstract class CompactionTest extends BookKeeperClusterTestCase {
Checkpointer.NULL,
NullStatsLogger.INSTANCE);
storage.start();
- long startTime = MathUtils.now();
+ long startTime = System.currentTimeMillis();
storage.gcThread.enableForceGC();
storage.gcThread.triggerGC().get(); //major
storage.gcThread.triggerGC().get(); //minor
@@ -1174,7 +1173,7 @@ public abstract class CompactionTest extends BookKeeperClusterTestCase {
storage.gcThread.suspendMajorGC();
Thread.sleep(1000);
- long startTime = MathUtils.now();
+ long startTime = System.currentTimeMillis();
majorCompactions = stats.getCounter("storage.gc." + MAJOR_COMPACTION_COUNT).get().intValue();
Thread.sleep(conf.getMajorCompactionInterval() * 1000
+ conf.getGcWaitTime());
@@ -1194,7 +1193,7 @@ public abstract class CompactionTest extends BookKeeperClusterTestCase {
storage.gcThread.suspendMinorGC();
Thread.sleep(1000);
- startTime = MathUtils.now();
+ startTime = System.currentTimeMillis();
minorCompactions = stats.getCounter("storage.gc." + MINOR_COMPACTION_COUNT).get().intValue();
Thread.sleep(conf.getMajorCompactionInterval() * 1000
+ conf.getGcWaitTime());