github-actions[bot] commented on code in PR #69277:
URL: https://github.com/apache/doris/pull/69277#discussion_r4320790528
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/WarmupSelectCommand.java:
##########
@@ -216,10 +231,26 @@ public void sendAggregatedBlackholeResults(Map<Long,
TQueryStatisticsResult> sta
executor.sendResultSet(resultSet);
}
+ @Override
+ public void run(ConnectContext ctx, StmtExecutor executor) throws
Exception {
+ if (indexTable != null) {
+ LanceIndexPrewarm.run(ctx, executor, indexTable, indexColumns,
Review Comment:
[P2] Resolve prepared table defaults separately for each execution
With indexed `items` tables in Lance databases A and B, prepare `WARM UP
SELECT * FROM items PROPERTIES (read_index_only=true)`, execute it in A, then
`USE B` and execute the same statement again. This command retains the same
`indexTable`; `LanceIndexPrewarm.run` calls its mutating `analyze(ctx)`, which
fills missing catalog/database only once. The second execution therefore still
warms A, unlike the normal prepared relation path, which resolves the original
identifier parts against the current execution context. Resolve into an
execution-local table name instead. This is distinct from the existing
projection-qualifier thread: it uses only `*`, occurs after an earlier
execution, and persists even if qualifier validation is moved to execution.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceIndexPrewarm.java:
##########
@@ -0,0 +1,274 @@
+// 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.datasource.lance;
+
+import org.apache.doris.analysis.ResourceTypeEnum;
+import org.apache.doris.analysis.TableName;
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.ScalarType;
+import org.apache.doris.catalog.TableIf;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.Config;
+import org.apache.doris.common.UserException;
+import org.apache.doris.datasource.CatalogIf;
+import org.apache.doris.datasource.lance.index.LanceIndexInspectionExecutor;
+import org.apache.doris.datasource.lance.index.LanceIndexSegmentGroup;
+import org.apache.doris.datasource.lance.metadata.LanceTableMetadata;
+import org.apache.doris.info.TableNameInfo;
+import org.apache.doris.mysql.privilege.PrivPredicate;
+import org.apache.doris.proto.InternalService.PLanceIndexPrewarmRequest;
+import org.apache.doris.proto.InternalService.PLanceIndexPrewarmResponse;
+import org.apache.doris.qe.ConnectContext;
+import org.apache.doris.qe.ShowResultSet;
+import org.apache.doris.qe.ShowResultSetMetaData;
+import org.apache.doris.qe.StmtExecutor;
+import org.apache.doris.resource.computegroup.ComputeGroup;
+import org.apache.doris.rpc.BackendServiceProxy;
+import org.apache.doris.system.Backend;
+import org.apache.doris.system.BeSelectionPolicy;
+import org.apache.doris.thrift.TNetworkAddress;
+import org.apache.doris.thrift.TStatusCode;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.OptionalInt;
+import java.util.Set;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.function.BooleanSupplier;
+
+/** Pins table access and coordinates bounded synchronous prewarm RPCs. */
+public final class LanceIndexPrewarm {
+ static final int MAX_IN_FLIGHT = 8;
+ private static final ShowResultSetMetaData RESULT_META =
ShowResultSetMetaData.builder()
+ .addColumn(new Column("Table", ScalarType.createStringType()))
+ .addColumn(new Column("Index", ScalarType.createStringType()))
+ .addColumn(new Column("DatasetVersion", ScalarType.BIGINT))
+ .addColumn(new Column("BackendCount", ScalarType.INT))
+ .addColumn(new Column("ElapsedMs", ScalarType.BIGINT)).build();
+
+ private LanceIndexPrewarm() {
+ }
+
+ public static ShowResultSetMetaData getResultSetMetaData() {
+ return RESULT_META;
+ }
+
+ public static void run(ConnectContext ctx, StmtExecutor executor,
TableNameInfo tableName,
+ List<String> columns, BooleanSupplier cancelled) throws Exception {
+ long started = System.nanoTime();
+ long elapsedMs = ctx.getStartTime() > 0 ? Math.max(0,
System.currentTimeMillis() - ctx.getStartTime()) : 0;
+ long remainingMs = TimeUnit.SECONDS.toMillis(ctx.getQueryTimeoutS()) -
elapsedMs;
+ long deadline = started + TimeUnit.MILLISECONDS.toNanos(Math.max(0,
remainingMs));
+ tableName.analyze(ctx);
+ checkPrivileges(ctx, tableName);
+ List<Backend> targets = selectBackends(resolveComputeGroup(ctx));
+ // Resolve catalog/table initialization and snapshot IO in the bounded
metadata pool.
+ // The worker owns native resources until it returns, even if this
caller stops waiting.
+ String catalogName = tableName.getCtl();
+ String databaseName = tableName.getDb();
+ String name = tableName.getTbl();
+ LanceTableMetadata metadata = LanceIndexInspectionExecutor.execute(()
-> {
+ CatalogIf<?> catalog =
Env.getCurrentEnv().getCatalogMgr().getCatalog(catalogName);
+ if (!(catalog instanceof LanceExternalCatalog)) {
+ throw new AnalysisException("Index-only WARM UP SELECT
requires a Lance catalog table");
+ }
+ TableIf table =
catalog.getDbOrAnalysisException(databaseName).getTableOrAnalysisException(name);
+ if (!(table instanceof LanceExternalTable)) {
+ throw new AnalysisException("Index-only WARM UP SELECT
requires a Lance catalog table");
+ }
+ checkActive(deadline, cancelled);
+ // Use the query access path, including REST-vended credentials,
exactly once.
+ return ((LanceExternalTable) table).loadMetadata();
Review Comment:
[P2] Include usable legacy scalar indexes in prewarm discovery
This query-metadata loader reaches
`LanceDatasetIndexDiscovery.describeUserIndexes`, which deliberately omits
legacy indexes whose segments lack `index_details`. Ordinary SDK scans can
still use those scalar indexes, and the pinned SDK prewarm path also opens them
through its [legacy scalar
fallback](https://github.com/lance-format/lance/blob/68c12dfd7efe02f90ad2d7f3a239a7eb64884e57/rust/lance/src/index/scalar.rs#L689-L736).
For an older-writer BTREE index named `id` on column `id`, `WARM UP SELECT *`
or `SELECT id` consequently reports no matching indexes if it is the only
index; with another modern index, `*` succeeds while silently skipping it.
Discover raw user-index names/field IDs from the same pinned dataset for
prewarm rather than inheriting the planner-specific exclusion, and cover
legacy-only and mixed datasets.
##########
thirdparty/download-thirdparty.sh:
##########
@@ -718,25 +718,29 @@ if [[ " ${TP_ARCHIVES[*]} " =~ " AZURE " ]]; then
echo "Finished patching ${AZURE_SOURCE}"
fi
-# Foyer remains a local patch until its cache interface is accepted upstream.
-# All search fixes are supplied by the immutable lance-c dependency revision.
+# Keep local cache integration and prewarm field resolution on the pinned
lance-c revision.
if [[ " ${TP_ARCHIVES[*]} " =~ " LANCE_C " ]]; then
- foyer_patch_checksum="$(cksum < "${TP_PATCH_DIR}/lance-c-foyer.patch")"
-
foyer_patch_marker="${TP_SOURCE_DIR}/${LANCE_C_SOURCE}/${PATCHED_MARK}_foyer"
- # A new local patch must also replace previously patched cached sources.
- # Empty markers from older builds cannot identify the applied patch
version.
- if [[ -f "${foyer_patch_marker}" ]] &&
- [[ "$(cat "${foyer_patch_marker}")" != "${foyer_patch_checksum}" ]];
then
+ lance_c_patches=(lance-c-foyer.patch lance-c-prewarm-field.patch)
+ lance_c_patch_checksum="$(
+ for patch_file in "${lance_c_patches[@]}"; do
+ cat "${TP_PATCH_DIR}/${patch_file}"
+ done | cksum
+ )"
+
lance_c_patch_marker="${TP_SOURCE_DIR}/${LANCE_C_SOURCE}/${PATCHED_MARK}_lance_c"
Review Comment:
[P2] Migrate the existing patch harness to the combined marker
The downloader now writes only `patched_mark_lance_c`, but
`thirdparty/test/lance-prefilter-patch-test.sh:78` still unconditionally
requires `patched_mark_foyer`. Running that harness with the pinned archive
therefore fails its first fresh-source check with `missing Foyer patch marker`.
Its other fixtures also rely on the old protocol: stale-cache cases modify only
the obsolete marker, and the incompatible-source case expects failure even
though this new path re-extracts and repairs it. Update the harness alongside
the marker/reset change so it verifies both patches, the combined fingerprint,
cache reuse and recovery under the new behavior.
--
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]
