[5.0] treat a `end_block_num=0` as "forever" in snapshot scheduler for compatibility with leap 4.0 behavior by spoonincode · Pull Request #2402 · AntelopeIO/leap

spoonincode · 2024-08-16T15:55:37Z

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

leap/plugins/producer_plugin/include/eosio/producer_plugin/producer_plugin.hpp

Lines 85 to 89 in 01dab51

    
           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,

leap/plugins/producer_plugin/include/eosio/producer_plugin/snapshot_scheduler.hpp

Lines 94 to 99 in 01dab51

    
           // 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,

leap/libraries/chain/include/eosio/chain/snapshot_scheduler.hpp

Lines 47 to 54 in 0242caa

    
           // 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,

leap/libraries/chain/include/eosio/chain/snapshot_scheduler.hpp

Lines 40 to 45 in 0242caa

    
           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,

leap/plugins/producer_plugin/producer_plugin.cpp

Lines 1538 to 1544 in 0242caa

    
           // 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.

leap/libraries/chain/snapshot_scheduler.cpp

Lines 30 to 31 in 0242caa

    
           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 · 2024-10-02T14:11:37Z

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

treat end_block_num=0 as "forever" similiar to leap4

dc42b77

heifner approved these changes Aug 16, 2024

View reviewed changes

greg7mdp approved these changes Aug 16, 2024

View reviewed changes

spoonincode merged commit 9fb5719 into release/5.0 Aug 17, 2024

spoonincode deleted the ss_end_block_num=0_50 branch August 17, 2024 19:08

spoonincode mentioned this pull request Aug 17, 2024

[5.0 -> 1.0] treat a end_block_num=0 as "forever" in snapshot scheduler for compatibility with leap 4.0 behavior AntelopeIO/spring#571

Merged

spoonincode linked an issue Aug 17, 2024 that may be closed by this pull request

Snapshot Scheduling Broken in 5.0.x #2261

Closed

spoonincode mentioned this pull request Aug 19, 2024

[1.0 - > main] treat a end_block_num=0 as "forever" in snapshot scheduler for compatibility with leap 4.0 behavior AntelopeIO/spring#584

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

[5.0] treat a `end_block_num=0` as "forever" in snapshot scheduler for compatibility with leap 4.0 behavior#2402
spoonincode merged 1 commit intorelease/5.0from
ss_end_block_num=0_50

spoonincode commented Aug 16, 2024

Uh oh!

ericpassmore commented Oct 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	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 = "";

	// 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);
	}

	// 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;
	};

	// 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 : ""
	};

	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)

Conversation

spoonincode commented Aug 16, 2024

Uh oh!

ericpassmore commented Oct 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants