deardeng commented on code in PR #68995:
URL: https://github.com/apache/doris/pull/68995#discussion_r4293074686
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudTabletRebalancer.java:
##########
@@ -1302,16 +1266,39 @@ private void
handleWarmupBatchFailure(List<WarmupTabletTask> tasks, Exception e)
}
private void revertWarmupState(WarmupTabletTask task) {
- updateBeToTablets(task.pickedTabletId, task.destBe, task.srcBe,
- futureBeToTabletsGlobal, futureBeToTabletsInTable,
futurePartitionToTablets);
- tabletToInfightTask.remove(new InfightTablet(task.pickedTabletId,
task.clusterId));
+ InfightTablet key = new InfightTablet(task.pickedTabletId,
task.clusterId);
+ InfightTask inflight = tabletToInfightTask.get(key);
+ if (inflight == null || inflight.srcBe != task.srcBe ||
inflight.destBe != task.destBe
+ || !tabletToInfightTask.remove(key, inflight)) {
Review Comment:
Thanks for the detailed interleaving. I agree that the checker can retain an
InfightTask after the daemon removes it. That task ownership race predates this
PR, although lazy views may turn it into an extra migration in the scenario
described.
The finding is based on static analysis; we do not yet have a runtime
reproduction or evidence of data loss. Serializing completion, rollback, and
the other task removal paths would expand this performance PR substantially and
introduce its own concurrency risk. I propose to keep this PR scoped and track
this race separately, with a deterministic reproducer before changing the task
lifecycle.
This reply accepts the possible migration churn; it does not claim the race
is fixed.
--
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]
