feat(state): add core support for Pebble Notices

[benhoyt]

This implements the core state aspects of Pebble Notices -- spec JU048.

The key user-visible methods are State.AddNotice to add a new notices, State.Notices and State.Notice to fetch and filter existing notices, and State.WaitNotices to wait (or long-poll) for notices as they occur.

The follow-on changes are in separate PRs for easier review:

• The notices API: /v1/notices and /v1/notices/{id} -- feat(daemon): add notices API #296
• The Go client for notices -- feat(client): add Go client for notices #297
• The CLI: pebble notices, pebble notice, and pebble notify) -- feat(cli): add CLI commands for notices (notices, notice, notify) #298
• Recording change-update notices whenever a change is spawned/updated - TODO
• Using notices to implement warnings (notices are basically a superset of warnings) - TODO

[hpidcock]

Looks reasonable and inline with what we discussed together.

[flotter]

Great piece of work and everything makes perfect sense for me. I have an overall comment about the concept of repeat, which probably makes up 70% of my comments directly or indirectly. The rest is all super nitty subjective preferences.

[niemeyer]

Thanks Ben, this looks like a great start.

[niemeyer]

If the way we process waiters is to, for each waiter, go over the entire list applying the filter, this whole mechanism feels a bit heavy for what it does. We could allow the code in WaitNotices itself to run this, and just have a single sync.Cond on the state which is broadcast to when notices are added. The cost of the process would be exactly the same, in that each waiter runs once through the list, and this woud save all these intermediate data structures and communication primitives.

[benhoyt]

I actually wrote it using sync.Cond initially, and it was a bit simpler, but unfortunately Cond with its Wait() method doesn't support cancellation/timeout. You can fake it by firing up a goroutine and pushing to a channel, but then you'll leak the goroutine if the timeout occurs before the event. So I went with a list of channels so that I can select on them.

Lots more reading here, where Brad Fitzpatrick suggests that Go adds a channel version of Cond.Wait to allow cancellation / selecting on it. But that hasn't been done or accepted yet.

[niemeyer]

Please have a look here: https://pkg.go.dev/context#AfterFunc

This seems to solve the problem by allowing cancellation of the context to broadcast on the Cond, so the function can simply Wait on the Cond, and when a notice arrives it calls Broadcast for the waiters to observe as desired.

Probably not as a coincidence, the example there turns out to be about Cond too.

[benhoyt]

Interesting! I hadn't seen context.AfterFunc before, and I hadn't thought of making the context cancellation do a broadcast. Unfortunately we can't use AfterFunc directly as it just arrived in Go 1.21 (we're on 1.20), but it's fairly easy to emulate what we need of it with a few lines of code.

Anyway, I've updated the PR to use sync.Cond (diff here). It's definitely simpler and avoids the tedious data structures and channel management, and will simplify further once we can use context.AfterFunc directly.

[hpidcock]

Looks good. Took a while to understand (and I should have read the comments) that the cond is hanging off the state's mutex. 👍

[pedronis]

Recording change-update notices whenever a change is spawned/updated - TODO

snapd has already grown some internal infrastructure in this area, maybe you can sync about this with @Meulengracht

[pedronis]

This looks aligned to what I would expect, using the sync.Cond doesn't have the issues I was worried about in the draft. I will ask the @Meulengracht to also take a look

[niemeyer]

Thank you, looks nice indeed. Just a couple of future trivials. Merging now so we can move on.

[niemeyer]

s/now/new/?

[benhoyt]

I actually meant "now", as in "after the above", or "at this point in time" ... but either works. I'll leave as is now. :-)

[niemeyer]

That's handy.