Skip to content

Commit

Permalink
Merge pull request redpanda-data#22676 from WillemKauf/topic_mount_mi…
Browse files Browse the repository at this point in the history
…nor_fixes

`cloud_storage`: minor fixes in `topic_mount_handler`
  • Loading branch information
WillemKauf authored Aug 1, 2024
2 parents 64d7f01 + 1cce41d commit 35cedc8
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 9 deletions.
17 changes: 9 additions & 8 deletions src/v/cloud_storage/tests/topic_mount_handler_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ static const model::topic_namespace test_tp_ns_override{
static ss::abort_source never_abort;
static constexpr model::cloud_credentials_source config_file{
model::cloud_credentials_source::config_file};

static cluster::topic_configuration
get_topic_configuration(cluster::topic_properties topic_props) {
auto topic_cfg = cluster::topic_configuration(
test_tp_ns.ns, test_tp_ns.tp, 1, 1);
topic_cfg.properties = std::move(topic_props);
return topic_cfg;
}

} // namespace

struct TopicMountHandlerFixture
Expand All @@ -64,14 +73,6 @@ struct TopicMountHandlerFixture
pool.stop().get();
}

cluster::topic_configuration
get_topic_configuration(cluster::topic_properties topic_props) const {
auto topic_cfg = cluster::topic_configuration(
test_tp_ns.ns, test_tp_ns.tp, 1, 1);
topic_cfg.properties = std::move(topic_props);
return topic_cfg;
}

ss::sharded<cloud_storage_clients::client_pool> pool;
ss::sharded<remote> remote;
};
Expand Down
6 changes: 6 additions & 0 deletions src/v/cloud_storage/topic_mount_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ ss::future<topic_mount_result> topic_mount_handler::check_mount(
_bucket, manifest_path, parent, existence_check_type::manifest);

// If the manifest exists, it is possible to mount the topic.
// If the result is anything but download_result::success, it is
// assumed that the mount manifest does not exist, or is unreachable for
// whatever reason. In that case, it is simple enough for the user to
// reissue a request to `mount_topic()`.
co_return (exists_result == download_result::success)
? topic_mount_result::success
: topic_mount_result::mount_manifest_does_not_exist;
Expand All @@ -72,6 +76,8 @@ ss::future<topic_mount_result> topic_mount_handler::commit_mount(
const auto delete_result = co_await _remote.delete_object(
_bucket, manifest_path, parent);

// If the upload_result is anything but upload_result::success,
// the mount manifest was not deleted, and we cannot mount the topic.
co_return (delete_result == upload_result::success)
? topic_mount_result::success
: topic_mount_result::mount_manifest_not_deleted;
Expand Down
2 changes: 1 addition & 1 deletion src/v/cloud_storage/topic_mount_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class topic_mount_handler {
ss::future<topic_mount_result> mount_topic(
const cluster::topic_configuration& topic_cfg, retry_chain_node& parent);

// Perform the mounting process by deleting the topic mount manifest.
// Perform the unmounting process by uploading the topic mount manifest.
// topic_cfg should be the recovered topic configuration from a topic
// manifest in cloud storage. If it has a value, the remote_label stored in
// the topic properties is used as the "source" label. Otherwise, the
Expand Down

0 comments on commit 35cedc8

Please sign in to comment.