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

[3.2] Implement JSON Snapshot Reader #25

Merged
merged 16 commits into from
Sep 8, 2022
Merged

Conversation

ClaytonCalabrese
Copy link
Contributor

@ClaytonCalabrese ClaytonCalabrese commented Aug 18, 2022

eosnetworkfoundation/mandel#710 ports a way to convert a snapshot to json. This PR enhances the --snapshot option to support reading a snapshot in JSON format if the snapshot has .json extension.

Resolves: eosnetworkfoundation/mandel#771

Previous discussion: eosnetworkfoundation/mandel#732

ClaytonCalabrese and others added 7 commits July 27, 2022 10:51
…erge snapshot and jsonsnapshot code handling
…d of adding a new option. Improved memory usage so current WAX snapshot can be supported. Modified the JSON output so that table rows are an array which reduces file size and memory usage. Put rapidjson inside an eosio_rapidjson namespace to avoid ODR issues. Change CMakeLists to not compile rapidjson but only include it as it is a header only library.
@ClaytonCalabrese ClaytonCalabrese added the OCI Work exclusive to OCI team label Aug 18, 2022
@ClaytonCalabrese ClaytonCalabrese self-assigned this Aug 18, 2022
@ClaytonCalabrese ClaytonCalabrese marked this pull request as draft August 18, 2022 16:22
…ent possible collision. Rename rapidjson in libraries.
@spoonincode
Copy link
Member

Is there any reasonable way to sniff the file and determine binary vs JSON, instead of relying on the file name?

@heifner
Copy link
Member

heifner commented Aug 25, 2022

Is there any reasonable way to sniff the file and determine binary vs JSON, instead of relying on the file name?

Maybe, is that worth doing? The snapshot-to-json generates with a .json file extension.

@spoonincode
Copy link
Member

spoonincode commented Sep 1, 2022

Maybe, is that worth doing?

Maybe. Being prescriptive about filenames can break more flexible invocations. Like maybe someone could set up a pipeline to modify a snapshot without ever writing it to disk via something like

nodeos --snapshot <(leap-util --snapshot-to-json snapshot.bin | program_mutates_json_somehow) --delete-all

But this could just be academic. The binary snapshots need to be seekable at the moment; I'm not sure if the JSON ones seek around or not, and it may be hard to accommodate something like the above command even for JSON, idk

@@ -5,6 +5,7 @@
#include <fc/variant_object.hpp>
#include <boost/core/demangle.hpp>
#include <ostream>
#include <memory>

namespace eosio { namespace chain {
Copy link
Member

Choose a reason for hiding this comment

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

Please rebase this PR to main (or start a new PR) and incorporate libraries/chain/include/eosio/chain/snapshot.hpp & libraries/chain/snapshot.cpp changes from #24 into it. Then we will be able to get the PR into main. The --snapshot-to-json portion of #24 can be added to leap-util later (keep that PR up so it can be easily incorporated into leap-util.

@heifner
Copy link
Member

heifner commented Sep 7, 2022

[@spoonincode sorry, somehow edited your comment instead of quoting it. I didn't even think that was possible.]

But this could just be academic. The binary snapshots need to be seekable at the moment; I'm not sure if the JSON ones seek around or not, and it may be hard to accommodate something like the above command even for JSON, idk

We could just try reading as binary which should quickly fail as it will not find the magic number of the binary file, then switch over to JSON.

Base automatically changed from snapshot_json_support to main September 8, 2022 18:40
@ClaytonCalabrese ClaytonCalabrese merged commit 782dfaf into main Sep 8, 2022
@ClaytonCalabrese ClaytonCalabrese deleted the cleanup_improve_json branch September 8, 2022 18:40
@ClaytonCalabrese ClaytonCalabrese restored the cleanup_improve_json branch September 12, 2022 16:19
heifner added a commit that referenced this pull request Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCI Work exclusive to OCI team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cleanup and improve json snapshot support
3 participants