kfaraz commented on code in PR #18314:
URL: https://github.com/apache/druid/pull/18314#discussion_r2236159432


##########
server/src/main/java/org/apache/druid/client/BrokerServerView.java:
##########
@@ -76,6 +80,12 @@ public class BrokerServerView implements TimelineServerView
   private final CountDownLatch initialized = new CountDownLatch(1);
   private final FilteredServerInventoryView baseView;
   private final BrokerViewOfCoordinatorConfig brokerViewOfCoordinatorConfig;
+  private final ConcurrentHashMap<RowKey, Long> totalSegmentAddCount = new 
ConcurrentHashMap<>();
+  private final ConcurrentHashMap<RowKey, Long> totalSegmentRemoveCount = new 
ConcurrentHashMap<>();
+  @GuardedBy("totalSegmentAddCount")
+  private Map<RowKey, Long> previousSegmentAddCount = new HashMap<>();
+  @GuardedBy("totalSegmentRemoveCount")
+  private Map<RowKey, Long> previousSegmentRemoveCount = new HashMap<>();

Review Comment:
   If you are adding a discovery event class, that can contain the RowKey 
itself. Then you don't really need the map. You could just have a list of 
events, which feels more natural.
   
   Drawback is that on the monitor side, you would need to distinguish added 
and removed events. So you would need to expose this DiscoveryEvent class, 
which seems unnecessary.
   
   See if you can just make do with a Boolean, or even keeping separate methods 
and lists for added/removed is fine.



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

To unsubscribe, e-mail: [email protected]

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