[dag] concurrent dag store #11505

ibalajiarun · 2023-12-22T23:30:32Z

Description

Separated in-memory component of Dag store from persistence.
PersistentDagStore handles both persistence and maintains the in-memory DAG store.

Test Plan

trunk-io · 2023-12-22T23:30:36Z

⏱️ 8h 12m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
windows-build	3h 55m	🟥 🟩 🟩 🟩 🟩 (+6 more)
rust-unit-tests	2h 33m	🟥 🟩 🟥 🟩 🟩 (+1 more)
rust-lints	30m	🟥 🟥 🟥 🟥 🟥 (+2 more)
run-tests-main-branch	26m	🟩 ⬜ 🟩 🟩 🟩 (+2 more)
check-dynamic-deps	22m	🟩 🟩 🟩 🟩 🟩 (+7 more)
general-lints	16m	🟩 🟩 🟩 🟩 🟩 (+1 more)
semgrep/ci	4m	🟩 🟩 🟩 🟩 🟩 (+8 more)
check	2m	🟥 ⬜ 🟥 🟥 🟥 (+3 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+2 more)
file_change_determinator	1m	🟩 ⬜ 🟩 ⬜ 🟩 (+3 more)
permission-check	28s	🟩 🟩 🟩 🟩 🟩 (+6 more)
permission-check	25s	🟩 🟩 🟩 🟩 🟩 (+6 more)
permission-check	22s	🟩 🟩 ⬜ 🟩 🟩 (+3 more)
permission-check	15s	🟩 ⬜ 🟩 ⬜ 🟩 (+3 more)

🚨 1 job on the last run was significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
windows-build	30m	18m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

ibalajiarun · 2023-12-22T23:30:45Z

Current dependencies on/for this PR:

main
- PR [dag] refactor/improve dag health monitoring #11362
  - PR [dag] stop voting based on pipeline latency #11407
    - PR [dag] concurrent dag store #11505 👈
      - PR [dag] parallel message handler implementation #11685
        
        PR [dag] support parallel rb voting #11743
        
        PR [dag] broadcast nodes within window till all validators ack #11751
        
        PR [dag] cache votes without re-counting #11808
        
        PR [dag] decouple payload 2/n #11918
        
        PR Balaji/dag forge #11433

This stack of pull requests is managed by Graphite.

sasha8 · 2024-01-10T20:31:47Z

consensus/src/dag/dag_store.rs

+        }
+    }
+
+    pub fn write(&self) -> DagWriteGuard {


I think this is redundant

sasha8 · 2024-01-26T00:23:29Z

consensus/src/dag/dag_driver.rs

@@ -46,7 +44,7 @@ use tokio_retry::strategy::ExponentialBackoff;
 pub(crate) struct DagDriver {
    author: Author,
    epoch_state: Arc<EpochState>,
-    dag: Arc<RwLock<Dag>>,
+    dag: Arc<PersistentDagStore>,


dag -> persistenet_dag to avoid confusion with the inner dag?

sasha8 · 2024-01-27T01:49:42Z

consensus/src/dag/dag_store.rs

nit:
You could take
let round_ref = self
.nodes_by_round
.entry(round)
.or_insert_with(|| vec![None; self.author_to_index.len()]);
out of validate and lock only that.

sasha8 · 2024-01-27T01:57:56Z

Much cleaner now!

zekun000 · 2024-02-04T00:01:12Z

consensus/src/dag/dag_store.rs

+        Ok(())
+    }
+
+    fn validate_new_node(&mut self, node: &CertifiedNode) -> anyhow::Result<()> {


hmm, it looks a bit weird to have &mut self for validation, but probably it makes insertion simpler

yes, it makes insertion and also last validation step simpler.

zekun000 · 2024-02-04T00:01:58Z

consensus/src/dag/dag_store.rs

@@ -408,3 +372,114 @@ impl Dag {
        self.nodes_by_round.is_empty() && self.start_round > 1
    }
 }
+
+pub struct PersistentDagStore {


I'd call this DagStore and Dag -> InMemDag

zekun000 · 2024-02-04T00:06:07Z

consensus/src/dag/dag_store.rs

+        let node = Arc::new(node);
+        let round_ref = self
+            .get_node_ref_mut(node.round(), node.author())
+            .expect("must be present");


hmm, this is not true if pruning happens in between?

yes, found this recently as well. we need a ensure! here for the round here. Also, need to fix the lowest_round() and highest_round() implementations.

github-actions · 2024-02-09T00:10:08Z

✅ Forge suite `compat` success on `aptos-node-v1.8.3` ==> `e769867995ebf107fee3ee8026361a221f00f581`

Compatibility test results for aptos-node-v1.8.3 ==> e769867995ebf107fee3ee8026361a221f00f581 (PR)
1. Check liveness of validators at old version: aptos-node-v1.8.3
compatibility::simple-validator-upgrade::liveness-check : committed: 4140 txn/s, latency: 6798 ms, (p50: 5600 ms, p90: 8700 ms, p99: 22100 ms), latency samples: 182160
2. Upgrading first Validator to new version: e769867995ebf107fee3ee8026361a221f00f581
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1781 txn/s, latency: 16052 ms, (p50: 19300 ms, p90: 22200 ms, p99: 22500 ms), latency samples: 92620
3. Upgrading rest of first batch to new version: e769867995ebf107fee3ee8026361a221f00f581
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1837 txn/s, latency: 15666 ms, (p50: 19100 ms, p90: 21900 ms, p99: 22300 ms), latency samples: 91860
4. upgrading second batch to new version: e769867995ebf107fee3ee8026361a221f00f581
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 3499 txn/s, latency: 9016 ms, (p50: 9800 ms, p90: 13500 ms, p99: 13900 ms), latency samples: 139960
5. check swarm health
Compatibility test for aptos-node-v1.8.3 ==> e769867995ebf107fee3ee8026361a221f00f581 passed
Test Ok

github-actions · 2024-02-09T00:10:31Z

✅ Forge suite `realistic_env_max_load` success on `e769867995ebf107fee3ee8026361a221f00f581`

two traffics test: inner traffic : committed: 7302 txn/s, latency: 5237 ms, (p50: 4800 ms, p90: 6900 ms, p99: 12900 ms), latency samples: 3161980
two traffics test : committed: 100 txn/s, latency: 2176 ms, (p50: 2100 ms, p90: 2500 ms, p99: 4800 ms), latency samples: 1760
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.227, avg: 0.203", "QsPosToProposal: max: 0.149, avg: 0.138", "ConsensusProposalToOrdered: max: 0.588, avg: 0.544", "ConsensusOrderedToCommit: max: 0.497, avg: 0.471", "ConsensusProposalToCommit: max: 1.052, avg: 1.015"]
Max round gap was 1 [limit 4] at version 1500894. Max no progress secs was 4.985149 [limit 15] at version 1500894.
Test Ok

ibalajiarun mentioned this pull request Dec 22, 2023

[dag] refactor/improve dag health monitoring #11362

Merged

This was referenced Dec 22, 2023

[dag] stop voting based on pipeline latency #11407

Merged

Balaji/dag forge #11433

Closed

ibalajiarun force-pushed the balaji/concurrent-dag-store branch from 4d63114 to 97d4ab8 Compare December 22, 2023 23:52

ibalajiarun marked this pull request as ready for review December 22, 2023 23:53

ibalajiarun requested review from zekun000 and sasha8 as code owners December 22, 2023 23:53

ibalajiarun force-pushed the balaji/vote-backpressure branch from 78bf7e0 to 2140a13 Compare January 8, 2024 23:39

ibalajiarun requested review from JoshLind and gregnazario as code owners January 8, 2024 23:39

ibalajiarun force-pushed the balaji/concurrent-dag-store branch from d314c8e to 74562ed Compare January 8, 2024 23:39

sasha8 reviewed Jan 10, 2024

View reviewed changes

consensus/src/dag/dag_store.rs Outdated

}

}

pub fn write(&self) -> DagWriteGuard {

Copy link

Contributor

sasha8 Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is redundant

ibalajiarun force-pushed the balaji/vote-backpressure branch from 2140a13 to 39e4b84 Compare January 10, 2024 22:23

ibalajiarun force-pushed the balaji/concurrent-dag-store branch from a215a13 to 7f86eb9 Compare January 10, 2024 23:40

ibalajiarun mentioned this pull request Jan 18, 2024

[dag] parallel message handler implementation #11685

Merged

ibalajiarun force-pushed the balaji/concurrent-dag-store branch 2 times, most recently from 60bacf0 to 78ffa8c Compare January 23, 2024 19:07

ibalajiarun requested a review from sasha8 January 23, 2024 19:07

This was referenced Jan 23, 2024

[dag] support parallel rb voting #11743

Merged

[dag] broadcast nodes within window till all validators ack #11751

Merged

ibalajiarun force-pushed the balaji/vote-backpressure branch from 39e4b84 to 9480471 Compare January 25, 2024 21:59

ibalajiarun force-pushed the balaji/concurrent-dag-store branch from a6e6ee3 to 0d87fcc Compare January 25, 2024 21:59

sasha8 approved these changes Jan 27, 2024

View reviewed changes

ibalajiarun mentioned this pull request Jan 28, 2024

[dag] cache votes without re-counting #11808

Merged

zekun000 reviewed Feb 4, 2024

View reviewed changes

ibalajiarun mentioned this pull request Feb 6, 2024

[dag] decouple payload 2/n #11918

Closed

ibalajiarun force-pushed the balaji/vote-backpressure branch from 9480471 to 0ffbe17 Compare February 6, 2024 05:54

Base automatically changed from balaji/vote-backpressure to main February 6, 2024 06:54

ibalajiarun added 4 commits February 6, 2024 07:39

[dag] concurrent dag store

fd31450

monitor handler methods

40c8f65

address feedback

f20234c

lint

e769867

ibalajiarun force-pushed the balaji/concurrent-dag-store branch from 0d87fcc to e769867 Compare February 6, 2024 19:31

ibalajiarun requested a review from zekun000 February 6, 2024 19:31

zekun000 approved these changes Feb 8, 2024

View reviewed changes

ibalajiarun enabled auto-merge (squash) February 8, 2024 23:43

This comment has been minimized.

Sign in to view

ibalajiarun merged commit 3163686 into main Feb 9, 2024
97 of 118 checks passed

ibalajiarun deleted the balaji/concurrent-dag-store branch February 9, 2024 00:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dag] concurrent dag store #11505

[dag] concurrent dag store #11505

ibalajiarun commented Dec 22, 2023 •

edited

Loading

trunk-io bot commented Dec 22, 2023 •

edited

Loading

ibalajiarun commented Dec 22, 2023 •

edited

Loading

sasha8 Jan 10, 2024

sasha8 Jan 26, 2024

sasha8 Jan 27, 2024

sasha8 commented Jan 27, 2024

zekun000 Feb 4, 2024

ibalajiarun Feb 6, 2024

zekun000 Feb 4, 2024

zekun000 Feb 4, 2024

ibalajiarun Feb 4, 2024

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Feb 9, 2024

github-actions bot commented Feb 9, 2024

[dag] concurrent dag store #11505

[dag] concurrent dag store #11505

Conversation

ibalajiarun commented Dec 22, 2023 • edited Loading

Description

Test Plan

trunk-io bot commented Dec 22, 2023 • edited Loading

ibalajiarun commented Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sasha8 commented Jan 27, 2024

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.

github-actions bot commented Feb 9, 2024

✅ Forge suite compat success on aptos-node-v1.8.3 ==> e769867995ebf107fee3ee8026361a221f00f581

github-actions bot commented Feb 9, 2024

✅ Forge suite realistic_env_max_load success on e769867995ebf107fee3ee8026361a221f00f581

ibalajiarun commented Dec 22, 2023 •

edited

Loading

trunk-io bot commented Dec 22, 2023 •

edited

Loading

ibalajiarun commented Dec 22, 2023 •

edited

Loading

✅ Forge suite `compat` success on `aptos-node-v1.8.3` ==> `e769867995ebf107fee3ee8026361a221f00f581`

✅ Forge suite `realistic_env_max_load` success on `e769867995ebf107fee3ee8026361a221f00f581`