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

ingest/ledgerbackend: Update captive-core config with new BucketlistDB parameters #5333

Merged
merged 11 commits into from
Jun 24, 2024

Conversation

urvisavla
Copy link
Contributor

@urvisavla urvisavla commented Jun 6, 2024

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into two PRs
    otherwise).
  • This PR's title starts with name of package that is most changed in the PR, ex.
    services/friendbot, or all or doc if the changes are broad or impact many
    packages.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or fixes.
  • I've updated any docs (developer docs, .md
    files, etc... affected by this change). Take a look in the docs folder for a given service,
    like this one.

Release planning

  • I've updated the relevant CHANGELOG (here for Horizon) if
    needed with deprecations, added features, breaking changes, and DB schema changes.
  • I've decided if this PR requires a new major/minor version according to
    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

  • Adds the new stellar-core BucketListDB related config parameter DEPRECATED_SQL_LEDGER_STATE. It also addresses the EXPERIMENTAL_BUCKETLIST_DB parameter, which will be completely removed in the next Stellar-core release, making DEPRECATED_SQL_LEDGER_STATE mandatory.

  • Horizon currently has a an option called CAPTIVE_CORE_USE_DB (to be deprecated) to use captive-core in in-memory mode. The default value of DEPRECATED_SQL_LEDGER_STATE is determined based on this flag: true for in-memory mode and false for the on-disk (preferred) mode. This check will not be necessary once the CAPTIVE_CORE_USE_DB flag is fully removed.

  • It also updates the parameter names: EXPERIMENTAL_BUCKETLIST_DB_INDEX_PAGE_SIZE_EXPONENT to BUCKETLIST_DB_INDEX_PAGE_SIZE_EXPONENT and EXPERIMENTAL_BUCKETLIST_DB_INDEX_CUTOFF to BUCKETLIST_DB_INDEX_CUTOFF

This should not be merged until protocol 21 has been voted on and everyone has had the chance to update to stellar-core 21.

Why

Addresses #5295

Known limitations

This PR completely removes the EXPERIMENTAL_BUCKETLIST_DB flag, which we currently use in our deployment. This means we will need to update our deployment scripts as part of the releasing this change.

@urvisavla urvisavla added the WIP label Jun 6, 2024
@urvisavla urvisavla force-pushed the 5295/update-bucketlist-config branch 5 times, most recently from 69597b1 to 2a4e60d Compare June 7, 2024 07:49
@urvisavla urvisavla changed the title (WIP) Update bucketlistdb config parameters ingest/ledgerbackend: Update captive-core config with new BucketlistDB parameters Jun 7, 2024
@urvisavla urvisavla force-pushed the 5295/update-bucketlist-config branch 3 times, most recently from 24525e6 to bba243e Compare June 12, 2024 00:24
@urvisavla urvisavla removed the WIP label Jun 12, 2024
@urvisavla urvisavla marked this pull request as ready for review June 12, 2024 21:08
@urvisavla urvisavla requested a review from a team June 12, 2024 22:27
Copy link
Contributor

@sreuland sreuland left a comment

Choose a reason for hiding this comment

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

nice work, left minor comments for consideration.

@sreuland
Copy link
Contributor

since EXPERIMENTAL_BUCKETLIST_DB is now removed, it effectively means any horizon ingestion node will only work when captive core bin on host is min v21 or higher. So, should this include a sanity check do be run as part of captive core init of horizon ingestion startup, to fail fast with error on command line, if captive core bin version is less than v21? via toml.go coreVersion.IsEqualOrAbove

@urvisavla urvisavla force-pushed the 5295/update-bucketlist-config branch from 827c6b0 to 1c64e10 Compare June 20, 2024 20:57
@urvisavla urvisavla force-pushed the 5295/update-bucketlist-config branch from 1c64e10 to 98f8b97 Compare June 20, 2024 23:03
@urvisavla urvisavla merged commit 90c18e2 into stellar:master Jun 24, 2024
23 checks passed
@urvisavla urvisavla deleted the 5295/update-bucketlist-config branch June 24, 2024 05:28
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.

3 participants