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

Programmable snapshots API #759

Merged
merged 55 commits into from
Mar 16, 2023
Merged

Programmable snapshots API #759

merged 55 commits into from
Mar 16, 2023

Conversation

vladtr
Copy link
Contributor

@vladtr vladtr commented Feb 28, 2023

This PR relates to #656 issue and introduces new feature - snapshot scheduling API to create, retrieve and delete/modify automated snapshot schedule.

In more details API specification with examples of it's use can be found here:
https://github.com/eosnetworkfoundation/product/blob/main/api-http/proposals/snapshot-api.md

This draft PR includes initial boost tests and will incorporate additional tests currently being finalized.

@vladtr vladtr self-assigned this Feb 28, 2023
@vladtr vladtr linked an issue Feb 28, 2023 that may be closed by this pull request
Programmable snapshots for future blocks #656
Closed
5 tasks
@vladtr vladtr marked this pull request as ready for review February 28, 2023 21:44
@vladtr vladtr removed their assignment Feb 28, 2023
heifner
heifner requested changes Mar 1, 2023
View reviewed changes
plugins/producer_plugin/include/eosio/producer_plugin/producer_plugin.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/producer_plugin.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/producer_plugin.hpp
@@ -105,7 +118,11 @@ class producer_plugin : public appbase::plugin<producer_plugin> {
void set_whitelist_blacklist(const whitelist_blacklist& params);

integrity_hash_information get_integrity_hash() const;

void create_snapshot(next_function<snapshot_information> next);
Copy link
Member

Choose a reason for hiding this comment

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

Are we going to deprecate create_snapshot?

plugins/producer_plugin/include/eosio/producer_plugin/producer_plugin.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Outdated Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Outdated Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Outdated Show resolved Hide resolved
@heifner
Copy link
Member

heifner commented Mar 2, 2023

Would like see an integration test for this functionality.

heifner
heifner previously requested changes Mar 2, 2023
View reviewed changes
plugins/producer_plugin/include/eosio/producer_plugin/producer_plugin.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_db_json.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_db_json.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp Outdated Show resolved Hide resolved
@heifner heifner requested a review from linh2931 March 15, 2023 22:09
@heifner heifner dismissed their stale review March 15, 2023 22:10

Lin is taking over for me.

@greg7mdp
Copy link
Contributor

greg7mdp commented Mar 15, 2023 •
edited
Loading

@766C6164 Thanks for all the changes! I think this is going to be a very popular feature :-).

Lin merged his large PR to main a few minutes ago. Maybe you should merge the latest changes from main (hopefully without too many conflicts), and then I'll do a last check and approve your PR, probably tomorrow morning.

@vladtr
Copy link
Contributor Author

vladtr commented Mar 15, 2023

@766C6164 Thanks for all the changes! I think this is going to be a very popular feature :-).

Lin merged his large PR to main a few minutes ago. Maybe you should merge the latest changes from main (hopefully without too main conflicts), and then I'll do a last check and approve your PR, probably tomorrow morning.

Thank you so much!!! Really appreciate all your time and effort you put into reviewing this PR! Luckily picked up changes from main with only 3 minor conflicts :)

linh2931
linh2931 reviewed Mar 16, 2023
View reviewed changes
Copy link
Member

@linh2931 linh2931 left a comment

Choose a reason for hiding this comment

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

Those are the first batch. They are not blockers and can be fixed in RC1.

I am continuing reviewing the rest of the code.

plugins/producer_plugin/include/eosio/producer_plugin/snapshot_db_json.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_db_json.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_db_json.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp Outdated Show resolved Hide resolved
plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp Outdated Show resolved Hide resolved
@vladtr vladtr merged commit e3b31cd into main Mar 16, 2023
@vladtr vladtr deleted the snapshot-api branch March 16, 2023 21:08
@bhazzard bhazzard mentioned this pull request Apr 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heifner heifner heifner left review comments

@linh2931 linh2931 linh2931 approved these changes

@greg7mdp greg7mdp greg7mdp approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Programmable snapshots for future blocks
4 participants
@vladtr @heifner @greg7mdp @linh2931