ct/l1/metastore: adjustments to pre-opening and add configurations#30849
Open
andrwng wants to merge 5 commits into
Open
ct/l1/metastore: adjustments to pre-opening and add configurations#30849andrwng wants to merge 5 commits into
andrwng wants to merge 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors LSM “pre-open” behavior into an asynchronous background prewarm task and wires new cloud-topics L1 metastore configuration knobs (block cache size, write buffer size, and prewarm concurrency) into the LSM open options.
Changes:
- Add
version::contains()and use it during prewarm to avoid warming SSTs that have fallen out of the live version. - Move SST pre-opening from
recover()to a best-effort background “prewarm” started after open (including readonly opens), and update the unit test accordingly. - Introduce new
cloud_topics_metastore_*configuration properties and pass them into metastore/read-replica LSM open options.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/v/lsm/db/version_set.h |
Adds version::contains() declaration for checking handle liveness in a version. |
src/v/lsm/db/version_set.cc |
Implements version::contains() via linear scan across levels/files. |
src/v/lsm/db/impl.h |
Declares new prewarm helpers (maybe_start_prewarm, prewarm). |
src/v/lsm/db/impl.cc |
Starts prewarm as a background task after open; adds skipping of non-live SSTs; adjusts readonly open path. |
src/v/lsm/db/tests/impl_test.cc |
Updates PreOpenFiles test to account for asynchronous prewarm (draining task queue). |
src/v/config/configuration.h |
Adds new cloud-topics metastore config properties. |
src/v/config/configuration.cc |
Defines new config properties (defaults + bounds + help text). |
src/v/cloud_topics/read_replica/snapshot_manager.cc |
Plumbs metastore prewarm + block cache config into readonly LSM open. |
src/v/cloud_topics/level_one/metastore/lsm/replicated_db.cc |
Plumbs metastore write buffer, prewarm, and block cache config into replicated LSM open. |
448abd8 to
15d0fcd
Compare
Collaborator
CI test resultstest results on build#86019
test results on build#86110
|
recover() previously had the option to pre-open every SST synchronously. For the metastore (using cloud cache persistenc), this meant it would download each SST and keep a file handle in the SST cache until it was evicted. At smaller scales this would be fine, but with larger SSTs, this prerequisite to download everything can be unstable, since requests to the metastore can't be served during this pre-opening phase. This commit makes this background work instead, such that the initial opening of the database is quick, but the work to pre-warm everything else still happens, just in the background.
Adds cloud_topics_metastore_max_pre_open_fibers cluster config, wired into the lsm::options used by replicated_database::open. When set to N > 0, lsm::db::impl::recover walks every level's files and warms the table cache via N concurrent fibers before returning from open. Because a metastore replicated_database is constructed each time a node takes leadership for a metastore_partition, this pays the SST open cost up front on leadership transfer, so subsequent reads hit warm file handles instead of contending on the per-handle table_cache loader lock (the cold-path wait covered by sst_loader_wait_*). Default is 10 (somewhat arbitrarily chosen, but seems like a decently balanced value).
Allow for configuring the write buffer size. I haven't used this in practice, but it can be helpful to e.g. define target sizes within each level if we need it.
Expose the LSM block cache size as a tunable. I left the default as its existing default, but allowing it to be tunable makes testing easier and also gives us flexibility to change it in the future in deployments that could benefit from it.
Also does some test plumbing to supply a real cache to snapshot_manager_test, since the pre-warming will require a cache.
15d0fcd to
4537d17
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes a few tweaks to the opening of the metastore:
Backports Required
Release Notes