cdc: Add highwater timestamp to crdb_internal.jobs #28156
Conversation
|
This change is |
There was a problem hiding this comment.
The documentation for cluster_logical_timestamp says "This function is reserved for testing purposes by CockroachDB developers and its definition may change without prior notice" so I'm a little hesitant to add it to SHOW JOBS. That said, I think we're pretty committed at this point to making this timestamp as decimal format work as an input to AS OF SYSTEM TIME. @bdarnell thoughts?
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale)
pkg/sql/show_test.go, line 822 at r1 (raw file):
out.details = in.details // Shift 10 decimals to the right, so that the logical
eh? what's doing this shift?
mrtracy
force-pushed
the
mtracy/system_jobs_highwater
branch
from
0376e94 to
69848d5
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/show_test.go, line 822 at r1 (raw file):
Previously, danhhz (Daniel Harrison) wrote…
eh? what's doing this shift?
Ah, that's left over from before I found tree.DecimalToHLC(). Removed.
mrtracy
force-pushed
the
mtracy/system_jobs_highwater
branch
from
69848d5 to
80a17ef
Compare
There was a problem hiding this comment.
What's the purpose of this field? Is it for end users (for monitoring that your changefeed jobs aren't falling too far behind, for example), or for our internal checkpointing purposes? Do we expect users to issue AS OF SYSTEM TIME queries using this value?
For monitoring purposes, I think it would be better to expose a TIMESTAMP column for ease of understanding. For checkpointing, I'd prefer a binary blob (encoded protobuf?) to give us flexibility to store other kinds of checkpoints (maybe including other kinds of jobs) without committing to implementation details (we might even want two separate fields, a TIMESTAMP and a blob, if we want to serve both needs). But if we expect this value to make its way back into AOST queries, decimal might be the right choice.
Reviewable status:
mrtracy
force-pushed
the
mtracy/system_jobs_highwater
branch
from
80a17ef to
14ca338
Compare
|
After talking with @danhhz, I believe we are going to remain with the DECIMAL notation; usage in AOST queries seems likely. |
mrtracy
force-pushed
the
mtracy/system_jobs_highwater
branch
from
14ca338 to
b76e953
Compare
mrtracy
force-pushed
the
mtracy/system_jobs_highwater
branch
from
b76e953 to
77fea65
Compare
|
@danhhz I found an error in the Admin API Jobs method that resulted from this change and fixed it; would you mind taking another look at this new code? |
There was a problem hiding this comment.
Reviewable status:
Adds column high_water_timestamp to the crdb_internal.jobs table, and thus also to the output of SHOW JOBS. This column is stored internally as a HLC timestamp, but is output to the user as a DECIMAL in the same fashion as the `cluster_logical_timestamp()` built-in. This column will be nil for existing jobs, which use fraction_completed to represent their progress. The Admin API Jobs method has also been modified to handle the possibly-null output of fraction_completed. Release note (general change): Added an additional column to crdb_internal.jobs and SHOW JOBS to represent the "high water timestamp" of Changefeed jobs. This is used to represent the progress of changefeeds, as an alternative to the fraction_completed column which is used by existing jobs. Release note: None
mrtracy
force-pushed
the
mtracy/system_jobs_highwater
branch
from
77fea65 to
9f0c6da
Compare
|
bors r+ |
28156: cdc: Add highwater timestamp to crdb_internal.jobs r=mrtracy a=mrtracy Adds column high_water_timestamp to the crdb_internal.jobs table, and thus also to the output of SHOW JOBS. This column is stored internally as a HLC timestamp, but is output to the user as a DECIMAL in the same fashion as the `cluster_logical_timestamp()` built-in. This column will be nil for existing jobs, which use fraction_completed to represent their progress. Release note: Added an additional column to crdb_internal.jobs and SHOW JOBS to represent the "high water timestamp" of Changefeed jobs. This is used to represent the progress of changefeeds, as an alternative to the fraction_completed column which is used by existing jobs. Co-authored-by: Matt Tracy <[email protected]>
Build succeeded |
Adds column high_water_timestamp to the crdb_internal.jobs table, and
thus also to the output of SHOW JOBS. This column is stored internally
as a HLC timestamp, but is output to the user as a DECIMAL in the same
fashion as the
cluster_logical_timestamp()built-in. This column willbe nil for existing jobs, which use fraction_completed to represent
their progress.
Release note: Added an additional column to crdb_internal.jobs and SHOW
JOBS to represent the "high water timestamp" of Changefeed jobs. This is
used to represent the progress of changefeeds, as an alternative to the
fraction_completed column which is used by existing jobs.