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.

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

Restructure executor crate #15008

Merged
merged 3 commits into from
Oct 22, 2024
Merged

Restructure executor crate #15008

merged 3 commits into from
Oct 22, 2024

Conversation

msmouse
Copy link
Contributor

@msmouse msmouse commented Oct 18, 2024

Description

For the three stages of execution,

  1. the outputs ExecutionOutput, StateCheckpointOutput, LedgerUpdateOutput live in executor-types, and them combined will be the final StateComputeResult
  2. the algorithms to make them, i.e. the actual logic of the three execution stages, namely DoGetExecutionOutput, DoStateCheckpoint, and DoLedgerUpdate live under executor/src/workflow.

How Has This Been Tested?

existing coverage

Key Areas to Review

Type of Change

  • Refactoring

Which Components or Systems Does This Change Impact?

  • Validator Node

Copy link

trunk-io bot commented Oct 18, 2024

⏱️ 3h 13m total CI duration on this PR
Slowest 15 Jobs Cumulative Duration Recent Runs
execution-performance / single-node-performance 1h 6m 🟩🟥🟩
rust-cargo-deny 14m 🟩🟩🟩🟩🟩 (+3 more)
forge-e2e-test / forge 14m 🟩
test-target-determinator 14m 🟩🟩🟩
execution-performance / test-target-determinator 14m 🟩🟩🟩
check 11m 🟩🟩🟩
rust-move-tests 10m 🟩
check-dynamic-deps 9m 🟩🟩🟩🟩🟩 (+3 more)
rust-doc-tests 5m 🟩
rust-doc-tests 5m 🟩
fetch-last-released-docker-image-tag 5m 🟩🟩🟩
general-lints 4m 🟩🟩🟩🟩🟩 (+3 more)
semgrep/ci 3m 🟩🟩🟩🟩🟩 (+3 more)
rust-doc-tests 3m 🟥
rust-move-tests 2m

settingsfeedbackdocs ⋅ learn more about trunk.io

@msmouse msmouse force-pushed the 1018-alden-immutable-outputs branch from 2c62bf0 to f650429 Compare October 18, 2024 18:58
Base automatically changed from 1017-alden-remove-txn-to-commit to main October 18, 2024 20:16
@msmouse msmouse force-pushed the 1018-alden-immutable-outputs branch 2 times, most recently from 22028c3 to 2a9ceed Compare October 18, 2024 21:50
@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 Oct 18, 2024
@msmouse msmouse changed the title extract ExecutionOutput, DoGetExecutionOutput Restructure executor crate Oct 18, 2024
@msmouse msmouse marked this pull request as ready for review October 18, 2024 23:41
@msmouse msmouse requested a review from zekun000 October 18, 2024 23:41

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.

@msmouse msmouse enabled auto-merge (rebase) October 21, 2024 20:39

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.

1. put ExecutionOutput under executor-types; next is to make
   StateComputeResult a combination of ExecutionOutput + StateCheckpointOutput
   + LedgerUpdateOutput
2. leave the algorithm in the executor crate, creating
   DoGetExecutionOutput
@msmouse msmouse force-pushed the 1018-alden-immutable-outputs branch from 2a9ceed to 0824245 Compare October 22, 2024 00:25

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 082424541082180be99e4b9a7553d867db7948d0

two traffics test: inner traffic : committed: 14218.64 txn/s, latency: 2793.35 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 3300 ms), latency samples: 5406280
two traffics test : committed: 100.07 txn/s, latency: 1620.69 ms, (p50: 1400 ms, p70: 1500, p90: 1600 ms, p99: 8200 ms), latency samples: 1800
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.243, avg: 0.224", "QsPosToProposal: max: 1.191, avg: 1.166", "ConsensusProposalToOrdered: max: 0.326, avg: 0.300", "ConsensusOrderedToCommit: max: 0.406, avg: 0.393", "ConsensusProposalToCommit: max: 0.706, avg: 0.692"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.94s no progress at version 2277441 (avg 0.21s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.64s no progress at version 2277439 (avg 8.64s) [limit 15].
Test Ok

Copy link
Contributor

✅ Forge suite framework_upgrade success on b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 082424541082180be99e4b9a7553d867db7948d0

Compatibility test results for b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 082424541082180be99e4b9a7553d867db7948d0 (PR)
Upgrade the nodes to version: 082424541082180be99e4b9a7553d867db7948d0
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1215.70 txn/s, submitted: 1218.12 txn/s, failed submission: 2.42 txn/s, expired: 2.42 txn/s, latency: 2547.35 ms, (p50: 2400 ms, p70: 2700, p90: 3900 ms, p99: 5100 ms), latency samples: 110560
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1128.30 txn/s, submitted: 1130.06 txn/s, failed submission: 1.76 txn/s, expired: 1.76 txn/s, latency: 2626.74 ms, (p50: 2400 ms, p70: 2700, p90: 3800 ms, p99: 5700 ms), latency samples: 102400
5. check swarm health
Compatibility test for b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 082424541082180be99e4b9a7553d867db7948d0 passed
Upgrade the remaining nodes to version: 082424541082180be99e4b9a7553d867db7948d0
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1186.42 txn/s, submitted: 1190.51 txn/s, failed submission: 4.08 txn/s, expired: 4.08 txn/s, latency: 2664.67 ms, (p50: 2400 ms, p70: 3000, p90: 3900 ms, p99: 5700 ms), latency samples: 104620
Test Ok

Copy link
Contributor

✅ Forge suite compat success on b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 082424541082180be99e4b9a7553d867db7948d0

Compatibility test results for b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 082424541082180be99e4b9a7553d867db7948d0 (PR)
1. Check liveness of validators at old version: b29f09f57e898d8d211c8bc3e303f6e50bba2266
compatibility::simple-validator-upgrade::liveness-check : committed: 15579.44 txn/s, latency: 2166.60 ms, (p50: 2000 ms, p70: 2100, p90: 2500 ms, p99: 5300 ms), latency samples: 503660
2. Upgrading first Validator to new version: 082424541082180be99e4b9a7553d867db7948d0
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 6987.52 txn/s, latency: 4054.95 ms, (p50: 4700 ms, p70: 4800, p90: 4900 ms, p99: 5100 ms), latency samples: 128980
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 6413.99 txn/s, latency: 5023.42 ms, (p50: 5100 ms, p70: 5200, p90: 6800 ms, p99: 7200 ms), latency samples: 218360
3. Upgrading rest of first batch to new version: 082424541082180be99e4b9a7553d867db7948d0
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 6389.45 txn/s, latency: 4473.44 ms, (p50: 5100 ms, p70: 5300, p90: 5500 ms, p99: 5600 ms), latency samples: 114260
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6787.13 txn/s, latency: 4748.02 ms, (p50: 5300 ms, p70: 5400, p90: 5500 ms, p99: 5600 ms), latency samples: 231240
4. upgrading second batch to new version: 082424541082180be99e4b9a7553d867db7948d0
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 8861.71 txn/s, latency: 3073.50 ms, (p50: 3100 ms, p70: 3600, p90: 4700 ms, p99: 5100 ms), latency samples: 158020
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 8394.97 txn/s, latency: 3791.91 ms, (p50: 3600 ms, p70: 4500, p90: 5400 ms, p99: 5800 ms), latency samples: 278740
5. check swarm health
Compatibility test for b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 082424541082180be99e4b9a7553d867db7948d0 passed
Test Ok

@msmouse msmouse merged commit fbef64e into main Oct 22, 2024
46 checks passed
@msmouse msmouse deleted the 1018-alden-immutable-outputs branch October 22, 2024 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants