sweepbatcher: customize initialDelay per sweep #910
Conversation
starius
changed the title
starius
force-pushed
the
sweepbatcher-initial-delay-func
branch
from
921714b to
798c32c
Compare
bhandras
left a comment
bhandras
left a comment
There was a problem hiding this comment.
LGTM, nice! I think this greedy logic will help batches a lot! 🎉
| // (especially important in case of a crashloop). If a sweep is about to | ||
| // expire (time until timeout is less that 2x initialDelay), then | ||
| // waiting is skipped. | ||
| initialDelayProvider InitialDelayProvider |
There was a problem hiding this comment.
nit: as this isn't really about init, but the first publish, for clarity we could rename it to firstPublishDelayProvider (and the firstPublishDelay below in the runloop). wdyt?
There was a problem hiding this comment.
I prefer the current name. We also have another option called PublishDelay. It could be confused with firstPublishDelayProvider if it is renamed. Also if we grep by PublishDelay, we would find both options.
starius
force-pushed
the
sweepbatcher-initial-delay-func
branch
from
798c32c to
535f619
Compare
|
Changed InitialDelayProvider signature: added the number of sweeps as an argument. |
sweepbatcher/sweep_batcher.go
Outdated
| type FeeRateProvider func(ctx context.Context, | ||
| swapHash lntypes.Hash) (chainfee.SatPerKWeight, error) | ||
|
|
||
| // InitialDelayProvider returns the duration after new batch creation before it |
There was a problem hiding this comment.
nit: InitialDelayProvider returns the duration after which a newly created batch is first published.
sweepbatcher/sweep_batch.go
Outdated
| b.primarySweepID[0:6], len(b.sweeps)) | ||
|
|
||
| for { | ||
| // If the batch if not empty, find earliest initialDelay. |
sweepbatcher/sweep_batch.go
Outdated
| ctx, len(b.sweeps), totalSweptAmt, | ||
| ) | ||
| if err != nil { | ||
| b.Warnf("initialDelayProvider failed: %v", err) |
There was a problem hiding this comment.
add to the warning that we use no initial delay in publishing this batch
sweepbatcher/sweep_batch.go
Outdated
| initialDelay = 0 | ||
| } | ||
| if initialDelay < 0 { | ||
| b.Warnf("negative delay: %v", initialDelay) |
Option WithInitialDelay now accepts a function returning initialDelay depending on sweep's data. This is needed to be able to wait longer for sweeps with low priority, but still sweeping high priority sweeps soon.
starius
force-pushed
the
sweepbatcher-initial-delay-func
branch
from
535f619 to
472bec0
Compare
3 participants
Option
WithInitialDelaynow accepts a function returning initialDelay depending on sweep's data.This is needed to be able to wait longer for sweeps with low priority, but still sweeping high priority sweeps soon.
Pull Request Checklist
release_notes.mdif your PR contains major features, breaking changes or bugfixes