Import export trie log #6363
Import export trie log #6363
Conversation
Signed-off-by: Simon Dudley <simon.dudley@consensys.net> Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net> Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
…tIdentifier.reset() Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
…emory Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
…abled Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
|
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
…ort-export-trie-log
# Conflicts: # besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogHelper.java # besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java # besu/src/test/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogHelperTest.java
| @Command( | ||
| name = "export", | ||
| description = | ||
| "This command prunes all trie log layers below the retention threshold, including orphaned trie logs.", |
There was a problem hiding this comment.
This is the pruning description not the export description
There was a problem hiding this comment.
thanks for catching this
| @Command( | ||
| name = "import", | ||
| description = | ||
| "This command prunes all trie log layers below the retention threshold, including orphaned trie logs.", |
There was a problem hiding this comment.
This description looks like the pruning description
There was a problem hiding this comment.
thanks for catching this!
| private CommandLine.Model.CommandSpec spec; // Picocli injects reference to command spec | ||
|
|
||
| @CommandLine.Option( | ||
| names = "--trie-log-hash", |
There was a problem hiding this comment.
Can the exported file contain more than one trielog? Would of expected the import to specify the filename and just import all trielogs in that file
There was a problem hiding this comment.
Not at the moment, this could be extended in the future to do so if needed.
besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java
Show resolved
Hide resolved
besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogHelper.java
Show resolved
Hide resolved
| final String trieLogFile = | ||
| dataDirectoryPath.resolve(DATABASE_PATH).resolve(trieLogHash.toString()).toString(); | ||
|
|
||
| var trieLog = readTrieLogsFromFile(trieLogFile); |
There was a problem hiding this comment.
nit: not sure how important this is so feel free to ignore it doesn't make sense. But having the format in RLP to match the storage in the database trielog column family might make comparison easier without the extra encoding of the IdentityMap. Depends on the purpose of this tool.
There was a problem hiding this comment.
I've added this subcommands to allow extraction and insertion of trie logs mostly for debug or troubleshooting purposes, but a version that imports and exports RLP could be very useful too.
There was a problem hiding this comment.
I'd lean towards a more readable format rather than binary as it's hard to work with. Agree RLP is a more useful middle ground than custom binary (if that's what this is).
e.g. I wonder if this could be used to import test data...compare with RlpBlockImporter.java and JsonBlockImporter
There was a problem hiding this comment.
I'd rather have us to extend the current feature to allow the export to RLP reusing this feature if necessary, it isn't quite simple to do that right now and it would impact the sub-command prune logic as well.
| paramLabel = DefaultCommandValues.MANDATORY_LONG_FORMAT_HELP, | ||
| description = "The hash of the block you want to import the trie log.", | ||
| arity = "1..1") | ||
| private String trieLogHash; |
There was a problem hiding this comment.
rename to blockHash? the variable name suggests this is the hash of the trielog
| paramLabel = DefaultCommandValues.MANDATORY_LONG_FORMAT_HELP, | ||
| description = "The hash of the block you want to import the trie log.", | ||
| arity = "1..1") | ||
| private String trieLogHash; |
There was a problem hiding this comment.
Could be useful to be able to specify a block number instead of block hash. But only a suggestion not sure which one we will use more in practice
There was a problem hiding this comment.
I think block hash is prob better is this case, as you could eventually request by number and not have the right block hash if your node forked at that specific block requested.
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
besu/src/main/java/org/hyperledger/besu/cli/options/stable/DataStorageOptions.java
Outdated
Show resolved
Hide resolved
besu/src/main/java/org/hyperledger/besu/cli/options/stable/DataStorageOptions.java
Outdated
Show resolved
Hide resolved
besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogHelper.java
Show resolved
Hide resolved
besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java
Show resolved
Hide resolved
| final String trieLogFile = | ||
| dataDirectoryPath.resolve(DATABASE_PATH).resolve(trieLogHash.toString()).toString(); | ||
|
|
||
| var trieLog = readTrieLogsFromFile(trieLogFile); |
There was a problem hiding this comment.
I'd lean towards a more readable format rather than binary as it's hard to work with. Agree RLP is a more useful middle ground than custom binary (if that's what this is).
e.g. I wonder if this could be used to import test data...compare with RlpBlockImporter.java and JsonBlockImporter
besu/src/test/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogHelperTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
…d or imported Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
…ort-export-trie-log
besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java
Outdated
Show resolved
Hide resolved
…d or imported Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
| @@ -97,16 +97,15 @@ private static void processTrieLogBatches( | |||
| final String batchFileNameBase) { | |||
|
|
|||
| for (long batchNumber = 1; batchNumber <= numberOfBatches; batchNumber++) { | |||
|
|
|||
| final String batchFileName = batchFileNameBase + "-" + batchNumber; | |||
There was a problem hiding this comment.
this might make more sense to have first/last block numbers included in the filename. Otherwise it won't be clear what is actually in the files after an export
There was a problem hiding this comment.
The batch filenames aren't used as part of import/export subcommands the filename is taken from command line args instead. This is only used for the prune subcommand
| @@ -285,11 +279,10 @@ private static void saveTrieLogsInFile( | |||
| } | |||
|
|
|||
| @SuppressWarnings("unchecked") | |||
| private static IdentityHashMap<byte[], byte[]> readTrieLogsFromFile( | |||
| final String batchFileNameBase, final long batchNumber) { | |||
| static IdentityHashMap<byte[], byte[]> readTrieLogsFromFile(final String batchFileName) { | |||
There was a problem hiding this comment.
Non blocking feedback -
I am all for code reuse, but if we are going to allow for arbitrary import and export, the import files should be more readable and "createable".
The ObjectOutputStream seems fine for backup/recovery of a pruning process, but when part of a general import/export process this file format is too inscrutable IMO.
At least for import/export we should serialize/deserialize these as json maps. Key as the hash string, and the trielog itself as hex (or as a rich json object if we wanted to be super transparent). In addition to being a bit more introspectable, it would allow us to create and import our own handcrafted trielogs when debugging
|
another cadillac feature would be the ability to specify a URL for the import files. This could have been used to recover nodes from the Jan 6 halting event. We could have provided import command instructions and a URL to delivery the correct trielog. |
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
…nto import-export-trie-log
| "--Xbonsai-trie-log-pruning-enabled", | ||
| "--Xbonsai-trie-log-pruning-limit", | ||
| "--Xbonsai-limit-trie-logs-enabled", | ||
| "--Xbonsai-trie-logs-pruning-limit", | ||
| "1"); | ||
| } | ||
|
|
||
| @Test | ||
| public void bonsaiTrieLogPruningLimitShouldBePositive() { | ||
| internalTestFailure( | ||
| "--Xbonsai-trie-log-pruning-limit=0 must be greater than 0", |
There was a problem hiding this comment.
| "--Xbonsai-trie-log-pruning-limit=0 must be greater than 0", | |
| "--Xbonsai-trie-logs-pruning-limit=0 must be greater than 0", |
There was a problem hiding this comment.
There's a few other Strings where this rename needs to happen too.
There was a problem hiding this comment.
will do this in a followup PR
PR description
This PR extends the x-trie-log subcommand to allow exporting and importing of a trie log.
This PR is built on top of #6303