morningman opened a new pull request, #68803:
URL: https://github.com/apache/doris/pull/68803
### What problem does this PR solve?
Issue Number: None
Related PR: #65586, #66315
Problem Summary:
**In short.** This ports the test coverage of branch-4.1 #65586
(credential-aware Hadoop FileSystem cache) to master. master already has the
feature (#66315), but the assertions that guard the object-store Hadoop maps
and the merge / vended-credential paths did not come along. Only tests are
added; no production code changes.
**Background.** #65586 stopped force-disabling the Hadoop FileSystem cache
(`fs.<scheme>.impl.disable.cache=true`) and instead made the cache
credential-aware: the Doris-patched `FileSystem.Cache.Key` folds a fingerprint
of the storage definition into the key. master ported this in #66315 with a
different layout. Instead of one scheme-less `doris.fs.cache.key`, every
storage publishes its fingerprint under each scheme it serves
(`doris.fs.cache.key.<scheme>`), so a merge of several storages keeps all of
them and no combined fingerprint is needed.
| | branch-4.1 #65586 | master #66315 |
|---|---|---|
| Cache key property | `doris.fs.cache.key` | `doris.fs.cache.key.<scheme>`,
one per scheme the storage serves |
| Several storages in one map | combined fingerprint | entries simply
coexist |
| Blanket `fs.<scheme>.impl.disable.cache=true` | removed | removed |
**The problem, and what it cost.** On master nothing asserts the shape of
the S3-family `toHadoopConfigurationMap()`, which is the map the Hadoop
FileSystem actually reads. The 4.1 `testS3DisableHadoopCache` assertions (COS,
GCS, OBS, OSS, S3) have no master counterpart, and
`StorageAdapterFsCacheFingerprintTest#testNoBlanketDisableCacheByDefault`
inspects the `AWS_*` backend map, which never held these flags. Today every FE
unit test still passes if a provider re-adds a blanket disable flag (silently
turning the cache off again), or publishes its fingerprint under fewer schemes
than it is opened with (COS is addressed as `cos://` but opened as `s3a`, so a
key published only under `cos` is never read, and two catalogs with different
credentials can share a FileSystem). The merge through `CredentialUtils` and
the vended-credential overlay were likewise only checked with mocks or not at
all.
**How this PR fixes it.** It adds the 4.1 assertions in master's terms:
| Test | New assertion |
|---|---|
| `S3` / `Cos` / `Obs` / `Oss` / `GcsFileSystemPropertiesTest` |
`toHadoopConfigurationMap()` has no `fs.<scheme>.impl.disable.cache`; it
carries `doris.fs.cache.key.<scheme>` equal to `fsCacheFingerprint()` for every
scheme the storage serves (S3 `{s3, s3a, s3n}`, COS `{cos, cosn, s3, s3a}`, OSS
`{oss, s3, s3a}`, OBS `{obs, s3, s3a}`, GCS `{gs, s3, s3a}`); it has no
scheme-less key; different credentials give a different fingerprint |
| `CredentialUtilsTest` | merging real HDFS and S3 adapters through
`getBackendPropertiesFromStorageMap` keeps `doris.fs.cache.key.hdfs` and
`doris.fs.cache.key.s3a` with each storage's own fingerprint |
| `DefaultConnectorContextVendTest` | a vended OSS token carries its own
`doris.fs.cache.key.oss` / `.s3a`, and a rotated access key yields different
values |
**Results.** The new tests pass on current master and fail if any of the
regressions above is introduced.
Original author: @CalvinKirs
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- `run-fe-ut.sh --run` on `S3FileSystemPropertiesTest`,
`CosFileSystemPropertiesTest`, `ObsFileSystemPropertiesTest`,
`OssFileSystemPropertiesTest`, `GcsFileSystemPropertiesTest`,
`CredentialUtilsTest`, `DefaultConnectorContextVendTest`,
`StorageAdapterFsCacheFingerprintTest`, `DorisFileSystemCacheKeyTest`,
`OutFileTest`: 137 tests, 0 failures
- `mvn checkstyle:check` on the six touched modules: 0 violations
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
