ObserverManager Notify should fan out

[bjorkstromm]

Peeked at the code for ObserverManager Notify, and I think we should fan out notifications. Notifying all observers in serial is not necessary in my opinion.

orleans/src/Orleans.Core/Utils/ObserverManager.cs

Lines 175 to 184 in d08e286

	try
	{
	await notification(observer.Value.Observer);
	}
	catch (Exception)
	{
	// Failing observers are considered defunct and will be removed..
	defunct ??= new List<TIdentity>();
	defunct.Add(observer.Key);
	}

[ReubenBond]

Good idea

[willg1983]

If the notifiation delegate had closure over grain state would that introduce either conurrency or re-entrant behaviour by stealth?

Separately, but related to that are a few other problems/limitations I've seen with the current implementation, which could be addressed at the same time:

Calling Notify from a re-entrant grain can result in an InvalidOperationException - Collection was modified; enumeration operation may not execute. I assume because whilst the ObserverManager is using foreach loop to notify each observer (and awaiting completion) a re-entrant grain is free to process subscribe/unsubscribe commands from other observers which modify the underlying Dictionary in the ObserverManager, leading to the exception when the loop resumes.

It would be useful (at least I would find it useful) to be able to set a timeout for the observer notification, so that a slow observer does not hang the grain and is treated as defunct and removed. Not sure if there's already a good mechanism for doing this? or whether something like #7837 would address this? I currently use WaitAsync peppered around my observer notifications but it feels like boilerplate code that could be moved to the ObserverManager.

Should we look at supporting serialization of the ObserverManager so that a grain activation with observers that fails, can be re-activated on another silo and continue sending to the set of observers already subscribed to it, without waiting for them to re-subsubscribe when they hit their expiry time.

It might also be useful to change the method name of Notify to NotifyAsync as per naming conventions and possibly add cancellation token support - although that has marginal benefit.

All this may suggest revisiting the implementation of ObserverManager to fix the re-entrant bug and make the state serializable, and maybe add an new method to support fan-out distribution explicitly (which can follow the async naming convention and take a cancellation token for example).