-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
*: use async get timestamp API && remove asyncGetTSWorker #3459
Conversation
go asyncGetTSWorker(s.store, s.txnFutureCh) | ||
func (tf *txnFuture) wait() (kv.Transaction, error) { | ||
startTS, err := tf.future.Wait() | ||
if err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to handle the error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error is always Canceled
, caused by network timeout.
The real reason may be pd leader change or server down or network problem...
But it doesn't matter, if err not nil, we'll call store.Begin()
in line 1070,
which would retry get timestamp until timeout.
txnFuture := s.getTxnFuture() | ||
goCtx, cancelFunc := goctx.WithCancel(goctx.Background()) | ||
s.txnFuture, s.goCtx, s.cancelFunc = txnFuture, goCtx, cancelFunc | ||
s.goCtx, s.cancelFunc = goctx.WithCancel(goctx.Background()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why store the Context inside a struct type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For historical reasons.
Each transaction goes along with a new context, transaction run in a session,
so the context is stored in the session.
LGTM |
LGTM |
With the new GetTSAsync API, a session doesn't need to bind a
asyncGetTSWorker
goroutine any more.This change brings those benefits:
session.Close
is not called.