chore: events: implement event observer deregister method

[frrist]

• guarded with test

Related Issues

Lily schedles jobs that use the events system to observe head changes. When these jobs exit their observer needs to be deregistered from the events system. Currently this is not possible, this PR addresses this.

Proposed Changes

Allow event observers to be deregistered

Checklist

Before you mark the PR ready for review, please make sure that:

• All commits have a clear commit message.
• The PR title is in the form of of <PR type>: <area>: <change being made>
  • example: fix: mempool: Introduce a cache for valid signatures
  • PR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, test
  • area: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps
• This PR has tests for new functionality or change in behaviour
• If new user-facing features are introduced, clear usage guidelines and / or documentation updates should be included in https://lotus.filecoin.io or Discussion Tutorials.
• CI is green

[magik6k]

Some things may care about the order this map in iterated in (mostly tests tho)

[frrist]

I could make a TipSetObservers type and implement an iterator pattern over it for consistent ordering, would that work? Something like:

for observers.HasNext() {
	if err := observers.Next().Revert(ctx, from, to); err != nil {
		log.Errorf("observer %T failed to apply tipset %s (%d) with: %s", obs, from.Key(), from.Height(), err)
	}
}

[codecov]

Codecov Report

Merging #8441 (20bf46f) into master (0fa74a7) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #8441      +/-   ##
==========================================
- Coverage   40.66%   40.62%   -0.04%     
==========================================
  Files         686      686              
  Lines       75398    75409      +11     
==========================================
- Hits        30661    30636      -25     
- Misses      39443    39461      +18     
- Partials     5294     5312      +18     

Impacted Files	Coverage Δ
chain/events/observer.go	73.79% <100.00%> (-6.06%)	⬇️
chain/events/message_cache.go	87.50% <0.00%> (-12.50%)	⬇️
miner/warmup.go	63.26% <0.00%> (-8.17%)	⬇️
storage/wdpost_sched.go	81.37% <0.00%> (-5.89%)	⬇️
chain/events/events_called.go	83.90% <0.00%> (-3.42%)	⬇️
miner/miner.go	58.03% <0.00%> (-1.97%)	⬇️
extern/storage-sealing/currentdealinfo.go	71.17% <0.00%> (-1.81%)	⬇️
chain/store/store.go	64.33% <0.00%> (-1.50%)	⬇️
chain/stmgr/searchwait.go	67.30% <0.00%> (-1.29%)	⬇️
... and 12 more

[Stebalien]

This seems complicated and expensive. Given that adding/removing observers is rare, can we notadd a Deregister method that:

1. Takes the same TipSetObserver interface.
2. Compares by-pointer with all elements in the observer list to find the observer to remove.
3. Creates a new list without that observer?

Do you need observer "IDs"?

[Stebalien]

Here's a sketch: #8445

[frrist]

Agreed, its a bit complicated to maintain consistent ordering with this approach. Yeah passing the TipSetObserver interface to an unregister method will work for Lily's use case, thanks for the sketch.