Add a NamedHandler, as well as the ability to remove handlers by their name.

[PaulSonOfLars]

What

Add a NamedHandler handler, which wraps existing handlers and exposes a stable name to remove them by.

With that, add methods to remove handlers.

And finally... Add some tests to make sure they all work as expected!

Impact

• Are your changes backwards compatible? Yes
• Have you included documentation, or updated existing documentation? Yes
• Do errors and log messages provide enough context? N/A

[celestix]

💕 UwU

Nice! I've finally got some stuff to play with once my exams get over 😀

[PaulSonOfLars]

I've put this on hold for now, because I'm not satisfied with the fact these methods aren't thread-safe.

On the other hand, I'm also not a fan of putting everything behind a RWMutex - I worry this would slow down update processing for many, when only a select few actually need to remove handlers.

In my mind, if you need to "remove" handlers, you should use a custom handler and build logic into the CheckUpdate methods to skip handlers which are "disabled", rather than actually remove them and risk concurrency issues. Difficult choices :)

Let me know here if you have any thoughts!

[ALiwoto]

@PaulSonOfLars you are right, this way it does have costs for both people who want the feature, and the ones who don't.
So how about adding a boolean field to config when we want to create the bot value then? 🤔

A boolean config var indicating whether the user wants the feature to be enabled or not; how does it sound like?
Those who want it, will accept the fact that a RWMutex is being used ( == slower update proccessing, remove handlers feature); but those who don't, simply won't suffer from it.

Let me know if this idea (or something like this) can solve the problem or not! (or, if there is a main problem behind it, other than this)

[PaulSonOfLars]

So how about adding a boolean field to config when we want to create the bot value then? 🤔

A boolean config var indicating whether the user wants the feature to be enabled or not; how does it sound like? Those who want it, will accept the fact that a RWMutex is being used ( == slower update proccessing, remove handlers feature); but those who don't, simply won't suffer from it.

I had a think about this, and the reason I didn't go with it is because that really smells of poorly designed code 😅 And would make it much easier to accidentally forget to use the right locks in the future.

I see another option; make the Dispatcher an interface, and implement it twice. One has removals and mutexes, and the other doesn't. All it needs are Start(...) and Stop() methods. Even allows for others to implement their own if they want to get their hands dirty!

What do you think?

[ALiwoto]

@PaulSonOfLars wow this sounds like a really really good option!
Not even it will fix out our current problem, but it will give end-users way way more flexibility by allowing them to even design their own dispatcher!
I totally agree with this way :)

[vassilit]

RWMutex - I worry this would slow down [...]
make the Dispatcher an interface

Converting to an interface will induce some overhead (allocation).

Also, there are alternatives to RWMutexes:

• atomic.Value (see the two examples): read are fast, write are very slow (must copy the entire map)
• sync.Mutex: if read contention is low, Mutex.Lock() is faster than RWMutex.RLock()
• sync.Map: if read contention is very high, might be faster than sync.RWMutex

[rusq]

LGTM

[rusq]

Maybe, once the lib is updated to go1.21, the delete element operations could be replaced with https://pkg.go.dev/slices#Delete. Easy to make a fatal slip with these 😂

This one seems fine, no fatal slips.

[PaulSonOfLars]

Absolutely agree - I was also uncertain about the thread safety of this, but the writemutex should ensure that we don't hit any weirdness here. Will wait a couple of months before moving to 1.21 and will make sure to add this when we do :)

[rusq]

I always feel uneasy when I have to modify the slice I'm iterating on. But I guess in this case it's ok, as you're returning immediately.