Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protectedts: MakeSchemaObjectsTarget should protect the schema object's metadata #127345

Open
stevendanna opened this issue Jul 17, 2024 · 3 comments
Labels
A-schema-catalog Related to the schema descriptors collection and the catalog API in general. branch-master Failures and bugs on the master branch. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-kv KV Team T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@stevendanna
Copy link
Collaborator

stevendanna commented Jul 17, 2024

Describe the problem

Users of the protected timestamp system (Changefeeds, LDR, BACKUP/RESTORE, etc) often want to assume that if they have a protected timestamp on a table, then they will be able to do things such as issue a historical SQL query, get the correct table descriptor to parse datums on that historical data, and so forth.

However, that currently is not the case. A protected timestamp on a table does not protect that table's namespace or descriptor table rows. While callers could be updated to additionally protect those two tables, those callsites then need to always understand all of the tables that should be protected to plan an arbitrary query against a given table.

It would be nice if we had an API for creating a protected timestamp that protected both the table and all relevant metadata for that table.

Jira issue: CRDB-40397

@stevendanna stevendanna added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. branch-master Failures and bugs on the master branch. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) T-kv KV Team labels Jul 17, 2024
@stevendanna stevendanna changed the title protectedts: MakeSchemaObjectsTarget should protect additional metadata required to query the data. protectedts: MakeSchemaObjectsTarget should protect the schema object's metadata Jul 17, 2024
@stevendanna
Copy link
Collaborator Author

This is written more as an "enhancement". I labelled it "bug" because I am almost certain that we have at least 1 CDC bug related to the existing API.

@rafiss
Copy link
Collaborator

rafiss commented Jul 30, 2024

I believe this is a similar request to #120391. For now, we likely won't prioritize it unless we hear of a bug that needs this to be addressed in order to fix the bug.

@exalate-issue-sync exalate-issue-sync bot removed the T-kv KV Team label Jul 30, 2024
@rafiss rafiss added T-kv KV Team A-schema-catalog Related to the schema descriptors collection and the catalog API in general. labels Jul 30, 2024
@rafiss
Copy link
Collaborator

rafiss commented Aug 5, 2024

It seems that when this was being implemented (in #74248), that the decision was made to have the user of the PTS decide if they wanted to protect the system.descriptors table as well. Our team wasn't involved in the design of this, so I don't know for sure, but I'm basing this off of this block of code, which shows changefeeds opting-in to protecting system.descriptors rather than assuming that it will get protected automatically:

func makeTargetToProtect(targets changefeedbase.Targets) *ptpb.Target {
// NB: We add 1 because we're also going to protect system.descriptors.
// We protect system.descriptors because a changefeed needs all of the history
// of table descriptors to version data.
tablesToProtect := make(descpb.IDs, 0, targets.NumUniqueTables()+1)
_ = targets.EachTableID(func(id descpb.ID) error {
tablesToProtect = append(tablesToProtect, id)
return nil
})
tablesToProtect = append(tablesToProtect, keys.DescriptorTableID)
return ptpb.MakeSchemaObjectsTarget(tablesToProtect)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-schema-catalog Related to the schema descriptors collection and the catalog API in general. branch-master Failures and bugs on the master branch. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-kv KV Team T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
No open projects
Status: Incoming
Status: Triage
Development

No branches or pull requests

2 participants