Skip to content

Commit

Permalink
Addressed review comments:
Browse files Browse the repository at this point in the history
- added missing @nonnull and @nullable annotations
- added missing final modifiers
- extracted helper methods
- added null check for `snapshotMetrics`

Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
  • Loading branch information
imalygin committed Sep 20, 2024
1 parent 3b721f5 commit 262119f
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ private SerializableStreamConstants() {}
public static final int VERSION_BYTES = Integer.BYTES;

/**
* The previous version of the serialization protocol implemented by {@link SerializableDataOutputStream} and
* The current version of the serialization protocol implemented by {@link SerializableDataOutputStream} and
* {@link SerializableDataInputStream}
*/
public static final int SERIALIZATION_PROTOCOL_VERSION = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1110,9 +1110,11 @@ public void createSnapshot(Path targetPath) {
throwIfDestroyed();
long start = System.nanoTime();
createSnapshot(this, targetPath);
snapshotMetrics
.getWriteStateToDiskTimeMetric()
.update(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start));
if (snapshotMetrics != null) {
snapshotMetrics
.getWriteStateToDiskTimeMetric()
.update(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start));
}
}

static void createSnapshot(MerkleRoot merkleRoot, Path targetPath) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import com.swirlds.platform.state.signed.SigSet;
import com.swirlds.platform.state.signed.SignedState;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
Expand Down Expand Up @@ -71,7 +72,15 @@ public static List<SavedStateInfo> getSavedStateFiles(
.getSavedStateFiles(mainClassName, platformId, swirldName);
}

public record StateFileData(MerkleRoot state, Hash hash, SigSet sigSet, int fileVersion) {}
/**
* This is a helper class to hold the data read from a state file.
* @param state the Merkle tree state
* @param hash the hash of the state
* @param sigSet the signature set
* @param fileVersion the version of the file
*/
public record StateFileData(
@NonNull MerkleRoot state, @NonNull Hash hash, @Nullable SigSet sigSet, int fileVersion) {}

/**
* Reads a SignedState from disk using the provided snapshot reader function. If the reader throws
Expand All @@ -95,16 +104,16 @@ public record StateFileData(MerkleRoot state, Hash hash, SigSet sigSet, int file
checkSignedStatePath(stateFile);

final DeserializedSignedState returnState;
StateFileData data = readStateFileData(stateFile, snapshotStateReader);
final StateFileData data = readStateFileData(stateFile, snapshotStateReader);

final StateFileData normalizedData;
if (data.sigSet == null) {
File sigSetFile =
final File sigSetFile =
stateFile.getParent().resolve(SIGNATURE_SET_FILE_NAME).toFile();
normalizedData = deserializeAndDebugOnFailure(
() -> new BufferedInputStream(new FileInputStream(sigSetFile)),
(final MerkleDataInputStream in) -> {
int fileVersion = readAndCheckSigSetFileVersion(in);
final int fileVersion = readAndCheckSigSetFileVersion(in);
final SigSet sigSet = in.readSerializable();
return new StateFileData(data.state, data.hash, sigSet, fileVersion);
});
Expand All @@ -129,8 +138,17 @@ public record StateFileData(MerkleRoot state, Hash hash, SigSet sigSet, int file
return returnState;
}

/**
* Reads a SignedState from disk using the provided snapshot reader function.
* @param stateFile the file to read from
* @param snapshotStateReader state snapshot reading function
* @return a signed state with it's associated hash (as computed when the state was serialized)
* @throws IOException if there is any problems with reading from a file
*/
@NonNull
public static StateFileData readStateFileData(
Path stateFile, CheckedBiFunction<MerkleDataInputStream, Path, MerkleRoot, IOException> snapshotStateReader)
final Path stateFile,
final CheckedBiFunction<MerkleDataInputStream, Path, MerkleRoot, IOException> snapshotStateReader)
throws IOException {
return deserializeAndDebugOnFailure(
() -> new BufferedInputStream(new FileInputStream(stateFile.toFile())),
Expand All @@ -139,29 +157,51 @@ public static StateFileData readStateFileData(

final Path directory = stateFile.getParent();
if (fileVersion == INIT_FILE_VERSION) {
try {
final MerkleRoot state = snapshotStateReader.apply(in, directory);
final Hash hash = in.readSerializable();
final SigSet sigSet = in.readSerializable();
return new StateFileData(state, hash, sigSet, fileVersion);
} catch (final IOException e) {
throw new IOException("Failed to read snapshot file " + stateFile.toFile(), e);
}
return readStateFileDataV1(stateFile, snapshotStateReader, in, directory, fileVersion);
} else if (fileVersion == SIG_SET_SEPARATE_VERSION) {
try {
final MerkleRoot state = snapshotStateReader.apply(in, directory);
final Hash hash = in.readSerializable();
return new StateFileData(state, hash, null, fileVersion);

} catch (final IOException e) {
throw new IOException("Failed to read snapshot file " + stateFile.toFile(), e);
}
return createStateFileDataV2(stateFile, snapshotStateReader, in, directory, fileVersion);
} else {
throw new IOException("Unsupported protocol version: " + fileVersion);
}
});
}

@NonNull
private static StateFileData createStateFileDataV2(
@NonNull Path stateFile,
@NonNull CheckedBiFunction<MerkleDataInputStream, Path, MerkleRoot, IOException> snapshotStateReader,
@NonNull MerkleDataInputStream in,
@NonNull Path directory,
int fileVersion)
throws IOException {
try {
final MerkleRoot state = snapshotStateReader.apply(in, directory);
final Hash hash = in.readSerializable();
return new StateFileData(state, hash, null, fileVersion);

} catch (final IOException e) {
throw new IOException("Failed to read snapshot file " + stateFile.toFile(), e);
}
}

@NonNull
private static StateFileData readStateFileDataV1(
@NonNull Path stateFile,
@NonNull CheckedBiFunction<MerkleDataInputStream, Path, MerkleRoot, IOException> snapshotStateReader,
@NonNull MerkleDataInputStream in,
@NonNull Path directory,
int fileVersion)
throws IOException {
try {
final MerkleRoot state = snapshotStateReader.apply(in, directory);
final Hash hash = in.readSerializable();
final SigSet sigSet = in.readSerializable();
return new StateFileData(state, hash, sigSet, fileVersion);
} catch (final IOException e) {
throw new IOException("Failed to read snapshot file " + stateFile.toFile(), e);
}
}

/**
* Check the path of a signed state file
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ public final class SignedStateFileUtils {
public static final int INIT_FILE_VERSION = 1;

/**
* The current version of the signed state file
* The current version of the signed state file. A file of this version no longer contains the signature set,
* instead the signature set is stored in a separate file.
*/
public static final int SIG_SET_SEPARATE_VERSION = 2;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.swirlds.platform.system.Round;
import com.swirlds.platform.system.SwirldState;
import com.swirlds.state.merkle.singleton.StringLeaf;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.IOException;
import java.util.Objects;
import java.util.concurrent.CountDownLatch;
Expand Down Expand Up @@ -84,6 +85,7 @@ public void handleConsensusRound(final Round round, final PlatformStateAccessor
/**
* {@inheritDoc}
*/
@NonNull
@Override
public BlockingSwirldState copy() {
throwIfImmutable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ default void unregisterCommitListener(@NonNull final StateChangeListener listene
/**
* {@inheritDoc}
*/
@NonNull
@Override
default State copy() {
throw new UnsupportedOperationException();
Expand All @@ -102,7 +103,7 @@ default void computeHash() {
* Creates a snapshots for the state. The state has to be hashed and immutable before calling this method.
* @param targetPath The path to save the snapshot.
*/
default void createSnapshot(Path targetPath) {
default void createSnapshot(final @NonNull Path targetPath) {
throw new UnsupportedOperationException();
}
}

0 comments on commit 262119f

Please sign in to comment.