Add ParallelStep method to Trace to support logging concurrent work with an explicit duration#348
Conversation
…ith an explicit duration Signed-off-by: Richa Banker <richabanker@google.com>
k8s-ci-robot
added
kind/feature
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: richabanker The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/M
| for _, stepOrTrace := range t.traceItems { | ||
| stepOrTrace.rLock() | ||
| stepOrTrace.writeItem(b, formatter, lastStepTime, stepThreshold) | ||
| lastStepTime = stepOrTrace.time() |
There was a problem hiding this comment.
@serathius @dashpole I am wondering how should lastStepTime behave for a parallel step?
If ParallelStep advances lastStepTime normally, any sequential step that follows is measured from when ParallelStep was recorded. But if we consider a case like
t=0 trace starts; background i/o goroutine launched, CPU work begins on main thread
t=50 i/o finishes - ParallelStep("i/o", 50ms) advances lastStepTime to t=50
t=70 CPU finishes - Step("cpu computation") shows 20ms ---> should this be 70ms ?
so, should ParalleStep freeze lastStepTime to support this case ?
|
cc @p0lyn0mial |
3 participants
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adds
ParallelStepmethod to Trace to support logging concurrent work with an explicit duration.Which issue(s) this PR fixes:
Issue #kubernetes/kubernetes#136002
Special notes for your reviewer:
Release note: