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 <marc...@apache.org> 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org