Skip to content

Commit fe24e5c

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. Signed-off-by: Atanas Atanasov <a.v.atanasov98@gmail.com> Signed-off-by: Joseph S. <121976561+jsync-swirlds@users.noreply.github.com>
1 parent 083f521 commit fe24e5c

File tree

28 files changed

+1395
-740
lines changed

28 files changed

+1395
-740
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/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/block-access/src/main/java/org/hiero/block/node/access/service/BlockAccessServicePlugin.java

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import static java.lang.System.Logger.Level.INFO;
66
import static java.lang.System.Logger.Level.TRACE;
77

8-
import com.hedera.hapi.block.stream.Block;
98
import com.swirlds.metrics.api.Counter;
109
import org.hiero.block.api.BlockAccessServiceInterface;
1110
import org.hiero.block.api.BlockRequest;
@@ -14,6 +13,7 @@
1413
import org.hiero.block.node.spi.BlockNodeContext;
1514
import org.hiero.block.node.spi.BlockNodePlugin;
1615
import org.hiero.block.node.spi.ServiceBuilder;
16+
import org.hiero.block.node.spi.historicalblocks.BlockAccessor;
1717
import org.hiero.block.node.spi.historicalblocks.HistoricalBlockFacility;
1818

1919
/**
@@ -44,12 +44,10 @@ public class BlockAccessServicePlugin implements BlockNodePlugin, BlockAccessSer
4444
*/
4545
public BlockResponse getBlock(BlockRequest request) {
4646
requestCounter.increment();
47-
4847
try {
4948
long blockNumberToRetrieve;
5049
// The block number and retrieve latest are mutually exclusive in
5150
// the proto definition, so no need to check for that here.
52-
5351
// if retrieveLatest is set, or the request is for the largest possible block number, get the latest block.
5452
if (request.hasBlockNumber() && request.blockNumber() >= 0) {
5553
blockNumberToRetrieve = request.blockNumber();
@@ -63,7 +61,6 @@ public BlockResponse getBlock(BlockRequest request) {
6361
LOGGER.log(INFO, "Invalid request, 'retrieve_latest' or a valid 'block number' is required.");
6462
return new BlockResponse(Code.INVALID_REQUEST, null);
6563
}
66-
6764
// Check if block is within the available range
6865
if (!blockProvider.availableBlocks().contains(blockNumberToRetrieve)) {
6966
long lowestBlockNumber = blockProvider.availableBlocks().min();
@@ -77,14 +74,21 @@ public BlockResponse getBlock(BlockRequest request) {
7774
responseCounterNotAvailable.increment();
7875
return new BlockResponse(Code.NOT_AVAILABLE, null);
7976
}
80-
8177
// Retrieve the block
82-
Block block = blockProvider.block(blockNumberToRetrieve).block();
83-
responseCounterSuccess.increment();
84-
return new BlockResponse(Code.SUCCESS, block);
85-
86-
} catch (RuntimeException e) {
87-
String message = "Failed to retrieve block number %d.".formatted(request.blockNumber());
78+
try (final BlockAccessor accessor = blockProvider.block(blockNumberToRetrieve)) {
79+
if (accessor != null) {
80+
// even though we have checked for the existence of the block
81+
// it may not be available anymore, so we have to ensure the accessor
82+
// still exists
83+
final BlockResponse response = new BlockResponse(Code.SUCCESS, accessor.block());
84+
responseCounterSuccess.increment();
85+
return response;
86+
} else {
87+
return new BlockResponse(Code.NOT_FOUND, null);
88+
}
89+
}
90+
} catch (final RuntimeException e) {
91+
final String message = "Failed to retrieve block number %d.".formatted(request.blockNumber());
8892
LOGGER.log(ERROR, message, e);
8993
responseCounterNotFound.increment(); // Should this be a failure counter?
9094
return new BlockResponse(Code.NOT_FOUND, null);

0 commit comments

Comments
 (0)