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

[cherry-pick][1.16][consensus] fix edge case of block retrieval #13903

Merged
merged 2 commits into from
Jul 9, 2024

Conversation

ibalajiarun
Copy link
Contributor

Description

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Other (specify)

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

Copy link

trunk-io bot commented Jul 2, 2024

⏱️ 8h 2m total CI duration on this PR
Job Cumulative Duration Recent Runs
test-fuzzers 2h 25m 🟩🟩🟩🟩
rust-smoke-tests 1h 5m 🟩🟩
rust-targeted-unit-tests 36m 🟥🟥🟥🟩
execution-performance / single-node-performance 32m 🟩
rust-move-tests 30m 🟥🟥🟩
forge-e2e-test / forge 29m 🟩🟩
forge-compat-test / forge 28m 🟩🟩
rust-images / rust-all 24m 🟩🟩
rust-lints 17m 🟩🟩🟩
forge-framework-upgrade-test / forge 16m 🟩
test-target-determinator 10m 🟥🟩
check 10m 🟩🟩
execution-performance / test-target-determinator 8m 🟥🟩
rust-build-cached-packages 8m 🟩🟩
general-lints 7m 🟩🟩🟩🟩
run-tests-main-branch 4m 🟥🟥🟥
rust-move-tests 4m 🟩
check-dynamic-deps 4m 🟩🟩🟩🟩
node-api-compatibility-tests / node-api-compatibility-tests 2m 🟩🟩
semgrep/ci 2m 🟩🟩🟩🟩
file_change_determinator 39s 🟩🟩🟩🟩
file_change_determinator 38s 🟩🟩🟩🟩
file_change_determinator 20s 🟩🟩
permission-check 16s 🟩🟩🟩🟩
permission-check 12s 🟩🟩🟩🟩
permission-check 11s 🟩🟩🟩🟩
permission-check 9s 🟩🟩🟩🟩
determine-docker-build-metadata 9s 🟩🟩
permission-check 6s 🟩🟩

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

Job Duration vs 7d avg Delta
execution-performance / single-node-performance 32m 20m +59%
check 6m 4m +56%
test-target-determinator 7m 5m +42%
execution-performance / test-target-determinator 6m 5m +25%

settingsfeedbackdocs ⋅ learn more about trunk.io

@ibalajiarun ibalajiarun changed the base branch from main to aptos-release-v1.16 July 2, 2024 22:53
Copy link
Contributor

@sherry-x sherry-x left a comment

Choose a reason for hiding this comment

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

how this is being tested? any test case we can add to prevent this?

@ibalajiarun ibalajiarun requested a review from sherry-x July 9, 2024 01:03
@sherry-x sherry-x enabled auto-merge (squash) July 9, 2024 11:24

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@ibalajiarun ibalajiarun force-pushed the balaji/cp-zekun-fix branch from a36a00a to d225f1e Compare July 9, 2024 14:59

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Jul 9, 2024

✅ Forge suite compat success on f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> d225f1e6a377caffde4adb93bb177b1e466752b8

Compatibility test results for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> d225f1e6a377caffde4adb93bb177b1e466752b8 (PR)
1. Check liveness of validators at old version: f648076a280621dbfd4e73b1ca83e3a3f52878ed
compatibility::simple-validator-upgrade::liveness-check : committed: 8835.109911902455 txn/s, latency: 3718.9498932729625 ms, (p50: 2700 ms, p90: 6300 ms, p99: 25400 ms), latency samples: 309200
2. Upgrading first Validator to new version: d225f1e6a377caffde4adb93bb177b1e466752b8
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 2757.029055688565 txn/s, latency: 9629.620833333332 ms, (p50: 12300 ms, p90: 14400 ms, p99: 14800 ms), latency samples: 69600
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 3252.412324507118 txn/s, latency: 9544.818997246775 ms, (p50: 9600 ms, p90: 14700 ms, p99: 15100 ms), latency samples: 138020
3. Upgrading rest of first batch to new version: d225f1e6a377caffde4adb93bb177b1e466752b8
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 2901.2451714176636 txn/s, latency: 8866.505173913043 ms, (p50: 11300 ms, p90: 13000 ms, p99: 13500 ms), latency samples: 69000
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3201.1212286367813 txn/s, latency: 9725.523693654895 ms, (p50: 9600 ms, p90: 14800 ms, p99: 15300 ms), latency samples: 139320
4. upgrading second batch to new version: d225f1e6a377caffde4adb93bb177b1e466752b8
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 6330.451822435189 txn/s, latency: 4396.796945912341 ms, (p50: 4900 ms, p90: 5400 ms, p99: 5800 ms), latency samples: 128680
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6234.048447505275 txn/s, latency: 5241.133858125227 ms, (p50: 5300 ms, p90: 6900 ms, p99: 8700 ms), latency samples: 221040
5. check swarm health
Compatibility test for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> d225f1e6a377caffde4adb93bb177b1e466752b8 passed
Test Ok

Copy link
Contributor

github-actions bot commented Jul 9, 2024

✅ Forge suite realistic_env_max_load success on d225f1e6a377caffde4adb93bb177b1e466752b8

two traffics test: inner traffic : committed: 8729.615684711716 txn/s, latency: 4490.759516208577 ms, (p50: 4500 ms, p90: 5200 ms, p99: 9000 ms), latency samples: 3771460
two traffics test : committed: 99.87758844703197 txn/s, latency: 2027.8295454545455 ms, (p50: 2000 ms, p90: 2300 ms, p99: 3400 ms), latency samples: 1760
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.216, avg: 0.213", "QsPosToProposal: max: 0.249, avg: 0.234", "ConsensusProposalToOrdered: max: 0.308, avg: 0.288", "ConsensusOrderedToCommit: max: 0.370, avg: 0.358", "ConsensusProposalToCommit: max: 0.657, avg: 0.647"]
Max round gap was 1 [limit 4] at version 1319443. Max no progress secs was 4.933145 [limit 15] at version 1319443.
Test Ok

Copy link
Contributor

github-actions bot commented Jul 9, 2024

✅ Forge suite framework_upgrade success on f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> d225f1e6a377caffde4adb93bb177b1e466752b8

Compatibility test results for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> d225f1e6a377caffde4adb93bb177b1e466752b8 (PR)
Upgrade the nodes to version: d225f1e6a377caffde4adb93bb177b1e466752b8
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1075.0624529887054 txn/s, submitted: 1077.4689826962504 txn/s, failed submission: 2.406529707544925 txn/s, expired: 2.406529707544925 txn/s, latency: 2919.776566951567 ms, (p50: 2100 ms, p90: 5100 ms, p99: 9900 ms), latency samples: 98280
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1143.6132484327625 txn/s, submitted: 1145.9362491405275 txn/s, failed submission: 2.3230007077651074 txn/s, expired: 2.3230007077651074 txn/s, latency: 2693.8812715823683 ms, (p50: 1800 ms, p90: 4700 ms, p99: 10800 ms), latency samples: 98460
5. check swarm health
Compatibility test for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> d225f1e6a377caffde4adb93bb177b1e466752b8 passed
Upgrade the remaining nodes to version: d225f1e6a377caffde4adb93bb177b1e466752b8
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1033.5985424725877 txn/s, submitted: 1035.1445231985083 txn/s, failed submission: 1.5459807259205374 txn/s, expired: 1.5459807259205374 txn/s, latency: 3065.810138888889 ms, (p50: 2400 ms, p90: 5400 ms, p99: 11400 ms), latency samples: 93600
Test Ok

@sherry-x sherry-x merged commit 85ff36f into aptos-release-v1.16 Jul 9, 2024
46 checks passed
@sherry-x sherry-x deleted the balaji/cp-zekun-fix branch July 9, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants