feat(Ledger_Index): DEFI-2676: Set index timer minimum interval#8974
feat(Ledger_Index): DEFI-2676: Set index timer minimum interval#8974mbjorkqvist wants to merge 3 commits intomasterfrom
Conversation
| state.retrieve_blocks_from_ledger_interval = retrieve_blocks_from_ledger_interval_seconds | ||
| .map(|interval| { | ||
| Duration::from_secs(interval).max(MIN_RETRIEVE_BLOCKS_FROM_LEDGER_INTERVAL) | ||
| }); |
There was a problem hiding this comment.
understanding: why clamping to MIN_RETRIEVE_BLOCKS_FROM_LEDGER_INTERVAL and not returning an error in case the interval is smaller than this?
There was a problem hiding this comment.
Returning an error is definitely an option. I'd be fine with going this route, although it does come with the drawback outlined in the other discussion.
| @@ -45,6 +45,10 @@ const ACCOUNTIDENTIFIER_BLOCK_IDS_MEMORY_ID: MemoryId = MemoryId::new(3); | |||
| const ACCOUNTIDENTIFIER_DATA_MEMORY_ID: MemoryId = MemoryId::new(4); | |||
There was a problem hiding this comment.
here for a lack of better place: shouldn't the corresponding retrieve_block_from_ledger tests be updated to not have a value of 0 for the interval (for clarity)? And shouldn't we test setting an interval with value less than 1 sec?
There was a problem hiding this comment.
In the init and upgrade arguments, we take as input the interval in seconds (as u64), so without changing the init and upgrade arguments, an interval of less than 1 second cannot be specified. There are certainly some options we could consider:
- Add a new upgrade argument, e.g.,
retrieve_blocks_from_ledger_interval_millis, which would allow for finer-grained configuring of the interval. - Disallow intervals of 0 seconds. There probably aren't any index canisters out there with this configuration, but for ones that may exist, we'd need to decide what to do, either fail the upgrade if the interval is set to 0 seconds, or forcefully change it to 1 second (which may not be so easy for a user to discover).
There is a potential use case (although not necessarily a reasonable one) for less than 1 second - if you want the index to poll the ledger every round, then 1 second is not enough if the subnet finalization rate is more than 1 block/s.
2 participants
Set a minimum timer interval for the ICP and ICRC index canister.