This is an automated email from the ASF dual-hosted git repository.
kturner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new ae6085c346 Adds NanoTime wrapper for System.nanoTime (#4364)
ae6085c346 is described below
commit ae6085c34677e752949ef093cb350281b41a275f
Author: Keith Turner <[email protected]>
AuthorDate: Wed Mar 13 14:08:17 2024 -0400
Adds NanoTime wrapper for System.nanoTime (#4364)
* Adds NanoTime wrapper for System.nanoTime
Adds a strong type for System.nanoTime() and uses it in a few places.
Could be used in many more places if this is merged.
Co-authored-by: EdColeman <[email protected]>
---
.../org/apache/accumulo/core/fate/ZooStore.java | 15 +-
.../apache/accumulo/core/util/time/NanoTime.java | 104 +++++++++++++
.../accumulo/core/util/time/NanoTimeTest.java | 162 +++++++++++++++++++++
.../java/org/apache/accumulo/manager/Manager.java | 9 +-
4 files changed, 280 insertions(+), 10 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/fate/ZooStore.java
b/core/src/main/java/org/apache/accumulo/core/fate/ZooStore.java
index 941c04c241..c3de5f29df 100644
--- a/core/src/main/java/org/apache/accumulo/core/fate/ZooStore.java
+++ b/core/src/main/java/org/apache/accumulo/core/fate/ZooStore.java
@@ -39,12 +39,12 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.concurrent.TimeUnit;
import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter;
import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeExistsPolicy;
import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeMissingPolicy;
import org.apache.accumulo.core.util.FastFormat;
+import org.apache.accumulo.core.util.time.NanoTime;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.KeeperException.NoNodeException;
import org.apache.zookeeper.KeeperException.NodeExistsException;
@@ -64,7 +64,7 @@ public class ZooStore<T> implements TStore<T> {
private ZooReaderWriter zk;
private String lastReserved = "";
private Set<Long> reserved;
- private Map<Long,Long> deferred; // use Long here to properly handle
System.nanoTime()
+ private Map<Long,NanoTime> deferred;
private long statusChangeEvents = 0;
private int reservationsWaiting = 0;
@@ -164,7 +164,7 @@ public class ZooStore<T> implements TStore<T> {
}
if (deferred.containsKey(tid)) {
- if (deferred.get(tid) - System.nanoTime() < 0) {
+ if (deferred.get(tid).elapsed().compareTo(Duration.ZERO) > 0) {
deferred.remove(tid);
} else {
continue;
@@ -203,10 +203,11 @@ public class ZooStore<T> implements TStore<T> {
if (deferred.isEmpty()) {
this.wait(5000);
} else {
- final long now = System.nanoTime();
- long minWait = deferred.values().stream().mapToLong(l -> l -
now).min().orElseThrow();
+ var now = NanoTime.now();
+ long minWait = deferred.values().stream()
+ .mapToLong(nanoTime ->
nanoTime.subtract(now).toMillis()).min().orElseThrow();
if (minWait > 0) {
- this.wait(Math.min(TimeUnit.NANOSECONDS.toMillis(minWait),
5000));
+ this.wait(Math.min(minWait, 5000));
}
}
}
@@ -284,7 +285,7 @@ public class ZooStore<T> implements TStore<T> {
}
if (deferTime.compareTo(Duration.ZERO) > 0) {
- deferred.put(tid, deferTime.toNanos() + System.nanoTime());
+ deferred.put(tid, NanoTime.nowPlus(deferTime));
}
this.notifyAll();
diff --git
a/core/src/main/java/org/apache/accumulo/core/util/time/NanoTime.java
b/core/src/main/java/org/apache/accumulo/core/util/time/NanoTime.java
new file mode 100644
index 0000000000..f081278589
--- /dev/null
+++ b/core/src/main/java/org/apache/accumulo/core/util/time/NanoTime.java
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.util.time;
+
+import java.time.Duration;
+
+import com.google.common.annotations.VisibleForTesting;
+
+/**
+ * This class implements a strong type for System.nanoTime() that offers the
limited operations that
+ * can be performed on a nanoTime. See the System.nanoTime() javadoc for
details - specifically
+ * these values are meaningful only when the difference between two such
values, obtained within the
+ * same instance of a Java virtual machine, are computed.
+ */
+public final class NanoTime implements Comparable<NanoTime> {
+ // In the System.nanoTime javadoc it describes the returned value as the
"nanoseconds since some
+ // fixed but arbitrary origin time (perhaps in the future, so values may be
negative)". This
+ // variable name is derived from that where AO is arbitrary origin.
+ private final long nanosSinceAO;
+
+ // This method should only be called by test inorder to test edge
conditions, that is why it is
+ // package private. Calling this outside of test makes it hard to reason
about the correctness of
+ // using this class.
+ @VisibleForTesting
+ NanoTime(long ntsao) {
+ this.nanosSinceAO = ntsao;
+ }
+
+ /**
+ * @return this.nanoTime - other.nanoTime as a Duration
+ */
+ public Duration subtract(NanoTime other) {
+ return Duration.ofNanos(nanosSinceAO - other.nanosSinceAO);
+ }
+
+ /**
+ * Determines the amount of time that has elapsed since this object was
created relative to the
+ * current nanoTime.
+ *
+ * @return System.nanoTime() - this.nanoTime
+ */
+ public Duration elapsed() {
+ return Duration.ofNanos(System.nanoTime() - nanosSinceAO);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (other instanceof NanoTime) {
+ return nanosSinceAO == ((NanoTime) other).nanosSinceAO;
+ }
+
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return Long.hashCode(nanosSinceAO);
+ }
+
+ @Override
+ public int compareTo(NanoTime other) {
+ // All operations w/ nanoTimes must use differences, can not directly
compare. This is because a
+ // nano time of Long.MAX_VALUE -10 is considered less than Long.MAX_VALUE
+10
+ long diff = nanosSinceAO - other.nanosSinceAO;
+
+ if (diff < 0) {
+ return -1;
+ } else if (diff > 0) {
+ return 1;
+ } else {
+ return 0;
+ }
+ }
+
+ /**
+ * @return a NanoTime created using System.nanoTime()
+ */
+ public static NanoTime now() {
+ return new NanoTime(System.nanoTime());
+ }
+
+ /**
+ * @return a NanoTime created using System.nanoTime() + duration.toNanos()
+ */
+ public static NanoTime nowPlus(Duration duration) {
+ return new NanoTime(System.nanoTime() + duration.toNanos());
+ }
+}
diff --git
a/core/src/test/java/org/apache/accumulo/core/util/time/NanoTimeTest.java
b/core/src/test/java/org/apache/accumulo/core/util/time/NanoTimeTest.java
new file mode 100644
index 0000000000..d306aafc39
--- /dev/null
+++ b/core/src/test/java/org/apache/accumulo/core/util/time/NanoTimeTest.java
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.util.time;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+public class NanoTimeTest {
+ @Test
+ public void testMultipleTimes() {
+ List<NanoTime> ntimes = new ArrayList<>();
+
+ NanoTime prev = NanoTime.now();
+ ntimes.add(prev);
+
+ for (int i = 0; i < 100; i++) {
+ NanoTime next = NanoTime.now();
+ while (prev.equals(next)) {
+ next = NanoTime.now();
+ }
+
+ ntimes.add(next);
+ prev = next;
+ }
+
+ long curr = System.nanoTime();
+ while (curr == System.nanoTime()) {}
+
+ var start = NanoTime.now();
+
+ while (start.equals(NanoTime.now())) {}
+
+ for (int i = 1; i < ntimes.size(); i++) {
+ var last = ntimes.get(i - 1);
+ var next = ntimes.get(i);
+ assertTrue(last.compareTo(next) < 0);
+ assertTrue(next.compareTo(last) > 0);
+ assertTrue(next.compareTo(next) == 0);
+ assertTrue(next.elapsed().toNanos() > 0);
+ assertEquals(next, next);
+ assertEquals(next.hashCode(), next.hashCode());
+ assertNotEquals(last, next);
+ assertNotEquals(last.hashCode(), next.hashCode());
+
+ var duration1 = next.elapsed();
+ var duration2 = start.subtract(last);
+ var duration3 = start.subtract(next);
+
+ assertTrue(duration2.compareTo(duration3) > 0);
+ assertTrue(duration1.compareTo(duration3) > 0);
+ }
+
+ var copy = List.copyOf(ntimes);
+ Collections.shuffle(ntimes);
+ Collections.sort(ntimes);
+ assertEquals(copy, ntimes);
+ }
+
+ @Test
+ public void testBoundry() {
+ // tests crossing the Long.MAX_VALUE boundry
+ long origin = Long.MAX_VALUE - 1000;
+
+ List<NanoTime> ntimes = new ArrayList<>();
+
+ // add times that start positive and then go negative
+ for (int i = 0; i < 20; i++) {
+ var nt = i * 100 + origin;
+ ntimes.add(new NanoTime(nt));
+ }
+
+ for (int i = 1; i < ntimes.size(); i++) {
+ var last = ntimes.get(i - 1);
+ var next = ntimes.get(i);
+ assertEquals(100, next.subtract(last).toNanos());
+ assertEquals(-100, last.subtract(next).toNanos());
+ assertTrue(next.compareTo(last) > 0);
+ assertTrue(last.compareTo(next) < 0);
+ assertTrue(next.compareTo(next) == 0);
+ }
+
+ var copy = List.copyOf(ntimes);
+ Collections.shuffle(ntimes);
+ Collections.sort(ntimes);
+ assertEquals(copy, ntimes);
+ }
+
+ @Test
+ public void testNowPlus() {
+
+ List<NanoTime> ntimes = new ArrayList<>();
+ for (int i = 0; i < 10; i++) {
+ ntimes.add(NanoTime.nowPlus(Duration.ofHours(i)));
+ }
+
+ for (int i = 1; i < ntimes.size(); i++) {
+ var last = ntimes.get(i - 1);
+ var next = ntimes.get(i);
+
+ var duration = next.subtract(last);
+
+ assertTrue(duration.compareTo(Duration.ofHours(1)) >= 0);
+ // This could fail if the test process were paused for more than 3
minutes
+ assertTrue(duration.compareTo(Duration.ofMinutes(63)) < 0);
+ assertTrue(next.elapsed().compareTo(Duration.ZERO) < 0);
+ }
+
+ var copy = List.copyOf(ntimes);
+ Collections.shuffle(ntimes);
+ Collections.sort(ntimes);
+ assertEquals(copy, ntimes);
+
+ ntimes.clear();
+
+ // nano time can compute elapsed times in a 290 year period which should
wrap Long.MAX_VALUE no
+ // matter where it starts
+ for (int i = 0; i < 290; i++) {
+ ntimes.add(NanoTime.nowPlus(Duration.ofDays(365 * i)));
+ }
+
+ for (int i = 1; i < ntimes.size(); i++) {
+ var last = ntimes.get(i - 1);
+ var next = ntimes.get(i);
+
+ var duration = next.subtract(last);
+
+ assertTrue(duration.compareTo(Duration.ofDays(365)) >= 0);
+ assertTrue(duration.compareTo(Duration.ofDays(366)) < 0);
+ assertTrue(next.elapsed().compareTo(Duration.ZERO) < 0);
+ }
+
+ copy = List.copyOf(ntimes);
+ Collections.shuffle(ntimes);
+ Collections.sort(ntimes);
+ assertEquals(copy, ntimes);
+ }
+
+}
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
index bdf73b9191..cbbba0fb7f 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
@@ -112,6 +112,7 @@ import org.apache.accumulo.core.util.Halt;
import org.apache.accumulo.core.util.Retry;
import org.apache.accumulo.core.util.threads.ThreadPools;
import org.apache.accumulo.core.util.threads.Threads;
+import org.apache.accumulo.core.util.time.NanoTime;
import org.apache.accumulo.manager.metrics.ManagerMetrics;
import org.apache.accumulo.manager.recovery.RecoveryManager;
import org.apache.accumulo.manager.state.TableCounts;
@@ -156,6 +157,7 @@ import org.apache.zookeeper.Watcher;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.Comparators;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.util.concurrent.RateLimiter;
import com.google.common.util.concurrent.Uninterruptibles;
@@ -1022,11 +1024,12 @@ public class Manager extends AbstractServer
}));
}
// wait at least 10 seconds
- final long nanosToWait = Math.max(SECONDS.toNanos(10),
MILLISECONDS.toNanos(rpcTimeout) / 3);
- final long startTime = System.nanoTime();
+ final Duration timeToWait =
+ Comparators.max(Duration.ofSeconds(10), Duration.ofMillis(rpcTimeout /
3));
+ final NanoTime startTime = NanoTime.now();
// Wait for all tasks to complete
while (!tasks.isEmpty()) {
- boolean cancel = ((System.nanoTime() - startTime) > nanosToWait);
+ boolean cancel = (startTime.elapsed().compareTo(timeToWait) > 0);
Iterator<Future<?>> iter = tasks.iterator();
while (iter.hasNext()) {
Future<?> f = iter.next();
