github-actions[bot] commented on code in PR #69997:
URL: https://github.com/apache/doris/pull/69997#discussion_r4443651956
##########
fe/fe-core/src/main/java/org/apache/doris/qe/PointQueryExecutor.java:
##########
@@ -117,28 +107,44 @@ private void updateCloudPartitionVersions() throws
RpcException {
void setScanRangeLocations() throws Exception {
OlapScanNode scanNode = shortCircuitQueryContext.scanNode;
- // compute scan range
- List<TScanRangeLocations> locations =
scanNode.lazyEvaluateRangeLocations();
- Preconditions.checkNotNull(locations);
- if (scanNode.getScanTabletIds().isEmpty()) {
- return;
- }
- Preconditions.checkState(scanNode.getScanTabletIds().size() == 1);
- this.tabletID = scanNode.getScanTabletIds().get(0);
-
- // update partition version if cloud mode
- if (Config.isCloudMode()
- &&
ConnectContext.get().getSessionVariable().enableSnapshotPointQuery) {
- // TODO: Optimize to reduce the frequency of version checks in the
meta service.
- updateCloudPartitionVersions();
- }
+ OlapTable table = scanNode.getOlapTable();
+ List<CloudPartition> snapshotPartitions = null;
+ // Partition pruning, tablet lookup and partition-version selection
need one metadata view.
+ // Reuse validation in ExecuteCommand can precede a concurrent
partition DDL commit.
+ table.readLock();
+ try {
+ List<TScanRangeLocations> locations =
scanNode.lazyEvaluateRangeLocations();
Review Comment:
[P1] Keep cloud auto-start outside the routing read lock
A cached prepared point query can reach this call after its compute group
has auto-suspended. `lazyEvaluateRangeLocations()` calls
`getBackendAlivePathHashes()`, which invokes
`Replica.getBackendIdWithoutException() -> CloudReplica.getBackendId() ->
CloudSystemInfoService.getCurrentClusterId() -> waitForAutoStart()`. That path
issues a Meta Service `alterCluster` RPC and can sleep for hundreds of retries
while this table read lock remains held, blocking partition DDL and causing
later readers to queue behind a waiting writer. `getComputeGroupSafely()` can
also trigger a virtual-group failover RPC. Moving only the explicit
partition-version fetch outside the lock leaves this blocking work in the
critical section. Resolve/wake the cloud routing context outside the lock and
reuse it throughout replica selection inside the protected metadata view. Add
coverage with the real `CloudReplica` path; the current cloud test retains
ordinary replicas and mocks compute-group resolution.
##########
fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java:
##########
@@ -864,7 +865,11 @@ public void computePartitionInfo() throws
AnalysisException {
setHasPartitionPredicate(false);
selectedPartitionIds = olapTable.getPartitionIds();
}
- selectedPartitionIds =
olapTable.selectNonEmptyPartitionIds(selectedPartitionIds, Optional.empty());
+ // Cloud empty-partition pruning can fetch versions from MS.
Point-query routing holds the table lock;
+ // let the backend point lookup determine whether the selected
partition contains a matching row.
+ if (!(Config.isCloudMode() && isPointQuery())) {
Review Comment:
[P1] Preserve single-tablet routing with an empty LIST default
Consider a cloud MoW row-store table with `UNIQUE KEY(id,d)`, `LIST(d)
(PARTITION p1 VALUES IN ('1'), PARTITION p_default)`, and hash distribution on
`id`. Insert only `(id=1,d=1)`, leaving the default empty:
```text
ResultSink
Filter(id = 1 AND d = 1)
OlapScan(p1, p_default)
```
This qualifies for short circuit, but `ListPartitionPrunerV2` inherits
`PartitionPrunerV2Base.handleDefaultPartition()`, which always appends the
default ID to the explicit match. Skipping `selectNonEmptyPartitionIds` now
retains both partitions; `computeTabletInfo` selects one hash tablet from each,
and `PointQueryExecutor.setScanRangeLocations` fails its `size() == 1`
precondition before the point lookup. Previously the empty default was filtered
out and this query succeeded. `PruneEmptyPartition` explicitly skips
short-circuit scans, so it does not rescue this case. Preserve an exact
single-partition route or fall back to an executor supporting multiple routes,
while keeping version RPCs outside the lock, and add cloud LIST/default
coverage.
--
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]
