jihoonson commented on issue #6698: do not watch unwanted tier's data node in HttpServerInventoryView URL: https://github.com/apache/incubator-druid/pull/6698#issuecomment-463345469 I don't think the current change is easy to understand because 1) `DataSegment` for the pair is nullable only in `HttpServerInventoryView`. This is because zk-based inventoryView doesn't need it as you said. 2) It's not defined when `DataSegment` is null. 3) As it's supposed from its name, `segmentPredicate` is intended for filtering segments, but it's being used for filtering servers in this PR. It's also not good for code maintenance because it's easy to be broken if someone adds a new segmentPredicate which doesn't check null segment. > On the other hand, I think ` if (input.rhs == null) { return true; }` is some kind of reasonable just from code perspective to make sure rhs is not null before `input.rhs.getDataSource()` is called I also don't think it's reasonable to return true when rhs is null. This is related to `2)`. What does null segment mean? > I found it is too complicated to implement the new interface you suggested. It need to add a finalServerPredicate like finalPredicate(which is an 'or' by defaultFilter and segmentPredicates, it should add a new 'or' by defaultServerFilter and serverPredicates) in 3 sub classes and many related classes like BrokerServerView, and apply finalServerPredicate before applying finalPredicate when filtering segments . And the default implement for the new interface is an empty function because a zk InventoryView implement doesn't actually need this. I'm not sure what're complicated. As you said, the default implementation would be empty and zk-based inventoryView doesn't have to implement it. Currently there's only one class adding these predicates which is BrokerServerView. Adding a new `finalServerPredicate` and using it look pretty straightforward to me.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
