Skip to content

Commit

Permalink
test: fixed file-leaks in tests 16946 (#17045)
Browse files Browse the repository at this point in the history
Signed-off-by: Alex Kuzmin <[email protected]>
  • Loading branch information
alex-kuzmin-hg authored Dec 12, 2024
1 parent a4d668f commit cae5cc2
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.stream.Stream;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -272,10 +273,11 @@ private void testMapSerialization(final VirtualMap<ExampleLongKeyFixedSize, Exam
out.writeMerkleTree(savedStateDirectory, map);
out.flush();

final List<Path> filesInDirectory = Files.list(savedStateDirectory).toList();
assertNotNull(filesInDirectory, "saved state directory is not a valid directory");
assertTrue(filesInDirectory.size() > 0, "there should be a non-zero number of files created");

try (final Stream<Path> filesInDirectory = Files.list(savedStateDirectory)) {
List list = filesInDirectory.toList();
assertNotNull(list, "saved state directory is not a valid directory");
assertTrue(list.size() > 0, "there should be a non-zero number of files created");
}
// Change default MerkleDb path, so data sources are restored into a different DB instance
final Path restoredDbDirectory =
LegacyTemporaryFileBuilder.buildTemporaryDirectory("merkledb-restored", CONFIGURATION);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.Logger;
import org.junit.jupiter.api.AfterEach;
Expand Down Expand Up @@ -172,7 +173,11 @@ void createDataFileCollection(FilesTestType testType) throws Exception {
count += 100;
}
// check 10 files were created
assertEquals(10, Files.list(tempFileDir.resolve(testType.name())).count(), "unexpected file count");
int filesCount;
try (Stream<Path> list = Files.list(tempFileDir.resolve(testType.name()))) {
filesCount = (int) list.count();
}
assertEquals(10, filesCount, "unexpected file count");
}

@Order(3)
Expand Down Expand Up @@ -223,13 +228,14 @@ void createDataFileCollectionWithLoadedDataCallback(final FilesTestType testType
reinitializeDirectMemoryUsage();
// check that the 10 files were created previously (in the very first unit test) still are
// readable
assertEquals(
10,
Files.list(tempFileDir.resolve(testType.name()))
.filter(f -> f.toString().endsWith(".pbj"))
.filter(f -> !f.toString().contains("metadata"))
.count(),
"Temp file should not have changed since previous test in sequence");
try (Stream<Path> list = Files.list(tempFileDir.resolve(testType.name()))) {
assertEquals(
10,
list.filter(f -> f.toString().endsWith(".pbj"))
.filter(f -> !f.toString().contains("metadata"))
.count(),
"Temp file should not have changed since previous test in sequence");
}
// examine loadedDataCallbackImpl content's map sizes as well as checking the data
assertEquals(
1000,
Expand Down Expand Up @@ -436,13 +442,14 @@ public <T extends Throwable> void forEach(final LongAction<T> action)
}
});
// check we only have 1 file left
assertEquals(
1,
Files.list(tempFileDir.resolve(testType.name()))
.filter(f -> f.toString().endsWith(".pbj"))
.filter(f -> !f.toString().contains("metadata"))
.count(),
"unexpected # of files #1");
try (Stream<Path> list = Files.list(tempFileDir.resolve(testType.name()))) {
assertEquals(
1,
list.filter(f -> f.toString().endsWith(".pbj"))
.filter(f -> !f.toString().contains("metadata"))
.count(),
"unexpected # of files #1");
}
// After merge is complete, there should be only 1 "fully written" file, and that it is
// empty.
List<DataFileReader> filesLeft = fileCollection.getAllCompletedFiles();
Expand Down Expand Up @@ -485,13 +492,14 @@ void changeSomeData(final FilesTestType testType) throws Exception {
}
fileCollection.endWriting(0, 1000).setFileCompleted();
// check we now have 2 files
assertEquals(
2,
Files.list(tempFileDir.resolve(testType.name()))
.filter(f -> f.toString().endsWith(".pbj"))
.filter(f -> !f.toString().contains("metadata"))
.count(),
"unexpected # of files");
try (Stream<Path> list = Files.list(tempFileDir.resolve(testType.name()))) {
assertEquals(
2,
list.filter(f -> f.toString().endsWith(".pbj"))
.filter(f -> !f.toString().contains("metadata"))
.count(),
"unexpected # of files");
}
}

@Order(201)
Expand Down Expand Up @@ -592,13 +600,14 @@ public <T extends Throwable> void forEach(final LongAction<T> action)
}
});
// check we 7 files left, as we merged 5 out of 11
assertEquals(
1,
Files.list(tempFileDir.resolve(testType.name()))
.filter(f -> f.toString().endsWith(".pbj"))
.filter(f -> !f.toString().contains("metadata"))
.count(),
"unexpected # of files");
try (Stream<Path> list = Files.list(tempFileDir.resolve(testType.name()))) {
assertEquals(
1,
list.filter(f -> f.toString().endsWith(".pbj"))
.filter(f -> !f.toString().contains("metadata"))
.count(),
"unexpected # of files");
}
}

private static DataFileCompactor createFileCompactor(
Expand Down Expand Up @@ -643,12 +652,13 @@ void mergeWorksAfterOpen(final FilesTestType testType) throws Exception {
// create 10x 100 item files
populateDataFileCollection(testType, fileCollection, storedOffsets);
// check 10 files were created and data is correct
assertEquals(
10,
Files.list(dbDir)
.filter(file -> file.getFileName().toString().startsWith(storeName))
.count(),
"expected 10 db files");
try (Stream<Path> list = Files.list(dbDir)) {
assertEquals(
10,
list.filter(file -> file.getFileName().toString().startsWith(storeName))
.count(),
"expected 10 db files");
}
assertSame(10, fileCollection.getAllCompletedFiles().size(), "Should be 10 files");
checkData(fileCollectionMap.get(testType), storedOffsetsMap.get(testType), testType, 0, 1000, 10_000);
// check all files are available for merge
Expand All @@ -669,15 +679,14 @@ void mergeWorksAfterOpen(final FilesTestType testType) throws Exception {
fileCompactor.compactFiles(storedOffsets, fileCollection2.getAllCompletedFiles(), 1);
// check 1 files were opened and data is correct
assertSame(1, fileCollection2.getAllCompletedFiles().size(), "Should be 1 files");
assertEquals(
1,
Files.list(dbDir)
.filter(file -> file.getFileName().toString().matches(storeName + ".*pbj"))
.filter(f -> !f.toString().contains("metadata"))
.count(),
"expected 1 db files but had ["
+ Arrays.toString(Files.list(dbDir).toArray())
+ "]");
try (Stream<Path> list = Files.list(dbDir)) {
assertEquals(
1,
list.filter(file -> file.getFileName().toString().matches(storeName + ".*pbj"))
.filter(f -> !f.toString().contains("metadata"))
.count(),
"expected 1 db files but had [" + Arrays.toString(list.toArray()) + "]");
}
checkData(fileCollectionMap.get(testType), storedOffsetsMap.get(testType), testType, 0, 1000, 10_000);
// close db
fileCollection2.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
Expand Down Expand Up @@ -212,11 +213,18 @@ int getMinNumberOfFilesToCompact() {
writeBatch(testType, store, 1500, 2000, 3500, 1234);
checkRange(testType, store, 0, 2000, 1234);
// check number of files created
assertEquals(3, Files.list(tempDir).count(), "unexpected # of files #1");
int filesCount;
try (Stream<Path> list = Files.list(tempDir)) {
filesCount = (int) list.count();
}
assertEquals(3, filesCount, "unexpected # of files #1");
// compact all files
dataFileCompactor.compact();
// check number of files after merge
assertEquals(1, Files.list(tempDir).count(), "unexpected # of files #2");
try (Stream<Path> list = Files.list(tempDir)) {
filesCount = (int) list.count();
}
assertEquals(1, filesCount, "unexpected # of files #2");
// check all data
checkRange(testType, store, 0, 2000, 1234);
// check metrics are reported
Expand Down Expand Up @@ -265,23 +273,28 @@ int getMinNumberOfFilesToCompact() {
checkRange(testType, store, 0, 2000, 8910);
checkRange(testType, store, 2000, 48_000, 56_000);
// check number of files created
assertEquals(2, Files.list(tempDir).count(), "unexpected # of files #3");
try (Stream<Path> list = Files.list(tempDir)) {
filesCount = (int) list.count();
}
assertEquals(2, filesCount, "unexpected # of files #3");

// create a snapshot
final Path tempSnapshotDir = testDirectory.resolve("DataFileTestSnapshot");
store.snapshot(tempSnapshotDir);
// check all files are in new dir
Files.list(tempDir).forEach(file -> {
assertTrue(Files.exists(tempSnapshotDir.resolve(file.getFileName())), "Expected file does not exist");
try {
assertEquals(
Files.size(file),
Files.size(tempSnapshotDir.resolve(file.getFileName())),
"Unexpected value from Files.size()");
} catch (IOException e) {
throw new RuntimeException(e);
}
});
try (Stream<Path> list = Files.list(tempDir)) {
list.forEach(file -> {
assertTrue(Files.exists(tempSnapshotDir.resolve(file.getFileName())), "Expected file does not exist");
try {
assertEquals(
Files.size(file),
Files.size(tempSnapshotDir.resolve(file.getFileName())),
"Unexpected value from Files.size()");
} catch (IOException e) {
throw new RuntimeException(e);
}
});
}
// open snapshot and check data
final LongListOffHeap snapshotIndex = new LongListOffHeap();
final MemoryIndexDiskKeyValueStore storeFromSnapshot = new MemoryIndexDiskKeyValueStore(
Expand Down

0 comments on commit cae5cc2

Please sign in to comment.