matanper opened a new pull request, #69278:
URL: https://github.com/apache/doris/pull/69278
### What problem does this PR solve?
Issue Number: close #68790
Related PR: None
Problem Summary:
Storage vaults support a sharded object key layout (`path_version = 1`,
`shard_num = N`, keys `data/{murmur64(tablet_id) %
N}/{tablet_id}/{rowset_id}/{seg}.dat`),
which spreads writes of sequentially allocated tablets across key prefixes
to avoid S3
`SlowDown`/503 throttling under heavy ingest. It could not be used for S3
vaults:
1. FE: `CREATE STORAGE VAULT ... ("path_version" = "1", "shard_num" =
"1024")` always failed
with `java.lang.UnsupportedOperationException`, because
`CreateStorageVaultCommand#validate`
called `remove()` on a Guava `ImmutableMap`. Invalid values (e.g.
`shard_num = 0`, which
divides by zero in BE) were not rejected.
2. Meta-service: the `ADD_S3_VAULT` branch built a new `StorageVaultPB` with
only id, name and
obj_info, so `path_format` was silently dropped (HDFS vaults kept it).
3. BE: `CloudMetaMgr::get_storage_vault_info` built S3 vaults with an empty
`PathFormat`, so
they always used version 0.
4. Even with the format propagated, version 1 was unsafe:
- File cache keys were the file name only; under version 1 every rowset's
first segment is
`0.dat`, so different rowsets/tablets would share one cache entry and
return wrong data.
- Recycler and checker hard-coded the version 0 layout, so version 1
objects would never be
deleted after DROP/TRUNCATE/compaction, and the checker would report
false data loss.
- The table size correctness check rebuilt version 0 paths.
Fix:
- FE: work on a mutable copy of the properties; validate `path_version` is 0
or 1, `shard_num`
is required and positive for version 1 and is rejected for version 0, and
report non integer
values as `AnalysisException`.
- Meta-service: validate `path_format` (version 0/1, positive shard_num for
version 1) for both
S3 and HDFS vaults, and persist it for S3 vaults.
- BE: pass `vault.path_format()` for S3 vaults (legacy obj infos stay on
version 0); reject a
non positive shard_num in `StorageResource`; use the rowset's own
`StorageResource` in the
table size check.
- BE file cache: add `BlockFileCache::doris_table_cache_file_name`, which
maps a version 1 path
`.../{rowset_id}/{file}` to the version 0 file name `{rowset_id}_{file}`
and leaves version 0
names unchanged. Use it for readers, upload cache allocation, packed
files, peer cache,
segment corruption retry and compaction GC. Because the result equals the
version 0 name,
`Segment::file_cache_key`, index file names, eviction and statistics keep
matching, and
version 0 cache keys are unchanged.
- Recycler/checker: port `murmur_hash64A` (identical to BE
`HashUtil::murmur_hash64A`, same
seed), make the path helpers take the vault `PathFormat` (default =
version 0, unchanged
output), keep each vault's format next to its accessor, and use it for
rowset/tablet deletion
(version 1 rowset prefix deletion also removes the delete bitmap, which
lives outside the
rowset directory), forward check and inverted check. Vaults with an
unsupported format get no
accessor rather than being recycled with a wrong layout.
Existing vaults and vaults without `path_format` keep using version 0;
nothing changes for them.
### Release note
S3 storage vaults can now be created with `path_version` = 1 and `shard_num`
to use the sharded
object key layout. Invalid `path_version`/`shard_num` values are rejected at
CREATE STORAGE VAULT.
### Check List (For Author)
- Test: Unit Test / Regression test (added, not run locally)
- FE UT: CreateStorageVaultCommandTest#testValidatePathFormat
- Meta-service UT: MetaServiceTest.CreateS3VaultWithPathFormat
- BE UT: StorageResourceTest.FileCacheName, invalid shard num death test
- Recycler UT: RecyclerTest.vault_path_format,
RecyclerTest.delete_rowset_data_path_version_1,
CheckerTest.path_version_1, CheckerTest.inverted_check_path_version_1
- Regression: vault_p0/create/test_create_s3_vault_path_version (.out to
be generated)
- None of these were compiled or run locally (no JDK/Maven/thirdparty
toolchain available);
C++ files formatted with clang-format 16 and
build-support/check-build-hygiene.sh passes.
- Behavior changed: Yes. CREATE STORAGE VAULT with path_version/shard_num
now succeeds (it
previously always failed) and rejects invalid combinations; S3 vaults
honor path_version 1.
Version 0 behavior and cache keys are unchanged.
- Does this need documentation: Yes, path_version/shard_num properties of
CREATE STORAGE VAULT.
### Notes for reviewers
- The regression suite `vault_p0/create/test_create_s3_vault_path_version`
needs its `.out` file generated on a cloud cluster with storage vault enabled
(`-genOut`); it was not run locally.
- Recycler/checker need the shard hash to match BE exactly; `murmur_hash64A`
is ported verbatim and pinned by the shared test vector tablet 10005, shard_num
1000 -> shard 611 (same as `StorageResourceTest.RemotePath`).
- `RefreshFSVaultVisitor` for S3 does not re-put the resource; this is fine
because a vault's `path_format` cannot be altered after CREATE.
- No new outbound requests or remote artifact loading are introduced.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
