[consensus] Proposal generator: fetch only to round-up over max_to_execute #14230

igor-aptos · 2024-08-06T22:55:10Z

Description

When due to backpressure we want to create small blocks - i.e. 20 transactions, allow QS to only pull minimal number of batches to have at least 20 transactions, and use max_to_execute to cut afterwards

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

trunk-io · 2024-08-06T22:55:14Z

⏱️ 4h 13m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
adhoc-forge-test / forge	47m	🟩
execution-performance / single-node-performance	44m	🟥 🟩
forge-e2e-test / forge	40m	🟩 🟩 🟩
test-target-determinator	17m	🟩 🟩 🟩 🟩
forge-framework-upgrade-test / forge	15m	🟩
forge-compat-test / forge	13m	🟩
rust-move-tests	9m	🟩
rust-cargo-deny	9m	🟩 🟩 🟩 🟩 🟩
general-lints	8m	🟩 🟩 🟩 🟩 🟩
rust-move-tests	8m	🟩
rust-move-tests	8m	🟩
check-dynamic-deps	7m	🟩 🟩 🟩 🟩 🟩 (+2 more)
rust-doc-tests	5m	🟩
rust-move-tests	4m	⬜
execution-performance / test-target-determinator	4m	🟩
check	4m	🟩
rust-move-tests	3m	⬜
semgrep/ci	3m	🟩 🟩 🟩 🟩 🟩 (+2 more)
file_change_determinator	1m	⬜ 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	44s	🟩 🟩 🟩 🟩
Backport PR	39s	🟥 🟥 🟩
permission-check	22s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	22s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	18s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	16s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	11s	🟩 🟩 🟩 🟩
determine-docker-build-metadata	10s	🟩 🟩 🟩 🟩
permission-check	10s	🟩 🟩 🟩
determine-forge-run-metadata	2s	🟩
rust-move-tests	1s	⬜

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

bchocho

LGTM. Do you think a counter or log that distinguishes whether the soft max or max (or both?) were hit would be useful?

vusirikala · 2024-08-06T23:29:15Z

config/src/config/consensus_config.rs

@@ -228,6 +231,8 @@ impl Default for ConsensusConfig {
                percentile: 0.5,
                target_block_time_ms: 250,
                min_block_time_ms_to_activate: 100,
+                // allow at least two spreading group from reordring in a single block, to utilize paralellism


What is a spreading group?

nit:reordering

https://github.com/aptos-foundation/AIPs/blob/main/aips/aip-68.md

igor-aptos · 2024-08-07T00:43:54Z

I think overall block size, and overall limits should be enough

…ecute

igor-aptos · 2024-08-07T02:26:00Z

forge test succeeded - https://github.com/aptos-labs/aptos-core/actions/runs/10276146477 , with metrics and logs looking better, and no latency increase. landing

github-actions · 2024-08-07T02:53:26Z

✅ Forge suite `compat` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `fed178923156b501727400e29d1346e4d62393ce`

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> fed178923156b501727400e29d1346e4d62393ce (PR)
1. Check liveness of validators at old version: 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5
compatibility::simple-validator-upgrade::liveness-check : committed: 8071.07 txn/s, latency: 3910.59 ms, (p50: 2700 ms, p90: 6400 ms, p99: 26200 ms), latency samples: 292260
2. Upgrading first Validator to new version: fed178923156b501727400e29d1346e4d62393ce
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7086.95 txn/s, latency: 3780.27 ms, (p50: 4200 ms, p90: 4600 ms, p99: 4800 ms), latency samples: 131620
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 7365.81 txn/s, latency: 4157.76 ms, (p50: 4300 ms, p90: 5100 ms, p99: 5600 ms), latency samples: 249480
3. Upgrading rest of first batch to new version: fed178923156b501727400e29d1346e4d62393ce
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 5862.79 txn/s, latency: 4908.84 ms, (p50: 5300 ms, p90: 6600 ms, p99: 6700 ms), latency samples: 112640
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6302.95 txn/s, latency: 5099.82 ms, (p50: 5700 ms, p90: 6300 ms, p99: 6500 ms), latency samples: 215640
4. upgrading second batch to new version: fed178923156b501727400e29d1346e4d62393ce
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 10356.93 txn/s, latency: 2670.53 ms, (p50: 2500 ms, p90: 4200 ms, p99: 5200 ms), latency samples: 181520
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 11274.86 txn/s, latency: 2738.70 ms, (p50: 2600 ms, p90: 3500 ms, p99: 5500 ms), latency samples: 378540
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> fed178923156b501727400e29d1346e4d62393ce passed
Test Ok

github-actions · 2024-08-07T02:54:08Z

✅ Forge suite `realistic_env_max_load` success on `fed178923156b501727400e29d1346e4d62393ce`

two traffics test: inner traffic : committed: 12247.98 txn/s, latency: 3248.17 ms, (p50: 3000 ms, p90: 3600 ms, p99: 4200 ms), latency samples: 4656920
two traffics test : committed: 99.94 txn/s, latency: 2898.89 ms, (p50: 2700 ms, p90: 3300 ms, p99: 11100 ms), latency samples: 1740
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.243, avg: 0.227", "QsPosToProposal: max: 0.401, avg: 0.367", "ConsensusProposalToOrdered: max: 0.333, avg: 0.321", "ConsensusOrderedToCommit: max: 0.696, avg: 0.646", "ConsensusProposalToCommit: max: 1.021, avg: 0.967"]
Max round gap was 1 [limit 4] at version 2575490. Max no progress secs was 8.634141 [limit 15] at version 2575490.
Test Ok

github-actions · 2024-08-07T02:56:00Z

✅ Forge suite `framework_upgrade` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `fed178923156b501727400e29d1346e4d62393ce`

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> fed178923156b501727400e29d1346e4d62393ce (PR)
Upgrade the nodes to version: fed178923156b501727400e29d1346e4d62393ce
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1196.22 txn/s, submitted: 1199.27 txn/s, failed submission: 3.05 txn/s, expired: 3.05 txn/s, latency: 2594.45 ms, (p50: 1800 ms, p90: 4800 ms, p99: 8600 ms), latency samples: 109740
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1220.42 txn/s, submitted: 1223.47 txn/s, failed submission: 3.05 txn/s, expired: 3.05 txn/s, latency: 2697.76 ms, (p50: 1800 ms, p90: 4800 ms, p99: 11700 ms), latency samples: 104040
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> fed178923156b501727400e29d1346e4d62393ce passed
Upgrade the remaining nodes to version: fed178923156b501727400e29d1346e4d62393ce
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1274.06 txn/s, submitted: 1276.52 txn/s, failed submission: 2.46 txn/s, expired: 2.46 txn/s, latency: 2481.84 ms, (p50: 2100 ms, p90: 4300 ms, p99: 6900 ms), latency samples: 114020
Test Ok

…ecute (#14230) Co-authored-by: Igor <[email protected]> (cherry picked from commit 0690055)

github-actions · 2024-08-07T06:14:10Z

💚 All backports created successfully

Status	Branch	Result
✅	aptos-release-v1.18

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

…ecute (#14230) (#14235) (cherry picked from commit 0690055)

igor-aptos requested review from bchocho, sasha8, gelash, zekun000, ibalajiarun, JoshLind and gregnazario as code owners August 6, 2024 22:55

igor-aptos requested review from vusirikala and sitalkedia August 6, 2024 22:55

igor-aptos force-pushed the igor/fetch_only_roundup_over_max_to_execute branch from b4bccd7 to 520d248 Compare August 6, 2024 22:58

igor-aptos added v1.18 CICD:run-forge-e2e-perf Run the e2e perf forge only labels Aug 6, 2024

igor-aptos force-pushed the igor/fetch_only_roundup_over_max_to_execute branch from 520d248 to 12f0f9b Compare August 6, 2024 23:03

bchocho approved these changes Aug 6, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

vusirikala reviewed Aug 6, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

vusirikala approved these changes Aug 6, 2024

View reviewed changes

[consensus] Proposal generator: fetch only to round-up over max_to_ex…

fed1789

…ecute

igor-aptos force-pushed the igor/fetch_only_roundup_over_max_to_execute branch from 12f0f9b to fed1789 Compare August 7, 2024 00:45

This comment has been minimized.

Sign in to view

igor-aptos enabled auto-merge (squash) August 7, 2024 02:26

This comment has been minimized.

Sign in to view

igor-aptos merged commit 0690055 into main Aug 7, 2024
89 of 94 checks passed

igor-aptos deleted the igor/fetch_only_roundup_over_max_to_execute branch August 7, 2024 06:13

github-actions bot pushed a commit that referenced this pull request Aug 7, 2024

[consensus] Proposal generator: fetch only to round-up over max_to_ex…

fd23fdd

…ecute (#14230) Co-authored-by: Igor <[email protected]> (cherry picked from commit 0690055)

github-actions bot mentioned this pull request Aug 7, 2024

[cp][aptos-release-v1.18] [consensus] Proposal generator: fetch only to round-up over max_to_execute #14235

Merged

igor-aptos pushed a commit that referenced this pull request Aug 7, 2024

[consensus] Proposal generator: fetch only to round-up over max_to_ex…

d1bf834

…ecute (#14230) (#14235) (cherry picked from commit 0690055)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[consensus] Proposal generator: fetch only to round-up over max_to_execute #14230

[consensus] Proposal generator: fetch only to round-up over max_to_execute #14230

igor-aptos commented Aug 6, 2024

trunk-io bot commented Aug 6, 2024 •

edited

Loading

bchocho left a comment

This comment has been minimized.

vusirikala Aug 6, 2024

vusirikala Aug 6, 2024

igor-aptos Aug 7, 2024

This comment has been minimized.

igor-aptos commented Aug 7, 2024

This comment has been minimized.

This comment has been minimized.

igor-aptos commented Aug 7, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Aug 7, 2024

github-actions bot commented Aug 7, 2024

github-actions bot commented Aug 7, 2024

github-actions bot commented Aug 7, 2024

[consensus] Proposal generator: fetch only to round-up over max_to_execute #14230

[consensus] Proposal generator: fetch only to round-up over max_to_execute #14230

Conversation

igor-aptos commented Aug 6, 2024

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

Checklist

trunk-io bot commented Aug 6, 2024 • edited Loading

bchocho left a comment

Choose a reason for hiding this comment

This comment has been minimized.

vusirikala Aug 6, 2024

Choose a reason for hiding this comment

vusirikala Aug 6, 2024

Choose a reason for hiding this comment

igor-aptos Aug 7, 2024

Choose a reason for hiding this comment

This comment has been minimized.

igor-aptos commented Aug 7, 2024

This comment has been minimized.

This comment has been minimized.

igor-aptos commented Aug 7, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Aug 7, 2024

✅ Forge suite compat success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> fed178923156b501727400e29d1346e4d62393ce

github-actions bot commented Aug 7, 2024

✅ Forge suite realistic_env_max_load success on fed178923156b501727400e29d1346e4d62393ce

github-actions bot commented Aug 7, 2024

✅ Forge suite framework_upgrade success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> fed178923156b501727400e29d1346e4d62393ce

github-actions bot commented Aug 7, 2024

💚 All backports created successfully

Questions ?

trunk-io bot commented Aug 6, 2024 •

edited

Loading

✅ Forge suite `compat` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `fed178923156b501727400e29d1346e4d62393ce`

✅ Forge suite `realistic_env_max_load` success on `fed178923156b501727400e29d1346e4d62393ce`

✅ Forge suite `framework_upgrade` success on `1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5` ==> `fed178923156b501727400e29d1346e4d62393ce`