Update gas estimation changes for UseCaseAware block reordering (AIP-68) #14669

igor-aptos · 2024-09-18T17:18:46Z

Description

AIP-68 implements spreading out of potentially conflicting transactions within a block. Consequence is that workload from single module doesn't increase gas prices for unrelated modules.

Updating gas estimation logic to not treat block as full if majority of block is from a single contract.

Type of Change

New feature

Which Components or Systems Does This Change Impact?

Full Node (API, Indexer, etc.)

How Has This Been Tested?

will run workload sweep forge test and check gas estimation.

Key Areas to Review

change to block_min_inclusion_price

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-09-18T17:18:50Z

⏱️ 7h 59m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
forge-compat-test / forge	2h 4m	🟩 🟩 🟥 🟩 🟩
execution-performance / single-node-performance	1h 31m	🟩 🟩 🟩 🟩
forge-e2e-test / forge	1h 3m	🟩 🟩 🟩 🟩
forge-framework-upgrade-test / forge	33m	🟩 🟩
test-target-determinator	18m	🟩 🟩 🟩 🟩
execution-performance / test-target-determinator	18m	🟩 🟩 🟩 🟩
check	14m	🟩 🟩 🟩 🟩
general-lints	11m	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-cargo-deny	10m	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
check-dynamic-deps	8m	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-doc-tests	5m	🟩
rust-doc-tests	5m	🟩
rust-doc-tests	5m	🟩
rust-doc-tests	5m	🟩
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose	4m	🟩 🟩 🟩
semgrep/ci	3m	🟩 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
Backport PR	43s	🟥 🟥 🟩
file_change_determinator	38s	🟩 🟩 🟩 🟩
permission-check	28s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	21s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	19s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	19s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	18s	🟩 🟩 🟩 🟩
determine-docker-build-metadata	11s	🟩 🟩 🟩 🟩
permission-check	9s	🟩 🟩 🟩

🚨 2 jobs on the last run were significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
check-dynamic-deps	3m	1m
forge-compat-test / forge	22m	18m

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

bchocho · 2024-09-18T18:58:08Z

api/src/context.rs

+            first,
+            last - first,
+            ledger_info.ledger_version.0,
+            user_use_case_spread_factor.is_some(),


Should we also have a local config here that can act as a kill switch if the gas estimates seem off?

igor-aptos · 2024-09-18T21:11:07Z

@bchocho added config to disable

bchocho

LGTM

bchocho · 2024-09-19T17:47:29Z

api/src/context.rs

+        ) {
+            Ok((prices_and_used, block_end_infos, majority_usecase_fraction)) => {
+                let is_full_block =
+                    if majority_usecase_fraction.map_or(false, |fraction| fraction > 0.5) {


nit: consistency usecase vs use_case

(cherry picked from commit 25499ae)

github-actions · 2024-09-19T22:15:53Z

💚 All backports created successfully

Status	Branch	Result
✅	aptos-release-v1.20

Questions ?

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

github-actions · 2024-09-19T22:45:22Z

✅ Forge suite `framework_upgrade` success on `25a081116546670e62ca927ba90478de78557056` ==> `e4dc9b5cc585fdf77bb2169123db7e9778aa7b15`

Compatibility test results for 25a081116546670e62ca927ba90478de78557056 ==> e4dc9b5cc585fdf77bb2169123db7e9778aa7b15 (PR)
Upgrade the nodes to version: e4dc9b5cc585fdf77bb2169123db7e9778aa7b15
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1212.24 txn/s, submitted: 1214.31 txn/s, failed submission: 2.07 txn/s, expired: 2.07 txn/s, latency: 2589.67 ms, (p50: 2300 ms, p70: 2600, p90: 4400 ms, p99: 5700 ms), latency samples: 105400
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1299.77 txn/s, submitted: 1301.47 txn/s, failed submission: 1.70 txn/s, expired: 1.70 txn/s, latency: 2559.04 ms, (p50: 2400 ms, p70: 2700, p90: 3600 ms, p99: 5100 ms), latency samples: 106960
5. check swarm health
Compatibility test for 25a081116546670e62ca927ba90478de78557056 ==> e4dc9b5cc585fdf77bb2169123db7e9778aa7b15 passed
Upgrade the remaining nodes to version: e4dc9b5cc585fdf77bb2169123db7e9778aa7b15
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1241.57 txn/s, submitted: 1243.87 txn/s, failed submission: 2.30 txn/s, expired: 2.30 txn/s, latency: 2510.53 ms, (p50: 2400 ms, p70: 2700, p90: 3600 ms, p99: 5700 ms), latency samples: 108040
Test Ok

github-actions · 2024-09-19T22:45:48Z

✅ Forge suite `realistic_env_max_load` success on `e4dc9b5cc585fdf77bb2169123db7e9778aa7b15`

two traffics test: inner traffic : committed: 13768.95 txn/s, submitted: 13771.42 txn/s, expired: 2.47 txn/s, latency: 2875.03 ms, (p50: 2700 ms, p70: 2800, p90: 3000 ms, p99: 8100 ms), latency samples: 5235240
two traffics test : committed: 99.98 txn/s, latency: 1597.18 ms, (p50: 1500 ms, p70: 1600, p90: 1800 ms, p99: 6000 ms), latency samples: 1980
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.246, avg: 0.224", "QsPosToProposal: max: 1.108, avg: 1.032", "ConsensusProposalToOrdered: max: 0.324, avg: 0.299", "ConsensusOrderedToCommit: max: 0.433, avg: 0.412", "ConsensusProposalToCommit: max: 0.730, avg: 0.711"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.85s no progress at version 8260 (avg 0.21s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.30s no progress at version 2062774 (avg 6.68s) [limit 15].
Test Ok

github-actions · 2024-09-19T22:52:47Z

✅ Forge suite `compat` success on `25a081116546670e62ca927ba90478de78557056` ==> `e4dc9b5cc585fdf77bb2169123db7e9778aa7b15`

Compatibility test results for 25a081116546670e62ca927ba90478de78557056 ==> e4dc9b5cc585fdf77bb2169123db7e9778aa7b15 (PR)
1. Check liveness of validators at old version: 25a081116546670e62ca927ba90478de78557056
compatibility::simple-validator-upgrade::liveness-check : committed: 16011.79 txn/s, latency: 2097.30 ms, (p50: 2000 ms, p70: 2100, p90: 2800 ms, p99: 3800 ms), latency samples: 522620
2. Upgrading first Validator to new version: e4dc9b5cc585fdf77bb2169123db7e9778aa7b15
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7922.02 txn/s, latency: 3438.23 ms, (p50: 3400 ms, p70: 3900, p90: 4700 ms, p99: 5100 ms), latency samples: 143740
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 7783.00 txn/s, latency: 4049.15 ms, (p50: 4100 ms, p70: 4300, p90: 6200 ms, p99: 6400 ms), latency samples: 261320
3. Upgrading rest of first batch to new version: e4dc9b5cc585fdf77bb2169123db7e9778aa7b15
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 7887.94 txn/s, latency: 3619.11 ms, (p50: 4100 ms, p70: 4200, p90: 4300 ms, p99: 4400 ms), latency samples: 144700
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6895.05 txn/s, latency: 4402.76 ms, (p50: 4400 ms, p70: 4500, p90: 4700 ms, p99: 6600 ms), latency samples: 259580
4. upgrading second batch to new version: e4dc9b5cc585fdf77bb2169123db7e9778aa7b15
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 10266.79 txn/s, latency: 2658.39 ms, (p50: 2600 ms, p70: 2800, p90: 4300 ms, p99: 4600 ms), latency samples: 195960
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 10334.70 txn/s, latency: 2944.67 ms, (p50: 2500 ms, p70: 2800, p90: 5700 ms, p99: 7800 ms), latency samples: 336740
5. check swarm health
Compatibility test for 25a081116546670e62ca927ba90478de78557056 ==> e4dc9b5cc585fdf77bb2169123db7e9778aa7b15 passed
Test Ok

(cherry picked from commit 25499ae) Co-authored-by: igor-aptos <[email protected]>

igor-aptos requested review from bchocho, msmouse and sitalkedia September 18, 2024 17:18

igor-aptos requested review from banool, gregnazario and 0xmaayan as code owners September 18, 2024 17:18

bchocho reviewed Sep 18, 2024

View reviewed changes

igor-aptos force-pushed the igor/exclude_majority_traffic_for_gas_estimation branch from eb3ec07 to a2297cd Compare September 18, 2024 21:10

igor-aptos requested a review from JoshLind as a code owner September 18, 2024 21:10

igor-aptos requested a review from bchocho September 18, 2024 21:10

vusirikala approved these changes Sep 18, 2024

View reviewed changes

igor-aptos added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Sep 18, 2024

This comment has been minimized.

Sign in to view

bchocho approved these changes Sep 19, 2024

View reviewed changes

gas estimation changes

e4dc9b5

igor-aptos force-pushed the igor/exclude_majority_traffic_for_gas_estimation branch from a2297cd to e4dc9b5 Compare September 19, 2024 19:37

This comment has been minimized.

Sign in to view

igor-aptos enabled auto-merge (squash) September 19, 2024 20:40

igor-aptos removed the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Sep 19, 2024

This comment has been minimized.

Sign in to view

igor-aptos merged commit 25499ae into main Sep 19, 2024
90 of 93 checks passed

igor-aptos deleted the igor/exclude_majority_traffic_for_gas_estimation branch September 19, 2024 22:13

igor-aptos added the v1.20 label Sep 19, 2024

github-actions bot pushed a commit that referenced this pull request Sep 19, 2024

gas estimation changes (#14669)

978df73

(cherry picked from commit 25499ae)

github-actions bot mentioned this pull request Sep 19, 2024

[cp][aptos-release-v1.20] Update gas estimation changes for UseCaseAware block reordering (AIP-68) #14699

Merged

This comment has been minimized.

Sign in to view

igor-aptos added a commit that referenced this pull request Sep 20, 2024

gas estimation changes (#14669) (#14699)

d891f37

(cherry picked from commit 25499ae) Co-authored-by: igor-aptos <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update gas estimation changes for UseCaseAware block reordering (AIP-68) #14669

Update gas estimation changes for UseCaseAware block reordering (AIP-68) #14669

igor-aptos commented Sep 18, 2024 •

edited

Loading

trunk-io bot commented Sep 18, 2024 •

edited

Loading

bchocho Sep 18, 2024

igor-aptos commented Sep 18, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bchocho left a comment

bchocho Sep 19, 2024

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.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 19, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 19, 2024

Update gas estimation changes for UseCaseAware block reordering (AIP-68) #14669

Update gas estimation changes for UseCaseAware block reordering (AIP-68) #14669

Conversation

igor-aptos commented Sep 18, 2024 • edited Loading

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 Sep 18, 2024 • edited Loading

bchocho Sep 18, 2024

Choose a reason for hiding this comment

igor-aptos commented Sep 18, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bchocho left a comment

Choose a reason for hiding this comment

bchocho Sep 19, 2024

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.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 19, 2024

💚 All backports created successfully

Questions ?

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 19, 2024

✅ Forge suite framework_upgrade success on 25a081116546670e62ca927ba90478de78557056 ==> e4dc9b5cc585fdf77bb2169123db7e9778aa7b15

github-actions bot commented Sep 19, 2024

✅ Forge suite realistic_env_max_load success on e4dc9b5cc585fdf77bb2169123db7e9778aa7b15

github-actions bot commented Sep 19, 2024

✅ Forge suite compat success on 25a081116546670e62ca927ba90478de78557056 ==> e4dc9b5cc585fdf77bb2169123db7e9778aa7b15

igor-aptos commented Sep 18, 2024 •

edited

Loading

trunk-io bot commented Sep 18, 2024 •

edited

Loading

✅ Forge suite `framework_upgrade` success on `25a081116546670e62ca927ba90478de78557056` ==> `e4dc9b5cc585fdf77bb2169123db7e9778aa7b15`

✅ Forge suite `realistic_env_max_load` success on `e4dc9b5cc585fdf77bb2169123db7e9778aa7b15`

✅ Forge suite `compat` success on `25a081116546670e62ca927ba90478de78557056` ==> `e4dc9b5cc585fdf77bb2169123db7e9778aa7b15`