Skip to content

Commit

Permalink
Revert unrelated changes
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
  • Loading branch information
tinker-michaelj committed Nov 27, 2024
1 parent 57d0699 commit 9244d71
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,7 @@ public Stream<DynamicTest> sharedKeyWorksAsExpected() {
@Order(15)
public Stream<DynamicTest> overlappingKeysTreatedAsExpected() {
var keyGen = OverlappingKeyGenerator.withAtLeastOneOverlappingByte(2);
final long scheduleLifetime = 6;

return defaultHapiSpec("OverlappingKeysTreatedAsExpectedAtExpiry")
.given(
Expand All @@ -717,7 +718,7 @@ public Stream<DynamicTest> overlappingKeysTreatedAsExpected() {
tinyBarsFromTo("aSender", ADDRESS_BOOK_CONTROL, 1),
tinyBarsFromTo("cSender", ADDRESS_BOOK_CONTROL, 1)))
.waitForExpiry()
.withRelativeExpiry(SENDER_TXN, 5)
.withRelativeExpiry(SENDER_TXN, scheduleLifetime)
.recordingScheduledTxn())
.then(
scheduleSign(DEFERRED_XFER).alsoSigningWith("aKey"),
Expand All @@ -743,9 +744,9 @@ public Stream<DynamicTest> overlappingKeysTreatedAsExpected() {
.hasWaitForExpiry()
.isNotExecuted()
.isNotDeleted()
.hasRelativeExpiry(SENDER_TXN, 5)
.hasRelativeExpiry(SENDER_TXN, scheduleLifetime)
.hasRecordedScheduledTxn(),
sleepFor(TimeUnit.SECONDS.toMillis(6)),
sleepFor(TimeUnit.SECONDS.toMillis(scheduleLifetime)),
cryptoCreate("foo"),
getScheduleInfo(DEFERRED_XFER).hasCostAnswerPrecheck(INVALID_SCHEDULE_ID),
getAccountBalance(ADDRESS_BOOK_CONTROL).hasTinyBars(changeFromSnapshot(BEFORE, +2)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,7 @@ public void saveRecords(
}

delegate.saveRecords(
firstLeafPath,
lastLeafPath,
pathHashRecordsToUpdate,
leaves.stream(),
leafRecordsToDelete,
isReconnectContext);
firstLeafPath, lastLeafPath, pathHashRecordsToUpdate, leaves.stream(), leafRecordsToDelete);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,18 +287,6 @@ private int getIndexInOut() {
}
}

/**
* If a dirty leaves stream is empty, returns {@code null}. If leaf path is empty, that
* is when {@code firstLeafPath} and/or {@code lastLeafPath} are zero or less, and
* dirty leaves stream is not empty, throws an {@link IllegalArgumentException}.
*
* @param hashReader A function to read hashes for clean paths
* @param sortedDirtyLeaves A stream of leaf records, sorted by path
* @param firstLeafPath First leaf path
* @param lastLeafPath Last leaf path
* @param listener Hash listener. May be null
* @param virtualMapConfig VirtualMap config
*/
public Hash hash(
final LongFunction<Hash> hashReader,
final Iterator<VirtualLeafRecord<K, V>> sortedDirtyLeaves,
Expand All @@ -308,6 +296,15 @@ public Hash hash(
final @NonNull VirtualMapConfig virtualMapConfig) {
requireNonNull(virtualMapConfig);

// If the first or last leaf path are invalid, then there is nothing to hash.
if (firstLeafPath < 1 || lastLeafPath < 1) {
return null;
}

if (!sortedDirtyLeaves.hasNext()) {
return null;
}

// We don't want to include null checks everywhere, so let the listener be NoopListener if null
if (listener == null) {
listener =
Expand All @@ -316,19 +313,6 @@ public Hash hash(
};
}

// Let the listener know we have started hashing.
listener.onHashingStarted();

if (!sortedDirtyLeaves.hasNext()) {
// Nothing to hash.
listener.onHashingCompleted();
return null;
} else {
if ((firstLeafPath < 1) || (lastLeafPath < 1)) {
throw new IllegalArgumentException("Dirty leaves stream is not empty, but leaf path range is empty");
}
}

this.hashReader = hashReader;
this.listener = listener;
this.cryptography = CryptographyHolder.get();
Expand Down Expand Up @@ -362,6 +346,9 @@ public Hash hash(
int firstLeafRank = Path.getRank(firstLeafPath);
int lastLeafRank = Path.getRank(lastLeafPath);

// Let the listener know we have started hashing.
listener.onHashingStarted();

// This map contains all tasks created, but not scheduled for execution yet
final HashMap<Long, ChunkHashTask> map = new HashMap<>();
// The result task is never executed but used as an output dependency for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,11 @@ public void onHashingCompleted() {
finalLeavesToFlush = leaves;
leaves = null;
}
assert !flushInProgress.get() : "Flush must not be in progress when hashing is complete";
flushInProgress.set(true);
// Nodes / leaves lists may be empty, but a flush is still needed to make sure
// all stale leaves are removed from the data source
flush(finalNodesToFlush, finalLeavesToFlush);
if (!finalNodesToFlush.isEmpty() || !finalLeavesToFlush.isEmpty()) {
assert !flushInProgress.get() : "Flush must not be in progress when hashing is complete";
flushInProgress.set(true);
flush(finalNodesToFlush, finalLeavesToFlush);
}
}

// Since flushes may take quite some time, this method is called outside synchronized blocks,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.swirlds.virtualmap.VirtualKey;
import com.swirlds.virtualmap.VirtualValue;
import com.swirlds.virtualmap.datasource.VirtualLeafRecord;
import com.swirlds.virtualmap.internal.Path;
import com.swirlds.virtualmap.internal.RecordAccessor;
import java.util.HashSet;
import java.util.Set;
Expand Down Expand Up @@ -147,12 +146,15 @@ public synchronized void newLeafNode(final long path, final K newKey) {
}

public synchronized void allNodesReceived() {
if (newLastLeafPath < 0) {
// Empty teacher
return;
}
// no-op if newLastLeafPath is greater or equal to oldLastLeafPath
logger.info(
RECONNECT.getMarker(),
"allNodesReceived(): newLastLeafPath = " + newLastLeafPath + ", oldLastLeafPath = " + oldLastLeafPath);
final long firstOldStalePath = (newLastLeafPath == Path.INVALID_PATH) ? 1 : newLastLeafPath + 1;
// No-op if newLastLeafPath is greater or equal to oldLastLeafPath
for (long p = firstOldStalePath; p <= oldLastLeafPath; p++) {
for (long p = newLastLeafPath + 1; p <= oldLastLeafPath; p++) {
final VirtualLeafRecord<K, ?> oldExtraLeafRecord = oldRecords.findLeafRecord(p, false);
assert oldExtraLeafRecord != null || p < oldFirstLeafPath;
if (oldExtraLeafRecord != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,48 +91,24 @@ void emptyStreamProducesNull() {
*/
@Test
@Tag(TestComponentTags.VMAP)
@DisplayName("Invalid leaf paths")
void invalidLeafPaths() {
@DisplayName("Invalid leaf paths returns null for hash")
void invalidFirstOrLastLeafPathProducesNull() {
final TestDataSource ds = new TestDataSource(Path.INVALID_PATH, Path.INVALID_PATH);
final VirtualHasher<TestKey, TestValue> hasher = new VirtualHasher<>();
final List<VirtualLeafRecord<TestKey, TestValue>> emptyLeaves = new ArrayList<>();
// Empty dirty leaves stream -> null hash
assertNull(
hasher.hash(
ds::loadHash, emptyLeaves.iterator(), Path.INVALID_PATH, Path.INVALID_PATH, VIRTUAL_MAP_CONFIG),
"Call should have produced null");
final List<VirtualLeafRecord<TestKey, TestValue>> leaves = new ArrayList<>();
leaves.add(appleLeaf(VirtualTestBase.A_PATH));
assertNull(
hasher.hash(ds::loadHash, emptyLeaves.iterator(), Path.INVALID_PATH, 2, VIRTUAL_MAP_CONFIG),
hasher.hash(ds::loadHash, leaves.iterator(), Path.INVALID_PATH, 2, VIRTUAL_MAP_CONFIG),
"Call should have produced null");
assertNull(
hasher.hash(ds::loadHash, emptyLeaves.iterator(), 1, Path.INVALID_PATH, VIRTUAL_MAP_CONFIG),
hasher.hash(ds::loadHash, leaves.iterator(), 1, Path.INVALID_PATH, VIRTUAL_MAP_CONFIG),
"Call should have produced null");
assertNull(
hasher.hash(ds::loadHash, emptyLeaves.iterator(), 0, 2, VIRTUAL_MAP_CONFIG),
hasher.hash(ds::loadHash, leaves.iterator(), 0, 2, VIRTUAL_MAP_CONFIG),
"Call should have produced null");
assertNull(
hasher.hash(ds::loadHash, emptyLeaves.iterator(), 1, 0, VIRTUAL_MAP_CONFIG),
hasher.hash(ds::loadHash, leaves.iterator(), 1, 0, VIRTUAL_MAP_CONFIG),
"Call should have produced null");
// Non-empty dirty leaves stream + empty leaf path range -> IllegalStateException
final List<VirtualLeafRecord<TestKey, TestValue>> nonEmptyLeaves = new ArrayList<>();
nonEmptyLeaves.add(appleLeaf(VirtualTestBase.A_PATH));
assertThrows(
IllegalArgumentException.class,
() -> hasher.hash(
ds::loadHash,
nonEmptyLeaves.iterator(),
Path.INVALID_PATH,
Path.INVALID_PATH,
VIRTUAL_MAP_CONFIG),
"Non-null leaves iterator + invalid paths should throw an exception");
assertThrows(
IllegalArgumentException.class,
() -> hasher.hash(ds::loadHash, nonEmptyLeaves.iterator(), 0, 2, VIRTUAL_MAP_CONFIG),
"Non-null leaves iterator + invalid paths should throw an exception");
assertThrows(
IllegalArgumentException.class,
() -> hasher.hash(ds::loadHash, nonEmptyLeaves.iterator(), 1, 0, VIRTUAL_MAP_CONFIG),
"Non-null leaves iterator + invalid paths should throw an exception");
}

/**
Expand Down

0 comments on commit 9244d71

Please sign in to comment.