Revert "Merge pull request #29673 from redpanda-data/l1-readahead"#29757
Merged
oleiman merged 1 commit intoredpanda-data:devfrom Mar 5, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reverts the previously introduced L1 “readahead/lookahead” functionality and the related “include object metadata” path for extent metadata fetching in the Cloud Topics L1 metastore/reader.
Changes:
- Removes
lookahead_objectsfromcloud_topic_log_reader_configand deletes the L1 reader lookahead buffer/code paths and associated tests. - Simplifies
metastore::get_extent_metadata_forwards(and RPC equivalents) by removing theinclude_object_metadataparameter and strippingobject_infofrom extent metadata. - Updates metastore implementations (simple/replicated/db/simple domain) and unit tests to match the reverted APIs.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/v/cloud_topics/log_reader_config.h |
Removes L1 lookahead configuration field. |
src/v/cloud_topics/level_one/metastore/metastore.h |
Removes include_object_metadata and object_info from extent metadata API/model. |
src/v/cloud_topics/level_one/metastore/simple_metastore.{h,cc} |
Updates extent-metadata forward path to match reverted API and drops object metadata population. |
src/v/cloud_topics/level_one/metastore/replicated_metastore.{h,cc} |
Updates forward extent-metadata RPC wrapper and conversion logic. |
src/v/cloud_topics/level_one/metastore/rpc_types.h |
Removes object-metadata fields from extent metadata / request serialization. |
src/v/cloud_topics/level_one/domain/{simple_domain_manager.cc,db_domain_manager.cc} |
Removes object-metadata mapping/fetching from extent metadata RPC handling. |
src/v/cloud_topics/level_one/metastore/extent_metadata_reader.cc |
Updates call site for reverted metastore API. |
src/v/cloud_topics/level_one/metastore/tests/simple_metastore_test.cc |
Updates tests for reverted API; removes object-metadata-specific test. |
src/v/cloud_topics/level_one/frontend_reader/level_one_reader.{h,cc} |
Removes lookahead buffer and switches back to single-object lookup via get_first_ge. |
src/v/cloud_topics/level_one/frontend_reader/tests/{l1_reader_fixture.h,reader_test.cc} |
Removes lookahead plumbing and tests. |
Comment on lines
12
to
22
| #include "cloud_topics/level_one/common/object_id.h" | ||
| #include "cloud_topics/level_one/metastore/domain_uuid.h" | ||
| #include "cloud_topics/level_one/metastore/offset_interval_set.h" | ||
| #include "cloud_topics/level_one/metastore/state_update.h" | ||
| #include "model/fundamental.h" | ||
| #include "serde/envelope.h" | ||
| #include "serde/rw/enum.h" | ||
| #include "serde/rw/envelope.h" | ||
| #include "serde/rw/optional.h" | ||
|
|
||
| #include <fmt/format.h> | ||
|
|
There was a problem hiding this comment.
rpc_types.h still uses std::optional (e.g., get_compaction_info_reply::earliest_dirty_ts is serialized), but this diff removed both <optional> and serde/rw/optional.h. This will either fail to compile or rely on transitive includes/serde specializations. Re-add the missing includes (or otherwise ensure std::optional and its serde read/write support are directly included here).
piyushredpanda
approved these changes
Mar 5, 2026
oleiman
force-pushed
the
ci/noticket/revert-l1-lookahead
branch
from
e14a60b to
a54a229
Compare
Collaborator
CI test resultstest results on build#81415 |
7 tasks
wdberkeley
added a commit
that referenced
this pull request
Mar 5, 2026
Re-applies the changes from #29673 (reverted in #29757) with two fixes: 1. TestGetExtentMetadataForwardsWithObjectMetadata now calls preregister_objects before add_objects, required after #29739 made preregistration mandatory. 2. l1_reader_cache::evict_stale() had a co_await inside its iteration loop. During suspension, take_reader() on the fetch scheduling group could erase entries from the intrusive list, invalidating the iterator. Fix by collecting stale readers without yielding, then closing them after the loop — matching the pattern already used by stop().
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 reverts commit 2ea7c06, reversing changes made to 250b0f7.
Backports Required
Release Notes