Skip to content

Commit aa81ffa

Browse files
ata-nasjsync-swirlds
authored andcommitted
chore: BlockAccessors Hardening & Improved Error Handling
* BlockAccessors are now Autocloseable * BlockAccessors now create hard links for the duration of their lifespan * This allows us to ensure data is accessible for the duration of the accessor's life * Tests are updated and migrated * Deleted redundant configuration tests * Added tests: * Tests for subsequent reads to ensure that closing an accessor does not affect the data or the ability for a new accessor to access it * Added a recursive deletion visitor * Introduced a BlockAccessorBatch * Added reuse of `ZipFileSystem` instances to cut down on repeated reread of large zip files and dictionaries. * Adjusted block accessor close semantics to avoid double-close. * Adjusted BlockFile to use a visitor for some file tree walks. Signed-off-by: Atanas Atanasov <a.v.atanasov98@gmail.com> Signed-off-by: Joseph S. <121976561+jsync-swirlds@users.noreply.github.com>
1 parent d1fb3dd commit aa81ffa

File tree

30 files changed

+1617
-827
lines changed

30 files changed

+1617
-827
lines changed

block-node/app/src/testFixtures/java/org/hiero/block/node/app/fixtures/blocks/BlockUtils.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,14 @@
44
import com.hedera.hapi.block.stream.Block;
55
import com.hedera.pbj.runtime.ParseException;
66
import com.hedera.pbj.runtime.io.buffer.Bytes;
7+
import edu.umd.cs.findbugs.annotations.NonNull;
8+
import edu.umd.cs.findbugs.annotations.Nullable;
79
import java.io.IOException;
10+
import java.nio.file.FileVisitResult;
11+
import java.nio.file.Files;
12+
import java.nio.file.Path;
13+
import java.nio.file.SimpleFileVisitor;
14+
import java.nio.file.attribute.BasicFileAttributes;
815
import java.util.zip.GZIPInputStream;
916
import org.hiero.block.internal.BlockUnparsed;
1017
import org.hiero.block.node.app.fixtures.TestUtils;
@@ -112,4 +119,31 @@ public long getBlockNumber() {
112119
return blockNumber;
113120
}
114121
}
122+
123+
/**
124+
* A simple file visitor to recursively delete files and directories up to
125+
* the provided root.
126+
*/
127+
private static class RecursiveFileDeleteVisitor extends SimpleFileVisitor<Path> {
128+
@Override
129+
@NonNull
130+
public FileVisitResult visitFile(@NonNull final Path file, @NonNull final BasicFileAttributes attrs)
131+
throws IOException {
132+
Files.delete(file);
133+
return FileVisitResult.CONTINUE;
134+
}
135+
136+
@Override
137+
@NonNull
138+
public FileVisitResult postVisitDirectory(@NonNull final Path dir, @Nullable final IOException e)
139+
throws IOException {
140+
if (e == null) {
141+
Files.delete(dir);
142+
return FileVisitResult.CONTINUE;
143+
} else {
144+
// directory iteration failed
145+
throw e;
146+
}
147+
}
148+
}
115149
}

block-node/app/src/testFixtures/java/org/hiero/block/node/app/fixtures/blocks/InMemoryBlockAccessor.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ public final class InMemoryBlockAccessor implements BlockAccessor {
1818
private final Block block;
1919
/** The block number of the accessible block */
2020
private final long blockNumber;
21+
/** simple flag to track calls to close */
22+
private boolean isClosed = false;
2123

2224
/**
2325
* Constructor. Enforces preconditions on the input block items.
@@ -85,4 +87,14 @@ public Bytes blockBytes(final Format format) {
8587
private Bytes zstdCompressBytes(final Bytes bytes) {
8688
return Bytes.wrap(Zstd.compress(bytes.toByteArray()));
8789
}
90+
91+
@Override
92+
public void close() {
93+
isClosed = true;
94+
}
95+
96+
@Override
97+
public boolean isClosed() {
98+
return isClosed;
99+
}
88100
}

block-node/app/src/testFixtures/java/org/hiero/block/node/app/fixtures/blocks/MinimalBlockAccessor.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
public final class MinimalBlockAccessor implements BlockAccessor {
1010
private final long blockNumber;
1111
private final Block block;
12+
private boolean isClosed = false;
1213

1314
public MinimalBlockAccessor(final long blockNumber, final Block block) {
1415
this.blockNumber = blockNumber;
@@ -32,4 +33,14 @@ public Bytes blockBytes(final Format format) {
3233
private Bytes zstdCompressBytes(final Bytes bytes) {
3334
return Bytes.wrap(Zstd.compress(bytes.toByteArray()));
3435
}
36+
37+
@Override
38+
public void close() {
39+
isClosed = true;
40+
}
41+
42+
@Override
43+
public boolean isClosed() {
44+
return isClosed;
45+
}
3546
}

block-node/base/src/main/java/org/hiero/block/node/base/BlockFile.java

Lines changed: 74 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
// SPDX-License-Identifier: Apache-2.0
22
package org.hiero.block.node.base;
33

4+
import static java.nio.file.FileVisitResult.CONTINUE;
5+
46
import java.io.IOException;
7+
import java.io.UncheckedIOException;
8+
import java.lang.System.Logger;
9+
import java.lang.System.Logger.Level;
10+
import java.nio.file.FileVisitResult;
11+
import java.nio.file.FileVisitor;
512
import java.nio.file.Files;
613
import java.nio.file.Path;
14+
import java.nio.file.attribute.BasicFileAttributes;
715
import java.text.DecimalFormat;
816
import java.text.NumberFormat;
917
import java.util.Comparator;
@@ -21,12 +29,14 @@
2129
@SuppressWarnings("unused")
2230
public final class BlockFile {
2331
/** The logger for this class. */
24-
private static final System.Logger LOGGER = System.getLogger(BlockFile.class.getName());
32+
private static final Logger LOGGER = System.getLogger(BlockFile.class.getName());
2533
/** The format for block numbers in file names */
2634
private static final NumberFormat BLOCK_NUMBER_FORMAT = new DecimalFormat("0000000000000000000");
2735
/** The extension for compressed block files */
2836
public static final String BLOCK_FILE_EXTENSION = ".blk";
2937

38+
private static final int MAX_FILE_SEARCH_DEPTH = 20;
39+
3040
/**
3141
* The block number format for use in file name. For example block 123 becomes "000000000000000123".
3242
*
@@ -134,7 +144,7 @@ public static long nestedDirectoriesMinBlockNumber(Path basePath, CompressionTyp
134144
break;
135145
}
136146
} catch (Exception e) {
137-
LOGGER.log(System.Logger.Level.ERROR, "Error reading directory: " + lowestPath, e);
147+
LOGGER.log(Level.ERROR, "Error reading directory: " + lowestPath, e);
138148
// handle exception
139149
break;
140150
}
@@ -178,7 +188,7 @@ public static long nestedDirectoriesMaxBlockNumber(Path basePath, CompressionTyp
178188
break;
179189
}
180190
} catch (Exception e) {
181-
LOGGER.log(System.Logger.Level.ERROR, "Error reading directory: " + highestPath, e);
191+
LOGGER.log(Level.ERROR, "Error reading directory: " + highestPath, e);
182192
// handle exception
183193
break;
184194
}
@@ -195,16 +205,18 @@ public static long nestedDirectoriesMaxBlockNumber(Path basePath, CompressionTyp
195205
* @return the minimum block number, or -1 if no block files are found
196206
*/
197207
public static Set<Long> nestedDirectoriesAllBlockNumbers(Path basePath, CompressionType compressionType) {
198-
final Set<Long> blockNumbers = new HashSet<>();
199-
final String fullExtension = BLOCK_FILE_EXTENSION + compressionType.extension();
200-
try (var stream = Files.walk(basePath)) {
201-
stream.filter(Files::isRegularFile)
202-
.filter(path -> path.getFileName().toString().endsWith(fullExtension))
203-
.forEach(blockFile -> blockNumbers.add(blockNumberFromFile(blockFile)));
208+
try {
209+
final Set<Long> blockNumbers = new HashSet<>();
210+
final String fullExtension = BLOCK_FILE_EXTENSION + compressionType.extension();
211+
Files.walkFileTree(
212+
basePath,
213+
Set.of(),
214+
MAX_FILE_SEARCH_DEPTH,
215+
new BlockNumberCollectionVisitor(blockNumbers, fullExtension));
216+
return blockNumbers;
204217
} catch (IOException e) {
205-
throw new RuntimeException(e);
218+
throw new UncheckedIOException(e);
206219
}
207-
return blockNumbers;
208220
}
209221

210222
/**
@@ -213,21 +225,60 @@ public static Set<Long> nestedDirectoriesAllBlockNumbers(Path basePath, Compress
213225
* extension.
214226
*
215227
* @param file the path for file to extract the block number from
216-
* @return the block number
228+
* @return the block number, or `-1` if the file does not match the expected format
229+
* or the file base name does not parse as a `long` value.
217230
*/
218231
public static long blockNumberFromFile(final Path file) {
219-
return blockNumberFromFile(file.getFileName().toString());
232+
try {
233+
if (file != null && file.getFileName() != null) {
234+
final String fileName = file.getFileName().toString();
235+
if (fileName != null && fileName.indexOf('.') > 0) {
236+
return Long.parseLong(fileName.substring(0, fileName.indexOf('.')));
237+
} else {
238+
return -1L;
239+
}
240+
} else {
241+
return -1L;
242+
}
243+
} catch (NumberFormatException e) {
244+
return -1L;
245+
}
220246
}
221247

222-
/**
223-
* Extracts the block number from a file name. The file name is expected to be in the format
224-
* {@code "0000000000000000000.blk.xyz"} where the block number is the first 19 digits and the rest is the file
225-
* extension.
226-
*
227-
* @param fileName the file name to extract the block number from
228-
* @return the block number
229-
*/
230-
public static long blockNumberFromFile(final String fileName) {
231-
return Long.parseLong(fileName.substring(0, fileName.indexOf('.')));
248+
private static class BlockNumberCollectionVisitor implements FileVisitor<Path> {
249+
private final String blockFileExtension;
250+
private final Set<Long> blockNumbersFound;
251+
252+
public BlockNumberCollectionVisitor(final Set<Long> blockNumbers, final String fullExtension) {
253+
blockFileExtension = fullExtension;
254+
blockNumbersFound = blockNumbers;
255+
}
256+
257+
@Override
258+
public FileVisitResult preVisitDirectory(final Path dir, final BasicFileAttributes attrs) throws IOException {
259+
return CONTINUE;
260+
}
261+
262+
@Override
263+
public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException {
264+
if (file.endsWith(blockFileExtension)) {
265+
blockNumbersFound.add(BlockFile.blockNumberFromFile(file));
266+
}
267+
return CONTINUE;
268+
}
269+
270+
@Override
271+
public FileVisitResult visitFileFailed(final Path file, final IOException exc) throws IOException {
272+
throw exc;
273+
}
274+
275+
@Override
276+
public FileVisitResult postVisitDirectory(final Path dir, final IOException exc) throws IOException {
277+
if (exc != null) {
278+
return CONTINUE;
279+
} else {
280+
throw exc;
281+
}
282+
}
232283
}
233284
}

block-node/base/src/main/java/org/hiero/block/node/base/tar/TaredBlockIterator.java

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
// SPDX-License-Identifier: Apache-2.0
22
package org.hiero.block.node.base.tar;
33

4+
import edu.umd.cs.findbugs.annotations.NonNull;
45
import java.nio.charset.StandardCharsets;
56
import java.text.DecimalFormat;
67
import java.text.NumberFormat;
78
import java.util.Arrays;
89
import java.util.Iterator;
910
import java.util.NoSuchElementException;
11+
import java.util.Objects;
1012
import org.hiero.block.node.spi.historicalblocks.BlockAccessor;
1113
import org.hiero.block.node.spi.historicalblocks.BlockAccessor.Format;
1214

@@ -59,13 +61,14 @@ enum State {
5961
* @param format the format of the block data files written to the tar file
6062
* @param blockIterator the iterator over the blocks to be written into the tar file
6163
*/
62-
public TaredBlockIterator(Format format, Iterator<BlockAccessor> blockIterator) {
63-
this.format = format;
64+
public TaredBlockIterator(@NonNull final Format format, @NonNull final Iterator<BlockAccessor> blockIterator) {
65+
this.format = Objects.requireNonNull(format);
6466
this.extension = switch (format) {
6567
case Format.JSON -> ".json";
6668
case Format.PROTOBUF -> ".blk";
67-
case Format.ZSTD_PROTOBUF -> ".blk.zstd";};
68-
this.blockIterator = blockIterator;
69+
case Format.ZSTD_PROTOBUF -> ".blk.zstd";
70+
};
71+
this.blockIterator = Objects.requireNonNull(blockIterator);
6972
this.currentBuffer = new byte[CHUNK_SIZE];
7073
this.bufferPosition = 0;
7174
}
@@ -98,9 +101,17 @@ public byte[] next() {
98101
case NEXT_FILE:
99102
if (blockIterator.hasNext()) {
100103
// get the next block from the iterator
101-
final BlockAccessor currentBlock = blockIterator.next();
102-
currentBlockBytes = currentBlock.blockBytes(format).toByteArray();
103-
currentBlockName = BLOCK_NUMBER_FORMAT.format(currentBlock.blockNumber()) + extension;
104+
final BlockAccessor currentBlockAccessor = blockIterator.next();
105+
currentBlockName = BLOCK_NUMBER_FORMAT.format(currentBlockAccessor.blockNumber()) + extension;
106+
currentBlockBytes =
107+
currentBlockAccessor.blockBytes(format).toByteArray();
108+
// close the accessor as we are done with it and we need
109+
// to free resources, but only if it isn't already closed.
110+
// @todo replace Iterator with BlockAccessorBatch for this
111+
// class so we don't need to call close here.
112+
if (!currentBlockAccessor.isClosed()) {
113+
currentBlockAccessor.close();
114+
}
104115
filePosition = 0;
105116
state = State.WRITE_HEADER;
106117
} else {

block-node/base/src/test/java/org/hiero/block/node/base/BlockFileTest.java

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,25 @@
11
// SPDX-License-Identifier: Apache-2.0
22
package org.hiero.block.node.base;
33

4-
import static org.junit.jupiter.api.Assertions.*;
4+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
5+
import static org.junit.jupiter.api.Assertions.assertEquals;
6+
import static org.junit.jupiter.api.Assertions.assertNotNull;
7+
import static org.junit.jupiter.api.Assertions.assertThrows;
8+
import static org.junit.jupiter.api.Assertions.fail;
59

610
import java.nio.file.Files;
711
import java.nio.file.Path;
812
import java.nio.file.Paths;
913
import java.util.Arrays;
1014
import java.util.Set;
15+
import java.util.stream.Stream;
16+
import org.junit.jupiter.api.DisplayName;
1117
import org.junit.jupiter.api.Test;
1218
import org.junit.jupiter.api.io.TempDir;
19+
import org.junit.jupiter.params.ParameterizedTest;
20+
import org.junit.jupiter.params.aggregator.ArgumentsAccessor;
21+
import org.junit.jupiter.params.provider.Arguments;
22+
import org.junit.jupiter.params.provider.MethodSource;
1323

1424
/**
1525
* Unit tests for the BlockFile class.
@@ -33,10 +43,41 @@ void testStandaloneBlockFilePath() {
3343
assertEquals(expectedPath, BlockFile.standaloneBlockFilePath(basePath, 123, CompressionType.ZSTD));
3444
}
3545

36-
@Test
37-
void testBlockNumberFromFile() {
38-
Path filePath = Paths.get("0000000000000000123.blk.zstd");
39-
assertEquals(123, BlockFile.blockNumberFromFile(filePath));
46+
@ParameterizedTest(name = "{0}")
47+
@MethodSource("org.hiero.block.node.base.BlockFileTest#blockNumbersAndFilePaths")
48+
@DisplayName("Block Number From File")
49+
void testBlockNumberFromFile(final ArgumentsAccessor args) {
50+
final long expectedNumber = args.getLong(1);
51+
final Path file = args.get(2, Path.class);
52+
assertEquals(expectedNumber, BlockFile.blockNumberFromFile(file));
53+
}
54+
55+
/**
56+
* Method source for test cases to verify `blockNumberFromFile`.
57+
*/
58+
private static Stream<Arguments> blockNumbersAndFilePaths() {
59+
final String thirtySixZeros = "000000000000000000000000000000000000";
60+
return Stream.of(
61+
Arguments.of("Simple 'happy' test", 123L, Path.of("0000000000000000123.blk.zstd")),
62+
Arguments.of("a large block number", 1827329783712391L, Path.of("0001827329783712391.blk.zstd")),
63+
Arguments.of("max-long block number", Long.MAX_VALUE, Path.of(Long.MAX_VALUE + ".blk.zstd")),
64+
Arguments.of("number 0", 0L, Path.of("0.blk.zstd")),
65+
Arguments.of("number 0 with extra 0's prefix", 0L, Path.of(thirtySixZeros + "0000.blk.zstd")),
66+
Arguments.of("number 1984 with extra 0's prefix", 1984L, Path.of(thirtySixZeros + "1984.blk.zstd")),
67+
Arguments.of(
68+
"directory that looks like a block file",
69+
10L,
70+
Path.of("/000/000/000/010.blk.zip/nothing").getParent()),
71+
Arguments.of("Wrong extension returns -1", -1L, Path.of("marker.file")),
72+
Arguments.of("No extension returns -1", -1L, Path.of("file-without-extension")),
73+
Arguments.of("Filename is not a number returns -1", -1L, Path.of("copy-of-error.blk.zstd")),
74+
Arguments.of("Filename is out of range returns -1", -1L, Path.of(Long.MAX_VALUE + "9182.blk.zstd")),
75+
Arguments.of("Directory without an extension returns -1", -1L, Path.of("/000/000/000/010")),
76+
Arguments.of(
77+
"root path (getFileName is null) returns -1",
78+
-1L,
79+
Path.of("/000/000/000/010/").getRoot()),
80+
Arguments.of("null path returns -1", -1L, null));
4081
}
4182

4283
/**

0 commit comments

Comments
 (0)