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

[Forge] Add working_dir param to support running node on checkpoint dir, so that the existing data on disk is preserved. #4591

Merged
merged 1 commit into from
Dec 7, 2022
Merged

[Forge] Add working_dir param to support running node on checkpoint dir, so that the existing data on disk is preserved. #4591

merged 1 commit into from
Dec 7, 2022
+104 −15

Conversation

grao1991
Copy link
Contributor

@grao1991 grao1991 commented Sep 27, 2022 •
edited by geekflyer
Loading

For largedb test now, but could also be a useful feature in the future.

Description

Test Plan

This change is Reviewable

@grao1991 grao1991 added the CICD:build-images when this label is present github actions will start build+push rust images from the PR. label Sep 27, 2022
@grao1991 grao1991 force-pushed the grao_largedb branch 6 times, most recently from 484b008 to ab3c84c Compare September 29, 2022 00:09
@davidiw
Copy link
Contributor

davidiw commented Oct 28, 2022

closing stale pr -- feel free to re-open and lets get it reviewed if it isn't stale...

@davidiw davidiw closed this Oct 28, 2022
@davidiw davidiw deleted the grao_largedb branch October 28, 2022 16:38
@grao1991 grao1991 restored the grao_largedb branch December 6, 2022 20:06
@grao1991 grao1991 reopened this Dec 6, 2022
@grao1991 grao1991 force-pushed the grao_largedb branch 4 times, most recently from 47fbb5e to c50b4ba Compare December 6, 2022 20:39
@grao1991 grao1991 marked this pull request as ready for review December 6, 2022 21:33
@grao1991 grao1991 enabled auto-merge (squash) December 6, 2022 21:43
@grao1991
[Forge] Add working_dir param to support running node on checkpoint d…
7b0074a 
…ir, so that the existing data on disk is preserved.
@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

github-actions bot commented Dec 6, 2022

✅ Forge suite land_blocking success on 7b0074ae234728f65f5c5f1e06e76fd6b27adab6

performance benchmark with full nodes : 6319 TPS, 6253 ms latency, 19300 ms p99 latency,(!) expired 880 out of 2699480 txns
Test Ok

@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 7b0074ae234728f65f5c5f1e06e76fd6b27adab6

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 7b0074ae234728f65f5c5f1e06e76fd6b27adab6 (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 7452 TPS, 5183 ms latency, 7800 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 7b0074ae234728f65f5c5f1e06e76fd6b27adab6
compatibility::simple-validator-upgrade::single-validator-upgrade : 5354 TPS, 7463 ms latency, 10100 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 7b0074ae234728f65f5c5f1e06e76fd6b27adab6
compatibility::simple-validator-upgrade::half-validator-upgrade : 4682 TPS, 8610 ms latency, 10900 ms p99 latency,no expired txns
4. upgrading second batch to new version: 7b0074ae234728f65f5c5f1e06e76fd6b27adab6
compatibility::simple-validator-upgrade::rest-validator-upgrade : 6584 TPS, 5935 ms latency, 9200 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 7b0074ae234728f65f5c5f1e06e76fd6b27adab6 passed
Test Ok

JoshLind
JoshLind reviewed Dec 7, 2022
View reviewed changes
Copy link
Contributor

@JoshLind JoshLind left a comment

Choose a reason for hiding this comment

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

Overall, LGTM. Only concern is if we've verified that changing the secure-data.json path is not going to break anything?

docker/compose/aptos-node/validator.yaml
@@ -16,7 +16,7 @@ consensus:
type: "local"
backend:
type: "on_disk_storage"
path: /opt/aptos/data/secure-data.json
path: secure-data.json
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we change this?

Copy link
Contributor

Choose a reason for hiding this comment

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

it will be prepended with working_dir which is /opt/aptos/data/ in this case.

lightmark
lightmark approved these changes Dec 7, 2022
View reviewed changes
docker/compose/aptos-node/validator.yaml
@@ -16,7 +16,7 @@ consensus:
type: "local"
backend:
type: "on_disk_storage"
path: /opt/aptos/data/secure-data.json
path: secure-data.json
Copy link
Contributor

Choose a reason for hiding this comment

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

it will be prepended with working_dir which is /opt/aptos/data/ in this case.

@grao1991 grao1991 merged commit 1d45ed0 into main Dec 7, 2022
@grao1991 grao1991 deleted the grao_largedb branch December 7, 2022 01:42
areshand pushed a commit to areshand/aptos-core-1 that referenced this pull request Dec 18, 2022
@grao1991 @areshand
[Forge] Add working_dir param to support running node on checkpoint d…
52c914e 
…ir, so that the existing data on disk is preserved. (aptos-labs#4591)
@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

@JoshLind JoshLind JoshLind left review comments

@msmouse msmouse msmouse approved these changes

@lightmark lightmark lightmark approved these changes

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

Assignees
No one assigned
Labels
CICD:build-images when this label is present github actions will start build+push rust images from the PR.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@grao1991 @davidiw @lightmark @msmouse @JoshLind