feat: EXC-1751: Charge idle canisters for full execution

[berestovskyy]

The idle canisters in front of the round schedule should be marked as fully executed, as they were scheduled first in the round.

This helps to rotate the round schedule faster.

[alin-at-dfinity]

I would add here that this is a lower-bound approximation of the canisters that would have gotten a "full round" had they had anything to execute. But that's good enough because in the next round the idle canisters we left out will be at the front of the schedule and very likely still idle, so we will charge them at that time.

However, having just written that, I realized there's an issue: if you have a canister with 100 compute allocation that always has something to execute (e.g. a heartbeat), it will always prevent any idle canisters from getting charged until they accumulate more than 100 priority (and the handful of other active canisters accumulate huge negative priority in the process).

Maybe you should stop after the 4th non-idle canister instead? But that's also not accurate, as (due to splitting canisters across cores) you may not even have tried to execute that canister (e.g. it could be the second canister on core 0 and most of the execution may have happened on cores 1, 2 and 3).

Off the top of my head, it seems to me like it makes most sense to keep iterating until the first non-idle canister after you've seen all fully_executed_canister_ids. Because you could argue that any idle canister that was in-between the active canisters you executed (including the first one you didn't execute for a full round) might have been executed had they had any inputs. I believe this should be a much better estimate, but I'm not 100% sure. I have to think about it for a while.

[berestovskyy]

The current approach looks simple and correct. While I agree it has some limitations, it's definitely an improvement for the current situation. Any other complications, such as backtracking or finding the first non-idle canister after all fully_executed_canister_ids, sounds challenging and I'm not sure it's worth implementing now...

[alin-at-dfinity]

There's no need to backtrack. It's just a different (and valid) estimate of how far we would have gotten with executing idle canisters.

Your current implementation will stop at the first DTS execution. Or as soon as it encounters a canister that was scheduled first, but didn't execute all its backlog. As described above, it can also stop every single round without ever charging an idle canister (something that we agree is a problem).

My proposed solution is to simply keep going until you've seen all the canisters that got a full execution round. Which makes perfect sense: if you've executed canisters 1, 5 and 7, but not canisters 3 and 9; and all other canisters up to 1000 are idle; then you could argue that canisters 2, 4, 6 and 8 were "executed" they just didn't have any inputs. This is definitely a better guess at the truth than saying "no idle canister was executed" simply because canister 1 did not consume its entire backlog.

To put it differently, stopping at the first non-idle canister is arbitrary. Stopping after the last canister that got a full round isn't.

[berestovskyy]

If we go extreme, we can argue that all idle canisters were fully executed...

I'm resolving the conversation for now to merge the PR. We can improve it in a follow up PRs...

[alin-at-dfinity]

It's not about going to extremes. It's about doing something that makes sense. This particular change makes just as much sense as charging the first 10 idle canisters: sometimes it will match, sometimes not. (Or, as in your example, same as always charging all idle canisters.)

There is no good reason whatsoever why you should stop at the first non-idle canister.