[Storage][Pruner] Split state k/v pruning to a separate pruner. #6829

grao1991 · 2023-03-01T01:14:36Z

Description

Rename state pruner to state merkle pruner.
Split state k/v pruner from ledger pruner.
Change StateValueWriter to use state_kv_db.

Test Plan

areshand · 2023-03-01T04:04:21Z

storage/aptosdb/src/lib.rs

-                self.state_store.clone(),
-                &mut batch,
-            )?;
+            LedgerPruner::prune_genesis(self.ledger_db.clone(), &mut batch)?;


curious why we need to prune ledger, statekv while not seeing state_merkel's genesis being pruned?

We do. see ~7 lines below.

areshand · 2023-03-01T05:11:33Z

storage/aptosdb/src/pruner/state_kv_pruner_worker.rs

+
+    // Loop that does the real pruning job.
+    pub(crate) fn work(&self) {
+        while !self.quit_worker.load(Ordering::Relaxed) {


are there multiple threads running the same StateKvPrunerWorker?

No, there is only one worker. We use atomic variable here just because the stop_pruning signal is from other thread (not the worker thread).

msmouse · 2023-03-02T03:36:57Z

config/src/config/storage_config.rs

+    /// Similar to the variable above but for state store pruner. It means the number of stale
+    /// nodes to prune a time.


update comment

msmouse · 2023-03-02T03:39:39Z

state-sync/storage-service/server/src/tests.rs

+    assert_eq!(peers_with_ready_subscriptions, vec![(
+        peer_network_1,
+        synced_ledger_info
+    )]);


hmm.. nit: previous commit needs cargo +nightly fmt

msmouse · 2023-03-02T03:58:25Z

storage/aptosdb/src/pruner/state_store/test.rs

-    if pruned {
-        assert!(state_store.get_usage(Some(version)).is_err())
-    } else {
-        assert!(state_store.get_usage(Some(version)).is_ok())
-    }


is this still verified somewhere?

Add several lines in txn store pruner test to check this.

msmouse · 2023-03-02T03:59:27Z

storage/aptosdb/src/pruner/state_store/state_value_pruner.rs

+            if index.stale_since_version > target_version {
+                break;
+            }
+            // Prune the stale state value index itself first.


"first"? they are the same batch, right?

Done...

@lightmark originally wrote this LOL

msmouse · 2023-03-04T23:34:06Z

storage/aptosdb/src/pruner/transaction_store/test.rs

+        let usage = StateStorageUsage::zero();
+        batch
+            .put::<VersionDataSchema>(&i, &usage.into())
+            .expect("Must succeed.");


🤣 come on, it's a test, just unwrap()

I felt it was weird when writing expect, but my brain was stuck at that time and couldn't find the unwrap to use. 🤣

github-actions · 2023-03-06T23:25:23Z

✅ Forge suite `framework_upgrade` success on `cb4ba0a57c998c60cbab65af31a64875d2588ca5` ==> `ddec83654581bf351bc472a92a3be6d3a3db6338`

Compatibility test results for cb4ba0a57c998c60cbab65af31a64875d2588ca5 ==> ddec83654581bf351bc472a92a3be6d3a3db6338 (PR)
Upgrade the nodes to version: ddec83654581bf351bc472a92a3be6d3a3db6338
framework_upgrade::framework-upgrade::full-framework-upgrade : 7170 TPS, 5382 ms latency, 8000 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for cb4ba0a57c998c60cbab65af31a64875d2588ca5 ==> ddec83654581bf351bc472a92a3be6d3a3db6338 passed
Test Ok

github-actions · 2023-03-06T23:29:31Z

✅ Forge suite `land_blocking` success on `ddec83654581bf351bc472a92a3be6d3a3db6338`

performance benchmark with full nodes : 5839 TPS, 6769 ms latency, 12200 ms p99 latency,(!) expired 1140 out of 2494560 txns
Test Ok

github-actions · 2023-03-06T23:30:08Z

✅ Forge suite `compat` success on `testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b` ==> `ddec83654581bf351bc472a92a3be6d3a3db6338`

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> ddec83654581bf351bc472a92a3be6d3a3db6338 (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 8226 TPS, 4640 ms latency, 6700 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: ddec83654581bf351bc472a92a3be6d3a3db6338
compatibility::simple-validator-upgrade::single-validator-upgrade : 4946 TPS, 8036 ms latency, 10700 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: ddec83654581bf351bc472a92a3be6d3a3db6338
compatibility::simple-validator-upgrade::half-validator-upgrade : 4802 TPS, 8252 ms latency, 10900 ms p99 latency,no expired txns
4. upgrading second batch to new version: ddec83654581bf351bc472a92a3be6d3a3db6338
compatibility::simple-validator-upgrade::rest-validator-upgrade : 7156 TPS, 5303 ms latency, 9800 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> ddec83654581bf351bc472a92a3be6d3a3db6338 passed
Test Ok

grao1991 force-pushed the grao_kv_pruner branch 8 times, most recently from 2f63e52 to 2a008bc Compare March 1, 2023 02:14

grao1991 marked this pull request as ready for review March 1, 2023 02:14

grao1991 requested review from msmouse, lightmark and JoshLind as code owners March 1, 2023 02:14

grao1991 requested a review from areshand March 1, 2023 02:15

grao1991 force-pushed the grao_kv_pruner branch 4 times, most recently from 898b3fd to 002e105 Compare March 1, 2023 03:20

areshand reviewed Mar 1, 2023

View reviewed changes

grao1991 requested a review from areshand March 1, 2023 07:07

areshand approved these changes Mar 1, 2023

View reviewed changes

msmouse reviewed Mar 2, 2023

View reviewed changes

grao1991 force-pushed the grao_kv_pruner branch from 002e105 to 10d7a01 Compare March 3, 2023 23:30

[Storage][Pruner] Rename state pruner to state merkle pruner.

759b85d

grao1991 force-pushed the grao_kv_pruner branch 3 times, most recently from 78fe983 to 3bd0238 Compare March 4, 2023 00:05

grao1991 requested a review from msmouse March 4, 2023 00:06

msmouse approved these changes Mar 4, 2023

View reviewed changes

grao1991 force-pushed the grao_kv_pruner branch from 3bd0238 to 567bb19 Compare March 6, 2023 19:48

grao1991 force-pushed the grao_kv_pruner branch from 567bb19 to df9d17b Compare March 6, 2023 19:50

[Storage][Pruner] Split state k/v pruning to a separate pruner.

ffa28c8

grao1991 force-pushed the grao_kv_pruner branch from df9d17b to ffa28c8 Compare March 6, 2023 19:56

grao1991 enabled auto-merge (squash) March 6, 2023 21:53

This comment has been minimized.

Sign in to view

[Storage] Change StateValueWriter to use state_kv_db.

ddec836

grao1991 mentioned this pull request Mar 6, 2023

[CP][Preview] State kv db pruner, and restore. #6960

Merged

This comment has been minimized.

Sign in to view

grao1991 merged commit e41788c into main Mar 6, 2023

grao1991 deleted the grao_kv_pruner branch March 6, 2023 23:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Storage][Pruner] Split state k/v pruning to a separate pruner. #6829

[Storage][Pruner] Split state k/v pruning to a separate pruner. #6829

grao1991 commented Mar 1, 2023 •

edited

Loading

areshand Mar 1, 2023

grao1991 Mar 1, 2023

areshand Mar 1, 2023

grao1991 Mar 1, 2023

msmouse Mar 2, 2023

grao1991 Mar 3, 2023

msmouse Mar 2, 2023

grao1991 Mar 3, 2023

msmouse Mar 2, 2023

grao1991 Mar 4, 2023

msmouse Mar 2, 2023

grao1991 Mar 3, 2023

msmouse Mar 4, 2023

grao1991 Mar 5, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Mar 6, 2023

github-actions bot commented Mar 6, 2023

github-actions bot commented Mar 6, 2023

		/// Similar to the variable above but for state store pruner. It means the number of stale
		/// nodes to prune a time.

[Storage][Pruner] Split state k/v pruning to a separate pruner. #6829

[Storage][Pruner] Split state k/v pruning to a separate pruner. #6829

Conversation

grao1991 commented Mar 1, 2023 • edited Loading

Description

Test Plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Mar 6, 2023

✅ Forge suite framework_upgrade success on cb4ba0a57c998c60cbab65af31a64875d2588ca5 ==> ddec83654581bf351bc472a92a3be6d3a3db6338

github-actions bot commented Mar 6, 2023

✅ Forge suite land_blocking success on ddec83654581bf351bc472a92a3be6d3a3db6338

github-actions bot commented Mar 6, 2023

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> ddec83654581bf351bc472a92a3be6d3a3db6338

grao1991 commented Mar 1, 2023 •

edited

Loading

✅ Forge suite `framework_upgrade` success on `cb4ba0a57c998c60cbab65af31a64875d2588ca5` ==> `ddec83654581bf351bc472a92a3be6d3a3db6338`

✅ Forge suite `land_blocking` success on `ddec83654581bf351bc472a92a3be6d3a3db6338`

✅ Forge suite `compat` success on `testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b` ==> `ddec83654581bf351bc472a92a3be6d3a3db6338`