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

DiskSpacePricing #11262

Merged
merged 1 commit into from
Dec 13, 2023
Merged

DiskSpacePricing #11262

merged 1 commit into from
Dec 13, 2023

Conversation

msmouse
Copy link
Contributor

@msmouse msmouse commented Dec 8, 2023

Description

Refactor and consolidate storage fee charging logic to DiskSpacePricing. Prepare to inroduce different charging scheme.

Test Plan

existing coverage

Copy link

trunk-io bot commented Dec 8, 2023

⏱️ 67h 7m total CI duration on this PR
Job Cumulative Duration Recent Runs
execution-performance / single-node-performance 9h 49m 🟩🟩🟩🟩🟩 (+30 more)
windows-build 9h 31m 🟩🟩🟩🟩🟩 (+31 more)
rust-unit-tests 8h 🟥🟩 (+16 more)
rust-smoke-tests 6h 24m 🟩🟩 (+15 more)
forge-framework-upgrade-test / forge 5h 57m 🟩🟥 (+9 more)
forge-compat-test / forge 3h 49m 🟩🟩🟩 (+9 more)
rust-unit-coverage 3h 46m 🟩
rust-smoke-coverage 3h 5m 🟩
rust-images / rust-all 2h 55m 🟩🟩🟩 (+17 more)
forge-e2e-test / forge 2h 54m 🟥🟩🟩 (+9 more)
rust-lints 2h 18m 🟩🟩🟩🟩 (+16 more)
cli-e2e-tests / run-cli-tests 1h 58m 🟩🟩🟩 (+9 more)
run-tests-main-branch 1h 33m 🟩🟩🟩🟩 (+16 more)
check 1h 17m 🟩🟩🟩 (+24 more)
check-dynamic-deps 1h 17m 🟩🟩🟩🟩🟩 (+31 more)
general-lints 46m 🟩🟩🟩🟩 (+16 more)
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose 22m 🟩🟩🟩🟥 (+9 more)
rust-move-unit-coverage 18m 🟩
semgrep/ci 12m 🟩🟩🟩🟩🟩 (+31 more)
node-api-compatibility-tests / node-api-compatibility-tests 11m 🟩🟩🟩🟩 (+9 more)
execution-performance / file_change_determinator 7m 🟩🟩🟩🟩🟩 (+30 more)
docs-lint 6m 🟩🟩🟩🟩🟩 (+4 more)
execution-performance / sequential-execution-performance 4m 🟩🟩🟩🟩🟩 (+30 more)
file_change_determinator 4m 🟩🟩🟩🟩🟩 (+17 more)
execution-performance / parallel-execution-performance 4m 🟩🟩🟩🟩🟩 (+30 more)
file_change_determinator 4m 🟩🟩🟩🟩🟩 (+16 more)
file_change_determinator 4m 🟩🟩🟩🟩 (+21 more)
permission-check 3m 🟩🟩🟩🟩🟩 (+31 more)
permission-check 2m 🟩🟩🟩🟩🟩 (+31 more)
permission-check 1m 🟩🟩🟩🟩 (+25 more)
determine-docker-build-metadata 1m 🟩🟩🟩🟩🟩 (+17 more)
permission-check 1m 🟩🟩🟩🟩🟩 (+20 more)
permission-check 56s 🟩🟩🟩🟩 (+23 more)
upload-to-codecov 12s 🟩

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

Job Duration vs 7d avg Delta
forge-framework-upgrade-test / forge 44m 27m +67%
windows-build 11m 18m -38%

settingsfeedbackdocs ⋅ learn more about trunk.io

@msmouse
Copy link
Contributor Author

msmouse commented Dec 8, 2023

@msmouse msmouse marked this pull request as draft December 8, 2023 22:18
@msmouse msmouse requested review from igor-aptos, vgao1996 and a team December 8, 2023 22:19
@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 Dec 8, 2023
@msmouse msmouse force-pushed the 1206_consolidate_to_pricing branch 3 times, most recently from b3a2748 to faa9598 Compare December 8, 2023 23:27

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 force-pushed the 1206_consolidate_to_pricing branch 2 times, most recently from d34fb10 to 53d0a26 Compare December 9, 2023 00:43

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

codecov bot commented Dec 9, 2023

Codecov Report

Attention: 43 lines in your changes are missing coverage. Please review.

Comparison is base (ae06852) 68.7% compared to head (e2a17a7) 68.9%.
Report is 7 commits behind head on main.

❗ Current head e2a17a7 differs from pull request most recent head 94687c7. Consider uploading reports for the commit 94687c7 to get more accurate results

Files Patch % Lines
...e/aptos-vm-types/src/storage/change_set_configs.rs 86.8% 12 Missing ⚠️
aptos-move/aptos-vm-types/src/storage/mod.rs 57.1% 12 Missing ⚠️
...ptos-move/aptos-vm-types/src/storage/io_pricing.rs 71.4% 8 Missing ⚠️
aptos-move/aptos-abstract-gas-usage/src/algebra.rs 0.0% 5 Missing ⚠️
...aptos-gas-schedule/src/gas_schedule/transaction.rs 0.0% 3 Missing ⚠️
aptos-move/aptos-vm/src/aptos_vm_impl.rs 0.0% 2 Missing ⚠️
aptos-move/aptos-gas-profiling/src/profiler.rs 92.8% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main   #11262     +/-   ##
=========================================
+ Coverage    68.7%    68.9%   +0.1%     
=========================================
  Files         763      762      -1     
  Lines      176895   175963    -932     
=========================================
- Hits       121669   121325    -344     
+ Misses      55226    54638    -588     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

@msmouse msmouse changed the base branch from 1211-alden-io-gas to main December 13, 2023 00:02
@msmouse msmouse enabled auto-merge (rebase) December 13, 2023 00:03

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite compat success on aptos-node-v1.7.3 ==> 94687c77107d2fe564c80533fd9a9df9ff95c7f3

Compatibility test results for aptos-node-v1.7.3 ==> 94687c77107d2fe564c80533fd9a9df9ff95c7f3 (PR)
1. Check liveness of validators at old version: aptos-node-v1.7.3
compatibility::simple-validator-upgrade::liveness-check : committed: 5025 txn/s, latency: 6416 ms, (p50: 6900 ms, p90: 9200 ms, p99: 9600 ms), latency samples: 185960
2. Upgrading first Validator to new version: 94687c77107d2fe564c80533fd9a9df9ff95c7f3
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1781 txn/s, latency: 16015 ms, (p50: 19300 ms, p90: 22000 ms, p99: 22500 ms), latency samples: 92640
3. Upgrading rest of first batch to new version: 94687c77107d2fe564c80533fd9a9df9ff95c7f3
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1473 txn/s, latency: 18165 ms, (p50: 18500 ms, p90: 24200 ms, p99: 25200 ms), latency samples: 73680
4. upgrading second batch to new version: 94687c77107d2fe564c80533fd9a9df9ff95c7f3
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 3174 txn/s, latency: 9870 ms, (p50: 9600 ms, p90: 13300 ms, p99: 24800 ms), latency samples: 123820
5. check swarm health
Compatibility test for aptos-node-v1.7.3 ==> 94687c77107d2fe564c80533fd9a9df9ff95c7f3 passed
Test Ok

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 94687c77107d2fe564c80533fd9a9df9ff95c7f3

two traffics test: inner traffic : committed: 7760 txn/s, latency: 5051 ms, (p50: 4800 ms, p90: 6000 ms, p99: 13200 ms), latency samples: 3344580
two traffics test : committed: 100 txn/s, latency: 2669 ms, (p50: 2200 ms, p90: 3100 ms, p99: 12000 ms), latency samples: 1760
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.231, avg: 0.204", "QsPosToProposal: max: 0.165, avg: 0.156", "ConsensusProposalToOrdered: max: 0.613, avg: 0.568", "ConsensusOrderedToCommit: max: 0.554, avg: 0.500", "ConsensusProposalToCommit: max: 1.119, avg: 1.068"]
Max round gap was 1 [limit 4] at version 1415460. Max no progress secs was 9.068394 [limit 10] at version 1415460.
Test Ok

@msmouse msmouse merged commit 74dade4 into main Dec 13, 2023
70 of 87 checks passed
@msmouse msmouse deleted the 1206_consolidate_to_pricing branch December 13, 2023 00:35
Copy link
Contributor

❌ Forge suite framework_upgrade failure on aptos-node-v1.7.3 ==> 94687c77107d2fe564c80533fd9a9df9ff95c7f3

Compatibility test results for aptos-node-v1.7.3 ==> 94687c77107d2fe564c80533fd9a9df9ff95c7f3 (PR)
Upgrade the nodes to version: 94687c77107d2fe564c80533fd9a9df9ff95c7f3
Test Failed: API error: Unknown error error sending request for url (http://aptos-node-3-validator.forge-framework-upgrade-pr-11262.svc:8080/v1/estimate_gas_price): error trying to connect: dns error: failed to lookup address information: Name or service not known

Stack backtrace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: __libc_start_main
  13: <unknown>
Trailing Log Lines:
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: __libc_start_main
  13: <unknown>


Swarm logs can be found here: See fgi output for more information.
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ApiError: namespaces "forge-framework-upgrade-pr-11262" not found: NotFound (ErrorResponse { status: "Failure", message: "namespaces \"forge-framework-upgrade-pr-11262\" not found", reason: "NotFound", code: 404 })

Caused by:
    namespaces "forge-framework-upgrade-pr-11262" not found: NotFound

Stack backtrace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: <unknown>
  15: __libc_start_main
  16: <unknown>', testsuite/forge/src/backend/k8s/swarm.rs:676:18
stack backtrace:
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Debugging output:

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