github-actions[bot] commented on code in PR #66913:
URL: https://github.com/apache/doris/pull/66913#discussion_r3891465416
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hudi/source/HudiScanNode.java:
##########
@@ -552,59 +634,311 @@ private void initPrunedPartitions() throws UserException
{
throw new UserException(ExceptionUtils.getRootCauseMessage(e), e);
}
partitionInit = true;
+ ensureHmsRuntimeGeneration();
}
@Override
public void startSplit(int numBackends) {
+ ensureHmsRuntimeGeneration();
if (prunedPartitions.isEmpty()) {
splitAssignment.finishSchedule();
+ releaseFsViewOnce();
return;
}
- AtomicInteger numFinishedPartitions = new AtomicInteger(0);
+ acquireFsView();
Review Comment:
[P1] Acquire/sync this view under the catalog authenticator. `tryAcquire()`
now reloads the Hudi timeline via `fsView.sync()`, but batch planning reaches
this call with no HMS `ExecutionAuthenticator` scope at all; the synchronous
sibling likewise acquires at line 606 before its authenticated block begins at
line 608. On Kerberized or otherwise identity-scoped storage, the refresh can
therefore run as the ambient process identity and fail before split planning
reaches authenticated work. Please execute acquisition/sync under the exact HMS
authenticator for both planning modes, while keeping remote I/O outside the
generation-owner monitor, and add a scope-sensitive regression test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -117,11 +127,18 @@ public IcebergMetadataOps(ExternalCatalog dorisCatalog,
Catalog catalog) {
this.catalog = catalog;
nsCatalog = (SupportsNamespaces) catalog;
this.executionAuthenticator = dorisCatalog.getExecutionAuthenticator();
+ this.threadPoolWithPreAuth = dorisCatalog.getThreadPoolWithPreAuth();
+ this.catalogProperties = Collections.unmodifiableMap(new
HashMap<>(dorisCatalog.getProperties()));
+ this.icebergCatalogType =
catalogProperties.get(IcebergExternalCatalog.ICEBERG_CATALOG_TYPE);
Review Comment:
[P2] Preserve the normalized Iceberg catalog type here. The detached ALTER
validator accepts subtype values case-insensitively and publishes their
original spelling, so changing an existing HMS catalog to the equivalent
`iceberg.catalog.type=HMS` still initializes the normalized HMS runtime. This
snapshot retains `HMS`, though, and the later lowercase-only checks reject
`CREATE DATABASE ... PROPERTIES`/`location` that HMS supports. Please snapshot
the initialized metastore subtype (as the old call did) or normalize this value
with `Locale.ROOT`, and add a mixed-case ALTER regression test.
--
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]
