You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by sk...@apache.org on 2021/11/01 11:15:59 UTC
[ignite-3] branch main updated: IGNITE-15842 Fixed
RocksDbStorageTest#testSnapshot fail on Windows platform. Fixes #417
This is an automated email from the ASF dual-hosted git repository.
sk0x50 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 0996363 IGNITE-15842 Fixed RocksDbStorageTest#testSnapshot fail on Windows platform. Fixes #417
0996363 is described below
commit 099636376042d927f5d1e316150c6bc2ae388988
Author: Slava Koptilin <sl...@gmail.com>
AuthorDate: Mon Nov 1 14:15:31 2021 +0300
IGNITE-15842 Fixed RocksDbStorageTest#testSnapshot fail on Windows platform. Fixes #417
---
.../apache/ignite/internal/util/IgniteUtils.java | 94 +++++++++++++++++-----
.../server/persistence/RocksDBKeyValueStorage.java | 2 +-
.../org/apache/ignite/raft/jraft/util/Utils.java | 52 ++----------
.../storage/AbstractPartitionStorageTest.java | 9 ++-
.../storage/rocksdb/RocksDbPartitionStorage.java | 3 +-
.../storage/rocksdb/RocksDbTableStorage.java | 4 +-
6 files changed, 89 insertions(+), 75 deletions(-)
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java b/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
index 5b06fd9..fd334c9 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
@@ -18,11 +18,13 @@
package org.apache.ignite.internal.util;
import java.io.IOException;
+import java.nio.file.AtomicMoveNotSupportedException;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.SimpleFileVisitor;
+import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.BasicFileAttributes;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
@@ -58,7 +60,8 @@ public class IgniteUtils {
/** Class loader used to load Ignite. */
private static final ClassLoader igniteClassLoader = IgniteUtils.class.getClassLoader();
- private static final boolean assertionsEnabled;
+ /** Indicates that assertions are enabled. */
+ private static final boolean assertionsEnabled = IgniteUtils.class.desiredAssertionStatus();
/**
* Gets the current monotonic time in milliseconds.
@@ -85,25 +88,6 @@ public class IgniteUtils {
private static final ConcurrentMap<ClassLoader, ConcurrentMap<String, Class<?>>> classCache =
new ConcurrentHashMap<>();
- /*
- Initializes enterprise check.
- */
- static {
- boolean assertionsEnabled0 = true;
-
- try {
- assert false;
-
- assertionsEnabled0 = false;
- }
- catch (AssertionError ignored) {
- assertionsEnabled0 = true;
- }
- finally {
- assertionsEnabled = assertionsEnabled0;
- }
- }
-
/**
* Get JDK version.
*
@@ -415,7 +399,7 @@ public class IgniteUtils {
}
/**
- * @return {@code True} if assertions enabled.
+ * @return {@code true} if assertions enabled.
*/
public static boolean assertionsEnabled() {
return assertionsEnabled;
@@ -542,4 +526,72 @@ public class IgniteUtils {
err.printStackTrace(System.err);
}
}
+
+ /**
+ * Atomically moves or renames a file to a target file.
+ *
+ * @param sourcePath The path to the file to move.
+ * @param targetPath The path to the target file.
+ * @param log Optional logger.
+ *
+ * @return The path to the target file.
+ *
+ * @throws IOException If the source file cannot be moved to the target.
+ */
+ public static Path atomicMoveFile(Path sourcePath, Path targetPath, @Nullable IgniteLogger log) throws IOException {
+ // Move temp file to target path atomically.
+ // The code comes from
+ // https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/util/AtomicFileWriter.java#L187
+ Objects.requireNonNull(sourcePath, "sourcePath");
+ Objects.requireNonNull(targetPath, "targetPath");
+
+ Path success;
+
+ try {
+ success = Files.move(sourcePath, targetPath, StandardCopyOption.ATOMIC_MOVE);
+ }
+ catch (final IOException e) {
+ // If it falls here that can mean many things. Either that the atomic move is not supported,
+ // or something wrong happened. Anyway, let's try to be over-diagnosing
+ if (log != null) {
+ if (e instanceof AtomicMoveNotSupportedException)
+ log.warn("Atomic move not supported. falling back to non-atomic move, error: {}.", e.getMessage());
+ else
+ log.warn("Unable to move atomically, falling back to non-atomic move, error: {}.", e.getMessage());
+
+ if (targetPath.toFile().exists() && log.isInfoEnabled())
+ log.info("The target file {} was already existing.", targetPath);
+ }
+
+ try {
+ success = Files.move(sourcePath, targetPath, StandardCopyOption.REPLACE_EXISTING);
+ }
+ catch (final IOException e1) {
+ e1.addSuppressed(e);
+
+ if (log != null) {
+ log.warn("Unable to move {} to {}. Attempting to delete {} and abandoning.",
+ sourcePath,
+ targetPath,
+ sourcePath);
+ }
+
+ try {
+ Files.deleteIfExists(sourcePath);
+ }
+ catch (final IOException e2) {
+ e2.addSuppressed(e1);
+
+ if (log != null)
+ log.warn("Unable to delete {}, good bye then!", sourcePath);
+
+ throw e2;
+ }
+
+ throw e1;
+ }
+ }
+
+ return success;
+ }
}
diff --git a/modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDBKeyValueStorage.java b/modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDBKeyValueStorage.java
index bc81180..b12709d 100644
--- a/modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDBKeyValueStorage.java
+++ b/modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDBKeyValueStorage.java
@@ -263,7 +263,7 @@ public class RocksDBKeyValueStorage implements KeyValueStorage {
try {
// Rename the temporary directory
- Files.move(tempPath, snapshotPath);
+ IgniteUtils.atomicMoveFile(tempPath, snapshotPath, null);
}
catch (IOException e) {
throw new IgniteInternalException("Failed to rename: " + tempPath + " to " + snapshotPath, e);
diff --git a/modules/raft/src/main/java/org/apache/ignite/raft/jraft/util/Utils.java b/modules/raft/src/main/java/org/apache/ignite/raft/jraft/util/Utils.java
index accb888..bad60cd 100644
--- a/modules/raft/src/main/java/org/apache/ignite/raft/jraft/util/Utils.java
+++ b/modules/raft/src/main/java/org/apache/ignite/raft/jraft/util/Utils.java
@@ -24,12 +24,9 @@ import java.net.InetAddress;
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
import java.nio.charset.StandardCharsets;
-import java.nio.file.AtomicMoveNotSupportedException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.util.Collection;
+import java.util.Objects;
import java.util.concurrent.Callable;
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
@@ -38,6 +35,7 @@ import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.regex.Pattern;
import com.codahale.metrics.MetricRegistry;
+import org.apache.ignite.internal.util.IgniteUtils;
import org.apache.ignite.lang.IgniteLogger;
import org.apache.ignite.raft.jraft.Closure;
import org.apache.ignite.raft.jraft.Status;
@@ -336,52 +334,12 @@ public final class Utils {
return Requires.requireNonNull(obj, "obj");
}
- @SuppressWarnings("ConstantConditions")
public static boolean atomicMoveFile(final File source, final File target, final boolean sync) throws IOException {
- // Move temp file to target path atomically.
- // The code comes from
- // https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/util/AtomicFileWriter.java#L187
- Requires.requireNonNull(source, "source");
- Requires.requireNonNull(target, "target");
- final Path sourcePath = source.toPath();
- final Path targetPath = target.toPath();
- boolean success;
- try {
- success = Files.move(sourcePath, targetPath, StandardCopyOption.ATOMIC_MOVE) != null;
- }
- catch (final IOException e) {
- // If it falls here that can mean many things. Either that the atomic move is not supported,
- // or something wrong happened. Anyway, let's try to be over-diagnosing
- if (e instanceof AtomicMoveNotSupportedException) {
- LOG.warn("Atomic move not supported. falling back to non-atomic move, error: {}.", e.getMessage());
- }
- else {
- LOG.warn("Unable to move atomically, falling back to non-atomic move, error: {}.", e.getMessage());
- }
+ Objects.requireNonNull(source, "source");
+ Objects.requireNonNull(target, "target");
- if (target.exists()) {
- LOG.info("The target file {} was already existing.", targetPath);
- }
+ boolean success = IgniteUtils.atomicMoveFile(source.toPath(), target.toPath(), LOG) != null;
- try {
- success = Files.move(sourcePath, targetPath, StandardCopyOption.REPLACE_EXISTING) != null;
- }
- catch (final IOException e1) {
- e1.addSuppressed(e);
- LOG.warn("Unable to move {} to {}. Attempting to delete {} and abandoning.", sourcePath, targetPath,
- sourcePath);
- try {
- Files.deleteIfExists(sourcePath);
- }
- catch (final IOException e2) {
- e2.addSuppressed(e1);
- LOG.warn("Unable to delete {}, good bye then!", sourcePath);
- throw e2;
- }
-
- throw e1;
- }
- }
if (success && sync) {
File dir = target.getParentFile();
// fsync on target parent dir.
diff --git a/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/AbstractPartitionStorageTest.java b/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/AbstractPartitionStorageTest.java
index cbbbc21..f79f873 100644
--- a/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/AbstractPartitionStorageTest.java
+++ b/modules/storage-api/src/test/java/org/apache/ignite/internal/storage/AbstractPartitionStorageTest.java
@@ -19,6 +19,7 @@ package org.apache.ignite.internal.storage;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
@@ -593,9 +594,13 @@ public abstract class AbstractPartitionStorageTest {
* @throws Exception If failed to take snapshot.
*/
@Test
- public void testSnapshot(@WorkDirectory Path snapshotDir) throws Exception {
+ public void testSnapshot(@WorkDirectory Path workDir) throws Exception {
List<DataRow> rows = insertBulk(10);
+ Path snapshotDir = Path.of(workDir.toString(), "snapshot");
+
+ Files.createDirectories(snapshotDir);
+
storage.snapshot(snapshotDir).get(1, TimeUnit.SECONDS);
storage.removeAll(rows);
@@ -608,7 +613,7 @@ public abstract class AbstractPartitionStorageTest {
/**
* Inserts and returns a given amount of data rows with {@link #KEY}_i as a key and {@link #VALUE}_i as a value
* where i is an index of the data row.
- *
+ *
* @param numberOfEntries Amount of entries to insert.
* @return List of inserted rows.
*/
diff --git a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbPartitionStorage.java b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbPartitionStorage.java
index 8e82889..ba2fccc 100644
--- a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbPartitionStorage.java
+++ b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbPartitionStorage.java
@@ -81,7 +81,6 @@ public class RocksDbPartitionStorage implements PartitionStorage {
* @param partId Partition id.
* @param db Rocks DB instance.
* @param columnFamily Column family to be used for all storage operations.
- * @param storage
* @throws StorageException If failed to create RocksDB instance.
*/
public RocksDbPartitionStorage(int partId, RocksDB db, ColumnFamily columnFamily) throws StorageException {
@@ -354,7 +353,7 @@ public class RocksDbPartitionStorage implements PartitionStorage {
try {
// Rename the temporary directory
- Files.move(tempPath, snapshotPath);
+ IgniteUtils.atomicMoveFile(tempPath, snapshotPath, null);
}
catch (IOException e) {
throw new IgniteInternalException("Failed to rename: " + tempPath + " to " + snapshotPath, e);
diff --git a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
index 167455e..f8f5bd0 100644
--- a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
+++ b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
@@ -67,10 +67,10 @@ public class RocksDbTableStorage implements TableStorage {
private static final String CF_META = "default";
/** Prefix for partitions column families names. */
- private static final String CF_PARTITION_PREFIX = "cf-part:";
+ private static final String CF_PARTITION_PREFIX = "cf-part-";
/** Prefix for SQL indexes column family names. */
- private static final String CF_INDEX_PREFIX = "cf-idx:";
+ private static final String CF_INDEX_PREFIX = "cf-idx-";
/** Name of comparator used in indexes column family. */
private static final String INDEX_COMPARATOR_NAME = "index-comparator";