A way to figure out if a certain event type has any listeners listening for it?
Opened this issue · 17 comments
I skimmed the jdocs and I couldn't find anything like it.
You can register a handler to listen for DeadEvent, and log a warning. Check the readme for details. That will at least tell you if no one is listening for an event. But that's the opposite of what you want. I don't think you can do it directly because that contradicts the design intent of an event bus, which is that publishers should not care about subscribers (i.e., who is or isn't listening.) I prefer to log at debug or trace level in every handler when an event is received, to verify, which is a useful way to see who is listening at runtime. If you need to know at compile time, I suppose you could use reflection or a 3rd party library to search for methods containing @handler or @Listener annotations.
Sent from my iPhone
On Jun 28, 2015, at 10:41 AM, Guilherme Espada notifications@github.com wrote:
I skimmed the jdocs and I couldn't find anything like it.
—
Reply to this email directly or view it on GitHub.
This is useful, for example, if my events are produced by polling, instead of pushing. If there are no listeners, then it isn't efficient to poll.
I can think of a solution to support this. Shouldn't be too hard to
implement. API suggestions are welcome.
On Jun 29, 2015 1:03 AM, "Guilherme Espada" notifications@github.com
wrote:
This is useful, for example, if my events are produced by polling, instead
of pushing. If there are no listeners, then it isn't efficient to poll.—
Reply to this email directly or view it on GitHub
#116 (comment).
The way I'd implement it would be having bus.post(message).asynchronously() return some kind of future that tells you if it actually got delivered or not, but I'm not sure if that's the best way.
Maybe bus.willDeliver(Class) or bus.wouldDeliver(Class)?
The most direct way would be to add a bus.hasListenersFor(Class<?>) method. It would also be possible to expose the subscriptions with a bus.subscriptionsFor(Class<?>) method. Exposing the subscriptions is of course opening up the internals but maybe it could be used to support other usecases. Can't think of any right now.
@3xp0n3nt , @dorkbox, @davidsowerby ....Maybe you got some comments on this?
I think it could definitely be useful for debugging purposes, but like I said, I believe that it encourages abuse because the reason to use a pub-sub mechanism, as opposed to the listener / observer pattern, is to decouple producers from consumers, and vice versa. Needing this kind of information in production code seems like a good indicator that something is wrong with the underlying design. It would be nice if this type of functionality was put into some type of debugging class or package, with extensive javadoc explaining good vs. bad use cases.
Or am I missing something here?
I think @3xp0n3nt is right on. Decoupling is the entire point of pub-sub, and we should prevent abuse of this pattern.
However, I think that it is reasonable to state that a system which polls for changes then publishes them, is a legitimate use of the pub-sub pattern. Polling could be an expensive operation, and I don't think that knowing if a publication would succeed before it's published is breaking the pattern.
How is knowing before-hand any different than knowing afterwards? (other than it's a double-hit on the performance of the system and that it would only be an estimate).
Exposing the subscriptions might be useful for debugging, but even then, I think that provides too much potential for abuse. If someone wanted them bad enough, it is easy to use reflection to get them.
Exposing the subscriptions might be useful for debugging, but even then, I think that provides too much potential for abuse. If someone wanted them bad enough, it is easy to use reflection to get them.
I agree. I think @bennidi's first suggestion preserves design intent & encapsulation the best, i.e., bus.hasListenersFor(Class<?>).
What about dealing with filters, conditions, subtype rejection, and garbage collection? I think hasListenersFor might be too low level in that case. Would that method really be useful if it returns true and yet is still not delivered? Maybe something more along the lines of @GUIpsp's suggestion, bus.isDeliverable(Class <?>)?
My understanding of the original use case would be something like ...
"Because ExpensivePublisher is an expensive operation, we only want it to publish when we know it has subscribers"
This seems to be a valid use case, and one that could come up in a number of situations. Assuming subscribers can be added / removed dynamically, which is most likely, I think there are two scenarios to solve:
- how do we stop the
ExpensivePublisherfrom doings its expensive thing when there are no longer any subscribers - how do we restart
ExpensivePublisherwhen the the first subscriber is added
With bus.subscriptionsFor(Class<?>), ExpensivePublisher would effectively be polling MBassador to see if there are any subscribers before carrying out its expensive operation - this of course would work for both the scenarios above. I understand the reservations of @3xp0n3nt and @dorkbox about the purity of the pubsub model - but I would expect a developer to make the right decision for their use case (given the right information of course). There might be an issue if bus.subscriptionsFor(Class<?>) is expensive itself.
I don't think the bus.hasListenersFor(Class<?>) really works - there would be too many occasions where a listener is not actively subscribed.
The only other solutions I can think of would be:
- for
ExpensivePublisherto actually register / de-register subscribers (and therefore replicate the standard Observer pattern listeners), or - to have some handshake mechanism between the subscribers and
ExpensivePublisherusing the MBassador bus. These all require that subscribers also publish some form of status message or acknowledgement, and would increase bus traffic.
Scenario 1 could be solved using a discovery message and the Dead Event Handler - though this then requires every subscriber to process two messages. It might be more efficient just to have ExpensivePublisher respond to the appropriate DeadEvent by switching to "inactive"
For scenario 2, the subscriber could either:
- publish back an acknowledgement (that might be ok until there are a high number of subscribers).
- publish a "I am here" message when it is instantiated, which the
ExpensivePublisherwould then use to put itself back into active mode if it was not already.
These could all be made to work (though not necessarily within MBassador itself, they are more likely to be implemented at application level ), but I can't really think of any substantial benefits over the bus.subscriptionsFor(Class<?>) approach.
I don't think the bus.hasListenersFor(Class<?>) really works - there would be too many occasions where a listener is not actively subscribed.
I agree, I think what @GUIpsp and @3xp0n3nt proposed: bus.isDeliverable(Class <?>) is a good solution. I imagine it will use the same logic as publish -- minus publishing, and return a boolean if it would have published something or not.
It's also possible to solve this problem with a future, so bus.isDeliverable(Class <?>), would return a future, where future.publish() would then run the publication. The framework already has something very similar to this (for publishing), and so supporting it would be straightforward. There is also the question of async publication, perhaps future.publishAsync()?
I agree, I think what @GUIpsp and @3xp0n3nt proposed: bus.isDeliverable(Class <?>) is a good solution. I imagine it will use the same logic as publish -- minus publishing, and return a boolean if it would have published something or not.
I have not studied the internals of MBassador enough to know what the best implementation would be - but in essence I agree. Assuming we do not care who the active subscribers are, only that there is one or more, then bus.isDeliverable(Class <?>) is a good solution
Returning either a boolean or a future would work - but there might be some efficiency advantage to the 'future' approach, as presumably the same preparation would be required for the "test" publish and the "real" publish.
Thank you for taking the time to present your views on this issue. I personally agree that exposing the subscriptions can be considered leaking internal details of the implementation. One could arguably expose the subscriptions in read only fashion but this would allow clients to introduce tighter coupling between senders and receivers by inspecting concrete listener implementations. I don't think that it is a library's responsibility to enforce correct usage but I it also should not encourage misuse.
Controlling an expensive polling operation to be only executed if there are actually receivers for the resulting messages is definitely a valid use case. I do not consider this a violation of the pub-sub pattern because it does not break decoupling of senders and receivers as they still don't need to know about each other in terms of concrete implementations. The check for existence of listeners is based on the message type which is the common protocol of the respective senders/receivers. Knowing whether a component is listening is not knowing what is listening and what its purpose might be.
In order to support this use case the implementation of hasListeners(Class) can be helpful but has several limitations as pointed out by @3xp0n3nt (a) can not account for weakly-referenced but already GCed listeners for which the reference cleanup has not yet run (b) can not account for filters and (c) can not guarantee that while being called a new listener is subscribed or the last existing listener is unsubscribed. This uncertainty seems an inherent property of (unsynchronized) concurrent code execution. I don't see the point of isDeliverable(Class) in favor of hasListeners(Class). The best one can do is to retrieve all subscriptions that match the given message type and compute their aggregated size and check size > 0. This is almost already implemented since the retrieval of matching subscriptions is the first step for message publication. Just checking if there are subscriptions - which is how I understand the proposed isDeliverable() - is even less accurate.
Listening for DeadMessage and throttling the polling interval might also be a viable alternative which can also be combined with the hasListeners() check.
Another idea that just occurred to me is to support callbacks for subscription level events.
// SubscriptionHandle is a proxy to the underlying subscription
for(SubscriptionHandle sub : bus.subscriptionsFor(ExpensiveEvent.class)){
sub.onSubscription(new Callback(){
// activate polling for expensive events
});
class ExpensivePollingController{
@Handler
public void deactivatePolling(DeadMessage pubEvent){
// if DeadMessage type is ExpensiveEvent then stop polling
}
}The subscription would invoke the callback on each invocation of its subscribe(Object) method.
I am just wondering how to support that without performance penalties for subscriptions that have no such callbacks. Maybe a strategy pattern would help? Or can we rely on JIT to optimize away unnecessary iterations over an empty array of callbacks? @dorkbox What do you think?
I don't see the point of isDeliverable(Class) in favor of hasListeners(Class).
Just to clarify, the only point in using isDeliverable(Class) was if it could be more high-level, i.e., accounting for garbage collection, filters, and "last-second" subscription changes, whereas hasListeners(Class) was supposed to be more low-level / naive, just looking for annotations with a matching type. Since @bennidi mentioned that it's not really possible to implement the higher-level isDeliverable(Class) functionality based on the current design constraints, then hasListeners(Class) makes much more sense in that context. It should be made clear in the javadoc that there is no guarantee as to actual delivery, even if there are listeners (i.e., returns true).
Also, as I originally mentioned, I like the DeadMessage solution because it's the only way to truly know if a message was undeliverable. DeadMessage is essentially equivalent to ! isDeliverable(Class), except that with DeadMessage, publication actually took place.
Now that I understand the design constraints better, I think @bennidi's hybrid approach of DeadMessage combined with hasListeners(Class) is the best solution. It should be up to the user/caller to determine if they want to use one other the other, or both in conjunction.
I made a dumb mistake in my first post, I said DeadEvent, but I meant DeadMessage. DeadEvent is from a "certain competitor's" eventbus library. 😜
I don't know enough to comment on the performance of callbacks.
I think isDeliverable should still be considered a valid solution, since the weak-reference-but-not-collected doesn't show up if you are mindful of where you {,de}register listeners. I even think there's room for both.
@3xp0n3nt Thanks for clarifying. I understand isDeliverable and hasListeners the other way round but apart from that I think we fully agree. @GUIpsp is probably right saying that both have the potential for being useful. An implementation is easy and I will ship it with the next release for sure.
Can anybody comment on the subscription level callback approach?
Sub-level callback-style might be useful in concurrent systems or for debug reporting, but currently I have no use for it.