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

services/horizon: Enable BucketListDB by default in captive core #4733

Merged
merged 1 commit into from
Jan 23, 2023

Conversation

SirTyson
Copy link
Contributor

@SirTyson SirTyson commented Jan 5, 2023

Addresses #4710.

This PR exposes BucketListDB related configuration parameters in the captive-core toml file and enables BucketListDB by default in captive-core when using --captive-core-use-db. When enabled, BucketListDB reduces captive-core disk usage by around 50% and significantly reduces startup and catchup time.

This update exposes the captive core flags EXPERIMENTAL_BUCKETLIST_DB, EXPERIMENTAL_BUCKETLIST_DB_INDEX_PAGE_SIZE_EXPONENT, and EXPERIMENTAL_BUCKETLIST_DB_INDEX_CUTOFF. These flags control the memory usage of BucketListDB. See the stellar-core docs for more information on configuring these flags.

By default, captive-core will enable BucketListDB with a page size of 4 KB. This is different than the validator's default page size of 16 KB. While SDF's validators have NVME storage, the Horizon SKU uses EBS, requiring a different page size. If a Horizon operator is using storage medium other than AWS EBS, the page size parameter should be tuned accordingly.

With the default BucketListDB configuration, captive-core will use an additional ~2 GB of memory. However, this is partially offset by the memory savings from reducing the size of the SQL database. Should this memory overhead be too high, memory usage can be configured using the exposed configuration parameters.

This update will not automatically trigger a state rebuild. However, if EXPERIMENTAL_BUCKETLIST_DB is set to false in the captive-core toml, a state rebuild will be triggered.

@SirTyson SirTyson force-pushed the bucketsdb-captive-core branch from 9d1637d to 112f0f5 Compare January 5, 2023 23:42
@sreuland
Copy link
Contributor

sreuland commented Jan 9, 2023

@SirTyson , thanks for incorporating theses changes, was checking out that one CI test failure in ubuntu-20.04, 1.18.6, 9.6.5, captive-core-remote-storage, 19, which seems to be failing on TestClaimableBalanceCreationOperationsAndEffects, maybe merge latest from master back in here, and let's see if that clears up?

@mollykarcher
Copy link
Contributor

Very excited to see the difference this ends up making, thanks for testing this out! Would love it if you could share the results (+process) of the benchmarking/calibration you're doing once that's complete.

@SirTyson SirTyson force-pushed the bucketsdb-captive-core branch 3 times, most recently from f989c0c to 6e1f21a Compare January 12, 2023 18:20
@SirTyson
Copy link
Contributor Author

Very excited to see the difference this ends up making, thanks for testing this out! Would love it if you could share the results (+process) of the benchmarking/calibration you're doing once that's complete.

What would be the best format for something like this? It's a bit of an involved process where you have to build a special version of core and use some some low-level bench marking tools, but I could put a process doc together with some results if it's helpful.

@SirTyson SirTyson force-pushed the bucketsdb-captive-core branch 5 times, most recently from 41e9e45 to fc09c83 Compare January 13, 2023 18:58
@sreuland sreuland self-requested a review January 14, 2023 00:41
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.

looks good, approved, but before merging, let's get a review from at least one other horizon team member to confirm this approach with core runtime version checking is acceptable approach:

#4733 (comment)

thanks!

@SirTyson SirTyson force-pushed the bucketsdb-captive-core branch from fc09c83 to c3d60c9 Compare January 20, 2023 01:38
@SirTyson
Copy link
Contributor Author

Looks like all the comments have been addressed and the regex stuff has been reviewed, should be good to merge?

@sreuland
Copy link
Contributor

Looks like all the comments have been addressed and the regex stuff has been reviewed, should be good to merge?

@SirTyson , yes, I resolved those older comments, hopefully the repo allows you to proceed on merge button, if not lmk can click it, thanks again, great work!

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.

5 participants