pkg/jobs: Scheduled jobs

[miretskiy]

Informs #49346

Add initial implementation of scheduled jobs framework.
Scheduled job is an extension of the system job, which can run
periodically.

Release notes: None

[cockroach-teamcity]

This change is 

[pbardea]

Reviewed 3 of 3 files at r1, 3 of 3 files at r2, 2 of 2 files at r3, 7 of 11 files at r4.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @miretskiy, and @pbardea)

---

pkg/jobs/executor_impl.go, line 31 at r4 (raw file):

// inlineScheduledJobExecutor implements ScheduledJobExecutor interface.
// this executor runs SQL statement "inline" -- that is, it executes statement

nit: capitalization for "this"

---

pkg/jobs/executor_impl.go, line 41 at r4 (raw file):

const retryFailedJobAfter = time.Minute

func (e *inlineScheduledJobExecutor) ExecuteJob(

It may be useful to add a comment here pointing to the ScheduledJobExecutor interface which may provide more documentation about this API in general. I've seen the format // ExecuteJob is part of the jobs.ScheduledJobExecutor interface.

---

pkg/jobs/executor_impl_test.go, line 30 at r4 (raw file):

	var tests = []struct {
		onerror jobspb.ScheduleDetails_ErrorHandlingBehavior

nit: s/onerror/onError/

---

pkg/jobs/executor_impl_test.go, line 31 at r4 (raw file):

	var tests = []struct {
		onerror jobspb.ScheduleDetails_ErrorHandlingBehavior
		nextRun time.Time

nit: I think it may be useful to name this expectedNextRun for readability.

---

pkg/jobs/job_scheduler.go, line 47 at r4 (raw file):

var prodJobSchedulerEnv jobSchedulerEnv = &prodJobSchedulerEnvImpl{}

const createdByName = "schedule"

I think it would be good to prefix this with crdb or something of the sort.

---

pkg/jobs/job_scheduler.go, line 101 at r4 (raw file):

}

// unmarshalScheduledJOb is a helper to desiralize a row returned by

nit: s/JOb/job/

---

pkg/jobs/job_scheduler.go, line 166 at r4 (raw file):

}

func (s *jobScheduler) findJobs(ctx context.Context, txn *kv.Txn) error {

The name of this method feels a bit off to me. I would not really expect findJobs to go ahead and execute the job.

---

pkg/jobs/job_scheduler_test.go, line 53 at r4 (raw file):

	} {
		t.Run(wait.String(), func(t *testing.T) {
			// Create job that with target wait behavior.

I think the'ers an extra "that" in here?

---

pkg/jobs/scheduled_job.go, line 304 at r4 (raw file):

}

// marhalChanges marshals all changes in the in-memory representation and returns

nit: s/marhalChanges/marshalChanges

---

pkg/sql/sqlbase/system.go, line 319 at r2 (raw file):

    executor_type    STRING NOT NULL,
    execution_args    BYTES NOT NULL,
    schedule_changes BYTES,

nit: Alignment here and above

[miretskiy]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @pbardea)

---

pkg/jobs/executor_impl.go, line 41 at r4 (raw file):

Previously, pbardea (Paul Bardea) wrote…

It may be useful to add a comment here pointing to the ScheduledJobExecutor interface which may provide more documentation about this API in general. I've seen the format // ExecuteJob is part of the jobs.ScheduledJobExecutor interface.

Done.

---

pkg/jobs/job_scheduler.go, line 47 at r4 (raw file):

Previously, pbardea (Paul Bardea) wrote…

I think it would be good to prefix this with crdb or something of the sort.

Done.

---

pkg/jobs/job_scheduler.go, line 166 at r4 (raw file):

Previously, pbardea (Paul Bardea) wrote…

The name of this method feels a bit off to me. I would not really expect findJobs to go ahead and execute the job.

renamed.

---

pkg/jobs/job_scheduler_test.go, line 53 at r4 (raw file):

Previously, pbardea (Paul Bardea) wrote…

I think the'ers an extra "that" in here?

Done.

---

pkg/sql/sqlbase/system.go, line 319 at r2 (raw file):

Previously, pbardea (Paul Bardea) wrote…

nit: Alignment here and above

Addressed in #49765

[miretskiy]

@pbardea Rebased; CI is now green. PTAL.

[pbardea]

Reviewed 9 of 11 files at r6.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @miretskiy, and @pbardea)

---

pkg/jobs/jobspb/schedule.proto, line 64 at r6 (raw file):

}

// Message representing sql statement to executo.

Missing 'r' at the end.

---

pkg/jobs/jobspb/schedule.proto, line 67 at r6 (raw file):

message SqlStatementExecutionArg {
  string statement = 1;
}

Reviewable doesn't seem happy about this line - is it missing a newline?

[miretskiy]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @pbardea)

---

pkg/jobs/jobspb/schedule.proto, line 64 at r6 (raw file):

Previously, pbardea (Paul Bardea) wrote…

Missing 'r' at the end.

Done.

---

pkg/jobs/jobspb/schedule.proto, line 67 at r6 (raw file):

Previously, pbardea (Paul Bardea) wrote…

Reviewable doesn't seem happy about this line - is it missing a newline?

Done.

[miretskiy]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)