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.

Sign up for GitHub

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

Jump to bottom

Track error logs lines in a counter and assert as success criteria #12689

Merged
merged 1 commit into from Mar 28, 2024
Merged

Track error logs lines in a counter and assert as success criteria #12689

merged 1 commit into from Mar 28, 2024

Conversation

igor-aptos
Copy link
Contributor

Description

Type of Change

  • New feature

Which Components or Systems Does This Change Impact?

  • Validator Node

How Has This Been Tested?

tests on CI

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

@igor-aptos igor-aptos added CICD:run-execution-performance-test Run execution performance test CICD:run-execution-performance-full-test Run execution performance test (full version) CICD:run-forge-e2e-perf Run the e2e perf forge only labels Mar 26, 2024
@trunk-io Trunk.io
Copy link

trunk-io bot commented Mar 26, 2024 •
edited

⏱️ 31h 31m total CI duration on this PR
Job Cumulative Duration Recent Runs
rust-unit-coverage 9h 3m ⬜ 🟩 🟩 
rust-smoke-coverage 7h 8m ⬜ 🟩 🟩 
execution-performance / single-node-performance 2h 28m 🟩 🟩 🟩 🟩 🟩 
windows-build 1h 54m 🟩 🟩 🟩 🟩 🟩 (+2 more)
rust-unit-tests 1h 20m 🟩 🟩 ⬜ 🟩 
run-forge-consensus-stress-test / forge 47m 🟩 
rust-move-tests 44m 🟩 🟩 ⬜ 🟩 
run-forge-fullnode-reboot-stress-test / forge 38m 🟩 
run-forge-realistic-env-load-sweep / forge 35m 🟩 
run-forge-realistic-env-workload-sweep / forge 34m 🟩 
rust-smoke-tests 33m 🟩 
run-forge-realistic-env-graceful-overload / forge 31m 🟩 
forge-e2e-test / forge 29m 🟩 🟩 
rust-images / rust-all 28m 🟩 🟩 
run-forge-pfn-const-tps-realistic-env / forge 27m 🟩 
rust-lints 25m 🟩 🟩 ⬜ 🟩 
run-forge-changing-working-quorum-test / forge 25m 🟥 
run-forge-realistic-network-tuned-for-throughput / forge 21m 🟩 
run-forge-workload-mix-test / forge 21m 🟩 
run-forge-changing-working-quorum-test-high-load / forge 20m 🟩 
run-forge-haproxy / forge 15m 🟩 
check 15m 🟩 🟩 ⬜ 🟩 
check-dynamic-deps 15m 🟩 🟩 🟩 🟩 🟩 (+2 more)
run-tests-main-branch 14m 🟩 🟩 ⬜ 🟩 
forge-compat-test / forge 14m 🟩 
run-forge-single-vfn-perf / forge 12m 🟩 
run-forge-compat / forge 10m 🟩 
general-lints 8m 🟩 🟩 ⬜ 🟩 
cli-e2e-tests / run-cli-tests 6m 🟩 
semgrep/ci 3m 🟩 🟩 🟩 🟩 🟩 (+2 more)
indexer-grpc-e2e-tests / test-indexer-grpc-docker-compose 2m 🟩 
determine-test-metadata 2m 🟥 
execution-performance / file_change_determinator 55s 🟩 🟩 🟩 🟩 🟩 
node-api-compatibility-tests / node-api-compatibility-tests 51s 🟩 
file_change_determinator 50s 🟩 🟩 🟩 🟩 
file_change_determinator 41s 🟩 🟩 🟩 🟩 
upload-to-codecov 29s 🟩 🟩 
file_change_determinator 24s 🟩 🟩 
permission-check 24s 🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check 23s 🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check 15s 🟩 🟩 🟩 🟩 
permission-check 12s 🟩 🟩 🟩 🟩 
determine-docker-build-metadata 8s 🟩 🟩 
permission-check 2s 🟩 

settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io

@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@igor-aptos igor-aptos removed CICD:run-execution-performance-test Run execution performance test CICD:run-execution-performance-full-test Run execution performance test (full version) CICD:run-forge-e2e-perf Run the e2e perf forge only labels Mar 26, 2024
@igor-aptos igor-aptos force-pushed the igor/track_and_assert_on_errors branch from dfc354c to 1bd9ee1 Compare March 26, 2024 23:41
@igor-aptos
Copy link
Contributor Author

moved the counter increment to be called even if error logs are disabled (i.e. in some tests for example)

@codecov Codecov
Copy link

codecov bot commented Mar 27, 2024 •
edited

Codecov Report

Attention: Patch coverage is 57.14286% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 69.8%. Comparing base (cbe35b1) to head (e0776f6).
Report is 23 commits behind head on main.

Files Patch % Lines
aptos-move/e2e-benchmark/src/main.rs 0.0% 3 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##             main   #12689       +/-   ##
===========================================
+ Coverage    64.1%    69.8%     +5.6%     
===========================================
  Files         819     2293     +1474     
  Lines      182397   436041   +253644     
===========================================
+ Hits       117059   304556   +187497     
- Misses      65338   131485    +66147

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

sitalkedia
sitalkedia approved these changes Mar 27, 2024
View reviewed changes
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.

Is there a follow up to enable check no error for selective Forge runs?

aptos-move/e2e-benchmark/src/main.rs Outdated
@@ -80,6 +80,7 @@ fn main() {
let executor = FakeExecutor::from_head_genesis();
let mut executor = executor.set_not_parallel();

// error!("something");
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove.

@igor-aptos igor-aptos force-pushed the igor/track_and_assert_on_errors branch from 1bd9ee1 to e0776f6 Compare March 27, 2024 17:01
@igor-aptos igor-aptos added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Mar 27, 2024
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite compat success on aptos-node-v1.9.5 ==> e0776f6a95a6c1d86cebf8ce99b9a02dc488f28d

Compatibility test results for aptos-node-v1.9.5 ==> e0776f6a95a6c1d86cebf8ce99b9a02dc488f28d (PR)
1. Check liveness of validators at old version: aptos-node-v1.9.5
compatibility::simple-validator-upgrade::liveness-check : committed: 4912 txn/s, latency: 6153 ms, (p50: 5000 ms, p90: 10200 ms, p99: 15400 ms), latency samples: 221060
2. Upgrading first Validator to new version: e0776f6a95a6c1d86cebf8ce99b9a02dc488f28d
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 690 txn/s, latency: 34973 ms, (p50: 38500 ms, p90: 54600 ms, p99: 57200 ms), latency samples: 57320
3. Upgrading rest of first batch to new version: e0776f6a95a6c1d86cebf8ce99b9a02dc488f28d
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 470 txn/s, submitted: 635 txn/s, expired: 165 txn/s, latency: 36414 ms, (p50: 44900 ms, p90: 57300 ms, p99: 58600 ms), latency samples: 40932
4. upgrading second batch to new version: e0776f6a95a6c1d86cebf8ce99b9a02dc488f28d
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2019 txn/s, latency: 14890 ms, (p50: 18000 ms, p90: 19800 ms, p99: 20200 ms), latency samples: 96920
5. check swarm health
Compatibility test for aptos-node-v1.9.5 ==> e0776f6a95a6c1d86cebf8ce99b9a02dc488f28d passed
Test Ok

@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite realistic_env_max_load success on e0776f6a95a6c1d86cebf8ce99b9a02dc488f28d

two traffics test: inner traffic : committed: 7921 txn/s, latency: 4955 ms, (p50: 4800 ms, p90: 5700 ms, p99: 9900 ms), latency samples: 3422100
two traffics test : committed: 100 txn/s, latency: 1894 ms, (p50: 1800 ms, p90: 2100 ms, p99: 3300 ms), latency samples: 1780
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.210, avg: 0.202", "QsPosToProposal: max: 0.244, avg: 0.217", "ConsensusProposalToOrdered: max: 0.472, avg: 0.443", "ConsensusOrderedToCommit: max: 0.381, avg: 0.361", "ConsensusProposalToCommit: max: 0.816, avg: 0.803"]
Max round gap was 1 [limit 4] at version 1650658. Max no progress secs was 4.673741 [limit 15] at version 1650658.
Test Ok

@igor-aptos igor-aptos merged commit 6f3b22c into main Mar 28, 2024
110 of 118 checks passed
@igor-aptos igor-aptos deleted the igor/track_and_assert_on_errors branch March 28, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msmouse msmouse msmouse approved these changes

@sitalkedia sitalkedia sitalkedia approved these changes

@grao1991 grao1991 Awaiting requested review from grao1991 grao1991 is a code owner

@JoshLind JoshLind Awaiting requested review from JoshLind JoshLind is a code owner

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@igor-aptos @msmouse @sitalkedia