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

[Consensus Observer] Make subscription creation asynchronous. #14603

Merged
merged 4 commits into from
Sep 17, 2024
Merged

Conversation

JoshLind
Copy link
Contributor

@JoshLind JoshLind commented Sep 12, 2024

Note:

Description

This PR makes subscription creation asynchronous, instead of blocking the consensus observer thread. The PR offers the following commits:

  1. Make subscription creation asynchronous by spawning a dedicated thread. Most of this commit is just moving subscription logic into a new file so that the tasks can be spawned asynchronously.
  2. Move the subscription health check to subscription.rs and update the tests.
  3. Update the tests for the subscription manager.
  4. Update the tests for the subscription utilities file.

Testing Plan

New and existing test infrastructure.

Copy link

trunk-io bot commented Sep 12, 2024

⏱️ 16h 52m total CI duration on this PR
Job Cumulative Duration Recent Runs
execution-performance / single-node-performance 4h 22m 🟩🟩🟩🟩🟩 (+7 more)
forge-compat-test / forge 2h 53m 🟩🟩🟩🟩🟩 (+5 more)
forge-e2e-test / forge 2h 14m 🟩🟥🟩🟩🟩 (+5 more)
execution-performance / test-target-determinator 1h 6m 🟩🟩🟩🟩🟩 (+7 more)
test-target-determinator 1h 1m 🟩🟩🟩🟩🟩 (+6 more)
check 40m 🟩🟩🟩🟩 (+7 more)
forge-framework-upgrade-test / forge 36m 🟩🟩
general-lints 25m 🟩🟥🟥🟩🟩 (+7 more)
rust-cargo-deny 21m 🟩🟩🟩🟩🟩 (+7 more)
check-dynamic-deps 16m 🟩🟩🟩🟩🟩 (+8 more)
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose 15m 🟩🟥🟩🟩🟩 (+5 more)
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 8m 🟩
rust-doc-tests 8m 🟥🟩
rust-move-tests 6m
semgrep/ci 5m 🟩🟩🟩🟩🟩 (+8 more)
rust-doc-tests 5m 🟩
rust-doc-tests 5m 🟩
rust-doc-tests 5m 🟩
rust-doc-tests 5m 🟩
rust-doc-tests 5m 🟩
rust-doc-tests 5m 🟩
rust-doc-tests 5m 🟩
rust-doc-tests 4m 🟩
rust-doc-tests 4m 🟩
rust-doc-tests 4m 🟩
rust-move-tests 4m
file_change_determinator 2m 🟩🟩🟩🟩 (+8 more)
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+7 more)
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+6 more)
rust-move-tests 1m 🟩
permission-check 46s 🟩🟩🟩🟩🟩 (+7 more)
permission-check 44s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 39s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 39s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 36s 🟩🟩🟩🟩🟩 (+8 more)
determine-docker-build-metadata 21s 🟩🟩🟩🟩🟩 (+6 more)
Backport PR 8s 🟥🟥
permission-check 5s 🟩🟩
rust-move-tests 1s
rust-doc-tests 1s

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

Job Duration vs 7d avg Delta
forge-framework-upgrade-test / forge 17m 14m +24%
execution-performance / single-node-performance 10s 17m -99%

settingsfeedbackdocs ⋅ learn more about trunk.io

@JoshLind JoshLind 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 12, 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.

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.

@JoshLind JoshLind marked this pull request as ready for review September 13, 2024 21:35

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.

Base automatically changed from co_ref_9 to main September 17, 2024 19:35
@JoshLind JoshLind enabled auto-merge (rebase) September 17, 2024 19:36

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.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 9e835f70cc671b90ab3d03cdb9cb319f57912e04

two traffics test: inner traffic : committed: 13957.00 txn/s, submitted: 13958.19 txn/s, failed submission: 1.13 txn/s, expired: 1.18 txn/s, latency: 2848.80 ms, (p50: 2700 ms, p70: 2800, p90: 3000 ms, p99: 4000 ms), latency samples: 5306770
two traffics test : committed: 98.40 txn/s, submitted: 100.00 txn/s, failed submission: 1.60 txn/s, expired: 1.60 txn/s, latency: 2519.16 ms, (p50: 2400 ms, p70: 2500, p90: 2700 ms, p99: 9300 ms), latency samples: 1880
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.237, avg: 0.221", "QsPosToProposal: max: 0.310, avg: 0.278", "ConsensusProposalToOrdered: max: 0.328, avg: 0.298", "ConsensusOrderedToCommit: max: 0.432, avg: 0.421", "ConsensusProposalToCommit: max: 0.727, avg: 0.718"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.56s no progress at version 6007456 (avg 0.21s) [limit 15].
Max epoch-change gap was: 1 rounds at version 2846574 (avg 1.00) [limit 4], 10.20s no progress at version 2846574 (avg 10.20s) [limit 15].
Test Ok

Copy link
Contributor

✅ Forge suite framework_upgrade success on 70806ff543496aa6de7807feff49e7e1370efd20 ==> 9e835f70cc671b90ab3d03cdb9cb319f57912e04

Compatibility test results for 70806ff543496aa6de7807feff49e7e1370efd20 ==> 9e835f70cc671b90ab3d03cdb9cb319f57912e04 (PR)
Upgrade the nodes to version: 9e835f70cc671b90ab3d03cdb9cb319f57912e04
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1116.18 txn/s, submitted: 1117.68 txn/s, failed submission: 1.50 txn/s, expired: 1.50 txn/s, latency: 3054.24 ms, (p50: 2100 ms, p70: 3000, p90: 6300 ms, p99: 8400 ms), latency samples: 89320
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 925.05 txn/s, submitted: 926.38 txn/s, failed submission: 1.33 txn/s, expired: 1.33 txn/s, latency: 3303.46 ms, (p50: 2100 ms, p70: 3600, p90: 7200 ms, p99: 10400 ms), latency samples: 83360
5. check swarm health
Compatibility test for 70806ff543496aa6de7807feff49e7e1370efd20 ==> 9e835f70cc671b90ab3d03cdb9cb319f57912e04 passed
Upgrade the remaining nodes to version: 9e835f70cc671b90ab3d03cdb9cb319f57912e04
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 966.71 txn/s, submitted: 969.95 txn/s, failed submission: 2.85 txn/s, expired: 3.24 txn/s, latency: 3049.48 ms, (p50: 2100 ms, p70: 3300, p90: 6900 ms, p99: 8300 ms), latency samples: 88145
Test Ok

Copy link
Contributor

✅ Forge suite compat success on 70806ff543496aa6de7807feff49e7e1370efd20 ==> 9e835f70cc671b90ab3d03cdb9cb319f57912e04

Compatibility test results for 70806ff543496aa6de7807feff49e7e1370efd20 ==> 9e835f70cc671b90ab3d03cdb9cb319f57912e04 (PR)
1. Check liveness of validators at old version: 70806ff543496aa6de7807feff49e7e1370efd20
compatibility::simple-validator-upgrade::liveness-check : committed: 8246.30 txn/s, latency: 3418.38 ms, (p50: 2000 ms, p70: 2400, p90: 9300 ms, p99: 28600 ms), latency samples: 343600
2. Upgrading first Validator to new version: 9e835f70cc671b90ab3d03cdb9cb319f57912e04
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 5277.06 txn/s, latency: 5463.63 ms, (p50: 4500 ms, p70: 7300, p90: 7800 ms, p99: 8300 ms), latency samples: 96000
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 6952.40 txn/s, latency: 4580.60 ms, (p50: 4600 ms, p70: 4600, p90: 6700 ms, p99: 6900 ms), latency samples: 229920
3. Upgrading rest of first batch to new version: 9e835f70cc671b90ab3d03cdb9cb319f57912e04
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 7751.39 txn/s, latency: 3704.71 ms, (p50: 4200 ms, p70: 4300, p90: 4400 ms, p99: 4600 ms), latency samples: 143780
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 7762.61 txn/s, latency: 4113.34 ms, (p50: 4400 ms, p70: 4500, p90: 5100 ms, p99: 5400 ms), latency samples: 257280
4. upgrading second batch to new version: 9e835f70cc671b90ab3d03cdb9cb319f57912e04
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 2956.62 txn/s, latency: 9856.28 ms, (p50: 12000 ms, p70: 12800, p90: 13000 ms, p99: 13300 ms), latency samples: 56440
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 10664.61 txn/s, latency: 2931.76 ms, (p50: 2800 ms, p70: 2900, p90: 5000 ms, p99: 5800 ms), latency samples: 346000
5. check swarm health
Compatibility test for 70806ff543496aa6de7807feff49e7e1370efd20 ==> 9e835f70cc671b90ab3d03cdb9cb319f57912e04 passed
Test Ok

@JoshLind JoshLind disabled auto-merge September 17, 2024 21:16
@JoshLind JoshLind merged commit 4a87ad1 into main Sep 17, 2024
48 checks passed
@JoshLind JoshLind deleted the co_ref_10 branch September 17, 2024 21:16
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.

2 participants