github-actions[bot] commented on code in PR #67648:
URL: https://github.com/apache/doris/pull/67648#discussion_r3987854152
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -1945,6 +1945,18 @@ public long getRowCountForPartitionIndex(long
partitionId, long indexId, boolean
return index.getRowCount() == -1 ? 0 : index.getRowCount();
}
+ /**
+ * Whether the table has any partition that has ever been loaded.
+ */
+ public boolean hasData() {
+ for (Partition p : idToPartition.values()) {
+ if (p.hasData()) {
Review Comment:
**[P1] Use an authoritative cloud version for the empty-stats gate.**
`CloudPartition.hasData()` can accept a cached INIT version here, and this
analysis worker has no `ConnectContext`, so `getVisibleVersion()` uses the
default cache TTL of `Long.MAX_VALUE`. A follower that misses the best-effort
version push and is promoted, or an immediate-commit retry whose
already-visible response omits partition versions, can therefore retain version
1 after MetaService has version 2. This line then still persists empty stats
for the populated table, and the later cache repair cannot undo them. Use a
fresh MetaService snapshot for this correctness decision (preferably the batch
path) and cover stale-v1/authoritative-v2 behavior.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -1945,6 +1945,18 @@ public long getRowCountForPartitionIndex(long
partitionId, long indexId, boolean
return index.getRowCount() == -1 ? 0 : index.getRowCount();
}
+ /**
+ * Whether the table has any partition that has ever been loaded.
+ */
+ public boolean hasData() {
+ for (Partition p : idToPartition.values()) {
Review Comment:
**[P2] Batch the cloud partition-version lookup.** On a cold empty cloud
table, every `p.hasData()` reaches a synchronous `batchMode=false` MetaService
request and no partition can short-circuit the loop. One column task therefore
performs N serial RPCs; the default manual executor can start three column
tasks together, producing up to 3N cold calls. This turns the old in-memory
empty-table shortcut into remote work proportional to partition count even
though `CloudPartition.getSnapshotVisibleVersionFromMs()` and
`selectNonEmptyPartitionIds()` already provide a batch path. Batch this check
and add a multi-partition cloud test that bounds RPC count.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/OlapAnalysisTask.java:
##########
@@ -89,7 +89,9 @@ public void doExecute() throws Exception {
return;
}
// For empty table, write empty result directly, no need to run SQL to
collect stats.
- if (info.rowCount == 0 && tableSample != null) {
+ // Only take the fast path when no partition has ever been loaded.
+ if (info.rowCount == 0 && tableSample != null
+ && tbl instanceof OlapTable && !((OlapTable) tbl).hasData()) {
Review Comment:
**[P1] Do not reuse the known-stale zero in the sampling path.** This
condition establishes that row count 0 may be stale, but `doSample()`
immediately reads that same zero. `getSampleCollectInfo(0)` selects FULL and
removes the tablet hint/limit, turning the requested sample into a full-table
scan per column on a potentially huge first load. For a sole UNIQUE/AGG key,
the same stale value is rendered as literal NDV 0 while row count becomes
`COUNT(1)`; a populated scan then produces non-null min/max with NDV 0, which
`ColStatsData.isValid()` rejects and consumers read as UNKNOWN. Refresh the
estimate or force a bounded sampling plan, and derive sole-key NDV from the
full scan. The test should run this path instead of stubbing `doSample()`.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/OlapAnalysisTask.java:
##########
@@ -89,7 +89,9 @@ public void doExecute() throws Exception {
return;
}
// For empty table, write empty result directly, no need to run SQL to
collect stats.
- if (info.rowCount == 0 && tableSample != null) {
+ // Only take the fast path when no partition has ever been loaded.
+ if (info.rowCount == 0 && tableSample != null
Review Comment:
**[P1] Stop after cancellation of the cloud emptiness probe.** The new
`hasData()` call can block in retrying cloud-version RPCs before any
`StmtExecutor` exists. Analysis timeout sets `killed` and calls
`FutureTask.cancel(false)`, so it neither interrupts the lookup nor cancels a
statement. When the lookup returns, this method does not recheck `killed` and
can still append empty stats or start `doSample()`; the task timeout does not
set `AnalysisJob.killed`, so a late append can flush and replace FAILED with
FINISHED. Make the probe bounded/cancellable and recheck `killed` before either
branch writes or queries; cover cancellation while the version lookup is
blocked.
--
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]
