Re: [PR] fix: Fix clone historicals and inter-historical move handling of partial load profiles (druid)

["FrankChen021 (via GitHub)" <gi...@apache.org>]

FrankChen021 commented on code in PR #19843:
URL: https://github.com/apache/druid/pull/19843#discussion_r3695810366


##########
server/src/main/java/org/apache/druid/server/coordinator/duty/CloneHistoricals.java:
##########
@@ -150,6 +182,41 @@ private void loadSegmentOnTargetServer(
     }
   }
 
+  /**
+   * Returns the clone target to a full load of {@code segment}, for when the source has stopped holding it partially.
+   * <p>
+   * A plain load request on top of the existing replica does not achieve this. A historical that receives an unwrapped
+   * load request for a segment it already holds under a partial-load rule keeps that rule applied: its holds go on
+   * pinning the parts the rule selected, and the segment's info file goes on describing a partial load, which the
+   * historical reapplies and re-announces on its next restart. Dropping the replica does release the rule and retire
+   * the info file, so the next coordinator run sees a clone that is missing the segment and queues the ordinary full
+   * load.
+   * <p>
+   * A partial load that is still queued is cancelled and replaced by the full load within this run, since nothing has
+   * been applied on the historical yet. If the request has already gone out, the cancel fails and that load runs to
+   * completion; the drop path then converts the replica on a later run.
+   */
+  private void convertCloneReplicaToFullLoad(
+      DataSegment segment,
+      ServerHolder targetServer,
+      DruidCoordinatorRuntimeParams params
+  )
+  {
+    if (targetServer.isLoadingSegment(segment)) {
+      if (targetServer.cancelOperation(SegmentAction.LOAD, segment)) {
+        loadSegmentOnTargetServer(segment, null, targetServer, params);

Review Comment:
   [P1] Drop a served partial replica after cancelling its reload
   
   A target can simultaneously serve an existing partial replica and have another partial reload queued—for example, profile A is loaded while profile B is waiting when the source transitions to full. Cancellation succeeds here, but the subsequent plain load runs over the existing A replica. As documented above, the ordinary virtual-storage load path does not clear the applied rule or partial info file, while its plain announcement makes the coordinator record a null profile; later clone cycles therefore consider the target full and never repair it. After cancellation, check whether the target already serves a partial replica and drop it first; only queue the immediate full load when no replica is currently served.



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org