cloud_storage: re-adopt topic_manifest #19666
Merged
Conversation
This file contains 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
The file is huge and means that we are required to include v_cluster if we want to use the topic_manifest which just serializes the topic properties. This commit moves properties to their own file. A subsequent commit will make it its own independent library.
|
new failures in https://buildkite.com/redpanda/redpanda/builds/50129#0190095e-9bed-4d82-8181-4144f1b198ee: new failures in https://buildkite.com/redpanda/redpanda/builds/50151#01900b48-691c-4efb-93d3-fe7fd572d4bb: new failures in https://buildkite.com/redpanda/redpanda/builds/50151#01900bba-fff1-4c31-a003-90c278a4f0c6: |
|
CI failure: #14225 |
dotnwat
previously approved these changes
Jun 12, 2024
Comment on lines
-220
to
-222
| ../cloud_storage/topic_manifest.cc | ||
| HDRS | ||
| ../cloud_storage/topic_manifest.h |
This pulls out topic configuration out of types.h so it can more easily be depended on. I intend to have cloud_storage depend on just the topic configuration so the manifest code can move back into v_cloud_storage.
andrwng
force-pushed
the
cluster-topic-properties
branch
from
58f7aa0 to
d617040
Compare
dotnwat
reviewed
Jun 12, 2024
Now that topic_properties don't depend on the rest of types.h, this commit introduces a new v_cluster_topic_properties that cloud_storage can depend on without introducing a circular dependency. With this in hand, this commit pull topic_manifest back into v_cloud_storage. The motivation here is that I have some upcoming changes for downloading topic manifests that feel much more natural being in v_cloud_storage.
andrwng
force-pushed
the
cluster-topic-properties
branch
from
d617040 to
c730b83
Compare
dotnwat
approved these changes
Jun 12, 2024
andijcr
approved these changes
Jun 12, 2024
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.
First, moves topic configuration out of types.h.
The file is huge and means that we are required to include v_cluster if
we want to use the topic_manifest which just serializes the topic
properties.
Now that topic_properties don't depend on the rest of types.h, this
commit introduces a new v_cluster_topic_properties that cloud_storage
can depend on without introducing a circular dependency. With this in
hand, this commit pull topic_manifest back into v_cloud_storage.
The motivation here is that I have some upcoming changes for downloading
topic manifests that feel much more natural being in v_cloud_storage.
Backports Required
Release Notes