jonboulle / clockwork

a fake clock for golang
Apache License 2.0
625 stars 57 forks source link

Upcoming changes in Go 1.23 #84

Open sagikazarmark opened 1 month ago

sagikazarmark commented 1 month ago

https://tip.golang.org/doc/go1.23#timer-changes

DPJacques commented 1 month ago

"the timer channel associated with a Timer or Ticker is now unbuffered, with capacity 0." seems like a problem on first read.

We can't quickly iterate over multiple expirations during Advance() because if the receiver is taking an action on receive, they will be unlikely to receive and act on more than 1 expiration per call to Advance(). Ex: If a ticker ticks every second, and we advance 5 seconds, the receiver will register the first tick, but will be unavailable to receive the next 4 (because it is action on the first). However, we can't naively block on each tick for Tickers because slow receivers should not receive ticks.

So, there is no obvious way without an API modification.

So what should the API do? Some options:

  1. Nothing - the caller is responsible for advancing onto the next expiration, and they are responsible for knowing what that expiration is.
  2. Modify Advance() in some way to make it more usable
  3. Make an AdvanceToWaiter(max time.Duration) (remaining time.Duration) that advances to the next waiter expiration or a maximum duration, returning any duration that was short of the max. A return value of 0 means it advanced max duration.
    • Bonus: we could also return a count of expired waiters, so the caller can verify what happened.
  4. We can also add an AdvanceEach(context.Context) error which does positively block on each waiter receiving its tick.

I'll have to think on this for a while, but suggestions welcome. I don't think Option 2 is right. Option 1 is easy, but not user friendly. Option 4 is likely what most users expect.

DPJacques commented 1 month ago

More thoughts:

I think it is reasonable to offer 2 versions to the caller, a blocking and non-blocking version, #1 and #4 above.

Advance() should probably go unchanged, although it may break some callers due to the 1.23.

I was thinking... AdvanceBlocking(ctx context.Context) error as the version of advance that blocks on each watier.