sql: add cache for jobs to avoid querying system.jobs to find existing jobs#50848
sql: add cache for jobs to avoid querying system.jobs to find existing jobs#50848craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
| GRANT ALL ON * TO TEST | ||
| ---- | ||
| 160 | ||
| 50 |
b1b4f39 to
83a6476
Compare
|
Would we backport something like this? I'm not sure. |
ajwerner
left a comment
There was a problem hiding this comment.
The idea looks good to me. I think we would backport this to 20.1, it's a major regression and does not touch any durable state. The pointer map should get fixed. Otherwise just the naming nit from me.
Reviewed 1 of 4 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @RichardJCai and @rohany)
pkg/sql/conn_executor.go, line 942 at r2 (raw file):
jobs jobsCollection // jobsCache is a map of TableIDs to Jobs.
nit: maybe call this schemaChangeJobCache
ajwerner
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @RichardJCai and @rohany)
pkg/sql/planner.go, line 75 at r2 (raw file):
Previously, RichardJCai (Richard Cai) wrote…
Nvm I was being dumb, the pointer is necessary because we actually want the map to be reset by the call to resetExtraTxnState since it resets the cache. Without the pointer the cache doesn't get cleared when a txn is committed
Perhaps a better option is to clear the map rather than constructing a new one in resetExtraTxnState.
Good call, I'll do this instead. |
…ng job Currently, we perform a loop through all existing jobs which involves querying a system table to check if there is an existing job for the current table undergoing a schema change. To avoid this query, we can use a cache to map TableIDs to jobs. Fixes cockroachdb#50165, cockroachdb#50783 Release note: None
ajwerner
left a comment
There was a problem hiding this comment.
Reviewed 1 of 5 files at r3.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @RichardJCai and @rohany)
|
TFTR! |
|
bors r+ |
|
bors r=ajwerner |
Build failed (retrying...) |
Build succeeded |
Currently, we perform a loop through all existing jobs which involves
querying a system table to check if there is an existing job for the current
table undergoing a schema change. To avoid this query, we can use a cache
to map TableIDs to jobs.
Fixes #50165, #50783
Release note: None