kgyrtkirk commented on code in PR #18249:
URL: https://github.com/apache/druid/pull/18249#discussion_r2206845195


##########
processing/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java:
##########
@@ -68,15 +71,15 @@ public void emit(Event event)
    */
   public List<Event> getEvents()
   {
-    return events;
+    return new ArrayList<>(events);
   }
 
   /**
    * Gets all the metric events emitted since the previous {@link #flush()}.
    *
    * @return Map from metric name to list of events emitted for that metric.
    */
-  public Map<String, List<ServiceMetricEventSnapshot>> getMetricEvents()
+  public Map<String, Queue<ServiceMetricEventSnapshot>> getMetricEvents()
   {
     return metricEvents;

Review Comment:
   
   reading pre-existing events registrations seems dodgy to me; I feel like 
that's an artifact of the fact that the usage pattern is like:
   ```
   doSomething();
   waitSomething();
   ```
   which could have trouble if `doSomething` have already happened before 
`wait` could even launche....the "read older events" is a try to fix 
this...however the problem arises from the fact that the pattern itself is 
flawed...
   
   it should be more like
   ```
   w = registerWait()
   doSomething()
   wait(w)
   ```
   
   or possibly with some try-with-resources sugar
   ```
   try( waitForSomething() ) {
     doSomething();
   }
   ```
   
   so that the contract of `waitForSomething` would be:
   * only reads messages which are recieved after its registered
   * no need to track read point/redispatch old events  -  dispatch only the 
event caught during the `emit` to the current readers
   * it won't even need access to how the events are stored by the `StubEmitter`
   
   not sure if this would apply to all existing use cases - but I hope so...
   



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