[refactor](fs) Migrate fe-core storage consumers onto the fe-filesystem SPI facade - #66004
Merged
Merged
Conversation
CalvinKirs
requested review from
924060929,
Gabriel39,
englefly,
gavinchou,
liaoxin01,
luwei16,
morningman,
morrySnow and
starocean999
as code owners
July 24, 2026 08:47
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
CalvinKirs
force-pushed
the
fs-spi-a5-parity
branch
2 times, most recently
from
July 24, 2026 10:21
9400f70 to
7ce1594
Compare
Member
Author
|
/run buildall |
Member
Author
|
/review |
CalvinKirs
force-pushed
the
fs-spi-a5-parity
branch
2 times, most recently
from
July 24, 2026 10:53
bb45242 to
a80a7b2
Compare
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 30th, 2026 7:58 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Member
Author
|
/review |
Member
Author
|
run buildall |
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 30th, 2026 7:58 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Member
Author
|
run feut |
Member
Author
|
run external |
Member
Author
|
run performance |
Member
Author
|
run vault_p0 |
CalvinKirs
force-pushed
the
fs-spi-a5-parity
branch
from
July 24, 2026 12:25
a80a7b2 to
c5e1adf
Compare
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 22950 ms |
Contributor
TPC-DS: Total hot run time: 117683 ms |
Contributor
ClickBench: Total hot run time: 18.54 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
morningman
approved these changes
Jul 27, 2026
Member
Author
|
run feut |
Member
Author
|
run p0 |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
924060929
approved these changes
Jul 27, 2026
Contributor
FE UT Coverage ReportIncrement line coverage |
gavinchou
approved these changes
Jul 27, 2026
morningman
added a commit
that referenced
this pull request
Jul 27, 2026
…filesystem storage facade Upstream #66004 ("Migrate fe-core storage consumers onto the fe-filesystem SPI facade") deleted fe-core's entire typed storage hierarchy (`datasource/property/storage/*`, `fs/SchemaTypeMapper`, `fs/StoragePropertiesConverter`) and replaced it with the `datasource/storage` facade (StorageAdapter / StorageTypeId / StorageRegistry). Our connector-SPI bridge consumed the deleted types but was never touched by #66004, so git reported no conflict on these files — they simply stopped compiling. Re-typed onto the facade: - DefaultConnectorContext: storage supplier + vended-credentials map keyed by StorageTypeId/StorageAdapter; StorageProperties.createAll -> StorageAdapter.ofAll; LocationPath.of(uri, map) -> LocationPath.ofAdapters(uri, map). - PluginDrivenExternalCatalog: wires getStorageAdaptersMap() into the context. - 6 fe-core tests re-typed to match; `HdfsProperties.FS_HDFS_SUPPORT` inlined as the "fs.hdfs.support" literal, following upstream (the constant died with the hierarchy). Two further reconciliations that had no compile signal either: - fe-kerberos now depends on fe-foundation. #66004 made fe-common's ExecutionAuthenticator extend the foundation-level (Hadoop-free) doAs abstraction; P3b had moved that class into fe-kerberos, so the moved copy inherited a dependency its pom did not declare. - Three test fakes (FileSystemPluginManagerTest, DefaultConnectorContextStoragePropsTest, FileSystemFactoryBindAllTest) now declare supportsGuess(). #66004 tightened FileSystemPluginManager.bindAll so out-of-tree providers are selected by supportsExplicit/supportsGuess; a provider overriding only supports() is now logged and skipped. The fakes asserted binding, so they had to adopt the new contract — which is exactly what a real out-of-tree plugin must now do. Also repairs IOCallable/SimpleAuthenticationConfig, which git cross-contaminated by pairing them as a rename during the P3b replay (8-character conflict markers). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 27, 2026
…66004 storage facade) Also records an upstream-owned regression found while validating: #66004 itself breaks ForwardToMasterTest by double-nesting the /rest/v2/manager/node/* JSON payload (its parent 0dde273 is green, and so is this branch before the rebase). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 28, 2026
…filesystem storage facade Upstream #66004 ("Migrate fe-core storage consumers onto the fe-filesystem SPI facade") deleted fe-core's entire typed storage hierarchy (`datasource/property/storage/*`, `fs/SchemaTypeMapper`, `fs/StoragePropertiesConverter`) and replaced it with the `datasource/storage` facade (StorageAdapter / StorageTypeId / StorageRegistry). Our connector-SPI bridge consumed the deleted types but was never touched by #66004, so git reported no conflict on these files — they simply stopped compiling. Re-typed onto the facade: - DefaultConnectorContext: storage supplier + vended-credentials map keyed by StorageTypeId/StorageAdapter; StorageProperties.createAll -> StorageAdapter.ofAll; LocationPath.of(uri, map) -> LocationPath.ofAdapters(uri, map). - PluginDrivenExternalCatalog: wires getStorageAdaptersMap() into the context. - 6 fe-core tests re-typed to match; `HdfsProperties.FS_HDFS_SUPPORT` inlined as the "fs.hdfs.support" literal, following upstream (the constant died with the hierarchy). Two further reconciliations that had no compile signal either: - fe-kerberos now depends on fe-foundation. #66004 made fe-common's ExecutionAuthenticator extend the foundation-level (Hadoop-free) doAs abstraction; P3b had moved that class into fe-kerberos, so the moved copy inherited a dependency its pom did not declare. - Three test fakes (FileSystemPluginManagerTest, DefaultConnectorContextStoragePropsTest, FileSystemFactoryBindAllTest) now declare supportsGuess(). #66004 tightened FileSystemPluginManager.bindAll so out-of-tree providers are selected by supportsExplicit/supportsGuess; a provider overriding only supports() is now logged and skipped. The fakes asserted binding, so they had to adopt the new contract — which is exactly what a real out-of-tree plugin must now do. Also repairs IOCallable/SimpleAuthenticationConfig, which git cross-contaminated by pairing them as a rename during the P3b replay (8-character conflict markers). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 28, 2026
…66004 storage facade) Also records an upstream-owned regression found while validating: #66004 itself breaks ForwardToMasterTest by double-nesting the /rest/v2/manager/node/* JSON payload (its parent 0dde273 is green, and so is this branch before the rebase). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 28, 2026
…onnector-paimon Upstream #65984 adds a relation-scoped `@options('scan.snapshot-id'='1', ...)` clause for paimon. Its fe-core half is catalog-neutral and merged as-is; its paimon half landed in the fe-core paimon subsystem this branch deleted (P5-T29), so it moves into the connector. `@options` is just a sixth time-travel selector, so it rides the existing SPI pipeline rather than a new one: ConnectorTimeTravelSpec gains Kind.OPTIONS carrying the raw map, paimon's resolveTimeTravel validates the keys and freezes a mutable selector (scan.mode=latest, a tag, a wall clock) into an immutable pin, applySnapshot threads it onto the handle, and resolveScanTable applies it via PaimonScanParams.applyOptions. fe-core never inspects an option key. BindRelation rejects the clause up front through a new SUPPORTS_SCAN_PARAM_OPTIONS capability: @options only reaches a connector through the MVCC pin path, so a table that never enters it would silently answer a historical query with latest data. Two upstream changes needed no port: initialSelectedPartitions -> NOT_PRUNED and computeOutput's getFullSchema(scanParams) arm are already covered, because LogicalFileScan resolves partitions and schema against THIS reference's own version and an explicit pin yields empty (scan-all) partition maps. Upstream's per-relation resolve-once cache is likewise provided by StatementContext's version key, which includes the option map. #65984 also widens paimon system tables from "no scan params at all" to a per-view capability matrix. The generic pipeline for that already exists (resolveSysTableSnapshotPin + applyMvccSnapshotPin); it only needed a per-system-table question, so ConnectorScanPlanProvider gains supportsSystemTableIncrementalRead/Options and the PluginDrivenScanNode guard now names the missing capability instead of refusing wholesale. paimon's applySnapshot stops returning system handles unchanged so the pin reaches the view. Three independent hardenings ride along: the @incr null-reset widens from two keys to paimon's whole inherited read-state family; a $binlog @incr read vetoes COUNT(*) pushdown (its reader packs an UPDATE_BEFORE/AFTER pair into one logical row); and the JNI-forced system-table set picks up row_tracking, now sourced from the same capability table. Deliberately NOT ported, with the reasons recorded in plan-doc/HANDOFF.md so a later rebase does not re-add them: upstream re-adds jni.enable_file_reader_async to the backend option list, but no BE/JNI consumer for it exists anywhere, including upstream master itself (#65955 deleted it) -- this branch already guards that with a test; and the selector-free @options preload exemption, which would need the connector's option vocabulary before binding and only affects warmup latency. Tests: fe-connector-paimon 421/421, fe-connector-api 110/110, fe-connector-iceberg 1151/1151, fe-core 8335 tests / 0 failures (2 pre-existing errors: upstream #66004's NodeInfo JSON regression and HFUtilsTest needing network), full FE install + checkstyle green. Upstream's PaimonScanParamsTest is ported to the connector; its resolution cases now run against real local paimon tables because no connector module carries mockito. Three mutations verified RED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 28, 2026
…filesystem storage facade Upstream #66004 ("Migrate fe-core storage consumers onto the fe-filesystem SPI facade") deleted fe-core's entire typed storage hierarchy (`datasource/property/storage/*`, `fs/SchemaTypeMapper`, `fs/StoragePropertiesConverter`) and replaced it with the `datasource/storage` facade (StorageAdapter / StorageTypeId / StorageRegistry). Our connector-SPI bridge consumed the deleted types but was never touched by #66004, so git reported no conflict on these files — they simply stopped compiling. Re-typed onto the facade: - DefaultConnectorContext: storage supplier + vended-credentials map keyed by StorageTypeId/StorageAdapter; StorageProperties.createAll -> StorageAdapter.ofAll; LocationPath.of(uri, map) -> LocationPath.ofAdapters(uri, map). - PluginDrivenExternalCatalog: wires getStorageAdaptersMap() into the context. - 6 fe-core tests re-typed to match; `HdfsProperties.FS_HDFS_SUPPORT` inlined as the "fs.hdfs.support" literal, following upstream (the constant died with the hierarchy). Two further reconciliations that had no compile signal either: - fe-kerberos now depends on fe-foundation. #66004 made fe-common's ExecutionAuthenticator extend the foundation-level (Hadoop-free) doAs abstraction; P3b had moved that class into fe-kerberos, so the moved copy inherited a dependency its pom did not declare. - Three test fakes (FileSystemPluginManagerTest, DefaultConnectorContextStoragePropsTest, FileSystemFactoryBindAllTest) now declare supportsGuess(). #66004 tightened FileSystemPluginManager.bindAll so out-of-tree providers are selected by supportsExplicit/supportsGuess; a provider overriding only supports() is now logged and skipped. The fakes asserted binding, so they had to adopt the new contract — which is exactly what a real out-of-tree plugin must now do. Also repairs IOCallable/SimpleAuthenticationConfig, which git cross-contaminated by pairing them as a rename during the P3b replay (8-character conflict markers). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 28, 2026
…66004 storage facade) Also records an upstream-owned regression found while validating: #66004 itself breaks ForwardToMasterTest by double-nesting the /rest/v2/manager/node/* JSON payload (its parent 0dde273 is green, and so is this branch before the rebase). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 28, 2026
…onnector-paimon Upstream #65984 adds a relation-scoped `@options('scan.snapshot-id'='1', ...)` clause for paimon. Its fe-core half is catalog-neutral and merged as-is; its paimon half landed in the fe-core paimon subsystem this branch deleted (P5-T29), so it moves into the connector. `@options` is just a sixth time-travel selector, so it rides the existing SPI pipeline rather than a new one: ConnectorTimeTravelSpec gains Kind.OPTIONS carrying the raw map, paimon's resolveTimeTravel validates the keys and freezes a mutable selector (scan.mode=latest, a tag, a wall clock) into an immutable pin, applySnapshot threads it onto the handle, and resolveScanTable applies it via PaimonScanParams.applyOptions. fe-core never inspects an option key. BindRelation rejects the clause up front through a new SUPPORTS_SCAN_PARAM_OPTIONS capability: @options only reaches a connector through the MVCC pin path, so a table that never enters it would silently answer a historical query with latest data. Two upstream changes needed no port: initialSelectedPartitions -> NOT_PRUNED and computeOutput's getFullSchema(scanParams) arm are already covered, because LogicalFileScan resolves partitions and schema against THIS reference's own version and an explicit pin yields empty (scan-all) partition maps. Upstream's per-relation resolve-once cache is likewise provided by StatementContext's version key, which includes the option map. #65984 also widens paimon system tables from "no scan params at all" to a per-view capability matrix. The generic pipeline for that already exists (resolveSysTableSnapshotPin + applyMvccSnapshotPin); it only needed a per-system-table question, so ConnectorScanPlanProvider gains supportsSystemTableIncrementalRead/Options and the PluginDrivenScanNode guard now names the missing capability instead of refusing wholesale. paimon's applySnapshot stops returning system handles unchanged so the pin reaches the view. Three independent hardenings ride along: the @incr null-reset widens from two keys to paimon's whole inherited read-state family; a $binlog @incr read vetoes COUNT(*) pushdown (its reader packs an UPDATE_BEFORE/AFTER pair into one logical row); and the JNI-forced system-table set picks up row_tracking, now sourced from the same capability table. Deliberately NOT ported, with the reasons recorded in plan-doc/HANDOFF.md so a later rebase does not re-add them: upstream re-adds jni.enable_file_reader_async to the backend option list, but no BE/JNI consumer for it exists anywhere, including upstream master itself (#65955 deleted it) -- this branch already guards that with a test; and the selector-free @options preload exemption, which would need the connector's option vocabulary before binding and only affects warmup latency. Tests: fe-connector-paimon 421/421, fe-connector-api 110/110, fe-connector-iceberg 1151/1151, fe-core 8335 tests / 0 failures (2 pre-existing errors: upstream #66004's NodeInfo JSON regression and HFUtilsTest needing network), full FE install + checkstyle green. Upstream's PaimonScanParamsTest is ported to the connector; its resolution cases now run against real local paimon tables because no connector module carries mockito. Three mutations verified RED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 29, 2026
…filesystem storage facade Upstream #66004 ("Migrate fe-core storage consumers onto the fe-filesystem SPI facade") deleted fe-core's entire typed storage hierarchy (`datasource/property/storage/*`, `fs/SchemaTypeMapper`, `fs/StoragePropertiesConverter`) and replaced it with the `datasource/storage` facade (StorageAdapter / StorageTypeId / StorageRegistry). Our connector-SPI bridge consumed the deleted types but was never touched by #66004, so git reported no conflict on these files — they simply stopped compiling. Re-typed onto the facade: - DefaultConnectorContext: storage supplier + vended-credentials map keyed by StorageTypeId/StorageAdapter; StorageProperties.createAll -> StorageAdapter.ofAll; LocationPath.of(uri, map) -> LocationPath.ofAdapters(uri, map). - PluginDrivenExternalCatalog: wires getStorageAdaptersMap() into the context. - 6 fe-core tests re-typed to match; `HdfsProperties.FS_HDFS_SUPPORT` inlined as the "fs.hdfs.support" literal, following upstream (the constant died with the hierarchy). Two further reconciliations that had no compile signal either: - fe-kerberos now depends on fe-foundation. #66004 made fe-common's ExecutionAuthenticator extend the foundation-level (Hadoop-free) doAs abstraction; P3b had moved that class into fe-kerberos, so the moved copy inherited a dependency its pom did not declare. - Three test fakes (FileSystemPluginManagerTest, DefaultConnectorContextStoragePropsTest, FileSystemFactoryBindAllTest) now declare supportsGuess(). #66004 tightened FileSystemPluginManager.bindAll so out-of-tree providers are selected by supportsExplicit/supportsGuess; a provider overriding only supports() is now logged and skipped. The fakes asserted binding, so they had to adopt the new contract — which is exactly what a real out-of-tree plugin must now do. Also repairs IOCallable/SimpleAuthenticationConfig, which git cross-contaminated by pairing them as a rename during the P3b replay (8-character conflict markers). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 29, 2026
…66004 storage facade) Also records an upstream-owned regression found while validating: #66004 itself breaks ForwardToMasterTest by double-nesting the /rest/v2/manager/node/* JSON payload (its parent 0dde273 is green, and so is this branch before the rebase). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 29, 2026
…onnector-paimon Upstream #65984 adds a relation-scoped `@options('scan.snapshot-id'='1', ...)` clause for paimon. Its fe-core half is catalog-neutral and merged as-is; its paimon half landed in the fe-core paimon subsystem this branch deleted (P5-T29), so it moves into the connector. `@options` is just a sixth time-travel selector, so it rides the existing SPI pipeline rather than a new one: ConnectorTimeTravelSpec gains Kind.OPTIONS carrying the raw map, paimon's resolveTimeTravel validates the keys and freezes a mutable selector (scan.mode=latest, a tag, a wall clock) into an immutable pin, applySnapshot threads it onto the handle, and resolveScanTable applies it via PaimonScanParams.applyOptions. fe-core never inspects an option key. BindRelation rejects the clause up front through a new SUPPORTS_SCAN_PARAM_OPTIONS capability: @options only reaches a connector through the MVCC pin path, so a table that never enters it would silently answer a historical query with latest data. Two upstream changes needed no port: initialSelectedPartitions -> NOT_PRUNED and computeOutput's getFullSchema(scanParams) arm are already covered, because LogicalFileScan resolves partitions and schema against THIS reference's own version and an explicit pin yields empty (scan-all) partition maps. Upstream's per-relation resolve-once cache is likewise provided by StatementContext's version key, which includes the option map. #65984 also widens paimon system tables from "no scan params at all" to a per-view capability matrix. The generic pipeline for that already exists (resolveSysTableSnapshotPin + applyMvccSnapshotPin); it only needed a per-system-table question, so ConnectorScanPlanProvider gains supportsSystemTableIncrementalRead/Options and the PluginDrivenScanNode guard now names the missing capability instead of refusing wholesale. paimon's applySnapshot stops returning system handles unchanged so the pin reaches the view. Three independent hardenings ride along: the @incr null-reset widens from two keys to paimon's whole inherited read-state family; a $binlog @incr read vetoes COUNT(*) pushdown (its reader packs an UPDATE_BEFORE/AFTER pair into one logical row); and the JNI-forced system-table set picks up row_tracking, now sourced from the same capability table. Deliberately NOT ported, with the reasons recorded in plan-doc/HANDOFF.md so a later rebase does not re-add them: upstream re-adds jni.enable_file_reader_async to the backend option list, but no BE/JNI consumer for it exists anywhere, including upstream master itself (#65955 deleted it) -- this branch already guards that with a test; and the selector-free @options preload exemption, which would need the connector's option vocabulary before binding and only affects warmup latency. Tests: fe-connector-paimon 421/421, fe-connector-api 110/110, fe-connector-iceberg 1151/1151, fe-core 8335 tests / 0 failures (2 pre-existing errors: upstream #66004's NodeInfo JSON regression and HFUtilsTest needing network), full FE install + checkstyle green. Upstream's PaimonScanParamsTest is ported to the connector; its resolution cases now run against real local paimon tables because no connector module carries mockito. Three mutations verified RED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 29, 2026
…filesystem storage facade Upstream #66004 ("Migrate fe-core storage consumers onto the fe-filesystem SPI facade") deleted fe-core's entire typed storage hierarchy (`datasource/property/storage/*`, `fs/SchemaTypeMapper`, `fs/StoragePropertiesConverter`) and replaced it with the `datasource/storage` facade (StorageAdapter / StorageTypeId / StorageRegistry). Our connector-SPI bridge consumed the deleted types but was never touched by #66004, so git reported no conflict on these files — they simply stopped compiling. Re-typed onto the facade: - DefaultConnectorContext: storage supplier + vended-credentials map keyed by StorageTypeId/StorageAdapter; StorageProperties.createAll -> StorageAdapter.ofAll; LocationPath.of(uri, map) -> LocationPath.ofAdapters(uri, map). - PluginDrivenExternalCatalog: wires getStorageAdaptersMap() into the context. - 6 fe-core tests re-typed to match; `HdfsProperties.FS_HDFS_SUPPORT` inlined as the "fs.hdfs.support" literal, following upstream (the constant died with the hierarchy). Two further reconciliations that had no compile signal either: - fe-kerberos now depends on fe-foundation. #66004 made fe-common's ExecutionAuthenticator extend the foundation-level (Hadoop-free) doAs abstraction; P3b had moved that class into fe-kerberos, so the moved copy inherited a dependency its pom did not declare. - Three test fakes (FileSystemPluginManagerTest, DefaultConnectorContextStoragePropsTest, FileSystemFactoryBindAllTest) now declare supportsGuess(). #66004 tightened FileSystemPluginManager.bindAll so out-of-tree providers are selected by supportsExplicit/supportsGuess; a provider overriding only supports() is now logged and skipped. The fakes asserted binding, so they had to adopt the new contract — which is exactly what a real out-of-tree plugin must now do. Also repairs IOCallable/SimpleAuthenticationConfig, which git cross-contaminated by pairing them as a rename during the P3b replay (8-character conflict markers). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 29, 2026
…66004 storage facade) Also records an upstream-owned regression found while validating: #66004 itself breaks ForwardToMasterTest by double-nesting the /rest/v2/manager/node/* JSON payload (its parent 0dde273 is green, and so is this branch before the rebase). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Jul 29, 2026
…onnector-paimon Upstream #65984 adds a relation-scoped `@options('scan.snapshot-id'='1', ...)` clause for paimon. Its fe-core half is catalog-neutral and merged as-is; its paimon half landed in the fe-core paimon subsystem this branch deleted (P5-T29), so it moves into the connector. `@options` is just a sixth time-travel selector, so it rides the existing SPI pipeline rather than a new one: ConnectorTimeTravelSpec gains Kind.OPTIONS carrying the raw map, paimon's resolveTimeTravel validates the keys and freezes a mutable selector (scan.mode=latest, a tag, a wall clock) into an immutable pin, applySnapshot threads it onto the handle, and resolveScanTable applies it via PaimonScanParams.applyOptions. fe-core never inspects an option key. BindRelation rejects the clause up front through a new SUPPORTS_SCAN_PARAM_OPTIONS capability: @options only reaches a connector through the MVCC pin path, so a table that never enters it would silently answer a historical query with latest data. Two upstream changes needed no port: initialSelectedPartitions -> NOT_PRUNED and computeOutput's getFullSchema(scanParams) arm are already covered, because LogicalFileScan resolves partitions and schema against THIS reference's own version and an explicit pin yields empty (scan-all) partition maps. Upstream's per-relation resolve-once cache is likewise provided by StatementContext's version key, which includes the option map. #65984 also widens paimon system tables from "no scan params at all" to a per-view capability matrix. The generic pipeline for that already exists (resolveSysTableSnapshotPin + applyMvccSnapshotPin); it only needed a per-system-table question, so ConnectorScanPlanProvider gains supportsSystemTableIncrementalRead/Options and the PluginDrivenScanNode guard now names the missing capability instead of refusing wholesale. paimon's applySnapshot stops returning system handles unchanged so the pin reaches the view. Three independent hardenings ride along: the @incr null-reset widens from two keys to paimon's whole inherited read-state family; a $binlog @incr read vetoes COUNT(*) pushdown (its reader packs an UPDATE_BEFORE/AFTER pair into one logical row); and the JNI-forced system-table set picks up row_tracking, now sourced from the same capability table. Deliberately NOT ported, with the reasons recorded in plan-doc/HANDOFF.md so a later rebase does not re-add them: upstream re-adds jni.enable_file_reader_async to the backend option list, but no BE/JNI consumer for it exists anywhere, including upstream master itself (#65955 deleted it) -- this branch already guards that with a test; and the selector-free @options preload exemption, which would need the connector's option vocabulary before binding and only affects warmup latency. Tests: fe-connector-paimon 421/421, fe-connector-api 110/110, fe-connector-iceberg 1151/1151, fe-core 8335 tests / 0 failures (2 pre-existing errors: upstream #66004's NodeInfo JSON regression and HFUtilsTest needing network), full FE install + checkstyle green. Upstream's PaimonScanParamsTest is ported to the connector; its resolution cases now run against real local paimon tables because no connector module carries mockito. Three mutations verified RED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Jul 30, 2026
morningman
added a commit
that referenced
this pull request
Jul 31, 2026
…y the harness (#66195) ### What problem does this PR solve? FE unit tests dominate CI wall time, and most of that time is not spent running test logic. Full attribution of one `fe-core` surefire window (4092s x 12 forks = **49,104 fork-seconds**, 86% of an 79.7-min build): | component | fork-seconds | share | nature | |---|---|---|---| | JVM lifecycle *between* classes (1219x) | 17,317 | 35.3% | fixed overhead, proportional to class count | | non-FE classes: real test execution | 13,259 | 27.0% | **real work** | | FE startup + teardown (304x) | 7,066 | 14.4% | fixed overhead, proportional to FE-starting classes | | FE classes: class loading before FE start | 4,463 | 9.1% | fixed overhead | | FE classes: real test execution | 3,797 | 7.7% | **real work** | | non-FE classes: class-level setup | 3,172 | 6.5% | fixed overhead | **Real work 17,055s (34.7%) vs fixed overhead 32,019s (65.2%).** Unattributed: 30s. The overhead is per *class*, so the only lever with real magnitude is **reducing class count by merging test classes**. Eliminating one class is worth **55.2 fork-seconds if it starts an FE** (304 such classes) versus **16.0 if it does not** (915 classes) — FE-starting classes are 3.5x more valuable to merge. ### What is changed? Six commits, in dependency order: 1. **Avoid a 10s heartbeat wait in every FE unit test class.** `Daemon.run()` runs one cycle before sleeping, so `HeartbeatMgr`'s first cycle completes before `createDorisCluster()` registers a backend; `checkBEHeartbeat()` then waits a full `heartbeat_interval_second` (default 10). Sets the interval to 1 before the `Env` singleton is created, and makes `checkBEHeartbeatStatus()` check-then-sleep at 20ms granularity instead of sleeping 1s first. Timeout budget unchanged. 2. **Merge 12 tiny nereids test classes into 3 suites.** 3. **Give `DistributeHintTest` assertions and cut its runtime by 95%.** It enumerated a large parameter space while asserting almost nothing; now it asserts the distribution actually chosen and covers the space at a sane size. 4. **Restore spied `Env` fields after every test method**, to stop unbounded spy nesting leaking across methods. 5. **Merge six command privilege tests into one suite.** 6. **Migrate 25 legacy `UtFrameUtils` test classes to `TestWithFeService`.** This one saves no time by itself — it is the precondition for merging, since only classes on the same harness can share an FE. ### Measurements (CI, build 1009445 unless noted) | change | measured | note | |---|---|---| | heartbeat fix | -3.7s/class, **within noise** | **not the lever** — see caveat below | | merge 12 -> 3 classes | 614.4s -> 168.5s | -72.6% | | `DistributeHintTest` | 458.7s -> 64.2s | -86% | | merge 6 -> 1 class | 302.8s -> 63.9s | -78.9%, 13 test cases unchanged | | migrate 25 classes | `DropTableTest` 4.483s -> 4.566s | no gain by design; enables merging | The three verified items save roughly **840 test-seconds** in total. **Caveat, stated plainly:** at ~7x effective parallelism, and with per-build noise of 2 sigma = 151s, that lands as only about **2 minutes of wall clock and is partly masked by noise**. Do not expect the total-seconds number in the build summary to move cleanly. The measurement method that does work is per-class comparison against an unaffected control group, not the build total. The heartbeat fix in commit 1 in particular looked large in local single-fork measurement and turned out to be noise-level on CI — it is kept because it is correct and harmless, not because it is a win. Full-suite result on build 1009445: **SUCCESS, 8773 passed, 0 failed, 1 muted.** The muted one is `ForwardToMasterTest.testAddBeDropBe` (ClassCastException), which is known upstream issue #66004 and unrelated to this branch. ### Rebase note Rebased onto `af6dcff9051` today. Two files conflicted with upstream, both against commit 6: - `FrontendServiceImplTest` — #64304 moved `MCTransaction` / `MaxComputeExternalCatalog` out of `fe-core`; upstream's `WriteBlockAllocatingTransaction` mock kept verbatim. - `DynamicPartitionTableTest` — #65219 added 1478 lines of TIMESTAMPTZ tests. Resolved by taking upstream's file whole and replaying the harness migration onto it, then diffing against upstream to confirm only harness lines moved. All 241 `Assert.*` calls left untouched. Post-rebase verification: `test-compile` clean, `checkstyle:check` clean, and the two conflicted classes run **69/69 passing, 0 skipped** — counts matching upstream's annotation counts exactly. ### A note for reviewers on how this was verified The JUnit4 -> JUnit5 migration in commit 6 has two failure modes that **pass silently** rather than erroring: - `@Rule ExpectedException` is ignored outright by JUnit5, turning six exception tests into tests that assert nothing. Converted to `ExceptionChecker.expectThrowsWithMsg`, which matches JUnit4 semantics (`isInstance` for subclasses, `contains` for messages). - JUnit4 `Assert.assertEquals(msg, expected, actual)` versus JUnit5 `Assertions.assertEquals(expected, actual, msg)` have **reversed argument order**. When all three arguments are Strings this compiles fine and silently compares the wrong things. 36 such call sites exist across the migrated classes; message-first overloads were identified by argument *count*, not by whether the first argument looks like a string. Because of this, `BUILD SUCCESS` is not sufficient evidence for these commits. Every migrated class was accepted only after its `Tests run:` count matched the pre-migration count exactly. Reviewers checking this PR should apply the same standard. ### Release note None ### Check List - [x] Test - [x] Regression test — not applicable, this changes only the FE unit test harness and test classes - [x] Unit Test — full FE UT suite green on CI (build 1009445: 8773 passed, 0 failed); post-rebase spot check 69/69 on the two conflicted classes - [x] Behavior changed: No (test-only, no production code paths altered) - [x] Does this need documentation? No
morningman
added a commit
that referenced
this pull request
Aug 1, 2026
…mmon (#66324) ### What problem does this PR solve? Issue Number: #65185 Related PR: #66004 Problem Summary: Now that catalogs go through the connector/filesystem SPIs, fe-core and fe-common no longer have a reason to compile against hadoop, but a handful of source imports were still left over from the pre-SPI code. This removes them. Scope is deliberately narrow: **source imports only**. The hadoop pom dependencies stay exactly as they are, because they remain reachable at runtime — `ranger-plugins-common` inherits from `org.apache.hadoop.conf.Configuration`, `hive-exec` supplies the UDF base class that `CREATE FUNCTION` resolves, and `fe-kerberos` uses `UserGroupInformation`. Dropping the jars is a separate, larger question and is not attempted here. After this PR, `grep -rn "^import org.apache.hadoop" fe/fe-core/src fe/fe-common/src` returns exactly one line: `RangerHiveAuditHandler`, which is a sanctioned exception. The type is imposed by Ranger's own API, and `ranger-plugins-common` puts hadoop on the classpath regardless, so removing that import would remove no dependency. It is documented as such in the code rather than worked around. The six commits are independent steps and are easiest to review one at a time: | Commit | What moves | |---|---| | `4314ad2` | Dead and trivial imports: delete `CatalogConfigFileUtils` (no caller; `fe-filesystem-hdfs*` already carries the port) and `LocationPath.getTempWritePath` (no caller); inline two hadoop constants that were plain strings; document the Ranger exception | | `29649e2` | `hadoop-huaweicloud` moves from fe-core to the `fe-filesystem-obs` plugin, together with the `huawei-obs-sdk` repository declaration. The only FE reference is the `Class.forName` probe in `ObsFileSystemProperties`, which must resolve against that plugin's classloader to report the truth | | `90339cd` | `FileSplitter` takes a new fe-core `FileBlockLocation` instead of hadoop's `BlockLocation`. Only `getOffset`/`getLength`/`getHosts` were ever read | | `c55800a` | `LocationPath.toStorageLocation()` returns the existing `org.apache.doris.filesystem.Location` instead of a hadoop `Path`; `getPath()` is deleted in favour of the `fsIdentifier` the class already computes. **See the behaviour-change note below** | | `592e8aa` | The Azure OAuth2 backend map moves from `StorageAdapter` into `fe-filesystem-azure`, which is where the legacy `AzureProperties` owned it. This deletes ~120 lines of fe-core code that turned out to be unreachable | | `464efa4` | `StageUtil`'s `GlobExpander`/`GlobFilter` use is ported to a package-private `GlobPatterns`, compiled with re2j — the same engine hadoop's `GlobPattern` uses, and already a declared fe-core dependency | Two notes that are easy to miss on review: - `hadoop.fs.GlobFilter` reads like a wildcard predicate but its constructor is also a validator: it rejects `a[b`, `a{b`, a trailing backslash and `[z-a]`, and `analyzeGlob` surfaces that as a `DdlException`. Porting only the predicate would have accepted those globs and quietly listed an unintended object-store prefix instead of failing the statement, so the validation is ported with it. - `StorageAdapter.getHadoopStorageConfig()` had no caller anywhere in the tree; the only consumer was the adapter's own Azure OAuth2 arm. That map is still load-bearing (BE routes Microsoft Fabric OneLake locations to `FILE_HDFS`, and `hdfs_builder` feeds every entry into its JNI hadoop builder), so it is preserved key-for-key rather than dropped — it just lives in the Azure plugin now.
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
5 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
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=*.javareturns empty over ALL of fe-core sources, main and test alike.Approach
ExecutionAuthenticator(doAs abstraction, no Hadoop types) with UGI holders staying in impl modules; all 14 providers implementsupportsExplicit/supportsGuessand self-declare their metadata (storageFamilyName/legacyCacheSchemes/sensitivePropertyKeys);bindPrimary/bindAllmirror the legacy routing exactly (priority order, explicitfs.<x>.supportflags 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.datasource/storage):StorageAdapterwraps one SPI binding with the legacy consumer surface (backend map, Hadoop conf, storage name, schemas, type id, URI validation, broker params, AWS credentials providers);StorageRegistryis the single fe-core-side registry (bind order + type ids + uri-scheme table);S3ThriftAdapter(TS3StorageParam) andCloudObjectStoreAdapter(ObjectStoreInfoPB) are verbatim moves of the wire glue;S3ResourceCompatis the verbatim quarantine for the image/DDL-bearing S3 constants and map utilities soS3Resource/AzureResourcelogic stays untouched.validateAndNormalizeUrifailures surface as uncheckedStoragePropertiesExceptionwith 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
@SerializedNamebearer in the diff).Release note
None
Check List (For Author)
Check List (For Reviewer who merge this PR)