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

schemadb: make iterator lazy #13341

Merged
merged 1 commit into from
May 21, 2024
Merged

schemadb: make iterator lazy #13341

merged 1 commit into from
May 21, 2024

Conversation

msmouse
Copy link
Contributor

@msmouse msmouse commented May 20, 2024

Description

Meaning, do not advance underlying raw iter early
Sometimes the second advance on the state value CF is exetremely costly
due to deletions by the pruner.

Fixed issue where point-getting a state value can be extremely slow if the value gets historically updated a lot and stayed untouched within the ledger pruning window.

Type of Change

  • Refactoring

Which Components or Systems Does This Change Impact?

  • Validator Node

How Has This Been Tested?

Key Areas to Review

unit tests

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 May 20, 2024

⏱️ 30h 40m total CI duration on this PR
Job Cumulative Duration Recent Runs
windows-build 5h 39m 🟩🟩🟩🟩🟩 (+4 more)
execution-performance / single-node-performance 4h 33m 🟩🟩🟩🟩🟩 (+7 more)
rust-smoke-tests 4h 1m 🟩🟩🟩 (+6 more)
rust-targeted-unit-tests 2h 9m 🟩🟩🟩 (+8 more)
rust-images / rust-all 1h 49m 🟩🟩🟩 (+6 more)
forge-e2e-test / forge 1h 41m 🟩🟩🟩🟩 (+2 more)
forge-compat-test / forge 1h 30m 🟩🟩🟥🟩 (+2 more)
cli-e2e-tests / run-cli-tests 1h 30m 🟥🟥🟥🟥🟥 (+4 more)
forge-framework-upgrade-test / forge 1h 23m 🟥🟥🟥🟥🟩 (+2 more)
rust-lints 1h 8m 🟥🟥🟩🟩🟩 (+8 more)
run-tests-main-branch 51m 🟩🟩🟩🟩🟩 (+8 more)
rust-build-cached-packages 44m 🟩🟩🟩🟩🟩 (+6 more)
check 39m 🟩🟩🟩🟩🟩 (+6 more)
execution-performance / test-target-determinator 38m 🟩🟩🟩🟩🟩 (+6 more)
rust-move-tests 38m 🟩🟩🟩🟩🟩 (+8 more)
test-target-determinator 32m 🟩🟩🟩🟩🟩 (+6 more)
general-lints 23m 🟩🟩🟩🟩🟩 (+8 more)
check-dynamic-deps 20m 🟩🟩🟩🟩🟩 (+8 more)
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose 12m 🟩🟩🟩🟩🟩 (+2 more)
node-api-compatibility-tests / node-api-compatibility-tests 6m 🟩🟩🟩🟩🟩 (+2 more)
semgrep/ci 5m 🟩🟩🟩🟩🟩 (+8 more)
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+8 more)
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+8 more)
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+6 more)
permission-check 42s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 39s 🟩🟩🟩🟩🟩 (+6 more)
permission-check 38s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 31s 🟩🟩🟩🟩🟩 (+8 more)
determine-docker-build-metadata 30s 🟩🟩🟩🟩🟩 (+6 more)
permission-check 29s 🟩🟩🟩🟩🟩 (+8 more)

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

Job Duration vs 7d avg Delta
cli-e2e-tests / run-cli-tests 11m 7m +63%
rust-build-cached-packages 4m 5m -28%
rust-targeted-unit-tests 11m 19m -40%
forge-framework-upgrade-test / forge 17m 46m -63%
rust-move-tests 3m 9m -68%

settingsfeedbackdocs ⋅ learn more about trunk.io

@msmouse
Copy link
Contributor Author

msmouse commented May 20, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @msmouse and the rest of your teammates on Graphite Graphite

@msmouse msmouse mentioned this pull request May 20, 2024
8 tasks
@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 May 20, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@msmouse msmouse marked this pull request as ready for review May 20, 2024 04:57
@msmouse msmouse enabled auto-merge (rebase) May 20, 2024 15:49

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.

sherry-x pushed a commit that referenced this pull request May 20, 2024
* schemadb: make iterator lazy

Meaning, do not advance underlying raw iter early
Sometimes the second advance on the state value CF is exetremely costly
due to deletions by the pruner.

* [consensus] add missing check for commit cert

This comment has been minimized.

This comment has been minimized.

Meaning, do not advance underlying raw iter early
Sometimes the second advance on the state value CF is exetremely costly
due to deletions by the pruner.

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 compat success on 3ffe0986b5fe4acb76544ae7ae85d73b91a6a411 ==> a987f6788d092fa4cd0af1a110eef62e2d2b2a3f

Compatibility test results for 3ffe0986b5fe4acb76544ae7ae85d73b91a6a411 ==> a987f6788d092fa4cd0af1a110eef62e2d2b2a3f (PR)
1. Check liveness of validators at old version: 3ffe0986b5fe4acb76544ae7ae85d73b91a6a411
compatibility::simple-validator-upgrade::liveness-check : committed: 6928.261661635627 txn/s, latency: 4764.2294796975175 ms, (p50: 4900 ms, p90: 7200 ms, p99: 7800 ms), latency samples: 243320
2. Upgrading first Validator to new version: a987f6788d092fa4cd0af1a110eef62e2d2b2a3f
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1851.6922166756776 txn/s, latency: 15379.336228260869 ms, (p50: 19200 ms, p90: 21700 ms, p99: 22200 ms), latency samples: 92000
3. Upgrading rest of first batch to new version: a987f6788d092fa4cd0af1a110eef62e2d2b2a3f
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1849.5873711785791 txn/s, latency: 15455.60088005215 ms, (p50: 18800 ms, p90: 21700 ms, p99: 22300 ms), latency samples: 92040
4. upgrading second batch to new version: a987f6788d092fa4cd0af1a110eef62e2d2b2a3f
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 3586.9671914520177 txn/s, latency: 8737.105650899957 ms, (p50: 9700 ms, p90: 12400 ms, p99: 12700 ms), latency samples: 143340
5. check swarm health
Compatibility test for 3ffe0986b5fe4acb76544ae7ae85d73b91a6a411 ==> a987f6788d092fa4cd0af1a110eef62e2d2b2a3f passed
Test Ok

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on a987f6788d092fa4cd0af1a110eef62e2d2b2a3f

two traffics test: inner traffic : committed: 8170.874891838754 txn/s, latency: 4788.994514095942 ms, (p50: 4500 ms, p90: 5600 ms, p99: 10100 ms), latency samples: 3535060
two traffics test : committed: 100.03860500576802 txn/s, latency: 1848.5388888888888 ms, (p50: 1800 ms, p90: 2000 ms, p99: 2200 ms), latency samples: 1800
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.209, avg: 0.203", "QsPosToProposal: max: 0.255, avg: 0.209", "ConsensusProposalToOrdered: max: 0.483, avg: 0.440", "ConsensusOrderedToCommit: max: 0.368, avg: 0.345", "ConsensusProposalToCommit: max: 0.800, avg: 0.785"]
Max round gap was 1 [limit 4] at version 1658505. Max no progress secs was 5.12654 [limit 15] at version 1658505.
Test Ok

Copy link
Contributor

✅ Forge suite framework_upgrade success on 3ffe0986b5fe4acb76544ae7ae85d73b91a6a411 ==> a987f6788d092fa4cd0af1a110eef62e2d2b2a3f

Compatibility test results for 3ffe0986b5fe4acb76544ae7ae85d73b91a6a411 ==> a987f6788d092fa4cd0af1a110eef62e2d2b2a3f (PR)
Upgrade the nodes to version: a987f6788d092fa4cd0af1a110eef62e2d2b2a3f
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1063.9859318811148 txn/s, submitted: 1066.6260954837974 txn/s, failed submission: 2.6401636026826667 txn/s, expired: 2.6401636026826667 txn/s, latency: 2835.2414495450785 ms, (p50: 2200 ms, p90: 5200 ms, p99: 8400 ms), latency samples: 96720
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1269.1072144049747 txn/s, submitted: 1270.4278254501412 txn/s, failed submission: 1.3206110451664668 txn/s, expired: 1.3206110451664668 txn/s, latency: 2824.829843912591 ms, (p50: 2100 ms, p90: 5100 ms, p99: 7800 ms), latency samples: 96100
5. check swarm health
Compatibility test for 3ffe0986b5fe4acb76544ae7ae85d73b91a6a411 ==> a987f6788d092fa4cd0af1a110eef62e2d2b2a3f passed
Upgrade the remaining nodes to version: a987f6788d092fa4cd0af1a110eef62e2d2b2a3f
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1017.3796034698462 txn/s, submitted: 1018.7538217545871 txn/s, failed submission: 1.3742182847409 txn/s, expired: 1.3742182847409 txn/s, latency: 3055.1481764970736 ms, (p50: 2400 ms, p90: 5300 ms, p99: 9500 ms), latency samples: 88840
Test Ok

@msmouse msmouse merged commit 4bdd7f1 into main May 21, 2024
50 of 51 checks passed
@msmouse msmouse deleted the 0519-alden-lazy-iter branch May 21, 2024 23:45
heliuchuan pushed a commit that referenced this pull request Jun 5, 2024
* schemadb: make iterator lazy

Meaning, do not advance underlying raw iter early
Sometimes the second advance on the state value CF is exetremely costly
due to deletions by the pruner.

* [consensus] add missing check for commit cert
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.

None yet

3 participants