protectedts: add target field to pts record#74211
protectedts: add target field to pts record#74211craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
6425c34 to
0e7c029
Compare
|
Wanted to start getting some feedback on the skeleton work while we are still iterating on the RFC. |
0e7c029 to
2ed96a6
Compare
| // DeprecatedSpans are the spans which this Record protects. | ||
| repeated roachpb.Span deprecated_spans = 7 [(gogoproto.nullable) = false]; | ||
|
|
||
| // Target holds information about the objects on which the protected timestamp |
There was a problem hiding this comment.
[nit] Reword to avoid "objects"?
2ed96a6 to
826929d
Compare
irfansharif
left a comment
There was a problem hiding this comment.
The proto changes LGTM, and it makes sense to add a corresponding column to the protected timestamp system table in each tenant. I'll leave it to you what you want to do with this PR (either merge here, some side branch, or just leave open); perhaps others would prefer to see the corresponding RFC out for public review first.
826929d to
ad0328c
Compare
Protected timestamp records are moving away from the notion of protecting spans, and instead will operate on objects. Objects will be defined as: - Cluster - Tenant - Schema objects (database and table) This change deprecates the Spans field on `ptpb.Record`. Additionally, it adds a `oneof` target field that reflects which of the above objects the record corresponds to. This information will be needed by the SQLTranslator/Reconciler in future work to emit SpanConfigurations based on the type of object the job is protecting. Informs: cockroachdb#73727 Release note: None
ad0328c to
06bae23
Compare
|
I'm leaning towards checking atleast the protobuf changes in. I think the move to protect schema objects is pretty uncontroversial. I'll leave the remaining PRs as drafts until the RFC is generally accepted. |
arulajmani
left a comment
There was a problem hiding this comment.
Reviewed 13 of 15 files at r1, 4 of 4 files at r3, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @ajwerner, @dt, and @irfansharif)
|
Bazel failure is changefeedccl package timing out under race. TFTR! bors r=irfansharif,arulajmani |
|
Build succeeded: |
Protected timestamp records are moving away from
the notion of protecting spans, and instead will operate
on objects. Objects will be defined as:
This change deprecates the Spans field on
ptpb.Record.Additionally, it adds a
oneoftarget field that reflectswhich of the above objects the record corresponds to.
This information will be needed by the SQLTranslator/Reconciler
in future work to emit SpanConfigurations based on the type
of object the job is protecting.
Informs: #73727
Release note: None