util: define and implement core interfaces for async api #1591
Conversation
Signed-off-by: zyguan <[email protected]>
ti-chi-bot
bot
added
dco-signoff: yes
|
/retest |
|
@cfzjywxk PTAL |
| once sync.Once | ||
| e Executor | ||
| f func(T, error) | ||
| gs []func(T, error) (T, error) |
There was a problem hiding this comment.
f and g are common function names (in mathematics), and gs stands for a list of g (a common naming convention in FP).
| l.lock.Unlock() | ||
| return 0, errors.New("runloop: already executing") | ||
| } | ||
| // assert l.state == stateIdle |
There was a problem hiding this comment.
I leave this comment on purpose. It's not a runnable code in golang, just reminds us that the state should be idle in the following code.
| l.lock.Unlock() | ||
| return count, ctx.Err() | ||
| default: | ||
| f() |
There was a problem hiding this comment.
How about passing the ctx to f() so it could be terminated? Timeout cancel may be needed.
There was a problem hiding this comment.
I prefer keeping it simple, if f here accepts a ctx as an arg, then Callback functions (cb.f, cb.gs and call related methods) should accept a ctx as well.
In fact, ctx is typically referenced in the cb.f. eg.
func sendAll(ctx, reqs) {
completed := 0
e := NewExecutor(...)
for req in reqs {
cb := NewCallback(e, func(resp, err) {
handleResponse(ctx, resp, err)
completed++
}
cli.asyncSend(ctx, req, cb)
}
for completed < len(reqs) {
e.Exec()
}
}| l.Go(func() { atomic.StoreUint32(&n, 1) }) | ||
| require.Eventually(t, func() bool { return atomic.LoadUint32(&n) == 1 }, time.Second, time.Millisecond) | ||
|
|
||
| // use a customized pool |
There was a problem hiding this comment.
| // use a customized pool | |
| // Use a customized pool |
ti-chi-bot
bot
added
needs-1-more-lgtm
The scope shall be small, typically one per query/txn-cmd, here is an example.
Yes, but it depends on the caller, RunLoop itself cannot perceive the business logic. Eg. when we send multiple requests, we can either collect all errors, or just return the first error (drop the rest). |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfzjywxk, ekexium The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ref #1586