From c0f55ecf16c51ff3ff8953839f3c5f393b8a8d9e Mon Sep 17 00:00:00 2001 From: Artem Ananev Date: Mon, 1 Apr 2024 15:05:28 -0700 Subject: [PATCH] Default reconnect mode is set to 'push'. Added more comments to TwoPhasePessimisticTraversalOrder. Reverted JRS test config Signed-off-by: Artem Ananev --- .../VMerkleDual-NDReconnect-500-60m.json | 50 +++------------- .../virtualmap/config/VirtualMapConfig.java | 2 +- .../TwoPhasePessimisticTraversalOrder.java | 60 +++++++++---------- 3 files changed, 39 insertions(+), 73 deletions(-) diff --git a/platform-sdk/platform-apps/tests/PlatformTestingTool/src/main/resources/VMerkleDual-NDReconnect-500-60m.json b/platform-sdk/platform-apps/tests/PlatformTestingTool/src/main/resources/VMerkleDual-NDReconnect-500-60m.json index 8faf9a465569..a92655d5133e 100644 --- a/platform-sdk/platform-apps/tests/PlatformTestingTool/src/main/resources/VMerkleDual-NDReconnect-500-60m.json +++ b/platform-sdk/platform-apps/tests/PlatformTestingTool/src/main/resources/VMerkleDual-NDReconnect-500-60m.json @@ -35,7 +35,7 @@ }, "virtualMerkleConfig": { "samplingProbability": 0, - "assorted": false, + "assorted": true, "smartContractConfig": { "minKeyValuePairsDuringCreation": 100, "maxKeyValuePairsDuringCreation": 250, @@ -46,63 +46,31 @@ "addsDuringMethodExecution": 300 }, "sequential": [ - { - "type": "TYPE_VIRTUAL_MERKLE_CREATE", - "amount": 150000 - }, { - "type": "TYPE_VIRTUAL_MERKLE_CREATE_SMART_CONTRACT", - "amount": 5000 - }, - { - "type": "TYPE_VIRTUAL_MERKLE_SMART_CONTRACT_METHOD_EXECUTION", - "amount": 5000 + "type": "TYPE_VIRTUAL_MERKLE_CREATE", + "amount": 100000 }, { - "type": "TYPE_VIRTUAL_MERKLE_UPDATE", - "amount": 30000 + "type": "TYPE_VIRTUAL_MERKLE_DELETE", + "amount": 100000 }, - { - "type": "TYPE_VIRTUAL_MERKLE_DELETE", - "amount": 15000 - }, - { - "type": "TYPE_VIRTUAL_MERKLE_CREATE_SMART_CONTRACT", - "amount": 1000 - }, - { - "type": "TYPE_VIRTUAL_MERKLE_SMART_CONTRACT_METHOD_EXECUTION", - "amount": 3000 - }, { "type": "TYPE_VIRTUAL_MERKLE_UPDATE", - "amount": 50000 - }, - { - "type": "TYPE_VIRTUAL_MERKLE_CREATE", - "amount": 50000 + "amount": 100000 }, { "type": "TYPE_VIRTUAL_MERKLE_CREATE_SMART_CONTRACT", - "amount": 2000 - }, - { - "type": "TYPE_VIRTUAL_MERKLE_UPDATE", - "amount": 50000 + "amount": 15000 }, { "type": "TYPE_VIRTUAL_MERKLE_SMART_CONTRACT_METHOD_EXECUTION", - "amount": 5000 - }, - { - "type": "TYPE_VIRTUAL_MERKLE_UPDATE", - "amount": 200000 + "amount": 15000 } ] }, "fcmConfig": { "sequentialTest": false, - "_comment": "if sequentialTest is true, generate payload according to sequentialType, sequentialAmoutn, sequentialSize, this override typeDistribution defined in payloadConfig", + "_comment": "if sequentialTest is true, generate payload according to sequentialType, sequentialAmount, sequentialSize, this override typeDistribution defined in payloadConfig", "sequentials": [] } } diff --git a/platform-sdk/swirlds-virtualmap/src/main/java/com/swirlds/virtualmap/config/VirtualMapConfig.java b/platform-sdk/swirlds-virtualmap/src/main/java/com/swirlds/virtualmap/config/VirtualMapConfig.java index cf2e15f35c54..6ecb6277ae99 100644 --- a/platform-sdk/swirlds-virtualmap/src/main/java/com/swirlds/virtualmap/config/VirtualMapConfig.java +++ b/platform-sdk/swirlds-virtualmap/src/main/java/com/swirlds/virtualmap/config/VirtualMapConfig.java @@ -95,7 +95,7 @@ public record VirtualMapConfig( double percentHashThreads, // FUTURE WORK: We need to add min/max support for double values @Min(-1) @ConfigProperty(defaultValue = "-1") int numHashThreads, @Min(1) @Max(64) @ConfigProperty(defaultValue = "3") int virtualHasherChunkHeight, - @ConfigProperty(defaultValue = "pullTwoPhasePessimistic") String reconnectMode, + @ConfigProperty(defaultValue = "push") String reconnectMode, @Min(0) @ConfigProperty(defaultValue = "500000") int reconnectFlushInterval, @Min(0) @Max(100) @ConfigProperty(defaultValue = "25.0") double percentCleanerThreads, // FUTURE WORK: We need to add min/max support for double values diff --git a/platform-sdk/swirlds-virtualmap/src/main/java/com/swirlds/virtualmap/internal/reconnect/TwoPhasePessimisticTraversalOrder.java b/platform-sdk/swirlds-virtualmap/src/main/java/com/swirlds/virtualmap/internal/reconnect/TwoPhasePessimisticTraversalOrder.java index 7a4daeae2cdb..7e1eacda2513 100644 --- a/platform-sdk/swirlds-virtualmap/src/main/java/com/swirlds/virtualmap/internal/reconnect/TwoPhasePessimisticTraversalOrder.java +++ b/platform-sdk/swirlds-virtualmap/src/main/java/com/swirlds/virtualmap/internal/reconnect/TwoPhasePessimisticTraversalOrder.java @@ -174,7 +174,7 @@ public void nodeReceived(final long path, final boolean isClean) { chunkNextToCheckPaths.get(chunk).addFirst(Path.getParentPath(path)); } } else { - // At the chunk start rank, every other path (i.e. all right paths) is skipped by + // At the chunk start rank, every other path (i.e. all right paths) are skipped by // default. If a left sibling is clean, there is no need to check the right sibling, // as a request for the parent will be sent anyway. However, if the left sibling // is dirty, the right sibling may be either dirty, or clean, so a request for it @@ -201,24 +201,35 @@ public long getNextPathToSend() { final int chunk = (lastSentPathChunk + 1 + i) % chunkCount; // Check the queue first. If not empty, return a path from there (if not clean) final Deque toCheck = chunkNextToCheckPaths.get(chunk); - result = toCheck.isEmpty() ? -1 : toCheck.pollFirst(); - while ((result != -1) && hasCleanParent(result)) { - result = toCheck.isEmpty() ? -1 : toCheck.pollFirst(); + result = toCheck.isEmpty() ? Path.INVALID_PATH : toCheck.pollFirst(); + while ((result != Path.INVALID_PATH) && hasCleanParent(result)) { + result = toCheck.isEmpty() ? Path.INVALID_PATH : toCheck.pollFirst(); } - if (result != -1) { + if (result != Path.INVALID_PATH) { lastSentPathChunk = chunk; break; } // Otherwise proceed to the next pessimistic path at chunk start rank - result = cleanOrNext(chunk, chunkNextPessimisticPaths.get(chunk)); - if (result == -1) { - chunkNextPessimisticPaths.put(chunk, -1L); + final long nextPessimisticPath = chunkNextPessimisticPaths.get(chunk); + if (nextPessimisticPath == Path.INVALID_PATH) { continue; } + result = skipCleanPaths(nextPessimisticPath, getLastChunkPath(chunk)); + if (result == Path.INVALID_PATH) { + // All remaining paths at chunk start rank are clear. Mark the chunk as done + // and try the next chunk + chunkNextPessimisticPaths.put(chunk, Path.INVALID_PATH); + continue; + } + // Only left paths are sent at the chunk start rank. If a left path is dirty, then its + // right sibling is scheduled to check in nodeReceived(), otherwise a parent will be + // checked, and there is no need to send a request for the right sibling + assert Path.isLeft(result); lastSentPathChunk = chunk; + // Skip to the next left path long next = result + 2; if (next > getLastChunkPath(chunk)) { - next = -1; + next = Path.INVALID_PATH; } chunkNextPessimisticPaths.put(chunk, next); break; @@ -230,28 +241,6 @@ public long getNextPathToSend() { return result; } - private long cleanOrNext(final int chunk, final long path) { - if ((path == -1) || (getPathChunk(path) != chunk)) { - return -1; - } - return hasCleanParent(path) ? getNextPathInChunk(chunk, path) : path; - } - - private long getNextPathInChunk(final int chunk, final long lastPath) { - final int lastPathRank = Path.getRank(lastPath); - final int chunkStartRank = chunkStartRanks.get(chunk); - final int chunkHeight = chunkStartRank - chunksStopRank; - if (Path.isLeft(lastPath) && (chunkStartRank - lastPathRank < chunkHeight) && !hasCleanParent(lastPath)) { - return Path.getParentPath(lastPath); - } - // next path at chunk start rank - long path = Path.getLeftGrandChildPath(lastPath, chunkStartRank - lastPathRank) + 1; - final long chunkStartPath = chunkStartPaths.get(chunk); - final long chunkWidth = chunkWidths.get(chunk); - final long lastPathInChunk = chunkStartPath + chunkWidth - 1; - return skipCleanPaths(path, lastPathInChunk); - } - private long getNextLeafPath() { long path = lastLeafPath == Path.INVALID_PATH ? reconnectFirstLeafPath : lastLeafPath + 1; if ((path > reconnectLastLeafPath) || (reconnectFirstLeafPath < 0)) { @@ -287,6 +276,10 @@ private boolean hasCleanParent(final long path) { return clean; } + /** + * Skip all clean paths starting from the given path at the same rank, un until the limit. If + * all paths are clean to the very limit, Path.INVALID_PATH is returned + */ private long skipCleanPaths(long path, final long limit) { long result = skipCleanPaths(path); while ((result < limit) && (result != path)) { @@ -296,6 +289,11 @@ private long skipCleanPaths(long path, final long limit) { return (result <= limit) ? result : Path.INVALID_PATH; } + /** + * For a given path, find its highest parent path in cleanNodes. If such a parent exists, + * skip all paths at the original paths's rank in the parent sub-tree and return the first + * path after that. If no clean parent is found, the original path is returned + */ private long skipCleanPaths(final long path) { assert path > 0; long parent = Path.getParentPath(path);