clintropolis commented on a change in pull request #7185: Avoid many
unnecessary materializations of collections of 'all segments in cluster'
cardinality
URL: https://github.com/apache/incubator-druid/pull/7185#discussion_r262817348
##########
File path:
server/src/main/java/org/apache/druid/server/coordinator/helper/DruidCoordinatorSegmentInfoLoader.java
##########
@@ -42,8 +42,17 @@ public DruidCoordinatorRuntimeParams
run(DruidCoordinatorRuntimeParams params)
{
log.info("Starting coordination. Getting available segments.");
- // Display info about all available segments
- final Set<DataSegment> availableSegments =
coordinator.getOrderedAvailableDataSegments();
+ final TreeSet<DataSegment> availableSegments =
DruidCoordinatorRuntimeParams.createAvailableSegmentsSet();
+ for (DataSegment segment : coordinator.iterateAvailableDataSegments()) {
+ if (segment.getSize() < 0) {
+ log.makeAlert("No size on a segment")
Review comment:
I know this isn't introduced here, but is it actually useful? This check
looks like it has been here since the initial OSS commit, though in that form
it looks like it attempted to do something if size was less than 0.
It seems like this should happen, and we don't actually do anything other
than alert if it does happen, so does it really qualify as an alert?
The main reason I ask is it seems like the 3 places we are calling
`DruidCoordinatorRuntimeParams.createAvailableSegmentsSet()` we then
immediately fill it up afterwards, perhaps it could just take an
`Iterable<DataSegment>` instead and could just produce the populated `TreeSet`
directly if we didn't need this alert.
----------------------------------------------------------------
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]
