matanper commented on code in PR #69278:
URL: https://github.com/apache/doris/pull/69278#discussion_r4377070151
##########
cloud/src/meta-service/meta_service_resource.cpp:
##########
@@ -1538,6 +1570,9 @@ void
MetaServiceImpl::alter_storage_vault(google::protobuf::RpcController* contr
*instance.mutable_resource_ids()->Add() = vault.id();
*instance.mutable_storage_vault_names()->Add() = vault.name();
vault.mutable_obj_info()->MergeFrom(last_item);
+ if (request->vault().has_path_format()) {
+
vault.mutable_path_format()->CopyFrom(request->vault().path_format());
Review Comment:
Addressed. `check_s3_root_path_version` (959c6ae649b, ed0bfb846bb) rejects
an S3 vault whose path_version differs from any storage on the same root. It
reads every storage vault KV of the instance, not only the legacy `obj_info`.
It runs on both `ADD_S3_VAULT` and `add_vault_into_instance` (create_instance,
ADD_BUILT_IN_VAULT, ADD_HDFS_INFO with an obj info). Same-version vaults may
share a root: tablet ids are unique, and a v1 vault only deletes
`data/{shard}/{tablet_id}/`. A mixed v0/v1 root is now refused at creation, so
the recycling overlap you describe is not reachable.
5cf5de9416c also stops comparing the endpoint, so aliases of one bucket
(`https://…`, regional vs global, internal vs public) can't get around the
check. Test: `MetaServiceTest.CreateS3VaultRejectsMixedPathVersionOnSameRoot`
(both orders, trimmed prefixes, endpoint aliases, legacy obj_info,
add_vault_into_instance).
##########
be/src/cloud/cloud_meta_mgr.cpp:
##########
@@ -1909,18 +1909,18 @@ Status
CloudMetaMgr::get_storage_vault_info(StorageVaultInfos* vault_infos, bool
*is_vault_mode = resp.enable_storage_vault();
- auto add_obj_store = [&vault_infos](const auto& obj_store) {
+ // Legacy object store infos have no path format, they always use path
version 0
+ std::ranges::for_each(resp.obj_info(), [&](const auto& obj_store) {
vault_infos->emplace_back(obj_store.id(),
S3Conf::get_s3_conf(obj_store),
StorageVaultPB_PathFormat {});
- };
-
- std::ranges::for_each(resp.obj_info(), add_obj_store);
+ });
std::ranges::for_each(resp.storage_vault(), [&](const auto& vault) {
if (vault.has_hdfs_info()) {
vault_infos->emplace_back(vault.id(), vault.hdfs_info(),
vault.path_format());
}
if (vault.has_obj_info()) {
- add_obj_store(vault.obj_info());
+ vault_infos->emplace_back(vault.obj_info().id(),
S3Conf::get_s3_conf(vault.obj_info()),
+ vault.path_format());
Review Comment:
Addressed in f0c67f89fb8. `CloudSnapshotMgr::_create_rowset_meta` maps each
backup file (`{rowset}_{seg}.dat` and the V1/V2 index files) to its full target
path. It builds that path with the target `StorageResource`'s layout, using the
same `cpp/remote_path.h` functions BE reads with.
`CloudSnapshotLoader::download` writes to that path instead of appending a flat
name to `remote_tablet_path()`. Test:
`CloudSnapshotMgrTest.ConvertRowsetsMapsFilesToTargetLayout` (v1 with index
format V1 and V2, v0 unchanged).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
