github-actions[bot] commented on code in PR #67770:
URL: https://github.com/apache/doris/pull/67770#discussion_r4022045160
##########
fe/fe-core/src/main/java/org/apache/doris/fs/FileSystemPluginManager.java:
##########
@@ -170,7 +183,7 @@ public void loadPlugins(List<Path> pluginRoots) {
Set<String> sensitiveKeys;
try {
sensitiveKeys = provider.sensitivePropertyKeys();
- } catch (RuntimeException | LinkageError e) {
+ } catch (RuntimeException | LinkageError |
ServiceConfigurationError e) {
Review Comment:
[P2] Materialize the sensitive-key set inside this guard
The guarded call can return a plugin-owned lazy/custom `Set`; its iterator
is first consumed later by `registerSensitiveKeys()` via the case-insensitive
`addAll`. At that point `providers.add(provider)` has already made the
directory provider routable, so an iterator-time failure (or a null/invalid
element) aborts the family load with the provider active, its aliases
partially/not masked, no inventory row, and later successful handles skipped.
`loadBuiltins()` has the same root cause in the opposite publication order: it
writes the inventory row before traversing the set and does not roll it back on
failure. This is distinct from the connector engine-name thread because this
path controls filesystem admission and secret masking. Please materialize and
validate a host-owned set inside the guard before any publication in both
paths, discard on failure, and test iterator/content failures.
##########
fe/fe-core/src/main/java/org/apache/doris/backup/Repository.java:
##########
@@ -251,36 +272,113 @@ public void gsonPostProcess() {
LOG.info("Repository '{}': migrating legacy 'fs' field to
'fs_descriptor'", name);
Map<String, String> props = legacyFs.properties != null ?
legacyFs.properties : new HashMap<>();
String fsName = legacyFs.name != null ? legacyFs.name : "";
+ // Both binds run plugin code (bindPrimary probes every loaded
provider) at image load and
+ // edit-log replay: LinkageError included, so a half-installed
plugin costs this
+ // repository, not the FE - the same catch CatalogFactory uses for
catalogs.
try {
StorageAdapter storageAdapter = StorageAdapter.of(props);
Review Comment:
[P1] Preserve broker identity before routing legacy properties
This routes `prop` before consulting the registered broker name. A real
legacy `WITH BROKER broker_0` HDFS repository commonly stores
`fs.defaultFS`/`hadoop.username`; the HDFS provider therefore binds here, and
the next checkpoint permanently records HDFS instead of BROKER, so later backup
I/O bypasses `broker_0`. The catch-side name heuristic has the inverse edge
too: a legal broker actually named `HDFS` is left descriptor-less and cannot be
repaired by `ALTER`. These are distinct from the existing
typed-provider-failure thread because the provider succeeds and the old record
genuinely is BROKER. Please recognize ordinary registered brokers before typed
routing and retain/add an explicit discriminator or repair path for ambiguous
names, with checkpoint/reload tests for broker HDFS/S3 passthrough properties.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
