Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid looking for new epoch event in all transactions #15293

Merged
merged 2 commits into from
Nov 19, 2024
Merged

Avoid looking for new epoch event in all transactions #15293

merged 2 commits into from
Nov 19, 2024

Conversation

msmouse
Copy link
Contributor

@msmouse msmouse commented Nov 16, 2024 •
edited
Loading

Description

takes about 1% of total execution time.
it's guranteed by BlockSTM that only one transaction can possibly return reconfig and all statuses after it are retries.

How Has This Been Tested?

added and fixed tests

Type of Change

  • Performance improvement

Which Components or Systems Does This Change Impact?

  • Validator Node

@trunk-io Trunk.io
Copy link

trunk-io bot commented Nov 16, 2024 •
edited
Loading

⏱️ 9h 2m total CI duration on this PR
Slowest 15 Jobs Cumulative Duration Recent Runs
single-node-performance 3h 42m 🟩 🟩 🟩 🟩 🟩 (+1 more)
execution-performance / single-node-performance 1h 53m 🟥 🟥 🟩 🟥 🟩 (+2 more)
test-target-determinator 26m 🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-cargo-deny 21m 🟩 🟩 ⬜ 🟩 🟩 (+4 more)
execution-performance / test-target-determinator 19m 🟩 🟩 🟩 🟩 🟩 
test-target-determinator 17m 🟩 🟩 ⬜ 🟩 🟩 
rust-move-tests 13m 🟩 
rust-move-tests 12m 🟩 
rust-move-tests 12m 🟩 
rust-move-tests 12m 🟩 
rust-move-tests 12m 🟩 
rust-move-tests 12m 🟩 
check 7m 🟩 🟩 
check-dynamic-deps 7m 🟩 🟩 🟩 🟩 🟩 (+4 more)
semgrep/ci 6m 🟩 🟩 🟩 🟩 🟩 (+4 more)

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

Job Duration vs 7d avg Delta
rust-cargo-deny 10m 2m +426%
execution-performance / single-node-performance 25m 15m +65%
execution-performance / test-target-determinator 3m 5m -30%

settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io

@msmouse msmouse added the CICD:run-execution-performance-test Run execution performance test label Nov 18, 2024
@msmouse msmouse enabled auto-merge (rebase) November 19, 2024 00:55
zekun000
zekun000 approved these changes Nov 19, 2024
View reviewed changes
execution/executor-types/src/transactions_with_output.rs
(0..self.len())
.rev()
.find(|&i| Self::need_checkpoint(&self.transactions[i], self.epoch_ending_flags[i]))
.find(|&i| self.transactions[i].is_non_reconfig_block_ending())
Copy link
Contributor

Choose a reason for hiding this comment

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

why this is not the last one? for chunks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, for state sync

@msmouse msmouse added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Nov 19, 2024
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 8ed24d16563fd32776d2527750e0336625c8cade

two traffics test: inner traffic : committed: 14654.29 txn/s, latency: 2714.65 ms, (p50: 2700 ms, p70: 2700, p90: 2700 ms, p99: 3000 ms), latency samples: 5571860
two traffics test : committed: 100.04 txn/s, latency: 1392.61 ms, (p50: 1300 ms, p70: 1400, p90: 1500 ms, p99: 2900 ms), latency samples: 1700
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.978, avg: 1.563", "ConsensusProposalToOrdered: max: 0.317, avg: 0.289", "ConsensusOrderedToCommit: max: 0.365, avg: 0.358", "ConsensusProposalToCommit: max: 0.659, avg: 0.647"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.88s no progress at version 2103422 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.60s no progress at version 2103420 (avg 7.21s) [limit 15].
Test Ok

@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on be3ccc58ef518259d63fab7f3e3613f2a6214268 ==> 8ed24d16563fd32776d2527750e0336625c8cade

Compatibility test results for be3ccc58ef518259d63fab7f3e3613f2a6214268 ==> 8ed24d16563fd32776d2527750e0336625c8cade (PR)
Upgrade the nodes to version: 8ed24d16563fd32776d2527750e0336625c8cade
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1236.39 txn/s, submitted: 1238.81 txn/s, failed submission: 2.42 txn/s, expired: 2.42 txn/s, latency: 2401.64 ms, (p50: 2100 ms, p70: 2400, p90: 4200 ms, p99: 6000 ms), latency samples: 112480
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1225.57 txn/s, submitted: 1227.61 txn/s, failed submission: 2.04 txn/s, expired: 2.04 txn/s, latency: 2465.10 ms, (p50: 1800 ms, p70: 2400, p90: 4800 ms, p99: 6900 ms), latency samples: 108060
5. check swarm health
Compatibility test for be3ccc58ef518259d63fab7f3e3613f2a6214268 ==> 8ed24d16563fd32776d2527750e0336625c8cade passed
Upgrade the remaining nodes to version: 8ed24d16563fd32776d2527750e0336625c8cade
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1176.29 txn/s, submitted: 1178.77 txn/s, failed submission: 2.48 txn/s, expired: 2.48 txn/s, latency: 2699.73 ms, (p50: 2100 ms, p70: 2700, p90: 5600 ms, p99: 7100 ms), latency samples: 104540
Test Ok

@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite compat success on be3ccc58ef518259d63fab7f3e3613f2a6214268 ==> 8ed24d16563fd32776d2527750e0336625c8cade

Compatibility test results for be3ccc58ef518259d63fab7f3e3613f2a6214268 ==> 8ed24d16563fd32776d2527750e0336625c8cade (PR)
1. Check liveness of validators at old version: be3ccc58ef518259d63fab7f3e3613f2a6214268
compatibility::simple-validator-upgrade::liveness-check : committed: 8423.49 txn/s, submitted: 8423.79 txn/s, expired: 0.29 txn/s, latency: 2211.67 ms, (p50: 1600 ms, p70: 1800, p90: 2000 ms, p99: 28500 ms), latency samples: 522662
2. Upgrading first Validator to new version: 8ed24d16563fd32776d2527750e0336625c8cade
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 5005.46 txn/s, latency: 5790.75 ms, (p50: 6200 ms, p70: 6800, p90: 7000 ms, p99: 7200 ms), latency samples: 96980
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 4983.36 txn/s, latency: 6454.05 ms, (p50: 7200 ms, p70: 7300, p90: 8000 ms, p99: 8400 ms), latency samples: 171820
3. Upgrading rest of first batch to new version: 8ed24d16563fd32776d2527750e0336625c8cade
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 7674.44 txn/s, latency: 3771.41 ms, (p50: 4200 ms, p70: 4400, p90: 4500 ms, p99: 4600 ms), latency samples: 143760
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6770.26 txn/s, latency: 4462.80 ms, (p50: 4600 ms, p70: 4700, p90: 4700 ms, p99: 6800 ms), latency samples: 258520
4. upgrading second batch to new version: 8ed24d16563fd32776d2527750e0336625c8cade
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 2802.45 txn/s, submitted: 2802.60 txn/s, expired: 0.15 txn/s, latency: 2583.62 ms, (p50: 2600 ms, p70: 3000, p90: 3100 ms, p99: 4700 ms), latency samples: 198929
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 12789.42 txn/s, latency: 2492.00 ms, (p50: 2600 ms, p70: 2700, p90: 2800 ms, p99: 2900 ms), latency samples: 410520
5. check swarm health
Compatibility test for be3ccc58ef518259d63fab7f3e3613f2a6214268 ==> 8ed24d16563fd32776d2527750e0336625c8cade passed
Test Ok

@msmouse msmouse merged commit ee486a8 into main Nov 19, 2024
115 of 143 checks passed
@msmouse msmouse deleted the 1114-alden-parse-reconfig branch November 19, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zekun000 zekun000 zekun000 approved these changes

@areshand areshand areshand approved these changes

@lightmark lightmark Awaiting requested review from lightmark lightmark is a code owner

@grao1991 grao1991 Awaiting requested review from grao1991 grao1991 is a code owner

@davidiw davidiw Awaiting requested review from davidiw davidiw is a code owner

@wrwg wrwg Awaiting requested review from wrwg wrwg is a code owner

@vgao1996 vgao1996 Awaiting requested review from vgao1996 vgao1996 is a code owner

@georgemitenkov georgemitenkov Awaiting requested review from georgemitenkov georgemitenkov is a code owner

Assignees
No one assigned
Labels
CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR CICD:run-execution-performance-test Run execution performance test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@msmouse @areshand @zekun000