cockroachdb / cockroachdb/cockroach

jobs: migrate DR-owned code off the deprecated job-updater API

Open
#171,116 2 comments 0 reactions 1 assignee Claimed by @andrew-r-thomas View on GitHub
A-disaster-recovery C-cleanup P-2 T-disaster-recovery
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Summary**

The `DeprecatedUpdater` API in `pkg/jobs` (and the `DeprecatedNoTxn` / `DeprecatedWithTxn` entry points, the `DeprecatedJobMetadata` / `DeprecatedJobUpdater` types, and `DeprecatedNewChunkProgressLoggerForJob`) is deprecated in favor of the API in `pkg/jobs/job_info_storage.go`.

This issue catalogs the remaining usage in `pkg/backup`, `pkg/crosscluster/physical`, and `pkg/jobs`. Line numbers reference commit `0f497cf02`.

## Mechanical fraction/status updates

A non-deprecated sibling already exists for these cases: `ProgressStorage.Set`, `StatusStorage.Set`, `NewChunkProgressLoggerForJob`.

- `pkg/backup`:
- [backup_job.go:368](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/backup_job.go#L368)
- [restore_online.go:771](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/restore_online.go#L771)
- [restore_online.go:811](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/restore_online.go#L811)
- `pkg/jobs`:
- [ingeststopped/ingesting_checker.go:59](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/ingeststopped/ingesting_checker.go#L59)

## Typed progress-proto read-modify-write

Callback mutates `Progress.GetX()` fields and persists.

- `pkg/backup`:
- [restore_job.go:575](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/restore_job.go#L575)
- [restore_online.go:603](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/restore_online.go#L603)
- `pkg/crosscluster/physical`:
- [stream_ingestion_dist.go:88](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/stream_ingestion_dist.go#L88)
- [stream_ingestion_dist.go:134](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/stream_ingestion_dist.go#L134)
- [stream_ingestion_dist.go:285](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/stream_ingestion_dist.go#L285)
- [stream_ingestion_job.go:270](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/stream_ingestion_job.go#L270)
- [stream_ingestion_job.go:853](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/stream_ingestion_job.go#L853)
- [stream_ingestion_job.go:1179](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/stream_ingestion_job.go#L1179)

## Typed payload-proto read-modify-write

Callback mutates `Payload.GetX()` or `Payload.Details` and persists.

- `pkg/backup`:
- [backup_job.go:757](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/backup_job.go#L757)
- [compaction_job.go:360](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/compaction_job.go#L360)
- [restore_job.go:2085](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/restore_job.go#L2085)
- [restore_job.go:2347](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/restore_job.go#L2347)
- [restore_job.go:3064](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/restore_job.go#L3064)
- [restore_job.go:3322](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/restore_job.go#L3322)
- [restore_job.go:4197](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/restore_job.go#L4197)
- `pkg/crosscluster/physical`:
- [stream_ingestion_job.go:794](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/stream_ingestion_job.go#L794)
- [alter_replication_job.go:657](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/alter_replication_job.go#L657)
- [alter_replication_job.go:688](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/alter_replication_job.go#L688)

## Txn-coupled with non-jobs writes

The in-callback work shares the txn with a PTS record write, and that coupling is load-bearing.

- `pkg/crosscluster/physical`:
- [stream_ingestion_frontier_processor.go:351](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/stream_ingestion_frontier_processor.go#L351)
- `pkg/jobs`:
- [jobsprotectedts/jobs_protected_ts_manager.go:181](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/jobsprotectedts/jobs_protected_ts_manager.go#L181)
- [jobsprotectedts/jobs_protected_ts_manager.go:221](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/jobsprotectedts/jobs_protected_ts_manager.go#L221)

## Engine internals (state transitions and column-level UPDATEs)

These sites either drive a state transition (`started`/`succeeded`/`failed`/`reverted`/`canceled`/`revertFailed`/`Unpaused`/`CancelRequested`/`PauseRequested`/`load`) or perform the column-level UPDATE of `status`/`description`/`owner`/`error_msg`/`finished` done by `DeprecatedUpdater.update`.

- `pkg/crosscluster/physical`:
- [alter_replication_job.go:623](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/alter_replication_job.go#L623)
- `pkg/jobs`:
- [registry.go:184](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L184)
- [registry.go:875](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L875), [:898](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L898)
- [registry.go:1316](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L1316), [:1324](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L1324)
- [registry.go:1340](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L1340), [:1351](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L1351)
- [registry.go:1651](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L1651), [:1723](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L1723), [:1739](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L1739), [:1751](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L1751), [:1791](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L1791), [:1810](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/registry.go#L1810)
- [adopt.go:631](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/adopt.go#L631)
- [jobs.go:243-549](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/jobs.go#L243-L549)
- [jobs.go:894](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/jobs.go#L894)
- [progress.go:79](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/progress.go#L79)
- [testing_knobs.go:55](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/testing_knobs.go#L55)
- [validate.go:20](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/validate.go#L20), [:59](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/validate.go#L59), [:97](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/validate.go#L97)
- [metricspoller/job_statistics.go:272](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/metricspoller/job_statistics.go#L272)

## Cross-cutting notes

1. The `updateStatus` / `updateStatusInternal` helper in [stream_ingestion_job.go:263-292](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/stream_ingestion_job.go#L263-L292) is called from multiple sites in the "Typed progress-proto" section. Migrating the helper migrates all of its callers.
2. The callbacks under "Txn-coupled with non-jobs writes" perform PTS record writes inside the deprecated update callback because the callback's txn is the same one the PTS writes commit on. Replacing the deprecated API has to preserve that coupling.
3. [alter_replication_job.go:623](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/crosscluster/physical/alter_replication_job.go#L623) is the only site outside `pkg/jobs` that drives a state transition.
4. `pkg/backup/restore_job.go` has five near-identical `SetDetails` call sites that mutate `RestoreDetails`.
5. `progress.go` has a non-deprecated `NewChunkProgressLoggerForJob` ([line 62](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/jobs/progress.go#L62)) that uses the new API. The no-callback site in [backup_job.go:368](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/backup_job.go#L368) can switch to it without changes. The with-callback site in [restore_job.go:575](https://github.com/cockroachdb/cockroach/blob/0f497cf02/pkg/backup/restore_job.go#L575) would need the new logger to grow a per-chunk callback hook.

Jira issue: CRDB-64330

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.