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

Data robustness during restart #4071

Merged
merged 8 commits into from
Jun 27, 2023
Merged

Data robustness during restart #4071

merged 8 commits into from
Jun 27, 2023

Conversation

Leo-Besancon
Copy link
Collaborator

  • document all added functions
  • try in sandbox /simulation/labnet
  • unit tests on the added/changed features
    • make tests compile
    • make tests pass
  • add logs allowing easy debugging in case the changes caused problems
  • if the API has changed, update the API specification

@Leo-Besancon Leo-Besancon linked an issue Jun 13, 2023 that may be closed by this pull request
Make the API queries more robust in case of a network restart #4064
Closed
@Leo-Besancon Leo-Besancon marked this pull request as ready for review June 21, 2023 14:11
@Leo-Besancon
Copy link
Collaborator Author

@aoudiamoncef, if you think it's relevant to add a context info that we are in downtime on the API response of get_stakers, please guide me on how to do so ;)

@aoudiamoncef aoudiamoncef mentioned this pull request Jun 21, 2023
Closed
@aoudiamoncef
Copy link
Contributor

@Leo-Besanconwe have two ways to bring it to Massa, using a field in the response or a status codes.

JsonRPC https://www.jsonrpc.org/specification
gRPC https://grpc.github.io/grpc/core/md_doc_statuscodes.html

I'm voting for a status field in the response which is explicit

aoudiamoncef
aoudiamoncef approved these changes Jun 21, 2023
View reviewed changes
massa-final-state/src/final_state.rs Outdated
"Invalid cycle_history",
)))?;
let latest_snapshot_cycle = self.pos_state.cycle_history_cache.back().cloned().ok_or(
FinalStateError::SnapshotError(String::from("Invalid cycle_history")),
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't hesitate to add more détails in error messages, this is very helpful

massa-pos-exports/src/pos_final_state.rs Show resolved Hide resolved
@aoudiamoncef aoudiamoncef self-requested a review June 21, 2023 14:25
@Leo-Besancon
Copy link
Collaborator Author

@Leo-Besanconwe have two ways to bring it to Massa, using a field in the response or a status codes.

JsonRPC https://www.jsonrpc.org/specification gRPC https://grpc.github.io/grpc/core/md_doc_statuscodes.html

I'm voting for a status field in the response which is explicit

Ok thanks I'll look into it then ;)

@aoudiamoncef
Copy link
Contributor

aoudiamoncef commented Jun 23, 2023 •
edited
Loading

@Leo-Besancon please don't forget to propagate the changes to GetLargestStakers in gRPC API

@Leo-Besancon Leo-Besancon force-pushed the data_robustness_during_restart branch from 9cf8384 to ff5061a Compare June 27, 2023 09:00
aoudiamoncef
aoudiamoncef approved these changes Jun 27, 2023
View reviewed changes
Copy link
Contributor

@aoudiamoncef aoudiamoncef left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

@Leo-Besancon Leo-Besancon merged commit 2f14945 into testnet_24 Jun 27, 2023
@Leo-Besancon Leo-Besancon deleted the data_robustness_during_restart branch June 27, 2023 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aoudiamoncef aoudiamoncef aoudiamoncef approved these changes

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

Successfully merging this pull request may close these issues.

Make the API queries more robust in case of a network restart
2 participants
@Leo-Besancon @aoudiamoncef