You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by sd...@apache.org on 2022/06/30 10:06:08 UTC
[ignite-3] branch main updated: IGNITE-17273 Close streams returned by Files.list (#909)
This is an automated email from the ASF dual-hosted git repository.
sdanilov 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 723262cc2 IGNITE-17273 Close streams returned by Files.list (#909)
723262cc2 is described below
commit 723262cc218b012db2beb56dd156d70fe1576171
Author: Alexander Polovtcev <al...@gmail.com>
AuthorDate: Thu Jun 30 13:06:02 2022 +0300
IGNITE-17273 Close streams returned by Files.list (#909)
---
.../checkpoint/CheckpointMarkersStorage.java | 61 +++++++++++-----------
.../checkpoint/CheckpointMarkersStorageTest.java | 42 ++++++++-------
.../store/FilePageStoreManagerTest.java | 31 ++++++-----
3 files changed, 72 insertions(+), 62 deletions(-)
diff --git a/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointMarkersStorage.java b/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointMarkersStorage.java
index 743047dd1..840e55537 100644
--- a/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointMarkersStorage.java
+++ b/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointMarkersStorage.java
@@ -17,17 +17,14 @@
package org.apache.ignite.internal.pagememory.persistence.checkpoint;
-import static java.nio.file.Files.createDirectories;
-import static java.nio.file.Files.createFile;
-import static java.nio.file.Files.exists;
-import static java.nio.file.Files.isDirectory;
-import static java.nio.file.Files.list;
-import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toCollection;
import static java.util.stream.Collectors.toList;
import java.io.IOException;
+import java.nio.file.Files;
import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -37,6 +34,8 @@ import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.util.IgniteUtils;
import org.apache.ignite.lang.IgniteInternalCheckedException;
import org.jetbrains.annotations.Nullable;
@@ -66,21 +65,19 @@ public class CheckpointMarkersStorage {
* @param storagePath Storage path.
* @throws IgniteInternalCheckedException If failed.
*/
- public CheckpointMarkersStorage(
- Path storagePath
- ) throws IgniteInternalCheckedException {
+ public CheckpointMarkersStorage(Path storagePath) throws IgniteInternalCheckedException {
checkpointDir = storagePath.resolve("cp");
try {
- createDirectories(checkpointDir);
+ Files.createDirectories(checkpointDir);
} catch (IOException e) {
throw new IgniteInternalCheckedException("Could not create directory for checkpoint metadata: " + checkpointDir, e);
}
checkCheckpointDir(checkpointDir);
- try {
- checkpointIds = list(checkpointDir)
+ try (Stream<Path> checkpointMarkers = Files.list(checkpointDir)) {
+ checkpointIds = checkpointMarkers
.map(CheckpointMarkersStorage::parseCheckpointIdFromMarkerFile)
.collect(toCollection(ConcurrentHashMap::newKeySet));
} catch (IOException e) {
@@ -101,7 +98,7 @@ public class CheckpointMarkersStorage {
Path checkpointStartMarker = checkpointDir.resolve(checkpointMarkerFileName(checkpointId, CHECKPOINT_START_MARKER));
try {
- createFile(checkpointStartMarker);
+ Files.createFile(checkpointStartMarker);
} catch (IOException e) {
throw new IgniteInternalCheckedException("Could not create start checkpoint marker: " + checkpointStartMarker, e);
}
@@ -122,7 +119,7 @@ public class CheckpointMarkersStorage {
Path checkpointEndMarker = checkpointDir.resolve(checkpointMarkerFileName(checkpointId, CHECKPOINT_END_MARKER));
try {
- createFile(checkpointEndMarker);
+ Files.createFile(checkpointEndMarker);
} catch (IOException e) {
throw new IgniteInternalCheckedException("Could not create end checkpoint marker: " + checkpointEndMarker, e);
}
@@ -142,25 +139,32 @@ public class CheckpointMarkersStorage {
Path startMarker = checkpointDir.resolve(checkpointMarkerFileName(checkpointId, CHECKPOINT_START_MARKER));
Path endMarker = checkpointDir.resolve(checkpointMarkerFileName(checkpointId, CHECKPOINT_END_MARKER));
- if (exists(startMarker)) {
- startMarker.toFile().delete();
- }
-
- if (exists(endMarker)) {
- endMarker.toFile().delete();
- }
+ IgniteUtils.deleteIfExists(startMarker);
+ IgniteUtils.deleteIfExists(endMarker);
}
/**
* Checks that the directory contains only paired (start and end) checkpoint markers.
*/
private static void checkCheckpointDir(Path checkpointDir) throws IgniteInternalCheckedException {
- assert isDirectory(checkpointDir) : checkpointDir;
+ assert Files.isDirectory(checkpointDir) : checkpointDir;
- try {
- List<Path> notCheckpointMarkers = list(checkpointDir)
- .filter(path -> parseCheckpointIdFromMarkerFile(path) == null)
- .collect(toList());
+ try (Stream<Path> checkpointMarkers = Files.list(checkpointDir)) {
+ List<Path> notCheckpointMarkers = new ArrayList<>();
+
+ Map<UUID, List<Path>> checkpointMarkersById = new HashMap<>();
+
+ checkpointMarkers.forEach(marker -> {
+ UUID checkpointId = parseCheckpointIdFromMarkerFile(marker);
+
+ if (checkpointId == null) {
+ notCheckpointMarkers.add(marker);
+ } else {
+ checkpointMarkersById
+ .computeIfAbsent(checkpointId, id -> new ArrayList<>())
+ .add(marker);
+ }
+ });
if (!notCheckpointMarkers.isEmpty()) {
throw new IgniteInternalCheckedException(
@@ -168,10 +172,7 @@ public class CheckpointMarkersStorage {
);
}
- Map<UUID, List<Path>> checkpointMarkers = list(checkpointDir)
- .collect(groupingBy(CheckpointMarkersStorage::parseCheckpointIdFromMarkerFile));
-
- List<UUID> checkpointsWithoutEndMarker = checkpointMarkers.entrySet().stream()
+ List<UUID> checkpointsWithoutEndMarker = checkpointMarkersById.entrySet().stream()
.filter(e -> e.getValue().stream().noneMatch(path -> path.getFileName().toString().contains(CHECKPOINT_END_MARKER)))
.map(Entry::getKey)
.collect(toList());
diff --git a/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointMarkersStorageTest.java b/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointMarkersStorageTest.java
index ee844e492..62adee532 100644
--- a/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointMarkersStorageTest.java
+++ b/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointMarkersStorageTest.java
@@ -17,21 +17,19 @@
package org.apache.ignite.internal.pagememory.persistence.checkpoint;
-import static java.nio.file.Files.createDirectories;
-import static java.nio.file.Files.createFile;
-import static java.nio.file.Files.list;
import static java.util.stream.Collectors.toSet;
import static org.apache.ignite.internal.util.IgniteUtils.deleteIfExists;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.io.FileWriter;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.nio.file.Path;
-import java.util.Set;
import java.util.UUID;
+import java.util.stream.Stream;
import org.apache.ignite.internal.testframework.SystemPropertiesExtension;
import org.apache.ignite.internal.testframework.WorkDirectory;
import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
@@ -50,7 +48,7 @@ public class CheckpointMarkersStorageTest {
@Test
void testFailCreateCheckpointDir() throws Exception {
- Path testFile = createFile(workDir.resolve("testFile"));
+ Path testFile = Files.createFile(workDir.resolve("testFile"));
try (FileWriter fileWriter = new FileWriter(testFile.toFile(), StandardCharsets.UTF_8)) {
fileWriter.write("testString");
@@ -68,7 +66,7 @@ public class CheckpointMarkersStorageTest {
@Test
void testNotOnlyMarkersFiles() throws Exception {
- createFile(createDirectories(cpDir()).resolve("test"));
+ Files.createFile(Files.createDirectories(cpDir()).resolve("test"));
IgniteInternalCheckedException exception = assertThrows(
IgniteInternalCheckedException.class,
@@ -80,12 +78,12 @@ public class CheckpointMarkersStorageTest {
@Test
void testTmpMarkersFiles() throws Exception {
- createDirectories(cpDir());
+ Files.createDirectories(cpDir());
UUID id = UUID.randomUUID();
- createFile(cpDir().resolve(startMarkerFileName(id) + ".tmp"));
- createFile(cpDir().resolve(endMarkerFileName(id) + ".tmp"));
+ Files.createFile(cpDir().resolve(startMarkerFileName(id) + ".tmp"));
+ Files.createFile(cpDir().resolve(endMarkerFileName(id) + ".tmp"));
IgniteInternalCheckedException exception = assertThrows(
IgniteInternalCheckedException.class,
@@ -97,9 +95,9 @@ public class CheckpointMarkersStorageTest {
@Test
void testCheckpointWithoutEndMarker() throws Exception {
- createDirectories(cpDir());
+ Files.createDirectories(cpDir());
- createFile(startMarkerFilePath(UUID.randomUUID()));
+ Files.createFile(startMarkerFilePath(UUID.randomUUID()));
IgniteInternalCheckedException exception = assertThrows(
IgniteInternalCheckedException.class,
@@ -118,10 +116,12 @@ public class CheckpointMarkersStorageTest {
markersStorage.onCheckpointBegin(id0);
markersStorage.onCheckpointEnd(id0);
- assertThat(
- list(cpDir()).collect(toSet()),
- equalTo(Set.of(startMarkerFilePath(id0), endMarkerFilePath(id0)))
- );
+ try (Stream<Path> files = Files.list(cpDir())) {
+ assertThat(
+ files.collect(toSet()),
+ containsInAnyOrder(startMarkerFilePath(id0), endMarkerFilePath(id0))
+ );
+ }
deleteIfExists(cpDir());
@@ -157,10 +157,12 @@ public class CheckpointMarkersStorageTest {
markersStorage.onCheckpointBegin(id2);
markersStorage.onCheckpointEnd(id2);
- assertThat(
- list(cpDir()).collect(toSet()),
- equalTo(Set.of(startMarkerFilePath(id2), endMarkerFilePath(id2)))
- );
+ try (Stream<Path> files = Files.list(cpDir())) {
+ assertThat(
+ files.collect(toSet()),
+ containsInAnyOrder(startMarkerFilePath(id2), endMarkerFilePath(id2))
+ );
+ }
}
private Path cpDir() {
diff --git a/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/store/FilePageStoreManagerTest.java b/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/store/FilePageStoreManagerTest.java
index c3a65327c..ca53ae4c0 100644
--- a/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/store/FilePageStoreManagerTest.java
+++ b/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/store/FilePageStoreManagerTest.java
@@ -17,15 +17,14 @@
package org.apache.ignite.internal.pagememory.persistence.store;
-import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
import static org.apache.ignite.internal.pagememory.PageIdAllocator.INDEX_PARTITION;
import static org.apache.ignite.internal.pagememory.persistence.store.PageStore.TYPE_DATA;
import static org.apache.ignite.internal.pagememory.persistence.store.PageStore.TYPE_IDX;
import static org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.empty;
-import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -43,6 +42,7 @@ import java.nio.file.Path;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
+import java.util.stream.Stream;
import org.apache.ignite.internal.fileio.RandomAccessFileIoFactory;
import org.apache.ignite.internal.testframework.WorkDirectory;
import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
@@ -117,16 +117,21 @@ public class FilePageStoreManagerTest {
assertDoesNotThrow(() -> manager.initialize("test", 0, 2));
assertTrue(Files.isDirectory(testGroupDir));
- assertThat(Files.list(testGroupDir).collect(toList()), empty());
+
+ try (Stream<Path> files = Files.list(testGroupDir)) {
+ assertThat(files.count(), is(0L));
+ }
for (FilePageStore filePageStore : manager.getStores(0)) {
filePageStore.ensure();
}
- assertThat(
- Files.list(testGroupDir).map(Path::getFileName).map(Path::toString).collect(toSet()),
- equalTo(Set.of("index.bin", "part-0.bin", "part-1.bin"))
- );
+ try (Stream<Path> files = Files.list(testGroupDir)) {
+ assertThat(
+ files.map(Path::getFileName).map(Path::toString).collect(toSet()),
+ containsInAnyOrder("index.bin", "part-0.bin", "part-1.bin")
+ );
+ }
} finally {
manager.stop();
}
@@ -217,10 +222,12 @@ public class FilePageStoreManagerTest {
100
));
- assertThat(
- Files.list(workDir.resolve("db/group-test0")).map(Path::getFileName).map(Path::toString).collect(toSet()),
- equalTo(Set.of("index.bin", "part-0.bin"))
- );
+ try (Stream<Path> files = Files.list(workDir.resolve("db/group-test0"))) {
+ assertThat(
+ files.map(Path::getFileName).map(Path::toString).collect(toSet()),
+ containsInAnyOrder("index.bin", "part-0.bin")
+ );
+ }
} finally {
manager0.stop();
}