feat(sequencer): dynamic batch production#2291
Conversation
|
Note that the batch selection process is now quite unoptimized. This will be improved as a separate PR. |
| if selected.is_empty() { | ||
| return None; | ||
| BatchSelection::Empty | ||
| } else if budget.is_exhausted() { | ||
| // The selected transactions may exactly consume the budget without another candidate | ||
| // being available to trigger the "would exceed" branch above. | ||
| BatchSelection::Full(selected.build()) | ||
| } else { | ||
| BatchSelection::Partial(selected.build()) | ||
| } |
There was a problem hiding this comment.
This is still slightly suboptimal.
If the budget is not exhausted but all remaining transactions would make the selection go over the budget budget.is_exhausted() will be false and we're returning BatchSelection::Partial here. That in turn makes the scheduler wait for the partial-batch timeout.
If we were to return BatchSelection::Full instead then we could immediately create this batch and then continue with the rest of the candidates.
There was a problem hiding this comment.
I did debate back and forth on this. But perhaps I settled on the wrong conclusion.
At the moment this only really matters if we have transactions with massive amounts of notes (since the tx count restriction is more binary).
As an example, if we have two extreme txs each with close to the maximum number of output notes, then each batch can only contain one of these. But you could in theory squeeze additional smaller txs in if they arrive.
So I guess the question is, do we want to wait, or do we want to process these asap even if better opportunities might arrive.
There was a problem hiding this comment.
It really just depends on whether we're optimizing for latency or for throughput.
I think that in most cases it's latency that matters (better UX, as transactions get included faster). But maybe even that is a premature optimization at this point -- I'm fairly sure the block builder will need quite a few tweaks once these decisions start to matter...
Summary
This PR changes batch production from a fixed interval, to producing as many full batches as possible. The current
batch.interval: DurationCLI is instead now used to configure the time after which a non-full batch will be scheduled if no full batch can be produced.The new algorithm is essentially:
batch.intervalseconds, attempt to build a batch of any size.block.interval / 10seconds.In addition, I've removed the failure injection we had in place for batch and block production. These have actually never been used, so they're not really worth the LoC.
Closes #2287
Changelog