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

[CI] Adjust the replay verify job #5849

Merged
merged 1 commit into from
Dec 16, 2022
Merged

[CI] Adjust the replay verify job #5849

merged 1 commit into from
Dec 16, 2022

Conversation

runtian-zhou
Copy link
Contributor

@runtian-zhou runtian-zhou commented Dec 10, 2022 •
edited
Loading

Description

Made two changes to the replay verify pipeling:

  1. Enabled the recent feature implemented in [replay-verify] Add an option to skip txns that are known to broke backward compatibility #5747 so that we no longer need to replay from the middle of history.
  2. Added a manual CI job to scan through the currently published modules in production. related to [replay-verify] Add an option to re-validate all move modules in the snapshot #5793

Test Plan

TBD

@runtian-zhou runtian-zhou requested review from msmouse, lightmark and a team as code owners December 10, 2022 00:31
msmouse
msmouse reviewed Dec 10, 2022
View reviewed changes
execution/executor/src/chunk_executor.rs Outdated
.lock()
.persisted_and_latest_view()
.1
.version()
Copy link
Contributor

Choose a reason for hiding this comment

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

this is None for the chunk containing the genesis. Needs something like this #5848

msmouse
msmouse reviewed Dec 10, 2022
View reviewed changes
.github/actions/verify-modules/action.yaml Outdated Show resolved Hide resolved
.github/actions/verify-modules/action.yaml Outdated
- name: run replay-verify in parallel
shell: bash
run: |
mkdir $NAME
Copy link
Contributor

Choose a reason for hiding this comment

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

NAME not defined?

.github/actions/verify-modules/action.yaml Outdated Show resolved Hide resolved
.github/actions/verify-modules/action.yaml Outdated Show resolved Hide resolved
.github/workflows/verify-on-chain-modules-mainnet.yaml Outdated Show resolved Hide resolved
.github/workflows/verify-on-chain-modules-testnet.yaml Outdated
env:
BUCKET: aptos-testnet-backup-2223d95b
SUB_DIR: e1
HISTORY_START: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

not used?

.github/workflows/verify-on-chain-modules-mainnet.yaml Outdated
jobs:
replay-transactions:
timeout-minutes: 720
runs-on: high-perf-docker-with-local-ssd
Copy link
Contributor

Choose a reason for hiding this comment

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

Can use a different type? we have only one instance of this I think.

.github/workflows/verify-on-chain-modules-testnet.yaml Outdated
jobs:
replay-transactions:
timeout-minutes: 720
runs-on: high-perf-docker-with-local-ssd
Copy link
Contributor

Choose a reason for hiding this comment

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

can use a different type

msmouse
msmouse approved these changes Dec 10, 2022
View reviewed changes
Copy link
Contributor

@msmouse msmouse left a comment

Choose a reason for hiding this comment

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

please make sure a manual run (on at least the mainnet) goes through before landing.

.github/workflows/replay-verify-testnet.yaml Outdated
@@ -17,6 +17,7 @@ env:
BUCKET: aptos-testnet-backup-2223d95b
SUB_DIR: e1
HISTORY_START: 350000000
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

This version is larger than the two versions below..
Document if you intentionally do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. I intentionally set this because I don't yet know what are the other versions that broke the replay. I'm planning to run it manually to see the configs for testnet.

rustielin
rustielin requested changes Dec 12, 2022
View reviewed changes
.github/workflows/verify-on-chain-modules-mainnet.yaml Outdated
with:
ref: ${{ inputs.GIT_SHA }}
fetch-depth: 0 # get all the history because cargo xtest --change-since origin/main requires it.
- uses: ./.github/actions/replay-verify
Copy link
Contributor

Choose a reason for hiding this comment

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

./.github/actions/verify-modules ?

.github/workflows/verify-on-chain-modules-testnet.yaml Outdated
with:
ref: ${{ inputs.GIT_SHA }}
fetch-depth: 0 # get all the history because cargo xtest --change-since origin/main requires it.
- uses: ./.github/actions/replay-verify
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh great catch...

@runtian-zhou runtian-zhou enabled auto-merge (squash) December 15, 2022 19:42
@msmouse msmouse disabled auto-merge December 15, 2022 20:01
@runtian-zhou runtian-zhou enabled auto-merge (squash) December 15, 2022 22:35
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

✅ Forge suite land_blocking success on 4bf5ebfb48717532f897b86cdc2ed18ae52de0c5

performance benchmark with full nodes : 6269 TPS, 6342 ms latency, 9700 ms p99 latency,(!) expired 40 out of 2677160 txns
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 4bf5ebfb48717532f897b86cdc2ed18ae52de0c5

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 4bf5ebfb48717532f897b86cdc2ed18ae52de0c5 (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 7170 TPS, 5365 ms latency, 8200 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 4bf5ebfb48717532f897b86cdc2ed18ae52de0c5
compatibility::simple-validator-upgrade::single-validator-upgrade : 4520 TPS, 8766 ms latency, 12200 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 4bf5ebfb48717532f897b86cdc2ed18ae52de0c5
compatibility::simple-validator-upgrade::half-validator-upgrade : 4447 TPS, 8925 ms latency, 12300 ms p99 latency,no expired txns
4. upgrading second batch to new version: 4bf5ebfb48717532f897b86cdc2ed18ae52de0c5
compatibility::simple-validator-upgrade::rest-validator-upgrade : 6497 TPS, 6001 ms latency, 11000 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 4bf5ebfb48717532f897b86cdc2ed18ae52de0c5 passed
Test Ok

@runtian-zhou runtian-zhou merged commit 6f51689 into main Dec 16, 2022
@runtian-zhou runtian-zhou deleted the runtianz/replay_CO branch December 16, 2022 19:55
@msmouse msmouse mentioned this pull request Dec 17, 2022
Closed
runtian-zhou added a commit that referenced this pull request Dec 19, 2022
@runtian-zhou
[CI] Adjust the replay verify job (#5849)
b35917d 
* [replay-verify] Add adjust the CI task

* fixup! [replay-verify] Add adjust the CI task
@runtian-zhou runtian-zhou mentioned this pull request Dec 19, 2022
Merged
runtian-zhou added a commit that referenced this pull request Dec 20, 2022
@runtian-zhou
Cherry Pick PRs (#5930)
79b0872 
* [CI] Adjust the replay verify job (#5849)

* [replay-verify] Add adjust the CI task

* fixup! [replay-verify] Add adjust the CI task

* [aptos-vm] Fix binary format version compatibility. (#5898)
zekun000 pushed a commit that referenced this pull request Dec 21, 2022
@runtian-zhou @zekun000
[CI] Adjust the replay verify job (#5849)
7e5def4 
* [replay-verify] Add adjust the CI task

* fixup! [replay-verify] Add adjust the CI task
zekun000 pushed a commit that referenced this pull request Dec 21, 2022
@runtian-zhou @zekun000
[CI] Adjust the replay verify job (#5849)
9ad772f 
* [replay-verify] Add adjust the CI task

* fixup! [replay-verify] Add adjust the CI task
@Markuze Markuze mentioned this pull request Dec 26, 2022
Closed
@Markuze Markuze mentioned this pull request Jan 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rustielin rustielin rustielin approved these changes

@msmouse msmouse msmouse approved these changes

@davidiw davidiw davidiw approved these changes

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

@perryjrandall perryjrandall Awaiting requested review from perryjrandall

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@runtian-zhou @davidiw @msmouse @rustielin