luwei16 commented on code in PR #68138:
URL: https://github.com/apache/doris/pull/68138#discussion_r4056418805
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RowBinlogTableWrapper.java:
##########
@@ -79,6 +86,22 @@ public KeysType getKeysType() {
return KeysType.DUP_KEYS;
}
+ @Override
+ public List<Long> selectNonEmptyPartitionIds(Collection<Long> partitionIds,
+ Optional<StreamReadMode> streamReadMode) {
+ if (Config.isCloudMode() && !hasFixedVisibleVersions()) {
+ // A row-binlog scan can start immediately after its target
transaction becomes visible. Refresh
+ // cached-empty or unknown partitions so an older cache entry
cannot prune newly visible binlog data.
+ List<CloudPartition> partitions = partitionIds.stream()
+ .map(this::getPartition)
+ .filter(Objects::nonNull)
+ .map(partition -> (CloudPartition) partition)
+ .collect(Collectors.toList());
+ return CloudPartition.selectNonEmptyPartitionIdsFromMs(partitions);
Review Comment:
[Major] Avoid routing the newly covered snapshot-version-disabled path
through the two-pass cached-state classifier.
This new call does not check Config.enable_cloud_snapshot_version. The
delegated helper first collects partitions whose hasDataCached() result is
true, then performs a second pass to collect partitions whose result is false.
If a partition returns false in the first pass and another transaction/query
refreshes the shared cache to version greater than 1 between the passes, the
second pass returns true. The partition is then in neither nonEmptyPartitionIds
nor unknowns, is not sent to MetaService, and is silently pruned. A later
ScanNode version refresh cannot restore a partition already removed from the
plan, so committed binlog rows can be missed.
The helper's two-pass behavior under enable_cloud_snapshot_version=true
predates this PR and is not attributed here. The regression introduced by this
call is the newly covered supported false path: previously
OlapTable.selectNonEmptyPartitionIds used the per-partition hasData() fallback
there.
Please classify each partition exactly once in a single loop so it must
enter either the known-nonempty or unknown collection, then force-refresh the
unknown collection. Please also add a test covering a cached-empty partition
becoming cached-nonempty between the two current checks.
--
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]
