-
Notifications
You must be signed in to change notification settings - Fork 543
ingest: enable captive core online replay ledger sequence 2 when unbounded #5873
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
Conversation
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.
Pull request overview
This PR fixes an issue where captive core fails to replay ledger sequence 2 when configured with an unbounded range (end=0) starting from ledger 2. The fix modifies the captive core runner to set the CATCHUP_COMPLETE=true configuration flag in stellar-core when the starting ledger is less than 3.
Key changes:
- Added
CatchupCompletefield to the TOML configuration struct to support theCATCHUP_COMPLETEstellar-core config option - Modified the run-from logic to detect when ledger sequence < 3 is requested and set
CATCHUP_COMPLETE=trueinstead of running catchup - Implemented TOML cloning in
newWorkingDirto avoid side effects when modifying configuration for specific runs - Added comprehensive error handling for the new TOML cloning operations
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ingest/ledgerbackend/toml.go | Added CatchupComplete boolean field to support the stellar-core CATCHUP_COMPLETE configuration option |
| ingest/ledgerbackend/stellar_core_runner.go | Updated runFrom and catchup methods to handle errors from stream creation functions |
| ingest/ledgerbackend/run_from.go | Modified logic to skip catchup and enable CATCHUP_COMPLETE when starting from ledger sequence < 3; updated newRunFromStream to return errors |
| ingest/ledgerbackend/dir.go | Implemented TOML config cloning in newWorkingDir to prevent side effects; added enableCoreCatchupComplete method to set the flag |
| ingest/ledgerbackend/catchup.go | Updated newCatchupStream to handle and propagate errors from newWorkingDir |
| ingest/ledgerbackend/stellar_core_runner_test.go | Added TestRunFromRequestedSequence2 to verify correct behavior when starting from ledger 2 |
| CHANGELOG.md | Documented the bug fix for ledger sequence 2 replay issue |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Apply suggestion for evaluating db wipe conditions based on suggestion from @tamirms Co-authored-by: tamirms <[email protected]>
|
Hello @tamirms , I incorporated feedback, s/b ready for re-review, thanks! |
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
CHANGELOG.mdwithin the component folder structure. For example, if I changed horizon, then I updated (services/horizon/CHANGELOG.md. I add a new line item describing the change and reference to this PR. If I don't update a CHANGELOG, I acknowledge this PR's change may not be mentioned in future release notes.semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
Changed the captive core runner to modify toml to include CATCHUP_COMPLETE core config flag when an unbounded ledger range(end=0) and a from=2 is requested
Why
Client ingestion that uses the captive core runner with an unbounded range(end=0) and start=2 will get stuck waiting for sequence 2 to be emitted by core during
runon the meta pipe which by default core will start emitting from 3, unless CATCHUP_COMPLETE=true is configured.Closes #5866