Skip to content

Commit

Permalink
WIP cleanup
Browse files Browse the repository at this point in the history
Signed-off-by: Atanas Atanasov <[email protected]>
  • Loading branch information
ata-nas committed Nov 28, 2024
1 parent cda9403 commit abd9196
Show file tree
Hide file tree
Showing 19 changed files with 94 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,12 @@ public static long requireWhole(final long toCheck) {
public static long requireWhole(final long toCheck, final String errorMessage) {
if (toCheck >= 0) {
return toCheck;
} else {
final String message = Objects.isNull(errorMessage)
? "The input integer [%d] is required be whole.".formatted(toCheck)
: errorMessage;
throw new IllegalArgumentException(message);
}

final String message = Objects.isNull(errorMessage)
? "The input integer [%d] is required be whole.".formatted(toCheck)
: errorMessage;
throw new IllegalArgumentException(message);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion server/docker/update-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ fi

if [ true = "$is_smoke_test" ]; then
# add smoke test variables
echo "PERSISTENCE_STORAGE_TYPE=BLOCK_AS_LOCAL_DIRECTORY" >> .env #todo maybe in the future this needs to use BLOCK_AS_LOCAL_FILE?
# @todo(#372) - default persistence type should be BLOCK_AS_LOCAL_FILE
echo "PERSISTENCE_STORAGE_TYPE=BLOCK_AS_LOCAL_DIRECTORY" >> .env
echo "MEDIATOR_RING_BUFFER_SIZE=1024" >> .env
echo "NOTIFIER_RING_BUFFER_SIZE=1024" >> .env
echo "JAVA_OPTS='-Xms4G -Xmx4G'" >> .env
Expand Down
4 changes: 2 additions & 2 deletions server/docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ defaults and can be left unchanged. It is recommended to browse the properties b
| SERVICE_DELAY_MILLIS | Service shutdown delay in milliseconds | 500 |
| MEDIATOR_RING_BUFFER_SIZE | Size of the ring buffer used by the mediator (must be a power of 2) | 67108864 |
| NOTIFIER_RING_BUFFER_SIZE | Size of the ring buffer used by the notifier (must be a power of 2) | 2048 |
| SERVER_PORT | The port the server will listen on | 8080 |
| SERVER_MAX_MESSAGE_SIZE_BYTES | The maximum size of a message frame in bytes | 1048576 |
| SERVER_PORT | The port the server will listen on | 8080 |
| SERVER_MAX_MESSAGE_SIZE_BYTES | The maximum size of a message frame in bytes | 1048576 |
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,16 @@
*/
@ConfigData("persistence.storage")
public record PersistenceStorageConfig(
// @todo(#371) - the default life/archive root path must be absolute starting from /opt
@ConfigProperty(defaultValue = "") String liveRootPath,
// @todo(#371) - the default life/archive root path must be absolute starting from /opt
@ConfigProperty(defaultValue = "") String archiveRootPath,
@ConfigProperty(defaultValue = "BLOCK_AS_LOCAL_FILE") StorageType type) {
private static final System.Logger LOGGER = System.getLogger(PersistenceStorageConfig.class.getName());
// @todo(#371) - the default life/archive root path must be absolute starting from /opt
private static final String LIVE_ROOT_PATH =
Path.of("hashgraph/blocknode/data/live/").toAbsolutePath().toString();
// @todo(#371) - the default life/archive root path must be absolute starting from /opt
private static final String ARCHIVE_ROOT_PATH =
Path.of("hashgraph/blocknode/data/archive/").toAbsolutePath().toString();

Expand Down Expand Up @@ -130,9 +134,8 @@ private void createDirectoryPath(@NonNull final Path targetPath, @NonNull final
Files.createDirectories(targetPath);
} catch (final IOException e) {
final String classname = this.getClass().getName();
final String message =
"Unable to instantiate [%s]! Unable to create the [%s] path for the block storage at [%s]"
.formatted(classname, semanticPathName, targetPath);
final String message = "Unable to instantiate [%s]! Unable to create the [%s] path that was provided!"
.formatted(classname, semanticPathName);
throw new UncheckedIOException(message, e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.hedera.block.server.persistence.storage.path;

import com.hedera.block.common.utils.Preconditions;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.nio.file.Path;
import java.util.Objects;
Expand All @@ -24,29 +25,29 @@
* A Block path resolver for block-as-dir.
*/
public final class BlockAsLocalDirPathResolver implements BlockPathResolver {
private final Path blockStorageRoot;
private final Path liveRootPath;

/**
* Constructor.
*
* @param blockStorageRoot valid, {@code non-null} instance of {@link Path}
* that points to the root of the block storage
* @param liveRootPath valid, {@code non-null} instance of {@link Path}
* that points to the live root of the block storage
*/
private BlockAsLocalDirPathResolver(@NonNull final Path blockStorageRoot) {
this.blockStorageRoot = Objects.requireNonNull(blockStorageRoot);
private BlockAsLocalDirPathResolver(@NonNull final Path liveRootPath) {
this.liveRootPath = Objects.requireNonNull(liveRootPath);
}

Check warning on line 38 in server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalDirPathResolver.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalDirPathResolver.java#L36-L38

Added lines #L36 - L38 were not covered by tests

/**
* This method creates and returns a new instance of
* {@link BlockAsLocalDirPathResolver}.
*
* @param blockStorageRoot valid, {@code non-null} instance of {@link Path}
* that points to the root of the block storage
* @param liveRootPath valid, {@code non-null} instance of {@link Path}
* that points to the live root of the block storage
* @return a new, fully initialized instance of
* {@link BlockAsLocalDirPathResolver}
*/
public static BlockAsLocalDirPathResolver of(@NonNull final Path blockStorageRoot) {
return new BlockAsLocalDirPathResolver(blockStorageRoot);
public static BlockAsLocalDirPathResolver of(@NonNull final Path liveRootPath) {
return new BlockAsLocalDirPathResolver(liveRootPath);

Check warning on line 50 in server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalDirPathResolver.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalDirPathResolver.java#L50

Added line #L50 was not covered by tests
}

/**
Expand All @@ -55,6 +56,7 @@ public static BlockAsLocalDirPathResolver of(@NonNull final Path blockStorageRoo
@NonNull
@Override
public Path resolvePathToBlock(final long blockNumber) {
return blockStorageRoot.resolve(String.valueOf(blockNumber));
Preconditions.requireWhole(blockNumber);
return liveRootPath.resolve(String.valueOf(blockNumber));

Check warning on line 60 in server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalDirPathResolver.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalDirPathResolver.java#L59-L60

Added lines #L59 - L60 were not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,29 @@
*/
public final class BlockAsLocalFilePathResolver implements BlockPathResolver {
private static final int MAX_LONG_DIGITS = 19;
private final Path blockStorageRoot;
private final Path liveRootPath;

/**
* Constructor.
*
* @param blockStorageRoot valid, {@code non-null} instance of {@link Path}
* that points to the root of the block storage
* @param liveRootPath valid, {@code non-null} instance of {@link Path}
* that points to the live root of the block storage
*/
private BlockAsLocalFilePathResolver(@NonNull final Path blockStorageRoot) {
this.blockStorageRoot = Objects.requireNonNull(blockStorageRoot);
private BlockAsLocalFilePathResolver(@NonNull final Path liveRootPath) {
this.liveRootPath = Objects.requireNonNull(liveRootPath);
}

Check warning on line 41 in server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalFilePathResolver.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalFilePathResolver.java#L39-L41

Added lines #L39 - L41 were not covered by tests

/**
* This method creates and returns a new instance of
* {@link BlockAsLocalFilePathResolver}.
*
* @param blockStorageRoot valid, {@code non-null} instance of {@link Path}
* that points to the root of the block storage
* @param liveRootPath valid, {@code non-null} instance of {@link Path}
* that points to the live root of the block storage
* @return a new, fully initialized instance of
* {@link BlockAsLocalFilePathResolver}
*/
public static BlockAsLocalFilePathResolver of(@NonNull final Path blockStorageRoot) {
return new BlockAsLocalFilePathResolver(blockStorageRoot);
public static BlockAsLocalFilePathResolver of(@NonNull final Path liveRootPath) {
return new BlockAsLocalFilePathResolver(liveRootPath);

Check warning on line 53 in server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalFilePathResolver.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalFilePathResolver.java#L53

Added line #L53 was not covered by tests
}

/**
Expand All @@ -59,11 +59,11 @@ public static BlockAsLocalFilePathResolver of(@NonNull final Path blockStorageRo
@NonNull
@Override
public Path resolvePathToBlock(final long blockNumber) {
Preconditions.requirePositive(blockNumber); // todo do we have block number 0?
Preconditions.requireWhole(blockNumber);
final String rawBlockNumber = String.format("%0" + MAX_LONG_DIGITS + "d", blockNumber);
final String[] blockPath = rawBlockNumber.split("");
final String blockFileName = rawBlockNumber.concat(Constants.BLOCK_FILE_EXTENSION);
blockPath[blockPath.length - 1] = blockFileName;
return Paths.get(blockStorageRoot.toAbsolutePath().toString(), blockPath);
return Paths.get(liveRootPath.toString(), blockPath);

Check warning on line 67 in server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalFilePathResolver.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/hedera/block/server/persistence/storage/path/BlockAsLocalFilePathResolver.java#L62-L67

Added lines #L62 - L67 were not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public interface BlockPathResolver {
*
* @param blockNumber to be resolved the path for
* @return the resolved path to the given Block by a number
* @throws IllegalArgumentException if the blockNumber IS NOT a whole number
*/
Path resolvePathToBlock(final long blockNumber);
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static java.lang.System.Logger.Level.ERROR;
import static java.lang.System.Logger.Level.INFO;

import com.hedera.block.common.utils.Preconditions;
import com.hedera.block.server.persistence.storage.PersistenceStorageConfig;
import com.hedera.hapi.block.BlockItemUnparsed;
import com.hedera.hapi.block.BlockUnparsed;
Expand All @@ -47,9 +48,8 @@
* containing block items. The block items are stored as files within the block directory.
*/
public class BlockAsLocalDirReader implements LocalBlockReader<BlockUnparsed> {
private final Logger LOGGER = System.getLogger(getClass().getName());
private final Path blockNodeRootPath;
private final FileAttribute<Set<PosixFilePermission>> folderPermissions =
private static final Logger LOGGER = System.getLogger(BlockAsLocalDirReader.class.getName());
private static final FileAttribute<Set<PosixFilePermission>> DEFAULT_FOLDER_PERMISSIONS =
PosixFilePermissions.asFileAttribute(Set.of(
PosixFilePermission.OWNER_READ,
PosixFilePermission.OWNER_WRITE,
Expand All @@ -58,21 +58,16 @@ public class BlockAsLocalDirReader implements LocalBlockReader<BlockUnparsed> {
PosixFilePermission.GROUP_EXECUTE,
PosixFilePermission.OTHERS_READ,
PosixFilePermission.OTHERS_EXECUTE));
private final Path liveRootPath;

/**
* Constructor.
*
* @param config the configuration to retrieve the block node root path
* @param config the configuration to retrieve the root paths
*/
protected BlockAsLocalDirReader(@NonNull final PersistenceStorageConfig config) {
LOGGER.log(INFO, "Initializing FileSystemBlockReader");

final Path blockNodeRootPath = Path.of(config.liveRootPath());

LOGGER.log(INFO, config.toString());
LOGGER.log(INFO, "Block Node Root Path: " + blockNodeRootPath);

this.blockNodeRootPath = blockNodeRootPath;
LOGGER.log(INFO, "Initializing %s...".formatted(getClass().getName()));
this.liveRootPath = Path.of(config.liveRootPath());
}

/**
Expand Down Expand Up @@ -100,15 +95,15 @@ public static BlockAsLocalDirReader of(@NonNull final PersistenceStorageConfig c
@NonNull
@Override
public Optional<BlockUnparsed> read(final long blockNumber) throws IOException, ParseException {

// Verify path attributes of the block node root path
if (isPathDisqualified(blockNodeRootPath)) {
Preconditions.requireWhole(blockNumber);
// Verify path attributes of the block node live root path
if (isPathDisqualified(liveRootPath)) {
return Optional.empty();
}

// Verify path attributes of the block directory within the
// block node root path
final Path blockPath = blockNodeRootPath.resolve(String.valueOf(blockNumber));
// block node live root path
final Path blockPath = liveRootPath.resolve(String.valueOf(blockNumber));
if (isPathDisqualified(blockPath)) {
return Optional.empty();
}
Expand All @@ -133,15 +128,14 @@ public Optional<BlockUnparsed> read(final long blockNumber) throws IOException,
blockItems.add(blockItemOpt.get());
continue;
}

break;
}

builder.blockItems(blockItems);

// Return the Block
return Optional.of(builder.build());
} catch (IOException io) {
} catch (final IOException io) {
LOGGER.log(ERROR, "Error reading block: " + blockPath, io);
throw io;
}
Expand Down Expand Up @@ -190,7 +184,7 @@ private boolean isPathDisqualified(@NonNull final Path path) {
try {
// If resetting the permissions fails or
// if the path is still unreadable, return true.
setPerm(path, folderPermissions.value());
setPerm(path, DEFAULT_FOLDER_PERMISSIONS.value());
if (!path.toFile().canRead()) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.hedera.block.server.persistence.storage.read;

import com.hedera.block.common.utils.Preconditions;
import com.hedera.hapi.block.BlockUnparsed;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.Optional;
Expand Down Expand Up @@ -46,6 +47,7 @@ public static BlockAsLocalFileReader newInstance() {
@NonNull
@Override
public Optional<BlockUnparsed> read(final long blockNumber) {
Preconditions.requireWhole(blockNumber);
throw new UnsupportedOperationException("Not implemented yet");

Check warning on line 51 in server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsLocalFileReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsLocalFileReader.java#L50-L51

Added lines #L50 - L51 were not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public interface BlockReader<T> {
* @throws ParseException if the PBJ codec encounters a problem caused by I/O issues, malformed
* input data, or any other reason that prevents the parse() method from completing the
* operation when fetching the block.
* @throws IllegalArgumentException if the blockNumber IS NOT a whole number
*/
@NonNull
Optional<T> read(final long blockNumber) throws IOException, ParseException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static java.lang.System.Logger;
import static java.lang.System.Logger.Level.ERROR;

import com.hedera.block.common.utils.Preconditions;
import com.hedera.block.server.persistence.storage.path.BlockPathResolver;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.File;
Expand Down Expand Up @@ -63,10 +64,11 @@ public static BlockAsLocalDirRemover of(@NonNull final BlockPathResolver blockPa
*
* @param blockNumber the id of the block to remove
* @throws IOException if an I/O error occurs
* @throws IllegalArgumentException if the blockNumber IS NOT a whole number
*/
@Override
public void remove(final long blockNumber) throws IOException {
// todo should we add file permissions normalization?
Preconditions.requireWhole(blockNumber);
final Path blockPath = blockPathResolver.resolvePathToBlock(blockNumber);
if (Files.notExists(blockPath)) {
LOGGER.log(ERROR, "Block cannot be deleted as it does not exist: {0}", blockNumber);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.hedera.block.server.persistence.storage.remove;

import com.hedera.block.common.utils.Preconditions;
import java.io.IOException;

/**
Expand All @@ -39,13 +40,15 @@ public static BlockAsLocalFileRemover newInstance() {
}

/**
* Removes a block from the file system.
* Remove a block with the given block number.
*
* @param blockNumber the id of the block to remove
* @throws IOException if an I/O error occurs
* @param blockNumber the block number of the block to remove
* @throws IOException when failing to remove a block
* @throws IllegalArgumentException if the blockNumber IS NOT a whole number
*/
@Override
public void remove(final long blockNumber) throws IOException {
Preconditions.requireWhole(blockNumber);
throw new UnsupportedOperationException("Not implemented yet");

Check warning on line 52 in server/src/main/java/com/hedera/block/server/persistence/storage/remove/BlockAsLocalFileRemover.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/hedera/block/server/persistence/storage/remove/BlockAsLocalFileRemover.java#L51-L52

Added lines #L51 - L52 were not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ public interface BlockRemover {
/**
* Remove a block with the given block number.
*
* @param blockNumber the block number of the block to remove.
* @throws IOException when failing to remove a block.
* @param blockNumber the block number of the block to remove
* @throws IOException when failing to remove a block
* @throws IllegalArgumentException if the blockNumber IS NOT a whole number
*/
void remove(final long blockNumber) throws IOException;
}
Loading

0 comments on commit abd9196

Please sign in to comment.