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

[gha] Refactor Forge Stable to run individual tests #15082

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

ibalajiarun
Copy link
Contributor

@ibalajiarun ibalajiarun commented Oct 24, 2024

Description

Refactor Forge Stable workflow to achieve the following:

  • Make parallelism configurable. One job at a time is too limited in critical situations.
  • Add ability to run single test (GH doesn't support multiple choice, so we get single choice) or entire workflow.
  • Add new tests easily because the tests are defined as a JSON blob

The above is accomplished by dynamically generating a matrix strategy based on inputs before dispatching the forge jobs.

Copy link

trunk-io bot commented Oct 24, 2024

⏱️ 5h 3m total CI duration on this PR
Slowest 15 Jobs Cumulative Duration Recent Runs
rust-cargo-deny 1h 3m 🟩🟩🟩🟩🟩 (+32 more)
determine-test-metadata 1h 1m 🟩🟩🟩 (+36 more)
check-dynamic-deps 52m 🟩🟩🟩🟩🟩 (+33 more)
general-lints 19m 🟩🟩🟩🟩🟩 (+32 more)
semgrep/ci 15m 🟩🟩🟩🟩🟩 (+33 more)
file_change_determinator 7m 🟩🟩🟩🟩 (+33 more)
rust-doc-tests 5m 🟩
test-target-determinator 4m 🟩
execution-performance / test-target-determinator 4m 🟩
check 4m 🟩
rust-move-tests 2m
rust-move-tests 2m 🟩
permission-check 2m 🟩🟩🟩🟩🟩 (+41 more)
rust-move-tests 2m 🟩
rust-move-tests 2m 🟩

🚨 1 job on the last run was significantly faster/slower than expected

Job Duration vs 7d avg Delta
execution-performance / single-node-performance 12s 24m -99%

settingsfeedbackdocs ⋅ learn more about trunk.io

@ibalajiarun ibalajiarun force-pushed the balaji/forge-stable-refactor branch 29 times, most recently from 3f83ae8 to 3f5f69c Compare October 25, 2024 04:30
@ibalajiarun ibalajiarun force-pushed the balaji/forge-stable-refactor branch from f65768e to c4705db Compare October 25, 2024 04:52
@ibalajiarun ibalajiarun marked this pull request as ready for review October 25, 2024 04:53
@ibalajiarun ibalajiarun requested a review from a team as a code owner October 25, 2024 04:53
@ibalajiarun ibalajiarun requested a review from a team October 25, 2024 04:53
@ibalajiarun ibalajiarun force-pushed the balaji/forge-stable-refactor branch from c4705db to 81c0cbc Compare October 25, 2024 23:39
Copy link
Contributor

@perryjrandall perryjrandall left a comment

Choose a reason for hiding this comment

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

U da best

@ibalajiarun ibalajiarun force-pushed the balaji/forge-stable-refactor branch 6 times, most recently from a9f172d to fce73ff Compare October 30, 2024 19:26
core.debug(`Matrix: ${JSON.stringify(matrix)}`);

core.summary.addHeading('Forge Stable Info');
core.summary.addRaw("Image Tag: ${{ steps.determine-test-branch.outputs.IMAGE_TAG }}", true);
Copy link

Choose a reason for hiding this comment

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

There appears to be an extra double quote in }}" that should be removed. The correct syntax is ${{ steps.determine-test-branch.outputs.IMAGE_TAG }}.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

id: set-matrix
uses: actions/github-script@v7
env:
JOB_NAME: ${{ inputs.JOB_NAME }}
Copy link

Choose a reason for hiding this comment

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

The environment variable JOB_NAME should be renamed to TEST_NAME to match the workflow input parameter. Currently this causes a mismatch between inputs.TEST_NAME and the environment variable being referenced.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

Comment on lines 192 to 193
core.summary.addRaw("Image Tag: ${{ steps.determine-test-branch.outputs.IMAGE_TAG }}", true);
core.summary.addRaw("Branch: ${{ steps.determine-test-branch.outputs.BRANCH }}", true);
Copy link

Choose a reason for hiding this comment

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

The steps context references a step in the current job, but this job needs values from the determine-test-metadata job. Please update to use the needs context instead:

core.summary.addRaw("Image Tag: ${{ needs.determine-test-metadata.outputs.IMAGE_TAG }}", true);
core.summary.addRaw("Branch: ${{ needs.determine-test-metadata.outputs.BRANCH }}", true);

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@ibalajiarun ibalajiarun force-pushed the balaji/forge-stable-refactor branch 2 times, most recently from c045adc to c9e4fa8 Compare October 30, 2024 19:32
@ibalajiarun ibalajiarun force-pushed the balaji/forge-stable-refactor branch from c9e4fa8 to db36738 Compare October 30, 2024 19:35
@ibalajiarun ibalajiarun enabled auto-merge (squash) October 30, 2024 19:35

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on db3673883a725538cbbfe9e3e274b04ec28f40a5

two traffics test: inner traffic : committed: 14429.34 txn/s, latency: 2754.46 ms, (p50: 2700 ms, p70: 2700, p90: 2900 ms, p99: 3100 ms), latency samples: 5486500
two traffics test : committed: 100.09 txn/s, latency: 1426.42 ms, (p50: 1400 ms, p70: 1400, p90: 1500 ms, p99: 6200 ms), latency samples: 1820
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 2.040, avg: 1.557", "ConsensusProposalToOrdered: max: 0.318, avg: 0.292", "ConsensusOrderedToCommit: max: 0.378, avg: 0.362", "ConsensusProposalToCommit: max: 0.672, avg: 0.655"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 1.20s no progress at version 32216 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.44s no progress at version 2043888 (avg 7.58s) [limit 15].
Test Ok

Copy link
Contributor

✅ Forge suite framework_upgrade success on 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> db3673883a725538cbbfe9e3e274b04ec28f40a5

Compatibility test results for 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> db3673883a725538cbbfe9e3e274b04ec28f40a5 (PR)
Upgrade the nodes to version: db3673883a725538cbbfe9e3e274b04ec28f40a5
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1129.72 txn/s, submitted: 1132.61 txn/s, failed submission: 2.89 txn/s, expired: 2.89 txn/s, latency: 2773.34 ms, (p50: 2400 ms, p70: 3000, p90: 4500 ms, p99: 6500 ms), latency samples: 101700
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1249.63 txn/s, submitted: 1251.88 txn/s, failed submission: 2.25 txn/s, expired: 2.25 txn/s, latency: 2422.47 ms, (p50: 2100 ms, p70: 2400, p90: 3300 ms, p99: 5300 ms), latency samples: 111100
5. check swarm health
Compatibility test for 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> db3673883a725538cbbfe9e3e274b04ec28f40a5 passed
Upgrade the remaining nodes to version: db3673883a725538cbbfe9e3e274b04ec28f40a5
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1326.45 txn/s, submitted: 1328.26 txn/s, failed submission: 1.82 txn/s, expired: 1.82 txn/s, latency: 2351.32 ms, (p50: 2400 ms, p70: 2700, p90: 3300 ms, p99: 4800 ms), latency samples: 116760
Test Ok

Copy link
Contributor

✅ Forge suite compat success on 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> db3673883a725538cbbfe9e3e274b04ec28f40a5

Compatibility test results for 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> db3673883a725538cbbfe9e3e274b04ec28f40a5 (PR)
1. Check liveness of validators at old version: 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd
compatibility::simple-validator-upgrade::liveness-check : committed: 17107.41 txn/s, latency: 1974.84 ms, (p50: 2000 ms, p70: 2100, p90: 2200 ms, p99: 2500 ms), latency samples: 553780
2. Upgrading first Validator to new version: db3673883a725538cbbfe9e3e274b04ec28f40a5
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 6038.43 txn/s, latency: 4792.84 ms, (p50: 5400 ms, p70: 5700, p90: 5900 ms, p99: 6000 ms), latency samples: 113500
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 5789.41 txn/s, latency: 5559.99 ms, (p50: 5800 ms, p70: 5900, p90: 7200 ms, p99: 7400 ms), latency samples: 193000
3. Upgrading rest of first batch to new version: db3673883a725538cbbfe9e3e274b04ec28f40a5
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 6450.51 txn/s, latency: 4198.23 ms, (p50: 4900 ms, p70: 5100, p90: 5400 ms, p99: 5800 ms), latency samples: 115880
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6640.22 txn/s, latency: 4839.12 ms, (p50: 5200 ms, p70: 5300, p90: 6600 ms, p99: 6800 ms), latency samples: 218720
4. upgrading second batch to new version: db3673883a725538cbbfe9e3e274b04ec28f40a5
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 7825.87 txn/s, latency: 3684.42 ms, (p50: 4000 ms, p70: 4300, p90: 4800 ms, p99: 5200 ms), latency samples: 143220
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 7551.24 txn/s, latency: 4143.48 ms, (p50: 4200 ms, p70: 4600, p90: 6400 ms, p99: 7000 ms), latency samples: 246480
5. check swarm health
Compatibility test for 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> db3673883a725538cbbfe9e3e274b04ec28f40a5 passed
Test Ok

@ibalajiarun ibalajiarun merged commit 347b8f9 into main Oct 30, 2024
85 of 95 checks passed
@ibalajiarun ibalajiarun deleted the balaji/forge-stable-refactor branch October 30, 2024 20:03
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