-
Notifications
You must be signed in to change notification settings - Fork 162
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
feat(status): Add snapshot progress RPC #5368
base: main
Are you sure you want to change the base?
feat(status): Add snapshot progress RPC #5368
Conversation
@LesnyRumcajs @elmattic Please 🙏 can any one of give it a review. This PR is not fully ready yet (naming, logging changes and need to remove extra command), just wanted to get some opinions on this, so I can proceed further. Edit: To review only the snapshot progress tracker take a look at the last two commits |
I'll let @hanabi1224 or @lemmih take a look; I don't have the capacity currently. |
use crate::lotus_json::lotus_json_with_self; | ||
|
||
#[derive(PartialEq, Default, Serialize, Deserialize, Debug, Clone, JsonSchema)] | ||
#[serde(rename_all = "camelCase")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#[serde(rename_all = "camelCase")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it's OK to keep it for consistency
Hey @hanabi1224 can you please approve the request for running the workflows. Also I have made some changes on top of your last review please do review and let me know what you think 🙏🏼. |
@akaladarshi Any ideas why some tests keep failing after a few retries? Please let us know if assistance is needed for investigating the failures. |
The Snapshot test mostly fails because it is not able to download it before timeout. I tried Devnet Check locally and it mostly failed because it was not able to download proofs and all. I have seen these two test fail mostly in previous PR'S as well. Will try to dig deep. |
@hanabi1224 While looking into this issue: I came across this weird behaviour. While adding another RPC call SyncSnapshotProgress inside the That's the reason why the Have you noticed this behaviour or can you give some pointers about it? |
Summary of changes
Changes introduced in this pull request:
Appcontext
indaemon
to hold common objectsSnapshotTracket
to track the snapshot progressFilecoin.SyncStatus
which reports the snapshot download progressReference issue to close (if applicable)
Closes: #4929
Other information and links
Change checklist