build: update patch with bug fix to runtime grunning #118907
Conversation
|
This change is |
|
Started TeamCity build |
aadityasondhi
force-pushed
the
20240205.grunning-patch
branch
from
d4d52ad to
84925d0
Compare
|
Previous patch was corrupted because I tried to minimize the lines changed. This one applies well but changes a lot more lines. Am I doing it correctly @rickystewart? Steps I followed:
|
aadityasondhi
force-pushed
the
20240205.grunning-patch
branch
from
84925d0 to
221e0e4
Compare
|
Can you put the diff for |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @aadityasondhi)
build/teamcity/internal/release/build-and-publish-patched-go/diff.patch line 113 at r3 (raw file):
--- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1004,6 +1004,9 @@ func casfrom_Gscanstatus(gp *g, oldval, newval uint32) {
We have two comments in the existing patch:
// We're transitioning into the running state, record the timestamp for
// subsequent use.
// We're transitioning out of running, record how long we were in the
// state.
can you add the relevant comment to these new changes too.
aadityasondhi
force-pushed
the
20240205.grunning-patch
branch
from
221e0e4 to
4cf5e85
Compare
| _32bit uintptr // size on 32bit platforms | ||
| _64bit uintptr // size on 64bit platforms | ||
| }{ | ||
| - {runtime.G{}, 252, 408}, // g, but exported for testing | ||
| + {runtime.G{}, 260, 416}, // g, but exported for testing | ||
| {runtime.Sudog{}, 56, 88}, // sudog, but exported for testing | ||
| } | ||
| diff -ru a/src/context/context.go b/src/context/context.go | ||
|
|
There was a problem hiding this comment.
Blank line here seems wrong
There was a problem hiding this comment.
To be clear: if the diff apply works, don't worry about removing the blank line. Just a thought I had as I was reviewing.
|
Trying another build, we'll see if this works. |
rickystewart
force-pushed
the
20240205.grunning-patch
branch
from
4cf5e85 to
b523584
Compare
|
Looks like it worked. I updated the PR and then we'll see if CI is happy. |
aadityasondhi
force-pushed
the
20240205.grunning-patch
branch
3 times, most recently
from
3960a3e to
11fbe3d
Compare
|
Had to rebase on top of the changes made in #118605. @rickystewart let me know if this is good to merge |
There was a problem hiding this comment.
Reviewable status:
build/teamcity/internal/release/build-and-publish-patched-go/diff.patch line 113 at r3 (raw file):
Previously, sumeerbhola wrote…
We have two comments in the existing patch:
// We're transitioning into the running state, record the timestamp for // subsequent use.// We're transitioning out of running, record how long we were in the // state.can you add the relevant comment to these new changes too.
Done.
There was a problem hiding this comment.
Reviewable status:
build/teamcity/internal/release/build-and-publish-patched-go/diff.patch line 187 at r4 (raw file):
Previously, rickystewart (Ricky Stewart) wrote…
To be clear: if the diff apply works, don't worry about removing the blank line. Just a thought I had as I was reviewing.
Done.
No. We need to build the SDK and incorporate it into your PR. I can get this process started for you. |
Thanks! let me know if I need to do anything to move this forward. |
|
@aadityasondhi Go ahead and apply this diff and we'll see if CI is okay with this. |
aadityasondhi
force-pushed
the
20240205.grunning-patch
branch
from
11fbe3d to
89289a8
Compare
In our previous version of this patch, we missed two entry points and one exit point into the grunning state of a Goroutine. This led to `grunning.Time()` being non-monotonic. This new patch adds those missing integration points. Fixes cockroachdb#95529. Release note: None
aadityasondhi
force-pushed
the
20240205.grunning-patch
branch
from
89289a8 to
6af173f
Compare
Build is looking good, feel free to bors if you think it is good to do so. |
|
bors r=sumeerbhola,rickystewart |
|
Build succeeded: |
This was fixed in cockroachdb#118907. This patch adds an assertion for test builds to ensure monotonic grunning time. Informs cockroachdb#95529 Release note: None
126424: kvadmission: remove grunning non-monotonic comment r=aadityasondhi a=aadityasondhi This was fixed in #118907. This patch adds an assertion for test builds to ensure monotonic grunning time. Informs #95529 Release note: None Co-authored-by: Aaditya Sondhi <[email protected]>
In our previous version of this patch, we missed two entry points and one exit point into the grunning state of a Goroutine. This led to
grunning.Time()being non-monotonic.This new patch adds those missing integration points.
Fixes #95529.
Release note: None