gianm commented on issue #7571: Optimize coordinator API to retrieve segments with overshadowed status URL: https://github.com/apache/incubator-druid/issues/7571#issuecomment-494659852 > Added to 0.15 milestone because If SegmentWithOvershadowedStatus will leak into Druid 0.15 API, it will be much harder to remove it from later (it will require transition PRs through several consecutive Druid versions, and temporary glue code). So I would really like to see it go away before Druid 0.15 if other people agree that needs to be done. Please provide arguments if you disagree. @leventov - which API leak are you concerned about? From context I'm guessing it's the HTTP API. I suggest we address that by not documenting it and treating it as an internal API. The info is still exposed through system tables (the original motivation for creating this HTTP API), and that won't change even if we end up wanting to alter the underlying API. By the way, I'm still not really sure that a mutable DataSegment is the way to go. It just feels wrong. It's a class that is meant to represent the 'payload' in the druid_segments table and the announcement in ZK of an available segment. In my experience these sorts of widely used modeling classes work best when they are immutable. Also, if we added mutable fields like 'isOvershadowed', it would often be invalid (for example: in code that is reading or writing an individual segment announcement in ZK, where the overshadowedness concept does not have meaning). Besides, I don't think that having mutable DataSegments would really do too much to reduce churn. DataSegments aren't updated very often. Most are either never updated, or are just updated once (during realtime-to-historical handoff). If someone is doing a deep storage migration they might update all their segments, but this would be quite infrequent. If we use wrappers in situations that want to track extra state, like overshadowedness, then those wrappers could be mutable to avoid churn in the wrappers. (Meaning: we could make SegmentWithOvershadowedStatus mutable.)
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
