-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
plan, tidb: support plan cache for SELECT statement #4644
Conversation
@zz-jason Good job! |
@shenli WIP, benchmark result will be provided later. |
When transaction changed from read only to not read only. The plan will be changed since dirty table exists. |
2. add cacheable checker
Please fix CI @zz-jason |
executor/compiler.go
Outdated
p, err := plan.Optimize(ctx, node, is) | ||
if err != nil { | ||
return nil, errors.Trace(err) | ||
} | ||
|
||
_, isDual := p.(*plan.TableDual) | ||
if isDual { | ||
cacheable = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why dual cannot be cached?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
create table t(a bigint);
select * from t where exists (select * from t);
this query will be optimized to a TableDual
@coocood PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest LGTM
plan/cache/simple_lru_test.go
Outdated
c.Assert(lru.size, Equals, lru.capacity) | ||
c.Assert(lru.size, Equals, int64(3)) | ||
|
||
// test for nonexistence elements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/ nonexistence/ non-existent
plan/cache/simple_lru_test.go
Outdated
c.Assert(element, IsNil) | ||
} | ||
|
||
// test for in existence elements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/ in existence/ existent
plan/cache/simple_lru_test.go
Outdated
lru.Put(keys[i], vals[i]) | ||
} | ||
|
||
// test for nonexistence elements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
plan/cacheable_checker.go
Outdated
switch expr := exprNode.(type) { | ||
case *ast.VariableExpr, *ast.ExistsSubqueryExpr: | ||
return false | ||
case *ast.UnaryOperationExpr: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed, V
will be visited.
plan/cacheable_checker.go
Outdated
// Enter implements Visitor interface. | ||
func (checker *cacheableChecker) Enter(in ast.Node) (out ast.Node, skipChildren bool) { | ||
switch node := in.(type) { | ||
case *ast.SelectStmt: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to do this, every expression in SelectStmt
will be visited.
plan/cacheable_checker.go
Outdated
} | ||
|
||
// Leave implements Visitor interface. | ||
func (checker *cacheableChecker) Leave(in ast.Node) (out ast.Node, skipChildren bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just switch the type of in
here will do.
executor/compiler.go
Outdated
if err := plan.Preprocess(node, is, ctx); err != nil { | ||
return nil, errors.Trace(err) | ||
// Compile compiles an ast.StmtNode to a physical plan. | ||
func (c *Compiler) Compile(ctx context.Context, node ast.StmtNode) (is infoschema.InfoSchema, p plan.Plan, expensive bool, cacheable bool, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function has too many return values, how about just return ExecStmt
?
session.go
Outdated
// Step3: Cache the physical plan if possiable. | ||
if cache.PlanCacheEnabled && cacheable && len(stmtNodes) == 1 { | ||
if _, isSelect := stmtNodes[0].(*ast.SelectStmt); isSelect { | ||
cache.GlobalPlanCache.Put(cacheKey, cache.NewSQLCacheValue(stmtNode, plan, expensive)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isSelect
is covered by cacheable
already.
plan/cache/sharded_lru.go
Outdated
// PlanCacheEnabled stores the global config "plan-cache-enabled". | ||
PlanCacheEnabled bool | ||
// PlanCacheShards stores the global config "plan-cache-shards". | ||
PlanCacheShards int64 = 200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a power of 2 value is more efficient.
plan/cache/key.go
Outdated
func (key *sqlCacheKey) Hash() []byte { | ||
if key.hash == nil { | ||
var ( | ||
userBytes = []byte(key.user) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use hack.Slice
to avoid memory allocation.
/run-all-test |
LGTM |
) | ||
|
||
// Cacheable checks whether the input ast is cacheable. | ||
func Cacheable(node ast.Node) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need some test for this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
plan/cacheable_checker.go
Outdated
// Enter implements Visitor interface. | ||
func (checker *cacheableChecker) Enter(in ast.Node) (out ast.Node, skipChildren bool) { | ||
switch in.(type) { | ||
case *ast.VariableExpr, *ast.ExistsSubqueryExpr: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Query contains non-deterministic function like Now(), RAND() should not be cacheable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. btw, rand() can be cached
session.go
Outdated
} | ||
sessionExecuteCompileDuration.Observe(time.Since(startTS).Seconds()) | ||
|
||
// Step3: Cache the physical plan if possiable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
config/config.go
Outdated
@@ -121,6 +129,11 @@ var defaultConf = Config{ | |||
XHost: "0.0.0.0", | |||
XPort: 14000, | |||
}, | |||
PlanCache: PlanCache{ | |||
Enabled: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set default to true is not safe for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only for the ci test, will be disabled in the last commit of this pr
LGTM |
main change:
func (s *session) Execute(sql string) ([]ast.RecordSet, error)
, we check the opportunity to use plan cache first