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

cloud_storage: remote labels #20778

Open
wants to merge 34 commits into
base: dev
Choose a base branch
from
Open

Conversation

andrwng
Copy link
Contributor

@andrwng andrwng commented Jun 29, 2024

This PR introduces remote labels as a new topic property. Remote labels today
only contain the originating cluster's UUID, though in the future this may be
extended to also include something user-provider. Once a remote label is set
for a topic, it cannot be changed.

When set, subcomponents within the partition of the topic will use this label
as the basis for generating paths for remote objects via a new
remote_path_provider that is owned by the archival_metadata_stm.

A large chunk of this PR entails clearing out vestiges of path-decision-making
from the cloud_storage::remote, instead relying on recently added
topic/partition manifest downloader classes, which take into account remote
labels.

The high level structure of this PR is:

  • preemptive change to offline_log_viewer to be able to read the remote label
    (needed for tests, which use the viewer to decode topic manifests)
  • introduce the remote label and plug it into the archival metadata stm,
    defaulting to the old style of naming for now
  • plug in the recently added topic_manifest_downloader code where appropriate
  • plug in the recently added partition_manifest_downloader code where
    appropriate
  • remove newly dead code subsumed by the downloaders
  • plug in path providers to the rest of the calls within partition
    (remote_partition, purger, segment merger, anomaly detector, etc)
  • update ducktape test utilities to support the new naming scheme, and support
    grouping metadata per label
  • plug in the actual cluster UUID into new topics, so they use new paths
  • remove newly dead code / rid manifest code of path generation
  • simple ducktape test to show things working

A new configuration is also added as a part of this PR that will be useful in
general for testing, to disable the addition of the remote label for new
topics.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

  • none

@andrwng
Copy link
Contributor Author

andrwng commented Jun 29, 2024

/ci-repeat

@andrwng
Copy link
Contributor Author

andrwng commented Jun 29, 2024

/ci-repeat

@andrwng
Copy link
Contributor Author

andrwng commented Jun 29, 2024

/cdt

andrwng added 25 commits July 1, 2024 08:26
This is a preemptive commit for the upcoming addition of remote labels
to the topic properties.
This plumbs the remote label (cluster uuid) associated with a topic from
the topics table into the archival meta stm, in the same fashion that
vcluster-id is plumbed into the rm_stm.

Conceptually, the archival STM will be the owner of the root remote path
provider used throughout the cloud_storage and archival subsystems.
This replaces path generation in ntp_archiver with variants that use the
path provider. Namely:
- topic manifest uploads
- partition manifest uploads
- segment uploads
- generating spillover manifests
This plumbs the archival stm's path provider into the manifest view. The
manifest view serves as the underlying abstraction to the
remote_partition class, so this will be necessary for getting the path
provider to be used on the remote read path.
Topic recovery need to be able to find a topic manifest whose format
(serde or json) is not yet known. This is handled transparently by the
topic manifest downloader, while also taking into account remote labels.

This commit updates the create topic path to use the downloader.
Automated topic recovery performs a list bucket operation to discover
what topics can be restored. This commit updates this to account for
topic manifests labeled with the cluster UUID.
Snapshot recovery needs to be able to find a partition manifest whose
format (serde or json) is not yet known. This is handled transparently
by the partition manifest downloader, while also taking remote labels
into account.

This commit updates the STM to use the downloader for recovery.
Read replicas need to be able to find a partition manifest whose format
(serde or json) is not yet known. This is handled transparently by the
partition manifest downloader, while also taking remote labels into
account.

This commit updates the archiver to use the downloader for the read
replica loop.
The partition recovery manager downloads the partition manifest and
remote segments to be able to create a "deep" copy of the remote replica
that is served by a local Raft log.

This recovery needs to be able to find a partition manifest whose format
(serde or json) is not yet known. This is handled transparently by the
partition manifest downloader, while also taking remote labels into
account.

On top of this, with the naming format of all objects (including
segments) changing, the recovery needs to be able to know what remote
label to use, if any.

To that end, this commit updates the partition recovery path to use a
path provider and partition manifest downloader.

Note, while the source cluster uuid of the path providers will
ultimately be the same, this is a different path provider than what will
be used at runtime by the partition. The latter is owned by the archival
metadata STM and its lifecycle is therefore tied to the partition. The
former is transient and only lives for as long as the recovery is
happening (it is constructed slightly before the partition is created).
Before allowing a recovery topic creation, the controller leader will
perform some validation on the partitions and segments.

During this process, Redpanda may not know the format of the partition
manifests (serde or json), and segment paths may include a cluster uuid
moving forward.

The former is handled transparently by the partition manifest
downloader, and the latter can be addressed by using a path provider
directly.
The unsafe reset method needs to be able to find a partition manifest
whose format (serde or json) is not yet known. This is handled
transparently by the partition manifest downlaoder, while also taking
remote labels into account.

This commit updates the method  to use the downloader.
…detector

The anomaly detector (and by extension, the scubber)  needs to be able
to find a partition manifest whose format (serde or json) is not yet
known. This is handled transparently by the partition manifest
downlaoder, while also taking remote labels into account.

This commit updates anomaly detector to use the downloader, and plugs in
the archiver's path provider from the scrubber.

As a part of this move, there's an anomalous case that is removed: when
we detect a JSON manifest that's a spillover manifest. Redpanda never
published JSON spillover manifests, so indeed this would be anomalous.
In practice this should never be the case, and this commit just removes
it, since it depends on knowing the downloaded format, which is no
longer the case when using the downloader.
This has been subsumed by the partition manifest downloader. Swaps out
the remaining (test-only) usages of the method for calls into the
partition_manifest_downloader.
Callers should use the partition_manifest_downloader instead.
The manifest view transparently downloads and caches spillover manifests
when Redpanda reads in the spillover region. This commit updates the
relevant path generation code to use the path provider.

The cursor doesn't need the path provider for anything but logging, so
instead of logging the full paths, this just has the cursor log the
filenames.
By extension, this also plugs the path provider into
remote_partition::erase(), which is only used by the purger.
Topic/partition manifests are by far the biggest offender of things that
generate paths without the path provider that really should be using the
path provider (because their paths naturally collide across clusters in
the same bucket).

Thus far, we've been switching the important usages of path generation
to use the path provider, which helps avoid collisions. This commit
tightens things up a bit, cleaning up the remaining usages of
manifest-generated paths.

This means removal of:
- static manifest path generation methods
- format-focused interfaces, which were generally not using the format
  for anything important (mostly logging)
- methods in the manifest mentioning "legacy" formats (now
  subsumed by path utils and the path provider)
- importantly, remote::upload_manifest(), the most important offender of
  using paths generated by the manifest without consulting the path
  provider

Several tests and log lines are updated to accomodate this cleanup.
This cleans up the remaining test-only usages of methods in the
partition manifest that would create remote segment paths without the
path provider.
This is no longer used, now that the topic_manifest class isn't in
charge of naming or deciding serialization format.
It's generally unimportant, and most admin endpoints don't require thie
field (they select one for the caller).
@andrwng andrwng force-pushed the remote-labels branch 2 times, most recently from 5b5661a to 38ee5f1 Compare July 1, 2024 16:32
@andrwng
Copy link
Contributor Author

andrwng commented Jul 1, 2024

/ci-repeat

BucketViews are typically used to examine the state of a bucket in the
context of a test. With objects proceeding to be include a given
cluster's UUID, this updates the view to know how to interpret the UUID.

This means most metadata scructures in the BucketView now account for a
label.
This is going to be useful in testing multiple clusters in a single
bucket.
This only takes effect for topics that aren't being read or restored
from the cloud.
With this activated in tests, updates several tests that now need to
account for the new paths.
Adds a basic test for remote labeling.
@andrwng andrwng force-pushed the remote-labels branch 2 times, most recently from 972ee44 to 1603f89 Compare July 1, 2024 22:31
@andrwng andrwng marked this pull request as ready for review July 1, 2024 23:30
@andrwng andrwng requested a review from a team as a code owner July 1, 2024 23:30
@andrwng andrwng added this to the v24.2.1-rc2 milestone Jul 2, 2024
@andrwng
Copy link
Contributor Author

andrwng commented Jul 2, 2024

There's still a CI failure (likely a test issue), but I think this otherwise ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant