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

Does db snapshots work for cumulus collators? #671

Closed
samelamin opened this issue Jan 2, 2023 · 4 comments
Closed

Does db snapshots work for cumulus collators? #671

samelamin opened this issue Jan 2, 2023 · 4 comments
Assignees

Comments

@samelamin
Copy link
Contributor

samelamin commented Jan 2, 2023

Looking at the code it seems like snapshots cannot be supported because we are writing the snapshot file to the chains dir but for collators, we also have the relay-data directory. A snapshot for a parachain needs both directories

https://github.com/paritytech/zombienet/blob/HEAD/javascript/packages/orchestrator/src/providers/native/nativeClient.ts#L283-L284

https://github1s.com/paritytech/zombienet/blob/HEAD/javascript/packages/orchestrator/src/providers/podman/podmanClient.ts#L383-L384

[Relaychain] Bootnode with peer id `12D3KooWQCkBm1BYtkHpocxCwMgR8yjitEeHGx8spzcDLGt2gkBm` is on a different chain (our genesis: 0x2d46…5d89 theirs: 0x2540…b2b2)  
@pepoviola
Copy link
Collaborator

Hi @samelamin, thanks for reporting this issue and provide feedback. I think the simplest solution is to match the node role to extract the data in the correct directory.
Thanks!

@pepoviola pepoviola self-assigned this Jan 9, 2023
@samelamin
Copy link
Contributor Author

No worries @pepoviola I have sent a pr so feel free to review it when you get a chance.

Wouldnt it be simpler to extract both at the root? because otherwise you need to match the node role as well as prepare the snapshot differently for both relay and parachain.

While if we extract at the root, everything remains consistent?

@pepoviola
Copy link
Collaborator

Hi @samelamin, thanks for the pr. I would review and check it asap.
Thanks!!

cc: @michalkucharczyk if we change this we will need to update the CI test for substrate.

@pepoviola
Copy link
Collaborator

pepoviola commented Feb 23, 2023

Hi @samelamin, closing here since the pr is already merged and included in the latest version.
Thanks!

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

No branches or pull requests

2 participants