x/time/rate: add Sometimes type

[gaal]

This proposal makes acting on simple rate throttlers more ergonomic. It adds an API that we use internally (I'm not the author). Adding it will ease open-sourcing a project I'm involved in.

package rate // import "golang.org/x/time/rate"

import "time"

// Sometimes performs an action occasionally.
// The public fields govern the behavior of Do, which performs the action.
// A zero Sometimes performs the action exactly once.
//
// C++ users familiar with the glog package can use this mechanism instead
// of LOG_FIRST_N, LOG_EVERY_N, LOG_EVERY_N_SEC:
//
//	var sometimes = rate.Sometimes{First: 3, Interval: 10*time.Second}
//	func Spammy() {
//	  sometimes.Do(func() { log.Info("here I am!") })
//	}
type Sometimes struct {
	First    int           // if non-zero, the first N calls to Do will run f.
	Every    int           // if non-zero, every Nth call to Do will run f.
	Interval time.Duration // if non-zero and Interval has elapsed since f's last run, Do will run f.
}

// Do runs f, as governed by First, Every, and Interval.
//
// The model is is a union of filters. The first call to Do
// always runs f. Subsequent calls run f if allowed by any
// one of the Sometimes fields.
//
// If Do is called multiple times simultaneously, calls will block
// and run serially. It is therefore intended for lightweight operations.
//
// Because a call to Do may block until f returns, if f causes Do
// to be called, it will deadlock.
func (s *Sometimes) Do(f func())

// 2022-08-21 Edited: moved from "x/time/rate".Sometimes -> "x/time/some".Times based on feedback.
// 2022-08-31 Edited: moved back to "x/time/rate".Sometimes.

[Sajmani]

I'm the original author of this type inside Google. It's simple, and the API has been stable for many years. If it's useful, I'm fine seeing this added to the existing rate package.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[gopherbot]

Change https://go.dev/cl/421915 mentions this issue: x/time/rate: add Sometimes, which runs a function occasionally.

[rsc]

Does anyone object to accepting this proposal?

[deefdragon]

I have 2 things.

One: given the code comment specifically mentions it being implemented as a union of the filters, it would be more flexible as an array of filters/filter functions which are then unioned. This implementation would come at the cost of complexity, where I acknowledge this is explicitly trying to be simple. This more of a personal preference but I think worth discussing.

Two: I continue to be of the opinion that before we expand rate any further, there should be a common rate limiter interface defined that different rate limiters can then implement.

[gaal]

One: given the code comment specifically mentions it being implemented as a union of the filters, it would be more flexible as an array of filters/filter functions which are then unioned. This implementation would come at the cost of complexity, where I acknowledge this is explicitly trying to be simple. This more of a personal preference but I think worth discussing.

Two: I continue to be of the opinion that before we expand rate any further, there should be a common rate limiter interface defined that different rate limiters can then implement.

There are many existing callers to this in the Google codebase, overwhelmingly rate-limited log statements. In those callsites, the simplicity of the API (as documented, modeled after sync.Once) is a big win. It decouples the particular log library from the Sometimes policy. And to date these have been the kinds of throttles that we've needed for these operations: I don't think adding abstraction would help. Adding flexibility might have allowed additional kinds of callers, but at the cost of complicating the setup, possibly significantly. I think it's worth the 20 LOC to be added as-is.

[deefdragon]

I don't think adding Checks [] func() bool to the struct would make setup that much more complicated as it could just be ignored if nil, but in retrospect, I also can not come up with much of a reasonable use of a Checks array where you wouldn't be better off just using some other mechanism. Also, adding the above can be done later if shown useful, so I withdraw my first remark.

I still believe in my second point that a common interface should be described before more changes to the rate package are added, but I can see this easily being made interface compatible to almost anything if one is ever introduced, so that's a bit of a consolation.

I have one final extra point. Is there similar code or similar acting helpers that are known elsewhere outside of google? I can see this being useful, but I want to make sure it meets the "address an important issue for many people" standard.

[rsc]

How often is this used for something other than a rate-limiting log statement? I'm curious whether it might belong nearer to logging if it's really log-specific.

[gaal]

The vast majority of Google callsites use this for logging. How would putting it closer to logging look like, though? Just copy the current type into packages log (and glog)? Does that need another proposal?

(BTW, while gathering this data, regexp lookaround assertions would have come in handy, though not strictly necessary.)

[gaal]

I forgot to mention, though, that it's also used to throttle t.Log and ctxlog. Those have similar requirements, but it would be odd to add Sometimes to packages testing and again to ctxlog for this; and it would also be odd to add Sometimes only to stdlib's log and have (say) tests do

var sometimes = log.Sometimes{...}
// ...
sometimes.Do(func() { t.Log(...) })

So personally I'm in favor of adding it to package rate.