clintropolis commented on a change in pull request #10660:
URL: https://github.com/apache/druid/pull/10660#discussion_r540512071



##########
File path: 
sql/src/main/java/org/apache/druid/sql/calcite/schema/DruidSchema.java
##########
@@ -448,25 +449,27 @@ void removeServerSegment(final DruidServerMetadata 
server, final DataSegment seg
   {
     synchronized (lock) {
       log.debug("Segment[%s] is gone from server[%s]", segment.getId(), 
server.getName());
-      if (server.getType().equals(ServerType.BROKER)) {
-        // a segment on a broker means a broadcast datasource, skip metadata 
because we'll also see this segment on the
-        // historical, however mark the datasource for refresh because it 
might no longer be broadcast or something
-        dataSourcesNeedingRebuild.add(segment.getDataSource());
-      } else {
-        final Map<SegmentId, AvailableSegmentMetadata> knownSegments = 
segmentMetadataInfo.get(segment.getDataSource());
-        final AvailableSegmentMetadata segmentMetadata = 
knownSegments.get(segment.getId());
-        final Set<DruidServerMetadata> segmentServers = 
segmentMetadata.getReplicas();
-        final ImmutableSet<DruidServerMetadata> servers = FluentIterable
-            .from(segmentServers)
-            .filter(Predicates.not(Predicates.equalTo(server)))
-            .toSet();
-
-        final AvailableSegmentMetadata metadataWithNumReplicas = 
AvailableSegmentMetadata
-            .from(segmentMetadata)
-            .withReplicas(servers)
-            .withRealtime(recomputeIsRealtime(servers))
-            .build();
-        knownSegments.put(segment.getId(), metadataWithNumReplicas);
+      final Map<SegmentId, AvailableSegmentMetadata> knownSegments = 
segmentMetadataInfo.get(segment.getDataSource());
+      if (knownSegments != null && !knownSegments.isEmpty()) {

Review comment:
       Oh, `segmentRemoved` does not get triggered from broker segments by 
`BrokerServerView` since it is ignoring the broker segments (`segmentRemoved` 
is fired only when segment is completely removed from timeline which doesn't 
contain broker segments).
   
   I'll just keep it this way for now I think, and add some comments about why 
the null check and how someday `DruidSchema` could consider tracking the broker 
segments when `BrokerServerView` is modified to track broker served segments.




----------------------------------------------------------------
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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to