kfaraz commented on code in PR #13334:
URL: https://github.com/apache/druid/pull/13334#discussion_r1017452780


##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -405,9 +394,16 @@ public String getType()
     }
 
     @Override
-    public boolean canAlsoHandle(Notice otherNotice)
+    public int hashCode()
     {
-      return otherNotice.getType().equals(TYPE);
+      return 0;
+    }
+
+    @Override
+    public boolean equals(Object obj)
+    {
+      // All RunNotices are the same. They are de-duplicated on insertion into 
the NoticesQueue "notices".
+      return obj != null && obj.getClass().equals(RunNotice.class);

Review Comment:
   Makes sense. 
   
   Since the RunNotice has no state, I wonder if we shouldn't just have a 
singleton like so:
   `static final RunNotice RUN_NOTICE = new RunNotice()` and just use that 
wherever needed `addNotice(RUN_NOTICE)`.
   
   It would also help clean up the time-based condition in `handle`, in that 
the `lastRunTime` can now be a field in `RunNotice` rather than in the 
`Supervisor`. Maybe this is not even needed now since we are already doing 
aggressive deduplication in the queue itself.



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