Skip to content

Default to retrying after errors forever.#125

Closed
winder wants to merge 1 commit intoalgorand:masterfrom
winder:will/default-retry
Closed

Default to retrying after errors forever.#125
winder wants to merge 1 commit intoalgorand:masterfrom
winder:will/default-retry

Conversation

@winder
Copy link
Contributor

@winder winder commented Jul 21, 2023

Summary

On an errors, Indexer will retry forever. Users have expressed an appreciation for this behavior because they can perform maintenance on algod and it doesn't disrupt service on the downstream applications.

This changes Conduit's default retry on error behavior to have unlimited attempts, which matches how Indexer 2.x works.

@winder winder requested a review from a team July 21, 2023 15:39
@winder winder self-assigned this Jul 21, 2023
@winder winder marked this pull request as ready for review July 21, 2023 15:40
@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #125 (1c2f299) into master (442791a) will increase coverage by 2.61%.
The diff coverage is 76.23%.

@@            Coverage Diff             @@
##           master     #125      +/-   ##
==========================================
+ Coverage   67.66%   70.27%   +2.61%     
==========================================
  Files          32       36       +4     
  Lines        1976     2493     +517     
==========================================
+ Hits         1337     1752     +415     
- Misses        570      646      +76     
- Partials       69       95      +26     
Impacted Files Coverage Δ
conduit/data/block_export_data.go 100.00% <ø> (+92.30%) ⬆️
conduit/metrics/metrics.go 100.00% <ø> (ø)
conduit/pipeline/metadata.go 69.11% <ø> (ø)
...duit/plugins/exporters/filewriter/file_exporter.go 81.63% <ø> (-1.06%) ⬇️
conduit/plugins/exporters/postgresql/util/prune.go 78.43% <ø> (ø)
conduit/plugins/importers/algod/metrics.go 100.00% <ø> (ø)
...gins/processors/filterprocessor/fields/searcher.go 77.50% <ø> (ø)
...ins/processors/filterprocessor/filter_processor.go 83.82% <ø> (+3.54%) ⬆️
...plugins/processors/filterprocessor/gen/generate.go 34.28% <ø> (ø)
conduit/plugins/processors/noop/noop_processor.go 64.70% <ø> (+6.81%) ⬆️
... and 16 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

# Number of retries to perform after a pipeline plugin error.
# Set to 0 to retry forever.
retry-count: 10
retry-count: 0
Copy link
Contributor

@tzaffi tzaffi Jul 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally against making this be the default. In a happy path setup for an experienced user this may very well make sense. But for a newbie starting out and with some configuration that's off the effect will be to have conduit hang instead of failing fast.

But I wasn't aware that this was the default for indexer 2.0 and others should express their thoughts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was surprised too. The current default was made for pretty much the reason you're describing. Failing fast seems like a good thing. That said, several users brought up the behavior as something they appreciated about Indexer.

We don't retry during Init, so plugins have a chance to kill the app during startup if they want.

That said, I've considered adding some sort of retry for the importer to smooth over the case where Conduit starts before Algod.

@winder winder closed this Jul 21, 2023
@winder winder deleted the will/default-retry branch July 21, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants