Improve reporting of aborts and retries during live load. #3313
Conversation
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @codexnull and @manishrjain)
dgraph/cmd/live/batch.go, line 82 at r1 (raw file):
start time.Time nreqs uint64
not sure what nreqs is just by looking at it. does it stand for "number of requests"? If so, maybe numReqs is better.
dgraph/cmd/live/batch.go, line 130 at r1 (raw file):
// the node certificate is created the name much match the request host name. e.g., localhost not // 127.0.0.1. func handleError(err error, nreq uint64, isRetry bool) {
nit: I think reqNum is a slightly better name.
Same for all the other instances of this variable name.
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @manishrjain and @martinmr)
dgraph/cmd/live/batch.go, line 82 at r1 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
not sure what nreqs is just by looking at it. does it stand for "number of requests"? If so, maybe
numReqsis better.
Done.
|
Defer to @martinmr for review. |
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @codexnull)
dgraph/cmd/live/batch.go, line 196 at r2 (raw file):
defer l.requestsWg.Done() for req := range l.reqs { nreq := atomic.AddUint64(&l.reqNum, 1)
Should this also be called reqNum to match the variable inside the loader? Same for all other instances of nreq.
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @martinmr)
dgraph/cmd/live/batch.go, line 196 at r2 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
Should this also be called
reqNumto match the variable inside the loader? Same for all other instances of nreq.
Yes. Fixed.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status:complete! all files reviewed, all discussions resolved
Summary of changes:
This change is