Add traces backfill option #615
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,11 +84,11 @@ func (e *Engine) Notify(block *models.Block) { | |
| return | ||
| } | ||
|
|
||
| go e.indexBlockTraces(block, cadenceID, false) | ||
| } | ||
|
|
||
| // indexBlockTraces iterates the block transaction hashes and tries to download the traces | ||
| func (e *Engine) indexBlockTraces(evmBlock *models.Block, cadenceBlockID flow.Identifier, skipExisting bool) { | ||
| ctx, cancel := context.WithTimeout(context.Background(), downloadTimeout) | ||
| defer cancel() | ||
|
|
||
|
|
@@ -107,9 +107,17 @@ func (e *Engine) indexBlockTraces(evmBlock *models.Block, cadenceBlockID flow.Id | |
|
|
||
| l := e.logger.With(). | ||
| Str("tx-id", h.String()). | ||
| Uint64("evm-height", evmBlock.Height). | ||
| Str("cadence-block-id", cadenceBlockID.String()). | ||
| Logger() | ||
|
|
||
| if skipExisting { | ||
| if _, err := e.traces.GetTransaction(h); err == nil { | ||
| l.Debug().Msg("trace already downloaded") | ||
| return | ||
| } | ||
| } | ||
|
|
||
| err := retry.Fibonacci(ctx, time.Second*1, func(ctx context.Context) error { | ||
| trace, err := e.downloader.Download(h, cadenceBlockID) | ||
| if err != nil { | ||
|
|
@@ -140,3 +148,44 @@ func (e *Engine) Error() <-chan error { | |
| func (e *Engine) Stop() { | ||
| e.MarkStopped() | ||
| } | ||
|
|
||
| // Backfill redownloads traces for blocks from EVM start to end height. | ||
| func (e *Engine) Backfill(start uint64, end uint64) { | ||
| select { | ||
| case <-e.Ready(): | ||
| case <-e.Done(): | ||
| return | ||
| } | ||
|
|
||
| e.logger.Info().Uint64("start", start).Uint64("end", end).Msg("backfilling traces") | ||
| for height := start; height <= end; height++ { | ||
| select { | ||
| case <-e.Done(): | ||
| return | ||
| case <-e.Stopped(): | ||
| return | ||
| default: | ||
| } | ||
|
|
||
| l := e.logger.With().Uint64("evm-height", height).Logger() | ||
|
|
||
| block, err := e.blocks.GetByHeight(height) | ||
| if err != nil { | ||
| l.Error().Err(err).Msg("failed to get block by height") | ||
| return | ||
| } | ||
|
Comment on lines
+175
to
+178
There was a problem hiding this comment. Modify error handling to continue backfilling despite retrieval errors. Currently, the backfill process exits if it encounters an error when retrieving a block or its Cadence ID (lines 175-178 and 185-188). This means that an error on a single block halts the entire backfill process. To improve robustness, consider logging the error and continuing with the next block instead of returning. Apply this diff to adjust the error handling: --- a/services/traces/engine.go
+++ b/services/traces/engine.go
@@ -176,7 +176,7 @@
if err != nil {
l.Error().Err(err).Msg("failed to get block by height")
- return
+ continue
}
@@ -186,7 +186,7 @@
if err != nil {
l.Error().Err(err).Msg("failed to get cadence block ID")
- return
+ continue
}Also applies to: 185-188 There was a problem hiding this comment. returning is intentional. if these blocks don't exist, it's unlikely future blocks will There was a problem hiding this comment. @peterargue, understood. If returning is intentional due to future blocks being unlikely to exist, no changes are necessary. (_/) ✏️ Learnings added
|
||
|
|
||
| if len(block.TransactionHashes) == 0 { | ||
| continue | ||
| } | ||
|
|
||
| cadenceID, err := e.blocks.GetCadenceID(block.Height) | ||
| if err != nil { | ||
| l.Error().Err(err).Msg("failed to get cadence block ID") | ||
| return | ||
| } | ||
|
|
||
| e.indexBlockTraces(block, cadenceID, true) | ||
|
There was a problem hiding this comment. 🛠️ Refactor suggestion Consider managing concurrency during backfill to prevent resource exhaustion. When calling There was a problem hiding this comment.
There was a problem hiding this comment. @peterargue, thank you for the clarification. It's good to know that 🐰😔 ✏️ Learnings added
|
||
| } | ||
| e.logger.Info().Uint64("start", start).Uint64("end", end).Msg("done backfilling traces") | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed something that could be problematic, though not really related to this PR.
The
traces-gcp-bucketaccepts a single bucket name, but formainnetwe already have 2:mainnet26-evm-execution-traces1mainnet25-evm-execution-traces1We need to make sure that when we back-fill, the provided heights exist in the appropriate bucket, but I don't know what is contained in those 2 buckets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a check that will log a warning if the start height is before the init-cadence height (378e60f). this may not work for nodes that have been up for a while, but not sure what else we can do without adding a bunch of complexity to the startup logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense 👍 Not much we can do for the time being. Just thought of mentioning it.