[dag] refactor/improve dag health monitoring #11362

ibalajiarun · 2023-12-15T00:06:05Z

Description

Refactors Chain health backoff by decoupling it from LeaderReputationAdapter.
Adds PipelineBackpressure mechanism
Organizes both chain health and pipeline backpressure into a health module
Adds on chain configs for leader reputation adapter.

Test Plan

trunk-io · 2023-12-15T00:06:08Z

⏱️ 10h total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-unit-tests	4h 56m	🟩 🟩 🟩 ⬜ 🟩 (+5 more)
windows-build	2h 29m	🟩 🟩 🟩 🟩 🟩 (+5 more)
run-tests-main-branch	51m	🟩 🟩 🟩 🟩 🟩 (+6 more)
rust-lints	38m	🟥 🟥 🟥 🟥 🟥 (+5 more)
general-lints	27m	🟩 🟩 🟩 🟩 🟩 (+5 more)
check-dynamic-deps	19m	🟩 🟩 🟩 🟩 🟩 (+5 more)
check	11m	🟥 🟥 🟥 🟥 🟩 (+6 more)
semgrep/ci	3m	🟩 🟩 🟩 🟩 🟩 (+5 more)
file_change_determinator	2m	🟩 🟩 🟩 🟩 🟩 (+6 more)
file_change_determinator	2m	🟩 🟩 🟩 🟩 🟩 (+5 more)
permission-check	39s	🟩 🟩 🟩 🟩 🟩 (+6 more)
permission-check	31s	🟩 🟩 🟩 🟩 🟩 (+6 more)
permission-check	29s	🟩 🟩 🟩 🟩 🟩 (+6 more)
permission-check	23s	🟩 🟩 🟩 🟩 🟩 (+5 more)
permission-check	2s	🟩

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

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

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

ibalajiarun · 2023-12-18T19:50:56Z

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.

ibalajiarun · 2023-12-18T22:51:09Z

consensus/src/dag/bootstrap.rs

+        Arc<dyn AnchorElection>,
+        Option<Arc<LeaderReputationAdapter>>,
+        Option<Vec<CommitEvent>>,


This is possibly ugly. I couldn't cleanly separate them in this PR.

we probably should always populate the commit events (for the sake of completeness)

to make it look better, we can create another trait to get voting power ratio and having a no-op one always returns 1 so here can return Arc<dyn AnchorElection>, Arc<dyn CommitHistory>, Vec<CommitEvent>

I am not sure that we can populate always. How many events do we read? For AnchorElectionMode::LeaderReputation we have a config with num events to read, but not for RoundRobin Leader. Also, it seems unnecessary to read in the latter case?

Also, introduced the CommitHistory trait.

zekun000 · 2023-12-20T22:14:48Z

consensus/src/dag/bootstrap.rs

+        Arc<dyn AnchorElection>,
+        Option<Arc<LeaderReputationAdapter>>,
+        Option<Vec<CommitEvent>>,


we probably should always populate the commit events (for the sake of completeness)

zekun000 · 2023-12-20T22:17:45Z

consensus/src/dag/bootstrap.rs

+        Arc<dyn AnchorElection>,
+        Option<Arc<LeaderReputationAdapter>>,
+        Option<Vec<CommitEvent>>,


to make it look better, we can create another trait to get voting power ratio and having a no-op one always returns 1 so here can return Arc<dyn AnchorElection>, Arc<dyn CommitHistory>, Vec<CommitEvent>

zekun000 · 2023-12-20T22:26:53Z

consensus/src/dag/health/backoff.rs

+            .unwrap_or((u64::MAX, u64::MAX));
+        let voting_power_ratio = self.chain_health.voting_power_ratio(round);
+
+        let max_txns_per_round = vec![


nit: it's probably better to use [] than vec![] here

zekun000 · 2023-12-20T22:34:19Z

consensus/src/dag/bootstrap.rs

+            ordered_notifier.clone(),
+        );
+        let health_backoff =
+            HealthBackoff::new(self.epoch_state.clone(), chain_health, pipeline_health);


none of these actually stops the progress fully right?

correct, next PR stops voting.

sasha8 · 2024-01-09T22:33:28Z

consensus/src/dag/round_state.rs

-        };
-
+        let wait_time = self.minimal_wait_time.max(minimum_delay);
+


Line 163:
I think even if voting power == 3f+1 we need to wait for minimum_delay.
Currently this is not the case.

yes, fixed it.

zekun000 · 2024-02-03T23:30:34Z

consensus/src/dag/adapter.rs

@@ -89,6 +93,7 @@ pub(super) struct OrderedNotifierAdapter {
    parent_block_info: Arc<RwLock<BlockInfo>>,
    epoch_state: Arc<EpochState>,
    ledger_info_provider: Arc<RwLock<LedgerInfoProvider>>,
+    block_created_ts: Arc<RwLock<BTreeMap<Round, Instant>>>,


the name is confusing, I think you use block ordered time instead of creation time?

The block is created here for the first time, so I named it that way.

oh because dag doesn't have block, but still it's easy to confuse with jolteon block creation, I'd just call it block_ordered_ts

zekun000 · 2024-02-03T23:39:56Z

consensus/src/dag/bootstrap.rs

+    ) -> (
+        Arc<dyn AnchorElection>,
+        Arc<dyn CommitHistory>,
+        Option<Vec<CommitEvent>>,


roughly remember we discussed this, the abstraction here looks weird. I'd imagine we have a concrete CommitHistory struct that can return CommitEvent, and AnchorElection can use the CommitHistory (impl MetadataBackend) internally for reputation.

discussed offline, will follow-up

zekun000 · 2024-02-03T23:44:12Z

consensus/src/dag/health/backoff.rs

+        .min()
+        .expect("must not be empty");
+
+        let max_txns = max_txns_per_round.saturating_div(


hmm, do we verify payload limit on receivers today? this seems making it impossible to verify because of different views?

added a TODO

github-actions · 2024-02-06T05:44:24Z

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

Compatibility test results for aptos-node-v1.8.3 ==> bf1d625b09968169673c5da6799d8c27c239c204 (PR)
1. Check liveness of validators at old version: aptos-node-v1.8.3
compatibility::simple-validator-upgrade::liveness-check : committed: 4823 txn/s, latency: 6650 ms, (p50: 5700 ms, p90: 10400 ms, p99: 20100 ms), latency samples: 183280
2. Upgrading first Validator to new version: bf1d625b09968169673c5da6799d8c27c239c204
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1772 txn/s, latency: 15665 ms, (p50: 18900 ms, p90: 21800 ms, p99: 22500 ms), latency samples: 92180
3. Upgrading rest of first batch to new version: bf1d625b09968169673c5da6799d8c27c239c204
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1409 txn/s, latency: 20505 ms, (p50: 20800 ms, p90: 27600 ms, p99: 28800 ms), latency samples: 67640
4. upgrading second batch to new version: bf1d625b09968169673c5da6799d8c27c239c204
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2985 txn/s, latency: 9647 ms, (p50: 9800 ms, p90: 13700 ms, p99: 17500 ms), latency samples: 140320
5. check swarm health
Compatibility test for aptos-node-v1.8.3 ==> bf1d625b09968169673c5da6799d8c27c239c204 passed
Test Ok

github-actions · 2024-02-06T05:45:13Z

✅ Forge suite `realistic_env_max_load` success on `bf1d625b09968169673c5da6799d8c27c239c204`

two traffics test: inner traffic : committed: 6281 txn/s, latency: 6081 ms, (p50: 5700 ms, p90: 8100 ms, p99: 15000 ms), latency samples: 2720060
two traffics test : committed: 100 txn/s, latency: 2269 ms, (p50: 2200 ms, p90: 2600 ms, p99: 5200 ms), latency samples: 1820
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.280, avg: 0.213", "QsPosToProposal: max: 0.176, avg: 0.165", "ConsensusProposalToOrdered: max: 0.621, avg: 0.595", "ConsensusOrderedToCommit: max: 0.536, avg: 0.499", "ConsensusProposalToCommit: max: 1.138, avg: 1.094"]
Max round gap was 1 [limit 4] at version 1258237. Max no progress secs was 4.987812 [limit 15] at version 1258237.
Test Ok

ibalajiarun force-pushed the balaji/backpressure branch from cfd24a5 to e653a70 Compare December 18, 2023 19:50

ibalajiarun changed the base branch from main to balaji/dag-event-fix December 18, 2023 20:13

ibalajiarun changed the title ~~[dag][bugfix] properly read events after restart~~ [dag] refactor dag health monitoring Dec 18, 2023

ibalajiarun changed the title ~~[dag] refactor dag health monitoring~~ [dag] refactor/improve dag health monitoring Dec 18, 2023

ibalajiarun marked this pull request as ready for review December 18, 2023 20:56

ibalajiarun requested review from zekun000, sasha8, JoshLind and gregnazario as code owners December 18, 2023 20:56

ibalajiarun force-pushed the balaji/backpressure branch from f394e70 to b443006 Compare December 18, 2023 21:57

ibalajiarun commented Dec 18, 2023

View reviewed changes

ibalajiarun force-pushed the balaji/dag-event-fix branch from fec87c9 to ce5ed52 Compare December 18, 2023 23:42

ibalajiarun force-pushed the balaji/backpressure branch from b443006 to e9bcd97 Compare December 18, 2023 23:42

ibalajiarun mentioned this pull request Dec 18, 2023

[dag] stop voting based on pipeline latency #11407

Merged

ibalajiarun force-pushed the balaji/dag-event-fix branch 2 times, most recently from 89b8b17 to 82f79bd Compare December 19, 2023 00:33

ibalajiarun force-pushed the balaji/backpressure branch from e9bcd97 to 697fbe6 Compare December 19, 2023 00:37

Base automatically changed from balaji/dag-event-fix to main December 19, 2023 01:06

ibalajiarun force-pushed the balaji/backpressure branch from 697fbe6 to b0ba9d6 Compare December 19, 2023 01:14

ibalajiarun mentioned this pull request Dec 20, 2023

Balaji/dag forge #11433

Closed

ibalajiarun force-pushed the balaji/backpressure branch from b0ba9d6 to 1a912a9 Compare December 20, 2023 17:29

zekun000 reviewed Dec 20, 2023

View reviewed changes

ibalajiarun mentioned this pull request Dec 22, 2023

[dag] concurrent dag store #11505

Merged

ibalajiarun force-pushed the balaji/backpressure branch from 1a912a9 to 022dce0 Compare January 8, 2024 23:38

sasha8 reviewed Jan 9, 2024

View reviewed changes

ibalajiarun force-pushed the balaji/backpressure branch from 022dce0 to d93c02a Compare January 10, 2024 22:00

ibalajiarun requested review from zekun000 and sasha8 January 10, 2024 22:23

This was referenced Jan 18, 2024

[dag] parallel message handler implementation #11685

Merged

[dag] support parallel rb voting #11743

Merged

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

Merged

ibalajiarun force-pushed the balaji/backpressure branch from c6a0cbf to 694d0e5 Compare January 25, 2024 21:59

ibalajiarun mentioned this pull request Jan 28, 2024

[dag] cache votes without re-counting #11808

Merged

zekun000 reviewed Feb 3, 2024

View reviewed changes

zekun000 approved these changes Feb 5, 2024

View reviewed changes

sasha8 approved these changes Feb 5, 2024

View reviewed changes

ibalajiarun mentioned this pull request Feb 6, 2024

[dag] decouple payload 2/n #11918

Closed

ibalajiarun added 6 commits February 5, 2024 19:53

[dag] refactor dag health monitor

5e0a796

[dag] move reading commit events from order_rule

82a3d59

address feedback

b0eae0f

wait backoff time with 3f+1

d3c1c40

address feedback

c2e2d04

lint

901b273

ibalajiarun force-pushed the balaji/backpressure branch from 52d77cf to 901b273 Compare February 6, 2024 03:58

ibalajiarun enabled auto-merge (squash) February 6, 2024 03:59

ibalajiarun added 3 commits February 5, 2024 20:07

lint

75312f3

precommit config

9d34e90

lint

bf1d625

This comment has been minimized.

Sign in to view

ibalajiarun merged commit 7c0ada5 into main Feb 6, 2024
42 checks passed

ibalajiarun deleted the balaji/backpressure branch February 6, 2024 05:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dag] refactor/improve dag health monitoring #11362

[dag] refactor/improve dag health monitoring #11362

ibalajiarun commented Dec 15, 2023 •

edited

Loading

trunk-io bot commented Dec 15, 2023 •

edited

Loading

ibalajiarun commented Dec 18, 2023 •

edited

Loading

ibalajiarun Dec 18, 2023 •

edited

Loading

zekun000 Dec 20, 2023

zekun000 Dec 20, 2023

ibalajiarun Jan 10, 2024

ibalajiarun Jan 10, 2024

zekun000 Dec 20, 2023

zekun000 Dec 20, 2023

zekun000 Dec 20, 2023

zekun000 Dec 20, 2023

ibalajiarun Jan 10, 2024

sasha8 Jan 9, 2024

ibalajiarun Jan 10, 2024

zekun000 Feb 3, 2024

ibalajiarun Feb 4, 2024

zekun000 Feb 4, 2024

zekun000 Feb 3, 2024

ibalajiarun Feb 6, 2024

zekun000 Feb 3, 2024

ibalajiarun Feb 6, 2024

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Feb 6, 2024

github-actions bot commented Feb 6, 2024

		};

		let wait_time = self.minimal_wait_time.max(minimum_delay);

[dag] refactor/improve dag health monitoring #11362

[dag] refactor/improve dag health monitoring #11362

Conversation

ibalajiarun commented Dec 15, 2023 • edited Loading

Description

Test Plan

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

ibalajiarun commented Dec 18, 2023 • edited Loading

ibalajiarun Dec 18, 2023 • edited Loading

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

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 6, 2024

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

github-actions bot commented Feb 6, 2024

✅ Forge suite realistic_env_max_load success on bf1d625b09968169673c5da6799d8c27c239c204

ibalajiarun commented Dec 15, 2023 •

edited

Loading

trunk-io bot commented Dec 15, 2023 •

edited

Loading

ibalajiarun commented Dec 18, 2023 •

edited

Loading

ibalajiarun Dec 18, 2023 •

edited

Loading

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

✅ Forge suite `realistic_env_max_load` success on `bf1d625b09968169673c5da6799d8c27c239c204`