-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(toolkit): add --exclude-historical-balance for lite snapshot #6690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
| import java.util.concurrent.Callable; | ||
| import java.util.stream.Collectors; | ||
| import java.util.stream.LongStream; | ||
| import java.util.stream.Stream; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import me.tongfei.progressbar.ProgressBar; | ||
| import org.rocksdb.RocksDBException; | ||
|
|
@@ -57,6 +58,8 @@ public class DbLite implements Callable<Integer> { | |
| private static final String TRANSACTION_HISTORY_DB_NAME = "transactionHistoryStore"; | ||
| private static final String PROPERTIES_DB_NAME = "properties"; | ||
| private static final String TRANS_CACHE_DB_NAME = "trans-cache"; | ||
| private static final String BALANCE_TRACE_DB_NAME = "balance-trace"; | ||
| private static final String ACCOUNT_TRACE_DB_NAME = "account-trace"; | ||
|
|
||
| private static final List<String> archiveDbs = Arrays.asList( | ||
| BLOCK_DB_NAME, | ||
|
|
@@ -65,6 +68,10 @@ public class DbLite implements Callable<Integer> { | |
| TRANSACTION_RET_DB_NAME, | ||
| TRANSACTION_HISTORY_DB_NAME); | ||
|
|
||
| private static final List<String> traceDbs = Arrays.asList( | ||
| BALANCE_TRACE_DB_NAME, | ||
| ACCOUNT_TRACE_DB_NAME); | ||
|
|
||
| enum Operate { split, merge } | ||
|
|
||
| enum Type { snapshot, history } | ||
|
|
@@ -105,8 +112,25 @@ enum Type { snapshot, history } | |
| private String datasetPath; | ||
|
|
||
| @CommandLine.Option( | ||
| names = {"--help", "-h"}, | ||
| names = {"--exclude-historical-balance"}, | ||
| defaultValue = "false", | ||
| description = "only used with `operate=split -t snapshot`: when true, balance-trace " | ||
| + "and account-trace are excluded from the lite snapshot. " | ||
| + "Default: ${DEFAULT-VALUE} (legacy behavior; trace stores stay in the snapshot). " | ||
| + "This flag only has a functional impact when the source full node ran with " | ||
| + "`historyBalanceLookup=true` (off by default; most operators are unaffected). " | ||
| + "WARNING: when historyBalanceLookup was enabled, this loss is permanent: a lite " | ||
| + "node booted from such a snapshot cannot answer historical balance lookups " | ||
| + "(getBlockBalance / getAccountBalance), and running merge afterwards will NOT " | ||
| + "restore the feature. If you need to keep historyBalanceLookup working on the " | ||
| + "resulting lite node, do NOT enable this flag. `split -t history` and `merge` " | ||
| + "ignore this flag.", | ||
| order = 5) | ||
| private boolean excludeHistoricalBalance; | ||
|
|
||
| @CommandLine.Option( | ||
| names = {"--help", "-h"}, | ||
| order = 6) | ||
| private boolean help; | ||
|
|
||
|
|
||
|
|
@@ -119,6 +143,7 @@ public Integer call() { | |
| try { | ||
| switch (this.operate) { | ||
| case split: | ||
| warnIfExcludingHistoricalBalance(); | ||
| if (Type.snapshot == this.type) { | ||
| generateSnapshot(fnDataPath, datasetPath); | ||
| } else if (Type.history == type) { | ||
|
|
@@ -253,12 +278,50 @@ public void completeHistoryData(String historyDir, String liteDir) { | |
| spec.commandLine().getOut().format("Merge history finished, take %d s.", during).println(); | ||
| } | ||
|
|
||
| /** | ||
| * Compute the directories to exclude from the lite snapshot. | ||
| * <p> | ||
| * Default ({@code --exclude-historical-balance=false}): the legacy archive set | ||
| * (5 dbs); {@link #BALANCE_TRACE_DB_NAME} / {@link #ACCOUNT_TRACE_DB_NAME} | ||
| * stay with the snapshot as state-style stores. | ||
| * <p> | ||
| * Opt-in ({@code --exclude-historical-balance=true}): the trace stores are | ||
| * additionally excluded, producing a smaller lite snapshot at the cost of | ||
| * dropping historical balance lookup support on the resulting lite node. | ||
| * Only {@code split -t snapshot} consults this. {@code split -t history} | ||
| * and {@code merge} always use the legacy archive set. | ||
| */ | ||
| private List<String> snapshotExclusion() { | ||
| if (!excludeHistoricalBalance) { | ||
| return archiveDbs; | ||
| } | ||
| return Stream.concat(archiveDbs.stream(), traceDbs.stream()) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MUST] When this exclusion is enabled on a source node that had For blocks that are still retained in the snapshot So this flag does not merely "disable" historical balance lookup; it can silently return wrong account balances. That is a correctness issue, and it does not match the current help / warning / README wording of "cannot answer". Please make this mode fail closed before merge, for example by persisting a marker in the snapshot and rejecting |
||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| private void warnIfExcludingHistoricalBalance() { | ||
| if (!excludeHistoricalBalance) { | ||
| return; | ||
| } | ||
| String msg = "WARNING: --exclude-historical-balance is enabled. balance-trace / account-trace " | ||
| + "will be excluded from the lite snapshot. This only matters when the source full " | ||
| + "node ran with historyBalanceLookup=true (off by default; most operators are " | ||
| + "unaffected). When that switch was enabled, this loss is permanent: lite nodes " | ||
| + "booted from this snapshot cannot answer historical balance lookups " | ||
| + "(getBlockBalance / getAccountBalance), and running merge afterwards will NOT " | ||
| + "restore the feature. If you need to keep historyBalanceLookup working on the " | ||
| + "resulting lite node, do NOT use this flag."; | ||
| logger.warn(msg); | ||
| spec.commandLine().getErr().println(msg); | ||
| } | ||
|
|
||
| private List<String> getSnapshotDbs(String sourceDir) { | ||
| List<String> snapshotDbs = Lists.newArrayList(); | ||
| File basePath = new File(sourceDir); | ||
| List<String> excluded = snapshotExclusion(); | ||
| Arrays.stream(Objects.requireNonNull(basePath.listFiles())) | ||
| .filter(File::isDirectory) | ||
| .filter(dir -> !archiveDbs.contains(dir.getName())) | ||
| .filter(dir -> !excluded.contains(dir.getName())) | ||
| .forEach(dir -> snapshotDbs.add(dir.getName())); | ||
| return snapshotDbs; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,14 @@ | ||
| package org.tron.plugins; | ||
|
|
||
| import static org.junit.Assert.assertFalse; | ||
| import static org.junit.Assert.assertTrue; | ||
| import static org.tron.common.utils.PublicMethod.getRandomPrivateKey; | ||
|
|
||
| import io.grpc.ManagedChannel; | ||
| import io.grpc.ManagedChannelBuilder; | ||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.Paths; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.junit.After; | ||
|
|
@@ -89,10 +92,19 @@ public void clear() { | |
|
|
||
| public void testTools(String dbType, int checkpointVersion) | ||
|
halibobo1205 marked this conversation as resolved.
|
||
| throws InterruptedException, IOException { | ||
| logger.info("dbType {}, checkpointVersion {}", dbType, checkpointVersion); | ||
| testTools(dbType, checkpointVersion, false); | ||
| } | ||
|
|
||
| public void testTools(String dbType, int checkpointVersion, boolean excludeHistoricalBalance) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [SHOULD] This opt-in test path never enables
Please add a case with |
||
| throws InterruptedException, IOException { | ||
| logger.info("dbType {}, checkpointVersion {}, excludeHistoricalBalance {}", | ||
| dbType, checkpointVersion, excludeHistoricalBalance); | ||
| init(dbType); | ||
| final String[] argsForSnapshot = | ||
| new String[] {"-o", "split", "-t", "snapshot", "--fn-data-path", | ||
| final String[] argsForSnapshot = excludeHistoricalBalance | ||
| ? new String[] {"-o", "split", "-t", "snapshot", "--fn-data-path", | ||
| dbPath + File.separator + databaseDir, "--dataset-path", | ||
| dbPath, "--exclude-historical-balance"} | ||
| : new String[] {"-o", "split", "-t", "snapshot", "--fn-data-path", | ||
| dbPath + File.separator + databaseDir, "--dataset-path", | ||
| dbPath}; | ||
| final String[] argsForHistory = | ||
|
|
@@ -114,6 +126,16 @@ public void testTools(String dbType, int checkpointVersion) | |
| FileUtil.deleteDir(Paths.get(dbPath, databaseDir, "trans-cache").toFile()); | ||
| // generate snapshot | ||
| cli.execute(argsForSnapshot); | ||
|
halibobo1205 marked this conversation as resolved.
|
||
| Path snapshotDir = Paths.get(dbPath, "snapshot"); | ||
| if (excludeHistoricalBalance) { | ||
| // when --exclude-historical-balance=true, the lite snapshot must not ship | ||
| // balance-trace / account-trace | ||
| assertFalse(snapshotDir.resolve("balance-trace").toFile().exists()); | ||
| assertFalse(snapshotDir.resolve("account-trace").toFile().exists()); | ||
| } else { | ||
| assertTrue(snapshotDir.resolve("balance-trace").toFile().exists()); | ||
| assertTrue(snapshotDir.resolve("account-trace").toFile().exists()); | ||
| } | ||
| // start fullNode | ||
| startApp(); | ||
| // produce transactions | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package org.tron.plugins.rocksdb; | ||
|
|
||
| import java.io.IOException; | ||
| import org.junit.Test; | ||
| import org.tron.plugins.DbLiteTest; | ||
|
|
||
| public class DbLiteExcludeHistoricalBalanceRocksDbTest extends DbLiteTest { | ||
|
|
||
| @Test | ||
| public void testToolsWithExcludeHistoricalBalance() throws InterruptedException, IOException { | ||
| testTools("ROCKSDB", 1, true); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[NIT]
warnIfExcludingHistoricalBalance()runs for everysplit, sosplit -t history --exclude-historical-balancewill still print a snapshot-specific permanent-loss warning even though history split ignores this flag.