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

Calculate cur_txns in pull proofs method accurately #13716

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

vusirikala
Copy link
Contributor

@vusirikala vusirikala commented Jun 15, 2024

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 Jun 15, 2024

⏱️ 8h 7m total CI duration on this PR
Job Cumulative Duration Recent Runs
rust-smoke-tests 1h 49m 🟩🟩🟩
forge-e2e-test / forge 53m 🟩🟩🟩
rust-images / rust-all 48m 🟩🟩🟩
forge-compat-test / forge 41m 🟩🟩🟩
rust-targeted-unit-tests 39m 🟩🟩🟩🟩
test-fuzzers 35m 🟩
rust-lints 22m 🟩🟩🟩🟩
run-tests-main-branch 22m 🟩🟩🟩🟩🟩
cli-e2e-tests / run-cli-tests 21m 🟩🟩🟩
rust-move-tests 20m 🟩🟩🟩🟩
rust-build-cached-packages 16m 🟩🟩🟩
check 12m 🟩🟩🟩
execution-performance / test-target-determinator 12m 🟩🟩🟩
test-target-determinator 12m 🟩🟩🟩
general-lints 7m 🟩🟩🟩🟩
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose 5m 🟩🟩🟩
check-dynamic-deps 5m 🟩🟩🟩🟩
node-api-compatibility-tests / node-api-compatibility-tests 3m 🟩🟩🟩
semgrep/ci 1m 🟩🟩🟩🟩
file_change_determinator 51s 🟩🟩🟩🟩
file_change_determinator 38s 🟩🟩🟩🟩
file_change_determinator 35s 🟩🟩🟩
execution-performance / single-node-performance 23s 🟩🟩🟩
permission-check 12s 🟩🟩🟩🟩
permission-check 11s 🟩🟩🟩🟩
permission-check 10s 🟩🟩🟩
permission-check 9s 🟩🟩🟩🟩
permission-check 9s 🟩🟩🟩🟩
determine-docker-build-metadata 8s 🟩🟩🟩

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

Job Duration vs 7d avg Delta
cli-e2e-tests / run-cli-tests 10m 6m +54%
execution-performance / test-target-determinator 3m 4m -26%
rust-targeted-unit-tests 8m 16m -48%
rust-move-tests 3m 9m -67%

settingsfeedbackdocs ⋅ learn more about trunk.io

@vusirikala vusirikala changed the title [Fix a bug in pull proofs method [Draft] Fix a bug in pull proofs method Jun 15, 2024
@vusirikala vusirikala requested review from sitalkedia and zekun000 and removed request for sasha8 and gelash June 15, 2024 00:07
@vusirikala vusirikala marked this pull request as draft June 15, 2024 00:07
@vusirikala vusirikala added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Jun 15, 2024
@vusirikala vusirikala marked this pull request as ready for review June 15, 2024 02:37

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@vusirikala vusirikala changed the title [Draft] Fix a bug in pull proofs method Fix a bug in pull proofs method Jun 17, 2024
@vusirikala vusirikala changed the title Fix a bug in pull proofs method [Draft] Fix a bug in pull proofs method Jun 17, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

@sitalkedia sitalkedia left a comment

Choose a reason for hiding this comment

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

LGTM

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@vusirikala vusirikala changed the title [Draft] Fix a bug in pull proofs method Fix a bug in pull proofs method Jun 18, 2024
Copy link
Contributor

@bchocho bchocho left a comment

Choose a reason for hiding this comment

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

LG. Maybe the title can be a bit more specific. It sounds a bit ominous :P

@vusirikala vusirikala changed the title Fix a bug in pull proofs method Calculate cur_txns in pull proofs method accurately Jun 25, 2024
@vusirikala vusirikala enabled auto-merge (squash) June 25, 2024 22:14

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite compat success on f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace

Compatibility test results for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace (PR)
1. Check liveness of validators at old version: f648076a280621dbfd4e73b1ca83e3a3f52878ed
compatibility::simple-validator-upgrade::liveness-check : committed: 4406.662169528157 txn/s, latency: 7378.2927319451765 ms, (p50: 6500 ms, p90: 13700 ms, p99: 18200 ms), latency samples: 151760
2. Upgrading first Validator to new version: 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 3347.208846159755 txn/s, latency: 9331.709208601522 ms, (p50: 9500 ms, p90: 14100 ms, p99: 14500 ms), latency samples: 136720
3. Upgrading rest of first batch to new version: 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3352.6625308320563 txn/s, latency: 9329.892754262199 ms, (p50: 9600 ms, p90: 13900 ms, p99: 14200 ms), latency samples: 136080
4. upgrading second batch to new version: 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6402.350581969856 txn/s, latency: 5099.910175857948 ms, (p50: 4800 ms, p90: 8200 ms, p99: 9300 ms), latency samples: 234280
5. check swarm health
Compatibility test for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace passed
Test Ok

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace

two traffics test: inner traffic : committed: 8679.29802158402 txn/s, latency: 4519.699831827792 ms, (p50: 4400 ms, p90: 5400 ms, p99: 9600 ms), latency samples: 3746160
two traffics test : committed: 100.08718982362151 txn/s, latency: 1976.6241758241758 ms, (p50: 1900 ms, p90: 2200 ms, p99: 3300 ms), latency samples: 1820
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.222, avg: 0.213", "QsPosToProposal: max: 0.256, avg: 0.241", "ConsensusProposalToOrdered: max: 0.308, avg: 0.290", "ConsensusOrderedToCommit: max: 0.357, avg: 0.349", "ConsensusProposalToCommit: max: 0.645, avg: 0.639"]
Max round gap was 1 [limit 4] at version 1671454. Max no progress secs was 4.95142 [limit 15] at version 1671454.
Test Ok

Copy link
Contributor

✅ Forge suite framework_upgrade success on f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace

Compatibility test results for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace (PR)
Upgrade the nodes to version: 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1134.8322354822274 txn/s, submitted: 1137.2486417134128 txn/s, failed submission: 2.4164062311855403 txn/s, expired: 2.4164062311855403 txn/s, latency: 2560.8125145180024 ms, (p50: 1800 ms, p90: 4800 ms, p99: 10200 ms), latency samples: 103320
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1119.2112544884142 txn/s, submitted: 1120.6336792981856 txn/s, failed submission: 1.4224248097713377 txn/s, expired: 1.4224248097713377 txn/s, latency: 2828.6446727388266 ms, (p50: 2100 ms, p90: 5100 ms, p99: 11100 ms), latency samples: 94420
5. check swarm health
Compatibility test for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace passed
Upgrade the remaining nodes to version: 6c4e588d9a835ba7ac1b5d1183ba88150a0b6ace
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1039.2457035945629 txn/s, submitted: 1041.4840100851823 txn/s, failed submission: 2.238306490619347 txn/s, expired: 2.238306490619347 txn/s, latency: 2931.2304329097565 ms, (p50: 2100 ms, p90: 5400 ms, p99: 9300 ms), latency samples: 92860
Test Ok

@vusirikala vusirikala merged commit eef1404 into main Jun 25, 2024
89 checks passed
@vusirikala vusirikala deleted the satya/pull_proof_bug branch June 25, 2024 22:50
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.

3 participants