admission: move kv admission under tracing span setup#77538
admission: move kv admission under tracing span setup#77538craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
sumeerbhola
left a comment
There was a problem hiding this comment.
looks ok to me, but can you get someone from KV to approve -- there may be some subtleties that I am unaware of.
Reviewable status:
complete! 0 of 0 LGTMs obtained
andreimatei
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/server/node.go, line 979 at r1 (raw file):
var pErr *roachpb.Error br, pErr = n.stores.Send(ctx, *args) n.admissionController.AdmittedKVWorkDone(handle)
can we defer this?
cucaroach
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/server/node.go, line 979 at r1 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
can we
deferthis?
@sumeerbhola ? I can't think of compelling argument either way given how simple the function is. I guess it boils down to do we want callComplete blocking the call to AdmittedKVWorkDone.
sumeerbhola
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/server/node.go, line 979 at r1 (raw file):
Previously, cucaroach (Tommy Reilly) wrote…
@sumeerbhola ? I can't think of compelling argument either way given how simple the function is. I guess it boils down to do we want callComplete blocking the call to AdmittedKVWorkDone.
defer-ing is fine, since callComplete is cpu work.
Release justification: Important for full tracing visibility Release note: none
|
TFTRs |
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build succeeded: |
Release justification: Important for full tracing visibility
Release note: none