[feat](fs) Add GCS, MinIO and Ozone filesystem plugins on a shared S3 base - #65834
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
Member
Author
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Jul 25th, 2026 6:03 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
CalvinKirs
force-pushed
the
fs-spi-gcs-local
branch
from
July 21, 2026 06:07
aab4408 to
9a7d4de
Compare
Member
Author
|
run buildall |
Member
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Request changes. The shared S3-base split and provider-owned property models are generally clear,
but four blocking issues remain: the standard clean FE build does not package the three new
dialect modules before unpacking them; normal legacy MinIO/Ozone requests lose their dialect
identity and still select generic S3; explicit Ozone canonical maps lose non-default signing
regions; and explicit DEFAULT credentials on raw GCP resource maps are silently rewritten to
anonymous. The inline comments contain the concrete triggers and fix directions.
Critical checkpoint conclusions:
- Goal and proof: GCS/MinIO/Ozone are intended to route to dedicated plugins on a shared S3
implementation. GCS has a durableprovider=GCPpath, but MinIO/Ozone do not across legacy
conversion, so the stated end-to-end goal is not achieved. Unit coverage is broad but does not
prove the real converter-to-provider path or clean distribution packaging. - Scope and reuse: extracting
fe-filesystem-s3-baseis focused and reuses the existing S3
implementation. The remaining defects are integration-boundary omissions rather than a need for
broader architecture. - Concurrency and lifecycle: no new request-time concurrency or shared mutable I/O state is
introduced. Plugin discovery/load lifecycle, service uniqueness, classloader identity, and
S3ObjStorageclose behavior were traced; no additional lifecycle defect was found once the
modules are actually built. - Configuration and compatibility: no dynamic configuration item is added. Compatibility is not
preserved for legacy MinIO/Ozone routing, canonical Ozone region propagation, or raw GCP
DEFAULTcredential mode. No storage-format, function-symbol, or rolling FE-BE protocol change is
involved. - Parallel paths and conditions: raw-map and legacy-
StoragePropertiespaths were both reviewed.
Generic S3 escape behavior remains valid. A suspected conflicting explicit-provider/foreign-guess
case was dismissed because no supported producer emits that contradictory map. - Tests and results: the new unit tests cover many aliases, defaults, validation failures, and URI
schemes, but use impossible dialect markers or default-valued fixtures that mask the accepted
issues; no converter/arbitration or build-packaging smoke test is present. Per the review-runner
contract, this review performed no local build or test execution. - Persistence, transactions, data writes, FE-BE variables, observability, and performance: these
mechanisms are not materially changed. Provider predicates remain cheap map checks, and existing
plugin-load logging is sufficient; no additional issue was found in these checkpoints. - User focus: no additional focus points were provided; the entire PR was reviewed.
| * {@code fs.s3.support=true}). Dialect providers must not guess against such a map, and the S3 | ||
| * provider uses it as its escape hatch from the dialect yield rule. | ||
| * | ||
| * <p>Note the {@code _STORAGE_TYPE_} marker is intentionally NOT consulted: the converter stamps |
Contributor
There was a problem hiding this comment.
[P1] Preserve a dialect signal across legacy conversion
The normal FE path does not pass the raw map to these providers: StoragePropertiesConverter rewrites every AbstractS3CompatibleProperties to canonical AWS_* keys plus _STORAGE_TYPE_=S3 (the inherited getStorageName() value). GCS keeps provider=GCP, but MinIO and Ozone keep neither their fs.*.support flag nor a provider hint. Consequently both dedicated supports methods return false, while generic S3 treats the marker as explicit and wins. Please preserve an authoritative MinIO/Ozone signal at the conversion boundary and add a test that runs the real legacy object through conversion and provider arbitration; the current _STORAGE_TYPE_=MINIO/OZONE fixtures are not values the converter emits.
| description = "The endpoint of the Ozone S3 Gateway.") | ||
| private String endpoint = ""; | ||
|
|
||
| @ConnectorProperty(names = {REGION, "s3.region"}, |
Contributor
There was a problem hiding this comment.
[P1] Bind the canonical Ozone region key
A map explicitly routed with provider=OZONE carries its normalized region as AWS_REGION (and the legacy converter will do the same once its dialect signal is preserved). This alias list accepts the other canonical AWS_* fields but not AWS_REGION, so eu-west-2 binds as the default us-east-1 and that wrong value reaches the SDK signer. Please add AWS_REGION here and test a non-default canonical value; the current us-east-1 fixture cannot distinguish successful binding from this loss. Today MERGED-1 masks this on the legacy route because generic S3 still wins, but fixing that routing exposes this independent failure.
| // Legacy AbstractS3CompatibleProperties emits this marker for EVERY S3-compatible | ||
| // dialect when no HMAC pair is configured; without it the shared S3 client falls back | ||
| // to the AWS default provider chain and signs requests with ambient env credentials. | ||
| if (!hasStaticCredentials()) { |
Contributor
There was a problem hiding this comment.
[P1] Preserve an explicitly requested DEFAULT credential mode
A validated raw GCP resource map can set AWS_CREDENTIALS_PROVIDER_TYPE=DEFAULT with no inline AK/SK. Before this routing change, generic S3 preserved that mode and used the environment/system/profile/instance credential chain. The GCS provider now wins, validation explicitly accepts DEFAULT, but this branch overwrites it with ANONYMOUS, so the same request becomes unsigned. Please preserve a supplied DEFAULT and synthesize anonymous only when no mode was requested (or reject DEFAULT explicitly if that is the intended contract), with an end-to-end provider/delegate test. This concerns the raw resource path; legacy converted maps without keys were already anonymous.
| # Each sub-directory is one storage backend loaded at runtime by FileSystemPluginManager. | ||
| FS_PLUGIN_DIR="${DORIS_OUTPUT}/fe/plugins/filesystem" | ||
| for fs_module in s3 azure oss cos obs hdfs oss-hdfs jfs local broker http; do | ||
| for fs_module in s3 gcs minio ozone azure oss cos obs hdfs oss-hdfs jfs local broker http; do |
Contributor
There was a problem hiding this comment.
[P1] Add the new plugins to the FE Maven selection list
This deployment loop now unpacks GCS, MinIO, and Ozone, but the FE_MODULES loop at lines 720-724 still selects only the pre-existing filesystem modules. Because the build runs mvn package -pl ... -am, it builds s3-base as a dependency of S3 but never builds these three new dependent modules. On a clean checkout (and deterministically with ./build.sh --fe --clean), their source directories pass the guard and the first missing target/doris-fe-filesystem-gcs.zip makes unzip abort under set -e. Please add all three modules to the selection loop (and ideally assert parity between the build and deployment lists).
… base Route GCS, MinIO and Ozone to dedicated filesystem plugins instead of letting them fall through to the generic S3 provider. Extract fe-filesystem-s3-base first, following the existing fe-filesystem-hdfs-base precedent: a plain library with no provider and no META-INF/services, holding the S3 protocol client (S3FileSystem, S3ObjStorage, S3FileSystemProperties, credentials provider factory) that the dialects reuse. Alongside it live AbstractDelegatingS3Properties, the base for dialects that speak the S3 protocol but differ in property spelling, defaults and credential surface, and S3CompatSignals, which centralizes dialect detection. Each dialect then gets its own plugin module, matching the module-per-dialect layout already used by cos, obs and oss: one package, one provider, one META-INF/services entry and one plugin zip each, packaged by build.sh. Each owns a typed properties class, so properties and filesystem stay bound together and the credential surface can be enforced at parse time: all three accept static HMAC access_key/secret_key only and reject the AWS-specific mechanisms (role ARN, instance profile) they cannot serve. GCS keeps HMAC interoperability access; the Google native SDK is not used because it is OAuth2-only and incompatible with the existing HMAC parameters. Public aliases, defaults and validation follow the legacy fe-core properties classes, including the anonymous credentials fallback that every S3-compatible dialect applies when no access key and secret key are configured. The dialects differ in supported URI schemes (GCS gs/s3/s3a, MinIO s3/s3a, Ozone s3/s3a/s3n), so S3ObjStorage gains a constructor taking an explicit scheme set: delegating through S3FileSystemProperties would otherwise replace the dialect scheme set with the S3 defaults and reject the dialect's own URIs. Finally, the generic S3 provider yields. It claims any map carrying credentials plus an endpoint or region, which now includes every dialect map, and selection cannot fall back on META-INF/services order because the dialects live in separate jars where ServiceLoader order is not guaranteed. The decision follows from the property map alone: S3 yields when the map either names a dedicated dialect (provider=..., fs.<dialect>.support=true) or is recognized by that dialect's guess predicate, unless the user explicitly asked for plain S3, which stays the escape hatch for serving a dialect endpoint through the generic client. The predicates are ports of the legacy fe-core guessIsMe methods, so detection matches what the kernel already does when it picks a StorageProperties subclass. Ozone has no legacy guess and is therefore reachable only by explicit request, with an unflagged Ozone gateway map served by the generic S3 provider as before. fe-core is untouched: the kernel needs no knowledge of the individual dialects.
CalvinKirs
force-pushed
the
fs-spi-gcs-local
branch
from
July 21, 2026 07:23
9a7d4de to
1a30fe7
Compare
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29473 ms |
Contributor
TPC-DS: Total hot run time: 178304 ms |
Contributor
ClickBench: Total hot run time: 24.95 s |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Member
Author
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Jul 25th, 2026 6:03 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
morningman
approved these changes
Jul 24, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
the
approved
Member
Author
|
/review |
Contributor
There was a problem hiding this comment.
Request changes. The shared S3-base extraction, dedicated plugin packaging, and direct provider tests are mostly coherent, but two additional patch-scoped issues remain: authoritative GCP identity is lost for custom-endpoint external stages before provider arbitration, and the new Hadoop export does not preserve the validated DEFAULT/ANONYMOUS credential mode. The existing P1 thread about normal legacy MinIO/Ozone conversion also remains a blocker and is intentionally not duplicated here.
Critical checkpoint conclusions:
- Goal and proof: the PR partially accomplishes the dedicated-provider goal. Direct raw-map routing is covered, but the real ObjectInfo(GCP) stage path can still select generic S3 for custom endpoints, and there is no adapter-to-provider test for that supported path.
- Scope and reuse: moving the common S3 implementation into s3-base and keeping thin dialect modules is focused and reuses the existing client. Reactor selection, zip names/layout, service descriptors, and classloader identity were checked statically; no additional packaging issue remains at this head.
- Concurrency and lifecycle: no new request-time shared mutable state, locking, or special object lifetime was introduced. Plugin discovery/loading and S3ObjStorage ownership/close paths were traced without a new concurrency or lifecycle finding.
- Configuration and compatibility: no dynamic Doris configuration or persisted format is added. Compatibility is incomplete at two boundaries: the supported GCP stage adapter loses its provider identity, and the public HadoopStorageProperties export reverses or loses no-static credential semantics.
- Parallel paths and conditions: raw maps, legacy StorageProperties conversion, ObjectInfo stage conversion, SDK/backend export, and Hadoop export were compared. Synthetic conflicting provider/endpoint maps, duplicate case aliases, and direct unnormalized s3n inputs were investigated and dismissed because no supported producer was established.
- Error handling and data correctness: validation and exception propagation in the changed Java property models were reviewed. The accepted issues are silent semantic misrouting/authentication changes rather than missing exception checks; no separate memory-safety concern applies to these Java-only changes.
- Tests and results: unit coverage is broad for aliases, defaults, validation, URI schemes, and provider predicates, but it omits the real custom-endpoint ObjectInfo(GCP) arbitration path and DEFAULT/ANONYMOUS Hadoop export cases. Per the review-runner contract, no local build or test was run. Live CI at submission reports COMPILE, FE UT, CheckStyle, and the reported regressions passing; check_coverage_fe is failed.
- Observability: existing plugin selection/load logging is adequate for this scope; no new metric or log is required beyond fixing the deterministic semantic errors.
- Persistence, transactions, data writes, and FE-BE propagation: no edit-log, transaction, storage-format, or data-write protocol is changed. Backend canonical-map propagation was reviewed, including GCP tagging, with no additional issue beyond the provider identity boundary above.
- Performance: provider checks remain bounded map/string operations and the extraction adds no material hot-path allocation or I/O; no performance blocker was found.
- User focus: no additional focus points were supplied; all 42 changed paths and their integration boundaries were reviewed.
|
|
||
| @Override | ||
| public boolean supports(Map<String, String> properties) { | ||
| String providerValue = properties.get(S3CompatSignals.PROVIDER_KEY); |
Contributor
There was a problem hiding this comment.
[P1] Preserve GCP identity for custom-endpoint stages
ObjectInfoAdapter.toStorageProperties() maps an authoritative Provider.GCP stage to S3Properties, and its builder emits only s3.* keys. After conversion this branch therefore receives no provider=GCP; if the supported GCS endpoint is custom rather than ending in storage.googleapis.com, the guess is false and generic S3 wins. External stage/copy operations then bypass the dedicated GCS property and HMAC contract. Please preserve the GCP marker (or construct GCSProperties) through the adapter/converter and cover ObjectInfo(GCP, custom-endpoint) through real provider arbitration.
| cfg.put("fs.s3a.impl.disable.cache", "true"); | ||
| cfg.put("fs.s3a.endpoint", getEndpoint()); | ||
| cfg.put("fs.s3a.endpoint.region", getRegion()); | ||
| if (StringUtils.isNotBlank(getAccessKey())) { |
Contributor
There was a problem hiding this comment.
[P2] Preserve the no-static credential mode in the Hadoop export
The validated properties distinguish DEFAULT from ANONYMOUS and toS3CompatibleKv() now preserves that choice, but this export emits no provider whenever AK/SK is absent. MinIO/Ozone therefore turn explicit or implicit ANONYMOUS into Hadoop's ambient chain, while the GCS hook does the reverse and overwrites explicit DEFAULT with AnonymousAWSCredentialsProvider. Since these objects advertise HadoopStorageProperties, please map the resolved mode consistently (for example with S3CredentialsProviderFactory.hadoopClassName) and let GCS only add fs.gs.impl, with DEFAULT/ANONYMOUS tests for each dialect.
7 tasks
morningman
added a commit
that referenced
this pull request
Jul 24, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
morningman
added a commit
that referenced
this pull request
Jul 24, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
morningman
added a commit
that referenced
this pull request
Jul 24, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
morningman
added a commit
that referenced
this pull request
Jul 25, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
morningman
added a commit
that referenced
this pull request
Jul 26, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
morningman
added a commit
that referenced
this pull request
Jul 27, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
0AyanamiRei
pushed a commit
to 0AyanamiRei/doris
that referenced
this pull request
Jul 27, 2026
… base (apache#65834) ### What problem does this PR solve? Issue Number: close #xxx Problem Summary: GCS, MinIO and Ozone currently fall through to the generic S3 filesystem provider. This PR routes each to its own dedicated filesystem plugin, so their property spelling, defaults and credential surface are owned and enforced per dialect instead of leaking into the generic S3 path. **Approach** - **Extract `fe-filesystem-s3-base` first**, following the existing `fe-filesystem-hdfs-base` precedent: a plain library with no provider and no `META-INF/services`, holding the S3 protocol client (`S3FileSystem`, `S3ObjStorage`, `S3FileSystemProperties`, credentials-provider factory) that the dialects reuse. Alongside it live `AbstractDelegatingS3Properties` — the base for dialects that speak the S3 protocol but differ in property spelling, defaults and credential surface — and `S3CompatSignals`, which centralizes dialect detection. - **One plugin module per dialect** (`fe-filesystem-gcs` / `-minio` / `-ozone`), matching the module-per-dialect layout already used by `cos`, `obs` and `oss`: one package, one provider, one `META-INF/services` entry and one plugin zip each, packaged by `build.sh`. Each owns a typed properties class, so properties and filesystem stay bound together and the credential surface is enforced at parse time: all three accept static HMAC `access_key`/`secret_key` only and reject the AWS-specific mechanisms (role ARN, instance profile) they cannot serve. GCS keeps HMAC interoperability access; the Google native SDK is not used because it is OAuth2-only and incompatible with the existing HMAC parameters. - **Aliases, defaults and validation follow the legacy fe-core properties classes**, including the anonymous-credentials fallback that every S3-compatible dialect applies when no access/secret key is configured. The dialects differ in supported URI schemes (GCS `gs`/`s3`/`s3a`, MinIO `s3`/`s3a`, Ozone `s3`/`s3a`/`s3n`), so `S3ObjStorage` gains a constructor taking an explicit scheme set — delegating through `S3FileSystemProperties` would otherwise replace the dialect scheme set with the S3 defaults and reject the dialect's own URIs. - **The generic S3 provider yields, decided from the property map alone.** S3 claims any map carrying credentials plus an endpoint or region — which now includes every dialect map — and selection cannot fall back on `META-INF/services` order because the dialects live in separate jars where `ServiceLoader` order is not guaranteed. So S3 yields when the map either names a dedicated dialect (`provider=...`, `fs.<dialect>.support=true`) or is recognized by that dialect's guess predicate, unless the user explicitly asked for plain S3 (the escape hatch for serving a dialect endpoint through the generic client). The predicates are ports of the legacy fe-core `guessIsMe` methods, so detection matches what the kernel already does when it picks a `StorageProperties` subclass. Ozone has no legacy guess and is therefore reachable only by explicit request; an unflagged Ozone gateway map is served by the generic S3 provider as before. - **Scope of the legacy (converted) path — deliberate, no behavior change.** On the normal FE path a legacy `StorageProperties` is rewritten by `StoragePropertiesConverter` to canonical `AWS_*` keys (plus `_STORAGE_TYPE_=S3`), which strips the dialect-specific keys the guess predicates rely on. Only GCS survives this, because legacy `GCSProperties` stamps a durable `provider=GCP`; legacy `MinioProperties`/`OzoneProperties` stamp no provider, so their converted maps carry no dialect signal and are served by the generic S3 provider — exactly as they are today. This is intentional: MinIO and Ozone are S3-compatible, the converter has already resolved their aliases and defaults into the canonical map, so the generic S3 client serves them identically. Dedicated MinIO/Ozone routing (typed properties, HMAC-only credential surface, dialect URI schemes) applies to the raw/explicit path (`provider=MINIO|OZONE`, `fs.<dialect>.support=true`, or the `minio.*` guess). Preserving a dialect signal through legacy conversion would require an fe-core change, which this PR intentionally avoids (fe-core net diff is zero). **fe-core is untouched** — the kernel needs no knowledge of the individual dialects (net diff against fe-core is zero). ### Release note Add GCS, MinIO and Ozone as dedicated filesystem plugins built on a shared S3-protocol base module. ### Check List (For Author) - Test - [x] Unit Test - [ ] Regression test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. - Behavior changed: - [x] No. <!-- GCS/MinIO/Ozone routing and property semantics follow the existing legacy behavior; the change is where that behavior lives, not what it does. --> - Does this need documentation? - [x] No.
morningman
added a commit
that referenced
this pull request
Jul 27, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
CalvinKirs
added a commit
that referenced
this pull request
Jul 27, 2026
…em SPI facade (#66004) ### What problem does this PR solve? Issue Number: close #xxx Related PR: #65834 Problem Summary: fe-core carried its own typed storage-properties hierarchy (`datasource/property/storage/*`, `fs/SchemaTypeMapper`, `fs/StoragePropertiesConverter`) in parallel with the fe-filesystem SPI plugins. Every storage consumer in fe-core was coupled to those typed classes, keeping storage knowledge split across two stacks. This PR migrates every fe-core consumer onto the fe-filesystem SPI behind a single fe-core facade, and **deletes the legacy hierarchy entirely** (21 main classes + SchemaTypeMapper + StoragePropertiesConverter + 34 legacy-only tests). Gate: `grep -rE "datasource\.property\.storage\.|fs\.SchemaTypeMapper|StoragePropertiesConverter" fe/fe-core/src --include=*.java` returns empty over ALL of fe-core sources, main and test alike. **Approach** - **SPI readiness**: HDFS-family typed properties implement the SPI interfaces; auth is unified on a foundation-level `ExecutionAuthenticator` (doAs abstraction, no Hadoop types) with UGI holders staying in impl modules; all 14 providers implement `supportsExplicit`/`supportsGuess` and self-declare their metadata (`storageFamilyName`/`legacyCacheSchemes`/`sensitivePropertyKeys`); `bindPrimary`/`bindAll` mirror the legacy routing exactly (priority order, explicit `fs.<x>.support` flags disabling guesses, OSS-HDFS/OSS and JFS/HDFS exclusivity, default-HDFS fallback at index 0), and providers absent from the built-in registry are consulted after it in registration order — an out-of-tree S3-compatible plugin needs zero fe-core changes. - **fe-core facade** (`datasource/storage`): `StorageAdapter` wraps one SPI binding with the legacy consumer surface (backend map, Hadoop conf, storage name, schemas, type id, URI validation, broker params, AWS credentials providers); `StorageRegistry` is the single fe-core-side registry (bind order + type ids + uri-scheme table); `S3ThriftAdapter` (`TS3StorageParam`) and `CloudObjectStoreAdapter` (`ObjectStoreInfoPB`) are verbatim moves of the wire glue; `S3ResourceCompat` is the verbatim quarantine for the image/DDL-bearing S3 constants and map utilities so `S3Resource`/`AzureResource` logic stays untouched. - **Consumers**: CatalogProperty, LocationPath, BrokerDesc/StorageDesc, all TVFs, backup/restore, resources/vaults/policy, connectivity testers, vended credentials, iceberg/paimon/hive integrations and cloud copy-into are re-typed mechanically. Behavior was frozen during the migration by golden parity tests comparing the new path against the legacy classes; with the legacy package now deleted, the surviving tests assert the same expectations as fixed golden values (routing table, wire structs field-by-field, persisted S3Resource maps, guess batteries). - Deliberate, test-locked behavior deltas: S3-family no longer silently defaults region to us-east-1 (same error fe-core raised), OSS non-standard endpoints are rewritten like fe-core did, multi-provider backend/Hadoop map merge order is pinned to binding order, and `validateAndNormalizeUri` failures surface as unchecked `StoragePropertiesException` with unchanged messages. Deployments must ship the fe-filesystem plugin zips (FE plugin dir), including the broker plugin. Gson/image shapes, EditLog payloads, BE code and thrift IDL are unchanged (verified field-by-field for every `@SerializedName` bearer in the diff). ### Release note None ### Check List (For Author) - Test - [x] Unit test: golden tests covering bind routing, wire structs (thrift/PB field-by-field), persisted S3Resource maps, URI normalization matrices, guess batteries and out-of-tree plugin extensibility; full buildall regression matrix green (P0/NonConcurrent/cloud_p0/vault_p0/External/performance). - Behavior changed: - [x] No. - Does this need documentation? - [x] No. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
morningman
added a commit
that referenced
this pull request
Jul 27, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
morningman
added a commit
that referenced
this pull request
Jul 28, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
morningman
added a commit
that referenced
this pull request
Jul 28, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
morningman
added a commit
that referenced
this pull request
Jul 29, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
morningman
added a commit
that referenced
this pull request
Jul 29, 2026
…tream #65834's dedicated fe-filesystem-minio Upstream #65834 added dedicated fe-filesystem-{gcs,minio,ozone} plugins on a shared s3-base. Its MinioFileSystemProperties already owns every minio.* alias and the same legacy 100/10000/10000 connection-tuning defaults, and the generic S3 provider now yields minio.* maps to it (S3CompatSignals.guessIsMinio fires before S3's fallback credential/location check). That makes the minio.* aliases + applyLegacyMinioTuningDefaults we had folded into the generic S3 properties (FIX-C1-MINIO, from the P5 paimon migration) dead at the routing layer and redundant at the property layer. Remove them so s3-base carries zero minio knowledge, matching upstream's dialect separation. - S3FileSystemProperties: drop minio.* aliases from every @ConnectorProperty name list (keep the unrelated aws.glue.session-token), the MINIO_* tuning constants, applyLegacyMinioTuningDefaults() and its now-orphaned hasRawKey() helper. - S3FileSystemProvider: drop minio.* from the credential/endpoint/region name arrays used by the fallback supports() check (the yield to the dedicated MinIO provider fires first). - S3FileSystemPropertiesTest: drop the four minio-folding tests; the capability is covered by fe-filesystem-minio's MinioFileSystem{Properties,Provider}Test. Keep glueAliases (guards the aws.glue.session-token alias, not minio). fe-filesystem s3-base/s3/minio/gcs/ozone unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
28 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #xxx
Problem Summary:
GCS, MinIO and Ozone currently fall through to the generic S3 filesystem provider. This PR routes each to its own dedicated filesystem plugin, so their property spelling, defaults and credential surface are owned and enforced per dialect instead of leaking into the generic S3 path.
Approach
Extract
fe-filesystem-s3-basefirst, following the existingfe-filesystem-hdfs-baseprecedent: a plain library with no provider and noMETA-INF/services, holding the S3 protocol client (S3FileSystem,S3ObjStorage,S3FileSystemProperties, credentials-provider factory) that the dialects reuse. Alongside it liveAbstractDelegatingS3Properties— the base for dialects that speak the S3 protocol but differ in property spelling, defaults and credential surface — andS3CompatSignals, which centralizes dialect detection.One plugin module per dialect (
fe-filesystem-gcs/-minio/-ozone), matching the module-per-dialect layout already used bycos,obsandoss: one package, one provider, oneMETA-INF/servicesentry and one plugin zip each, packaged bybuild.sh. Each owns a typed properties class, so properties and filesystem stay bound together and the credential surface is enforced at parse time: all three accept static HMACaccess_key/secret_keyonly and reject the AWS-specific mechanisms (role ARN, instance profile) they cannot serve. GCS keeps HMAC interoperability access; the Google native SDK is not used because it is OAuth2-only and incompatible with the existing HMAC parameters.Aliases, defaults and validation follow the legacy fe-core properties classes, including the anonymous-credentials fallback that every S3-compatible dialect applies when no access/secret key is configured. The dialects differ in supported URI schemes (GCS
gs/s3/s3a, MinIOs3/s3a, Ozones3/s3a/s3n), soS3ObjStoragegains a constructor taking an explicit scheme set — delegating throughS3FileSystemPropertieswould otherwise replace the dialect scheme set with the S3 defaults and reject the dialect's own URIs.The generic S3 provider yields, decided from the property map alone. S3 claims any map carrying credentials plus an endpoint or region — which now includes every dialect map — and selection cannot fall back on
META-INF/servicesorder because the dialects live in separate jars whereServiceLoaderorder is not guaranteed. So S3 yields when the map either names a dedicated dialect (provider=...,fs.<dialect>.support=true) or is recognized by that dialect's guess predicate, unless the user explicitly asked for plain S3 (the escape hatch for serving a dialect endpoint through the generic client). The predicates are ports of the legacy fe-coreguessIsMemethods, so detection matches what the kernel already does when it picks aStoragePropertiessubclass. Ozone has no legacy guess and is therefore reachable only by explicit request; an unflagged Ozone gateway map is served by the generic S3 provider as before.Scope of the legacy (converted) path — deliberate, no behavior change. On the normal FE path a legacy
StoragePropertiesis rewritten byStoragePropertiesConverterto canonicalAWS_*keys (plus_STORAGE_TYPE_=S3), which strips the dialect-specific keys the guess predicates rely on. Only GCS survives this, because legacyGCSPropertiesstamps a durableprovider=GCP; legacyMinioProperties/OzonePropertiesstamp no provider, so their converted maps carry no dialect signal and are served by the generic S3 provider — exactly as they are today. This is intentional: MinIO and Ozone are S3-compatible, the converter has already resolved their aliases and defaults into the canonical map, so the generic S3 client serves them identically. Dedicated MinIO/Ozone routing (typed properties, HMAC-only credential surface, dialect URI schemes) applies to the raw/explicit path (provider=MINIO|OZONE,fs.<dialect>.support=true, or theminio.*guess). Preserving a dialect signal through legacy conversion would require an fe-core change, which this PR intentionally avoids (fe-core net diff is zero).fe-core is untouched — the kernel needs no knowledge of the individual dialects (net diff against fe-core is zero).
Release note
Add GCS, MinIO and Ozone as dedicated filesystem plugins built on a shared S3-protocol base module.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?