chore: Replace fast sync with batch processing #132
Conversation
gitferry
force-pushed
the
gai/replace-fast-sync-with-batch-processing
branch
from
e0a35a8 to
df7421a
Compare
gitferry
force-pushed
the
gai/replace-fast-sync-with-batch-processing
branch
from
df7421a to
9976e23
Compare
gitferry
requested review from
SebastianElvis,
bap2pecs,
KonradStaniec and
Lazar955
|
|
||
| latestFinalisedBlocks, err := fp.latestFinalizedBlocksWithRetry(1) | ||
| if err != nil { | ||
| return 0, err |
| // error will be returned if maximum retries have been reached or the query to the consumer chain fails | ||
| func (fp *FinalityProviderInstance) retrySubmitFinalitySignatureUntilBlockFinalized(targetBlock *types.BlockInfo) (*types.TxResponse, error) { | ||
| func (fp *FinalityProviderInstance) retrySubmitFinalitySignaturesUntilBlocksFinalized(targetBlocks []*types.BlockInfo) (*types.TxResponse, error) { |
There was a problem hiding this comment.
Nit: fcn name is quite long suggestion retrySubmitSignatures
| defaultSignatureSubmissionInterval = 1 * time.Second | ||
| defaultMaxSubmissionRetries = 20 | ||
| defaultBitcoinNetwork = "signet" | ||
| defaultDataDirname = "data" |
There was a problem hiding this comment.
Shouldn't we add a config for batch processing size? what's the current code behavior if it lags way behind?
There was a problem hiding this comment.
If the fp lags behind, the poller should quickly poll blocks (that's why we should have a short polling interval). Then the fp will do batch submission
finality-provider/config/poller.go
Outdated
| @@ -4,7 +4,7 @@ import "time" | |||
|
|
|||
| var ( | |||
| defaultBufferSize = uint32(1000) | |||
| defaultPollingInterval = 20 * time.Second | |||
| defaultPollingInterval = 100 * time.Millisecond | |||
There was a problem hiding this comment.
i think the polling interval shouldn't be less than the chain block-producing time. OP chain is usually 2-3 seconds. what's the block time for Babylon chain?
There was a problem hiding this comment.
I would argue that we should have a short polling interval for a quick catching up. Would ms level interval be a concern in practice?
There was a problem hiding this comment.
Catching up is not a common case. If it's way behind, people should just change the config file temporarily to speed up. imo default value should be for normal submission cases
There was a problem hiding this comment.
Makes sense. Changed it to 1s to cover most surprises and added notes there
|
I would wait for one more approval before merge |
Sure. |
| func (fp *FinalityProviderInstance) getAllBlocksFromChan() []*types.BlockInfo { | ||
| var pollerBlocks []*types.BlockInfo | ||
| for { | ||
| select { |
There was a problem hiding this comment.
I think you could also add:
case <-fp.quit:
to this select, to not wait long time for quit in case of many blocks pending on poller channel.
|
@gitferry do you plan to backport this to the base branch? |
|
@bap2pecs not sure it's efficient to backport every change to the base branch. Do we have a routine to periodically backport changes to the base branch? |
actually it's probably the opposite. I feel it's probably always better to backport right away or context will be lost. for example, #153 is not a straightforward backport due to some interface level difference between main and base branch. but I can quickly backport it b/c my memory was fresh so I know where to change also, i'd say for important changes like this, we should proritize. this one is currently blocking some of our works (e.g. e2e test refactoring) so we'd appreciate if it can be backported |
|
@bap2pecs Make sense. I'll backport it |
Closes #118. Notable changes: * remove fast sync completely * introduce batch process for catching up * introduce signature submission interval to config * introduce batch submission size to config
Closes #118. Notable changes: * remove fast sync completely * introduce batch process for catching up * introduce signature submission interval to config * introduce batch submission size to config
thanks a lot! I remember @vitsalis told me in Bangkok that this backport issue will be solved soon but i don't remember the conversation do you guys feel that backporting takes lots of engineering time for your team? @gitferry @SebastianElvis |
Closes #118. Notable changes: * remove fast sync completely * introduce batch process for catching up * introduce signature submission interval to config * introduce batch submission size to config
Closes #118. Notable changes: