jobs,schedule: Annotate context with schedule ID. #69922
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @dt)
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/backupccl/schedule_exec.go, line 112 at r2 (raw file):
func invokeBackup(ctx context.Context, backupFn sql.PlanHookRowFn) error { resultCh := make(chan tree.Datums) // No need to close
wait, why this change? If an error gets returned from backupFn then the context will be canceled hence No need to close.
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/backupccl/schedule_exec.go, line 112 at r2 (raw file):
Previously, ajwerner wrote…
wait, why this change? If an error gets returned from
backupFnthen the context will be canceled henceNo need to close.
I'm not sure the context will be cancelled. This is the context passed from the scheduler, and afaik, it is never cancelled.
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/backupccl/schedule_exec.go, line 112 at r2 (raw file):
Previously, miretskiy (Yevgeniy Miretskiy) wrote…
I'm not sure the context will be cancelled. This is the context passed from the scheduler, and afaik, it is never cancelled.
Notice the use of ctxgroup. The context passed to the GoCtx functions is canceled once any such function returns an error.
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/backupccl/schedule_exec.go, line 112 at r2 (raw file):
Previously, ajwerner wrote…
Notice the use of ctxgroup. The context passed to the
GoCtxfunctions is canceled once any such function returns an error.
Ahh.. You're right. I'll backout this change.
|
@ajwerner -- ctxgroup change backed out; keeping ctx annotation change. |
miretskiy
changed the title
Annotate context passed to job execution with the schedule ID. Release Justification: Low danger, observability imporovement; category 4. Release Notes: None
|
bors r+ |
|
Build succeeded: |
Annotate context passed to job execution with the schedule ID.
Release Justification: Low danger, observability imporovement; category 4.
Release Notes: None