github-actions[bot] commented on code in PR #69278:
URL: https://github.com/apache/doris/pull/69278#discussion_r4356985182
##########
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:
[P1] Make snapshot restore use the vault's rowset layout
`RESTORE ... PROPERTIES ("storage_vault_name"="<v1 S3 vault>")` now reaches
this version-1 resource, but `CloudSnapshotMgr::_create_rowset_meta` still maps
each destination segment to `{new_rowset_id}_{segment_id}.dat` and derives flat
index names. `CloudSnapshotLoader::download` appends those names to
`remote_tablet_path()`, writing `data/{shard}/{tablet}/{rowset}_0.dat`, while
the restored metadata makes normal reads request
`data/{shard}/{tablet}/{rowset}/0.dat`. The copy and metadata commit can
succeed and leave the restored table unreadable. Derive restore destinations,
including indexes, from the selected `StorageResource` and cover RESTORE into a
v1 vault.
##########
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:
[P1] Isolate S3 roots before enabling a different path layout
Two differently named modern S3 vaults can share the same bucket/root:
`ADD_S3_VAULT` only checks duplicate object configurations in legacy
`instance.obj_info()`, while new vaults are stored in separate KVs. If vault A
uses v0 with tablet 10005 and vault B uses v1 with `shard_num=100000`, B's live
tablet 44051 hashes to shard 10005 and stores files under
`data/10005/44051/<rowset>/`. Dropping A's table makes
`InstanceRecycler::recycle_tablet` recursively delete `data/10005/` through A's
accessor, deleting B's committed files too. Persisting the S3 v1 format makes
this cross-layout overlap reachable. Reject incompatible shared roots against
existing vaults, or keep their physical namespaces disjoint, and cover
recycling isolation.
--
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]
