services: implement Health.Watch#4930
Conversation
| private final Object watchLock = new Object(); | ||
|
|
||
| @GuardedBy("watchLock") | ||
| private final Multimap<String, StreamObserver<HealthCheckResponse>> watchers = |
There was a problem hiding this comment.
If possible I think it would be good if this was a Map<String, List>, to avoid guava deps
There was a problem hiding this comment.
Although Guava collections dependency is permissible for server-side code, making Multimap use identity hash set would require extra code and lose the benefits of being shorter. I changed to the plain Java collections.
| // Called when the client has closed the stream | ||
| public void cancelled(Context context) { | ||
| synchronized (watchLock) { | ||
| watchers.remove(service, responseObserver); |
There was a problem hiding this comment.
I don't think there is promise about the equality of observers. An interceptor may pass in a StreamObserver that has equality based on call state.
I think the multimap values should be an identity hash set. That would make removal O(1), and avoid equality.
There was a problem hiding this comment.
HashMultimap is O(1) removal, but the equality is a valid concern. Switch to identity hash set.
2 participants
Part of #4932