sql: add cache invalidation builtin functions#127414
sql: add cache invalidation builtin functions#127414craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
michae2
left a comment
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @mgartner)
pkg/sql/sem/builtins/builtins.go line 8807 at r1 (raw file):
return tree.DVoidDatum, nil }, Info: `This function is used to clear the query plan cache`,
Might be worth saying that this only affects the local / gateway node.
pkg/sql/sem/builtins/builtins.go line 8820 at r1 (raw file):
ReturnType: tree.FixedReturnType(types.Void), Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) { evalCtx.Planner.ClearTableStatsCache()
random idea: It might also be worth creating an overload that takes an int and calls InvalidateTableStats to flush just a single table's stats.
pkg/sql/sem/builtins/builtins.go line 8823 at r1 (raw file):
return tree.DVoidDatum, nil }, Info: `This function is used to clear the table statistics cache`,
(Same comment here about local / gateway node.)
mgartner
left a comment
There was a problem hiding this comment.
cc @Uzair5162 to update the changes in #127395 (before or after it merges) to use this new built-in and remove the retries.
Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status:
pkg/sql/sem/builtins/builtins.go line 8820 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
random idea: It might also be worth creating an overload that takes an int and calls
InvalidateTableStatsto flush just a single table's stats.
Good idea, but I'd prefer to only add it if we have a specific need for it.
pkg/ccl/logictestccl/testdata/logic_test/builtins line 91 at r1 (raw file):
statement ok SELECT crdb_internal.clear_query_plan_cache();
It would be cool to have a builtin like this defined as a stored procedure, rather than a function, because it never needs to be invoked in arbitrary scalar expressions, like a function can. I don't know if Postgres has precedence for built-in stored procedures, though.
This commit adds builtin functions that can be used to invalidate the query plan cache and table statistics cache. This is useful for preventing test flakes due to asynchronous cache updates, as well as for tests that involve the caches in general. Fixes cockroachdb#122543 Release note: None
5b02995 to
3d71cf3
Compare
DrewKimball
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/sem/builtins/builtins.go line 8807 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
Might be worth saying that this only affects the local / gateway node.
Done.
pkg/sql/sem/builtins/builtins.go line 8823 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
(Same comment here about local / gateway node.)
Done.
pkg/ccl/logictestccl/testdata/logic_test/builtins line 91 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
It would be cool to have a builtin like this defined as a stored procedure, rather than a function, because it never needs to be invoked in arbitrary scalar expressions, like a function can. I don't know if Postgres has precedence for built-in stored procedures, though.
That's an interesting idea. I think a bunch of our builtins might be nicer as procedures.
|
TFTRs! bors r+ |
This commit adds builtin functions that can be used to invalidate the query plan cache and table statistics cache. This is useful for preventing test flakes due to asynchronous cache updates, as well as for tests that involve the caches in general.
Fixes #122543
Release note: None