[GitHub] samarthjain commented on issue #7073: Remove unnecessary check for contains() in LoadRule

[GitBox <gi...@apache.org>]

samarthjain commented on issue #7073: Remove unnecessary check for contains() in LoadRule
URL: https://github.com/apache/incubator-druid/pull/7073#issuecomment-463425688
 
 
   > 👎 This isn't a true veto, but I caution all other reviewers to not merge this without much more careful inspection of all surrounding code. There is logic in the load rules that is not immediately obvious upon first reading it but can have dramatic impacts and completely destabilize entire clusters.
   
   Not sure I understand the concern here. It looks fairly straightforward to me. The only caller of LoadRule#run method is DruidCoordinatorRuleRunner.
   
   And the DruidCoordinatorRuleRunner#run method is effectively doing the contains check here
   ```
   for (DataSegment segment : params.getAvailableSegments()) {
   ```
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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