This is an automated email from the ASF dual-hosted git repository.
marcuse pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new 60675cc Remove and ban use of Instant.now
60675cc is described below
commit 60675cc2759db0c5629604279e70c51e10dfefd6
Author: Marcus Eriksson <[email protected]>
AuthorDate: Wed Mar 2 11:33:39 2022 +0100
Remove and ban use of Instant.now
Patch by marcuse; reviewed by Benedict Elliott Smith for CASSANDRA-17414
---
checkstyle.xml | 9 +++++++++
.../org/apache/cassandra/db/ColumnFamilyStore.java | 12 ++++++------
src/java/org/apache/cassandra/db/Keyspace.java | 3 ++-
src/java/org/apache/cassandra/db/SystemKeyspace.java | 3 ++-
.../cassandra/db/compaction/CompactionTask.java | 6 +++---
.../org/apache/cassandra/service/StorageService.java | 5 +++--
.../cassandra/service/snapshot/SnapshotManager.java | 3 ++-
src/java/org/apache/cassandra/utils/Clock.java | 1 -
src/java/org/apache/cassandra/utils/FBUtilities.java | 7 +++++++
.../cassandra/distributed/test/ResourceLeakTest.java | 4 ++--
.../unit/org/apache/cassandra/db/DirectoriesTest.java | 5 +++--
.../service/snapshot/SnapshotManagerTest.java | 15 ++++++---------
.../cassandra/service/snapshot/TableSnapshotTest.java | 19 ++++++++++---------
.../cassandra/utils/concurrent/LoadingMapTest.java | 5 +++--
14 files changed, 58 insertions(+), 39 deletions(-)
diff --git a/checkstyle.xml b/checkstyle.xml
index 19bfa86..5efe0fc 100644
--- a/checkstyle.xml
+++ b/checkstyle.xml
@@ -62,6 +62,15 @@
<property name="ignoreComments" value="true"/>
<property name="message" value="Avoid System for time, should use
org.apache.cassandra.utils.Clock.Global or org.apache.cassandra.utils.Clock
interface" />
</module>
+
+ <module name="RegexpSinglelineJava">
+ <!-- block Instant.now -->
+ <property name="id" value="blockInstantNow"/>
+ <property name="format" value="Instant\.now"/>
+ <property name="ignoreComments" value="true"/>
+ <property name="message" value="Avoid Instant.now() for time, should use
org.apache.cassandra.util.FBUtilities.now()" />
+ </module>
+
<module name="RegexpSinglelineJava">
<!-- block normal executors -->
<property name="id" value="blockExecutors"/>
diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
index aa2b165..cc0c294 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -113,6 +113,7 @@ import static
org.apache.cassandra.db.commitlog.CommitLog.instance;
import static org.apache.cassandra.db.commitlog.CommitLogPosition.NONE;
import static org.apache.cassandra.utils.Clock.Global.currentTimeMillis;
import static org.apache.cassandra.utils.Clock.Global.nanoTime;
+import static org.apache.cassandra.utils.FBUtilities.now;
import static org.apache.cassandra.utils.Throwables.maybeFail;
import static org.apache.cassandra.utils.Throwables.merge;
import static
org.apache.cassandra.utils.concurrent.CountDownLatch.newCountDownLatch;
@@ -1539,9 +1540,8 @@ public class ColumnFamilyStore implements
ColumnFamilyStoreMBean
// skip snapshot creation during scrub, SEE JIRA 5891
if(!disableSnapshot)
{
- long epochMilli = currentTimeMillis();
- Instant creationTime = Instant.ofEpochMilli(epochMilli);
- String snapshotName = "pre-scrub-" + epochMilli;
+ Instant creationTime = now();
+ String snapshotName = "pre-scrub-" + creationTime.toEpochMilli();
snapshotWithoutFlush(snapshotName, creationTime);
}
@@ -1858,7 +1858,7 @@ public class ColumnFamilyStore implements
ColumnFamilyStoreMBean
public TableSnapshot snapshotWithoutFlush(String snapshotName)
{
- return snapshotWithoutFlush(snapshotName, Instant.now());
+ return snapshotWithoutFlush(snapshotName, now());
}
public TableSnapshot snapshotWithoutFlush(String snapshotName, Instant
creationTime)
@@ -2051,7 +2051,7 @@ public class ColumnFamilyStore implements
ColumnFamilyStoreMBean
*/
public TableSnapshot snapshot(String snapshotName)
{
- return snapshot(snapshotName, false, null, null, Instant.now());
+ return snapshot(snapshotName, false, null, null, now());
}
/**
@@ -2075,7 +2075,7 @@ public class ColumnFamilyStore implements
ColumnFamilyStoreMBean
*/
public TableSnapshot snapshot(String snapshotName,
Predicate<SSTableReader> predicate, boolean ephemeral, boolean skipFlush)
{
- return snapshot(snapshotName, predicate, ephemeral, skipFlush, null,
null, Instant.now());
+ return snapshot(snapshotName, predicate, ephemeral, skipFlush, null,
null, now());
}
/**
diff --git a/src/java/org/apache/cassandra/db/Keyspace.java
b/src/java/org/apache/cassandra/db/Keyspace.java
index 606e7e8..4ef3d02 100644
--- a/src/java/org/apache/cassandra/db/Keyspace.java
+++ b/src/java/org/apache/cassandra/db/Keyspace.java
@@ -77,6 +77,7 @@ import org.apache.cassandra.utils.concurrent.Promise;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.NANOSECONDS;
import static org.apache.cassandra.utils.Clock.Global.currentTimeMillis;
+import static org.apache.cassandra.utils.FBUtilities.now;
import static org.apache.cassandra.utils.MonotonicClock.Global.approxTime;
/**
@@ -241,7 +242,7 @@ public class Keyspace
*/
public void snapshot(String snapshotName, String columnFamilyName) throws
IOException
{
- snapshot(snapshotName, columnFamilyName, false, null, null,
Instant.now());
+ snapshot(snapshotName, columnFamilyName, false, null, null, now());
}
/**
diff --git a/src/java/org/apache/cassandra/db/SystemKeyspace.java
b/src/java/org/apache/cassandra/db/SystemKeyspace.java
index 2205750..e68a788 100644
--- a/src/java/org/apache/cassandra/db/SystemKeyspace.java
+++ b/src/java/org/apache/cassandra/db/SystemKeyspace.java
@@ -75,6 +75,7 @@ import static
org.apache.cassandra.cql3.QueryProcessor.executeInternal;
import static org.apache.cassandra.cql3.QueryProcessor.executeOnceInternal;
import static org.apache.cassandra.utils.Clock.Global.currentTimeMillis;
import static org.apache.cassandra.utils.Clock.Global.nanoTime;
+import static org.apache.cassandra.utils.FBUtilities.now;
public final class SystemKeyspace
{
@@ -1477,7 +1478,7 @@ public final class SystemKeyspace
previous,
next));
- Instant creationTime = Instant.now();
+ Instant creationTime = now();
for (String keyspace : SchemaConstants.LOCAL_SYSTEM_KEYSPACE_NAMES)
Keyspace.open(keyspace).snapshot(snapshotName, null, false,
null, null, creationTime);
}
diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionTask.java
b/src/java/org/apache/cassandra/db/compaction/CompactionTask.java
index ec730c6..002f0c3 100644
--- a/src/java/org/apache/cassandra/db/compaction/CompactionTask.java
+++ b/src/java/org/apache/cassandra/db/compaction/CompactionTask.java
@@ -49,6 +49,7 @@ import org.apache.cassandra.utils.concurrent.Refs;
import static org.apache.cassandra.utils.Clock.Global.currentTimeMillis;
import static org.apache.cassandra.utils.Clock.Global.nanoTime;
+import static org.apache.cassandra.utils.FBUtilities.now;
public class CompactionTask extends AbstractCompactionTask
{
@@ -120,9 +121,8 @@ public class CompactionTask extends AbstractCompactionTask
if (DatabaseDescriptor.isSnapshotBeforeCompaction())
{
- long epochMilli = currentTimeMillis();
- Instant creationTime = Instant.ofEpochMilli(epochMilli);
- cfs.snapshotWithoutFlush(epochMilli + "-compact-" + cfs.name,
creationTime);
+ Instant creationTime = now();
+ cfs.snapshotWithoutFlush(creationTime.toEpochMilli() + "-compact-"
+ cfs.name, creationTime);
}
try (CompactionController controller =
getCompactionController(transaction.originals()))
diff --git a/src/java/org/apache/cassandra/service/StorageService.java
b/src/java/org/apache/cassandra/service/StorageService.java
index c47812d..1a7b457 100644
--- a/src/java/org/apache/cassandra/service/StorageService.java
+++ b/src/java/org/apache/cassandra/service/StorageService.java
@@ -164,6 +164,7 @@ import static
org.apache.cassandra.schema.MigrationManager.evolveSystemKeyspace;
import static org.apache.cassandra.utils.Clock.Global.currentTimeMillis;
import static org.apache.cassandra.utils.Clock.Global.nanoTime;
import static org.apache.cassandra.service.ActiveRepairService.*;
+import static org.apache.cassandra.utils.FBUtilities.now;
/**
* This abstraction contains the token/identifier of this node
@@ -3979,7 +3980,7 @@ public class StorageService extends
NotificationBroadcasterSupport implements IE
RateLimiter snapshotRateLimiter =
DatabaseDescriptor.getSnapshotRateLimiter();
- Instant creationTime = Instant.now();
+ Instant creationTime = now();
for (Keyspace keyspace : keyspaces)
{
@@ -4045,7 +4046,7 @@ public class StorageService extends
NotificationBroadcasterSupport implements IE
}
RateLimiter snapshotRateLimiter =
DatabaseDescriptor.getSnapshotRateLimiter();
- Instant creationTime = Instant.now();
+ Instant creationTime = now();
for (Entry<Keyspace, List<String>> entry :
keyspaceColumnfamily.entrySet())
{
diff --git
a/src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java
b/src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java
index 7f1c4e9..8d127a3 100644
--- a/src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java
+++ b/src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java
@@ -44,6 +44,7 @@ import com.google.common.annotations.VisibleForTesting;
import org.apache.cassandra.io.util.File;
import org.apache.cassandra.utils.ExecutorUtils;
import static
org.apache.cassandra.concurrent.ExecutorFactory.Global.executorFactory;
+import static org.apache.cassandra.utils.FBUtilities.now;
public class SnapshotManager {
@@ -133,7 +134,7 @@ public class SnapshotManager {
@VisibleForTesting
protected synchronized void clearExpiredSnapshots()
{
- Instant now = Instant.now();
+ Instant now = now();
while (!expiringSnapshots.isEmpty() &&
expiringSnapshots.peek().isExpired(now))
{
TableSnapshot expiredSnapshot = expiringSnapshots.peek();
diff --git a/src/java/org/apache/cassandra/utils/Clock.java
b/src/java/org/apache/cassandra/utils/Clock.java
index acdfc82..4d6bd1a 100644
--- a/src/java/org/apache/cassandra/utils/Clock.java
+++ b/src/java/org/apache/cassandra/utils/Clock.java
@@ -20,7 +20,6 @@ package org.apache.cassandra.utils;
import java.util.concurrent.TimeUnit;
import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import static
org.apache.cassandra.config.CassandraRelevantProperties.CLOCK_GLOBAL;
import static org.apache.cassandra.utils.Shared.Scope.SIMULATION;
diff --git a/src/java/org/apache/cassandra/utils/FBUtilities.java
b/src/java/org/apache/cassandra/utils/FBUtilities.java
index 3c7d210..cf2f846 100644
--- a/src/java/org/apache/cassandra/utils/FBUtilities.java
+++ b/src/java/org/apache/cassandra/utils/FBUtilities.java
@@ -31,6 +31,7 @@ import java.net.*;
import java.nio.ByteBuffer;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
+import java.time.Instant;
import java.util.*;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
@@ -454,6 +455,12 @@ public class FBUtilities
return (int) (currentTimeMillis() / 1000);
}
+ public static Instant now()
+ {
+ long epochMilli = currentTimeMillis();
+ return Instant.ofEpochMilli(epochMilli);
+ }
+
public static <T> List<T> waitOnFutures(Iterable<? extends Future<?
extends T>> futures)
{
return waitOnFutures(futures, -1, null);
diff --git
a/test/distributed/org/apache/cassandra/distributed/test/ResourceLeakTest.java
b/test/distributed/org/apache/cassandra/distributed/test/ResourceLeakTest.java
index 8358c2e..a5a2dce 100644
---
a/test/distributed/org/apache/cassandra/distributed/test/ResourceLeakTest.java
+++
b/test/distributed/org/apache/cassandra/distributed/test/ResourceLeakTest.java
@@ -24,7 +24,6 @@ import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.sql.Date;
import java.text.SimpleDateFormat;
-import java.time.Instant;
import java.util.function.Consumer;
import javax.management.MBeanServer;
@@ -41,6 +40,7 @@ import org.apache.cassandra.utils.SigarLibrary;
import static org.apache.cassandra.distributed.api.Feature.GOSSIP;
import static org.apache.cassandra.distributed.api.Feature.NATIVE_PROTOCOL;
import static org.apache.cassandra.distributed.api.Feature.NETWORK;
+import static org.apache.cassandra.utils.FBUtilities.now;
/* Resource Leak Test - useful when tracking down issues with in-JVM framework
cleanup.
* All objects referencing the InstanceClassLoader need to be garbage
collected or
@@ -65,7 +65,7 @@ public class ResourceLeakTest extends TestBaseImpl
final long finalWaitMillis = 0l; // Number of millis to wait before
final resource dump to give gc a chance
static final SimpleDateFormat format = new
SimpleDateFormat("yyyyMMddHHmmss");
- static final String when = format.format(Date.from(Instant.now()));
+ static final String when = format.format(Date.from(now()));
static String outputFilename(String base, String description, String
extension)
{
diff --git a/test/unit/org/apache/cassandra/db/DirectoriesTest.java
b/test/unit/org/apache/cassandra/db/DirectoriesTest.java
index d21c06c..28cbc47 100644
--- a/test/unit/org/apache/cassandra/db/DirectoriesTest.java
+++ b/test/unit/org/apache/cassandra/db/DirectoriesTest.java
@@ -62,6 +62,7 @@ import org.apache.cassandra.service.snapshot.SnapshotManifest;
import org.apache.cassandra.service.snapshot.TableSnapshot;
import org.apache.cassandra.utils.JVMStabilityInspector;
+import static org.apache.cassandra.utils.FBUtilities.now;
import static org.assertj.core.api.Assertions.assertThat;
import static org.apache.cassandra.utils.Clock.Global.nanoTime;
import static org.junit.Assert.assertEquals;
@@ -187,7 +188,7 @@ public class DirectoriesTest
if (createManifest)
{
File manifestFile =
Directories.getSnapshotManifestFile(snapshotDir);
- manifest = new
SnapshotManifest(Collections.singletonList(sstableDesc.filenameFor(Component.DATA)),
new DurationSpec("1m"), Instant.now());
+ manifest = new
SnapshotManifest(Collections.singletonList(sstableDesc.filenameFor(Component.DATA)),
new DurationSpec("1m"), now());
manifest.serializeToJsonFile(manifestFile);
}
@@ -312,7 +313,7 @@ public class DirectoriesTest
File manifestFile = directories.getSnapshotManifestFile(tag);
- SnapshotManifest manifest = new SnapshotManifest(files, new
DurationSpec("1m"), Instant.now());
+ SnapshotManifest manifest = new SnapshotManifest(files, new
DurationSpec("1m"), now());
manifest.serializeToJsonFile(manifestFile);
Set<File> dirs = new HashSet<>();
diff --git
a/test/unit/org/apache/cassandra/service/snapshot/SnapshotManagerTest.java
b/test/unit/org/apache/cassandra/service/snapshot/SnapshotManagerTest.java
index 20cc8e2..9530c95 100644
--- a/test/unit/org/apache/cassandra/service/snapshot/SnapshotManagerTest.java
+++ b/test/unit/org/apache/cassandra/service/snapshot/SnapshotManagerTest.java
@@ -18,12 +18,9 @@
package org.apache.cassandra.service.snapshot;
-import java.io.IOException;
import java.time.Instant;
import java.util.Arrays;
-import java.util.HashSet;
import java.util.List;
-import java.util.Set;
import java.util.stream.Stream;
import org.junit.BeforeClass;
@@ -32,11 +29,11 @@ import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.apache.cassandra.config.DatabaseDescriptor;
-import org.apache.cassandra.io.util.File;
import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.service.DefaultFSErrorHandler;
import static
org.apache.cassandra.service.snapshot.TableSnapshotTest.createFolders;
+import static org.apache.cassandra.utils.FBUtilities.now;
import static org.assertj.core.api.Assertions.assertThat;
public class SnapshotManagerTest
@@ -68,7 +65,7 @@ public class SnapshotManagerTest
@Test
public void testLoadSnapshots() throws Exception {
TableSnapshot expired = generateSnapshotDetails("expired",
Instant.EPOCH);
- TableSnapshot nonExpired = generateSnapshotDetails("non-expired",
Instant.now().plusSeconds(ONE_DAY_SECS));
+ TableSnapshot nonExpired = generateSnapshotDetails("non-expired",
now().plusSeconds(ONE_DAY_SECS));
TableSnapshot nonExpiring = generateSnapshotDetails("non-expiring",
null);
List<TableSnapshot> snapshots = Arrays.asList(expired, nonExpired,
nonExpiring);
@@ -88,7 +85,7 @@ public class SnapshotManagerTest
// Add 3 snapshots: expired, non-expired and non-expiring
TableSnapshot expired = generateSnapshotDetails("expired",
Instant.EPOCH);
- TableSnapshot nonExpired = generateSnapshotDetails("non-expired",
Instant.now().plusMillis(ONE_DAY_SECS));
+ TableSnapshot nonExpired = generateSnapshotDetails("non-expired",
now().plusMillis(ONE_DAY_SECS));
TableSnapshot nonExpiring = generateSnapshotDetails("non-expiring",
null);
manager.addSnapshot(expired);
manager.addSnapshot(nonExpired);
@@ -121,8 +118,8 @@ public class SnapshotManagerTest
// Add 2 expiring snapshots: one to expire in 2 seconds, another
in 1 day
int TTL_SECS = 2;
- TableSnapshot toExpire = generateSnapshotDetails("to-expire",
Instant.now().plusSeconds(TTL_SECS));
- TableSnapshot nonExpired = generateSnapshotDetails("non-expired",
Instant.now().plusMillis(ONE_DAY_SECS));
+ TableSnapshot toExpire = generateSnapshotDetails("to-expire",
now().plusSeconds(TTL_SECS));
+ TableSnapshot nonExpired = generateSnapshotDetails("non-expired",
now().plusMillis(ONE_DAY_SECS));
manager.addSnapshot(toExpire);
manager.addSnapshot(nonExpired);
@@ -153,7 +150,7 @@ public class SnapshotManagerTest
{
// Given
SnapshotManager manager = new SnapshotManager(1, 3, Stream::empty);
- TableSnapshot expiringSnapshot = generateSnapshotDetails("snapshot",
Instant.now().plusMillis(50000));
+ TableSnapshot expiringSnapshot = generateSnapshotDetails("snapshot",
now().plusMillis(50000));
manager.addSnapshot(expiringSnapshot);
assertThat(manager.getExpiringSnapshots()).contains(expiringSnapshot);
assertThat(expiringSnapshot.exists()).isTrue();
diff --git
a/test/unit/org/apache/cassandra/service/snapshot/TableSnapshotTest.java
b/test/unit/org/apache/cassandra/service/snapshot/TableSnapshotTest.java
index 691aa70..0b77472 100644
--- a/test/unit/org/apache/cassandra/service/snapshot/TableSnapshotTest.java
+++ b/test/unit/org/apache/cassandra/service/snapshot/TableSnapshotTest.java
@@ -34,6 +34,7 @@ import org.apache.cassandra.io.util.File;
import org.apache.cassandra.io.util.FileOutputStreamPlus;
import org.apache.cassandra.io.util.FileUtils;
+import static org.apache.cassandra.utils.FBUtilities.now;
import static org.assertj.core.api.Assertions.assertThat;
public class TableSnapshotTest
@@ -97,46 +98,46 @@ public class TableSnapshotTest
);
assertThat(snapshot.isExpiring()).isFalse();
- assertThat(snapshot.isExpired(Instant.now())).isFalse();
+ assertThat(snapshot.isExpired(now())).isFalse();
snapshot = new TableSnapshot(
"ks",
"tbl",
"some",
- Instant.now(),
+ now(),
null,
folders,
(File file) -> 0L
);
assertThat(snapshot.isExpiring()).isFalse();
- assertThat(snapshot.isExpired(Instant.now())).isFalse();
+ assertThat(snapshot.isExpired(now())).isFalse();
snapshot = new TableSnapshot(
"ks",
"tbl",
"some",
- Instant.now(),
- Instant.now().plusSeconds(1000),
+ now(),
+ now().plusSeconds(1000),
folders,
(File file) -> 0L
);
assertThat(snapshot.isExpiring()).isTrue();
- assertThat(snapshot.isExpired(Instant.now())).isFalse();
+ assertThat(snapshot.isExpired(now())).isFalse();
snapshot = new TableSnapshot(
"ks",
"tbl",
"some",
- Instant.now(),
- Instant.now().minusSeconds(1000),
+ now(),
+ now().minusSeconds(1000),
folders,
(File file) -> 0L
);
assertThat(snapshot.isExpiring()).isTrue();
- assertThat(snapshot.isExpired(Instant.now())).isTrue();
+ assertThat(snapshot.isExpired(now())).isTrue();
}
private Long writeBatchToFile(File file) throws IOException
diff --git
a/test/unit/org/apache/cassandra/utils/concurrent/LoadingMapTest.java
b/test/unit/org/apache/cassandra/utils/concurrent/LoadingMapTest.java
index d648734..822e9eb 100644
--- a/test/unit/org/apache/cassandra/utils/concurrent/LoadingMapTest.java
+++ b/test/unit/org/apache/cassandra/utils/concurrent/LoadingMapTest.java
@@ -38,6 +38,7 @@ import org.awaitility.Awaitility;
import org.awaitility.core.ConditionFactory;
import static
org.apache.cassandra.concurrent.ExecutorFactory.Global.executorFactory;
+import static org.apache.cassandra.utils.FBUtilities.now;
import static org.assertj.core.api.Assertions.assertThat;
public class LoadingMapTest
@@ -58,10 +59,10 @@ public class LoadingMapTest
@After
public void afterTest() throws TimeoutException
{
- Instant deadline = Instant.now().plus(Duration.ofSeconds(5));
+ Instant deadline = now().plus(Duration.ofSeconds(5));
while (executor.getPendingTaskCount() > 0 ||
executor.getActiveTaskCount() > 0)
{
- if (Instant.now().isAfter(deadline))
+ if (now().isAfter(deadline))
throw new TimeoutException();
Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
