Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add retry to fetcher #2297

Merged
merged 3 commits into from
Jul 23, 2023
Merged

Conversation

victorkirov
Copy link
Contributor

@victorkirov victorkirov commented Jul 22, 2023

Resolves: #2248

Sometimes the transaction fetcher receives a non-json response from bitcoin core, resulting in a parsing error. It seems to happen due to load on the node. This is problematic because it will completely fail a batch of processed blocks. With the batch size set to 5000, if it fails a few thousand blocks in, it will be a lot of work to start over again.

This adds a retry which tries to fetch transactions with 5 retries which exponentially back off (Max being 3.2seconds).

@victorkirov
Copy link
Contributor Author

If this should go in, maybe it should be a back off retry and maybe a few more times (5 total maybe?). Let me know.

Copy link

@nick07002 nick07002 left a comment

Choose a reason for hiding this comment

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

Great stuff! This is going to help a lot of people overcome their indexing hiccups

src/index/fetcher.rs Outdated Show resolved Hide resolved
@victorkirov
Copy link
Contributor Author

Great. I'll fix up the logging and make it a back off retry a bit later today when I'm at my pc.

Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

Thanks, the logic looks good so far! Just some minor tweaks needed.

src/index/fetcher.rs Outdated Show resolved Hide resolved
src/index/fetcher.rs Outdated Show resolved Hide resolved
src/index/fetcher.rs Outdated Show resolved Hide resolved
src/index/fetcher.rs Outdated Show resolved Hide resolved
src/index/fetcher.rs Outdated Show resolved Hide resolved
src/index/fetcher.rs Outdated Show resolved Hide resolved
@victorkirov
Copy link
Contributor Author

Thanks for the review @raphjaph. I think it should all be sorted 🙂

@raphjaph raphjaph merged commit 919e010 into ordinals:master Jul 23, 2023
6 checks passed
@victorkirov victorkirov deleted the fix/fetch-error branch July 24, 2023 05:30
sidmorizon added a commit to OneKeyHQ/ord that referenced this pull request Aug 1, 2023
* Add contributing section (ordinals#2261)

* Implement clean index shutdown to prevent index corruption (with clippy updates for Rust 1.71) (ordinals#2275)

* gracefully shutdown index update thread to prevent index corruption

* Use `next_back()` instead of `rev().next()` for rust 1.71

---------

Co-authored-by: victorkirov <victor.kirov@gmail.com>

* Modify `ord list` output to include the end of each range (ordinals#1998)

* Don't create default data directory if --index overrides it (ordinals#1991)

* Fix docs inconsistency (ordinals#2276)

* Fix ordering for reinscriptions and show all reinscriptions for sat (ordinals#2279)

* Add satpoint and address to index export (ordinals#2284)

* Update bitcoin dependencies (ordinals#2281)

* Update redb (ordinals#2294)

* Add retry to fetcher (ordinals#2297)

* Clean up deploy scripts (ordinals#2298)

* Fix justfile recipe (ordinals#2299)

* Release 0.8.1 (ordinals#2300)

* Add `amount` field to `wallet inscriptions` output. (ordinals#1928)

* Fix dust limit for padding in `TransactionBuilder` (ordinals#1929)

* Inform user when redb starts in recovery mode (ordinals#2304)

* Fix remote RPC wallet commands (ordinals#1766)

* Select multiple utxos (ordinals#2303)

Co-authored-by: Greg Martin <gm7t2@gmail.com>

* feat: add outputs api

---------

Co-authored-by: raph <raphjaph@protonmail.com>
Co-authored-by: victorkirov <victor.kirov@gmail.com>
Co-authored-by: gmart7t2 <49558347+gmart7t2@users.noreply.github.com>
Co-authored-by: ordinally <11798624+veryordinally@users.noreply.github.com>
Co-authored-by: Carlos Alaniz <carlosglvn93@gmail.com>
Co-authored-by: Greg Martin <gm7t2@gmail.com>
Raylin51 pushed a commit to 0xaklabs/ord that referenced this pull request Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Couldn't receive txs expected value at line 1 column 1
3 participants