leventov edited a comment on issue #7571: Optimize coordinator API to retrieve segments with overshadowed status URL: https://github.com/apache/incubator-druid/issues/7571#issuecomment-496594002 > The wrapper object approach allows `DataSegment` objects to always be shared I realized that they can always be shared in my approach as well: `MetadataSegmentView` can "inject" the new version of a `SegmentWithOvershadowedStatus` (with `overshadowed=true`) inside `BrokerServerView`. > Senses of equality (or deep equality, as the case may be) can be murky when inheritance is involved. Is a SegmentWithOvershadowedStatus equal to a DataSegment if every field is equal other than overshadowed? Which behavior is best probably depends on where you are in the code. Composition through wrappers makes it very clear how the comparison works. `DataSegment` already checks just the `id` field. Again, please stop thinking about `DataSegment` as a "data class" with well-defined equality. It's not. Keeping `equals()` implemented in it (as suggested here: https://github.com/apache/incubator-druid/issues/6358#issuecomment-489102183) itself needed only to enable some other memory-saving *hacks* (see the link). We should also prohibit explicit calls to `segment.equals(otherSegment)` - it's never reasonable. Either do `segment.getId().equals(otherSegment.getId())`, or `segment.allDataEquals(otherSegment)`, where `allDataEquals` has firm semantics (e. g. of not comparing `overshadowed`).
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
