abhishekagarwal87 commented on code in PR #15274:
URL: https://github.com/apache/druid/pull/15274#discussion_r1375660841


##########
server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java:
##########
@@ -873,16 +872,14 @@ public Response isHandOffComplete(
       final DateTime now = DateTimes.nowUtc();
 
       // A segment that is not eligible for load will never be handed off
-      boolean notEligibleForLoad = true;
+      boolean eligibleForLoad = false;
       for (Rule rule : rules) {
         if (rule.appliesTo(theInterval, now)) {
-          if (rule instanceof LoadRule) {
-            notEligibleForLoad = false;
-          }
+          eligibleForLoad = rule.shouldSegmentBeLoaded();

Review Comment:
   ```suggestion
             if (rule instanceof LoadRule) {
               eligibleForLoad = ((LoadRule) rule).shouldSegmentBeLoaded();
             }
             
   ```



##########
server/src/main/java/org/apache/druid/server/coordinator/rules/Rule.java:
##########
@@ -50,4 +50,12 @@ public interface Rule
   boolean appliesTo(Interval interval, DateTime referenceTimestamp);
 
   void run(DataSegment segment, SegmentActionHandler segmentHandler);
+
+  /**
+   * @return Whether a segment that matches this rule needs to be loaded on a 
tier.
+   */
+  default boolean shouldSegmentBeLoaded()
+  {
+    return false;
+  }

Review Comment:
   This method is not really needed and could be left in LoadRule itself. In 
`DataSourceResource`, you can ask cast the rule to `rule` to `LoadRule` and 
then call `shouldSegmentBeLoaded`.  



##########
server/src/main/java/org/apache/druid/server/coordinator/rules/Rule.java:
##########
@@ -50,4 +50,12 @@ public interface Rule
   boolean appliesTo(Interval interval, DateTime referenceTimestamp);
 
   void run(DataSegment segment, SegmentActionHandler segmentHandler);
+
+  /**
+   * @return Whether a segment that matches this rule needs to be loaded on a 
tier.
+   */
+  default boolean shouldSegmentBeLoaded()
+  {
+    return false;
+  }

Review Comment:
   can you also add a comment that this method can be used in making handoff 
decisions? 



-- 
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]

Reply via email to