github-actions[bot] commented on code in PR #67186: URL: https://github.com/apache/doris/pull/67186#discussion_r3883420182
########## fe/fe-connector/fe-connector-hms/src/main/java/org/apache/doris/connector/hms/TrackingHiveMetaStoreClient.java: ########## @@ -0,0 +1,43 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.hms; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hive.metastore.HiveMetaHookLoader; +import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.apache.hadoop.hive.metastore.api.Partition; +import shade.doris.hive.org.apache.thrift.TException; + +import java.util.List; + +public final class TrackingHiveMetaStoreClient extends HiveMetaStoreClient implements IMetaStoreClient { + + public TrackingHiveMetaStoreClient(Configuration conf, HiveMetaHookLoader hookLoader, Boolean allowEmbedded) + throws MetaException { + super(conf, hookLoader, allowEmbedded); + } + + @Override + public List<Partition> getPartitionsByNames(String dbName, String tableName, List<String> partitionNames) + throws TException { + return HmsRemoteCallTracking.trackWireAttempt( + () -> super.getPartitionsByNames(dbName, tableName, partitionNames)); Review Comment: [P2] Keep partition-statistics writes inside the new HMS access boundary. `HiveConnectorTransaction.UpdateStatisticsTask` reaches `ThriftHmsClient.updatePartitionStatistics`, whose direct `client.getPartitionsByNames(...)` call enters this override without `HmsRemoteCallTracking.withTracker`; `CURRENT` is therefore null and every `RetryingMetaStoreClient` attempt is invisible. These commit/undo lookups consequently emit neither a WRITE logical event/query-profile span nor physical retry counts, even though the parallel write-planning lookup is source-aware. Carry the session/WRITE source into statistics updates and route this singleton read through the same access/tracker boundary, with a production-chain retry-count test. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/PreloadExternalMetadata.java: ########## @@ -57,7 +73,12 @@ public List<Rule> buildRules() { * Execute external metadata preload after relation collection and before internal table locks. */ public ExternalMetadataPreloadResult executePreload(StatementContext statementContext) { + return executePreload(statementContext, statementContext.getHints()); + } + + private ExternalMetadataPreloadResult executePreload(StatementContext statementContext, List<Hint> hints) { long preloadStartTime = TimeUtils.getStartTimeMs(); + preloadCloudMtmvRefreshContexts(statementContext, hints); Review Comment: [P2] Report cloud-MTMV seed work as an executed preload stage. This call can load snapshots and install refresh contexts under the separate cloud snapshot budget, but the following generic table-preload gate can still return `ExternalMetadataPreloadResult.skipped` (notably with the default `enable_preload_external_metadata=false`). `collectAndLockTable` then records no preload timing and logs that preload was skipped even though remote work just completed. Compose the cloud and table outcomes so any performed work preserves the total elapsed time/executed state, and add a default-generic-off cloud test that asserts the returned result and planner profile. -- 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]
