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

refactor: decouple snapshot-interval from state-sync #235

Closed
p0mvn opened this issue May 10, 2022 · 0 comments · Fixed by #236
Closed

refactor: decouple snapshot-interval from state-sync #235

p0mvn opened this issue May 10, 2022 · 0 comments · Fixed by #236
Assignees
Labels
frog T:Bug Something isn't working

Comments

@p0mvn
Copy link
Member

p0mvn commented May 10, 2022

Context

#140 , #187 refactored snapshot logic.

In these PRs, it was done so that the snapshot manager is not set (nil) if snapshot-interval is 0. However, snapshot manager is also responsible for state-sync. It might be the case that a node wants to catch up using state sync while turning off snapshots on their side.

Logs with this issue:

3:51AM INF Discovered new snapshot format=1 hash="\r��z��ܓ\x18Ws��\x19d�=ެ��=�\n\x05��\v���m" height=4329000 module=statesync
3:51AM INF Discovered new snapshot format=1 hash="\x008\x05(W���Y\b�b&��jڭ�\x10�,\a��o\x04c\a�E4" height=4327500 module=statesync
3:52AM INF VerifyHeader hash=6E992F698F0D454B2AC5458CB1C467BF9B393A11C86025CA986DA1235DF23238 height=4329001 module=light
3:52AM INF VerifyHeader hash=98CA996C1ED5ECA1731CEACA67EE08B03755C690F38745E50146AFAF2455360B height=4329002 module=light
3:52AM INF Offering snapshot to ABCI app format=1 hash="\r��z��ܓ\x18Ws��\x19d�=ެ��=�\n\x05��\v���m" height=4329000 module=statesync
3:46AM ERR snapshot manager not configured
3:46AM ERR State sync failed err="state sync aborted" module=statesync

We should decouple the logic so that the snapshot manager is created independently from the snapshot-inteval value in app.toml

Acceptance Criteria

  • state sync is enabled independently from the value of snapshot-interval in app.toml
  • unit tests added
  • snapshots and their configuration works as expected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frog T:Bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant