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

[5.0] treat a end_block_num=0 as "forever" in snapshot scheduler for compatibility with leap 4.0 behavior #2402

Merged
merged 1 commit into from
Aug 17, 2024

Conversation

spoonincode
Copy link
Member

leap 5's snapshot scheduling implementation changed the meaning of end_block_num=0 which breaks both requests that previously worked and any previously scheduled snapshots with that value (the scheduled snapshots will effectively be removed upon upgrade to leap5).

In leap4 requests were submitted as

struct snapshot_request_information {
uint32_t block_spacing = 0;
uint32_t start_block_num = 0;
uint32_t end_block_num = 0;
std::string snapshot_description = "";

This has the important behavior that if a request does not specify end_block_num it remains 0 and is written out to snapshot-schedule.json with "end_block_num":0. I believe what happens then is that end_block_num=0 gets treated as "forever" by it not being removed from the scheduled list here,
// cleanup - remove expired (or invalid) request
if((!req.start_block_num && !req.block_spacing) ||
(!req.block_spacing && height >= (req.start_block_num + 1)) ||
(req.end_block_num > 0 && height >= (req.end_block_num + 1))) {
unschedule_snapshot_request_ids.push_back(req.snapshot_request_id);
}

But leap5 changes things up. There is an additional indirection on the requests,

// this struct used to hold request params in api call
// it is differentiate between 0 and empty values
struct snapshot_request_params {
std::optional<uint32_t> block_spacing;
std::optional<uint32_t> start_block_num;
std::optional<uint32_t> end_block_num;
std::optional<std::string> snapshot_description;
};

before the similar snapshot_request_information is used,
struct snapshot_request_information {
uint32_t block_spacing = 0;
uint32_t start_block_num = 0;
uint32_t end_block_num = std::numeric_limits<uint32_t>::max();
std::string snapshot_description = "";
};

So, importantly, in 5.0 if the request does not specify the optional<uint32_t> end_block_num then it is set to UINT32_MAX,
// missing start/end is set to head block num, missing end to UINT32_MAX
chain::snapshot_scheduler::snapshot_request_information sri = {
.block_spacing = srp.block_spacing ? *srp.block_spacing : 0,
.start_block_num = srp.start_block_num ? *srp.start_block_num : head_block_num + 1,
.end_block_num = srp.end_block_num ? *srp.end_block_num : std::numeric_limits<uint32_t>::max(),
.snapshot_description = srp.snapshot_description ? *srp.snapshot_description : ""
};

And then leap5's schedule pruning does not treat end_block_num=0 as special because it expects "forever requests" to be end_block_num=UINT32_MAX; so any end_block_num=0 requests (be it new ones arriving on RPC, or old ones read from the JSON file) just get removed immediately.

bool marked_for_deletion = ((!req.block_spacing) && (height >= req.start_block_num + 1)) || // if one time snapshot executed or scheduled for the past, it should be gone
(height > 0 && ((height-1) >= req.end_block_num)); // any snapshot can expire by end block num (end_block_num can be max value)

So this PR replaces end_block_num=0 with end_block_num=UINT32_MAX both,

  • when reading existing snapshot-schedule.json entries with end_block_num=0, and
  • when handling schedule_snapshot RPC request, so that behavior with requests that worked in leap4 remain the same in leap5

Resolves #2261

@ericpassmore
Copy link
Contributor

Note:start
category: Other
component: Snapshots
summary: Treat a end_block_num=0 as "forever" in snapshot scheduler for compatibility with leap 4.0 behavior.
Note:end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snapshot Scheduling Broken in 5.0.x
4 participants