From 97ac6477854d8bd210bf26a6801eff6b9984d9e7 Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Tue, 21 Jun 2022 23:02:37 +0800 Subject: [PATCH 1/4] planner: introduce new cost formula for MPPAggs (#35436) ref pingcap/tidb#35240 --- planner/core/plan_cost.go | 31 +++++++++++++++++++++++++++---- planner/core/task.go | 4 ++-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/planner/core/plan_cost.go b/planner/core/plan_cost.go index 6e194965901cf..ee0af71c51149 100644 --- a/planner/core/plan_cost.go +++ b/planner/core/plan_cost.go @@ -312,7 +312,12 @@ func (p *PhysicalTableReader) GetPlanCost(taskType property.TaskType, costFlag u concurrency = float64(p.ctx.GetSessionVars().DistSQLScanConcurrency()) rowSize = getTblStats(p.tablePlan).GetAvgRowSize(p.ctx, p.tablePlan.Schema().Columns, false, false) seekCost = estimateNetSeekCost(p.tablePlan) - childCost, err := p.tablePlan.GetPlanCost(property.CopSingleReadTaskType, costFlag) + tType := property.MppTaskType + if p.ctx.GetSessionVars().CostModelVersion == modelVer1 { + // regard the underlying tasks as cop-task on modelVer1 for compatibility + tType = property.CopSingleReadTaskType + } + childCost, err := p.tablePlan.GetPlanCost(tType, costFlag) if err != nil { return 0, err } @@ -326,7 +331,8 @@ func (p *PhysicalTableReader) GetPlanCost(taskType property.TaskType, costFlag u // consider concurrency p.planCost /= concurrency // consider tidb_enforce_mpp - if isMPP && p.ctx.GetSessionVars().IsMPPEnforced() { + if isMPP && p.ctx.GetSessionVars().IsMPPEnforced() && + !hasCostFlag(costFlag, CostFlagRecalculate) { // show the real cost in explain-statements p.planCost /= 1000000000 } } @@ -892,12 +898,19 @@ func (p *PhysicalHashJoin) GetPlanCost(taskType property.TaskType, costFlag uint } // GetCost computes cost of stream aggregation considering CPU/memory. -func (p *PhysicalStreamAgg) GetCost(inputRows float64, isRoot bool, costFlag uint64) float64 { +func (p *PhysicalStreamAgg) GetCost(inputRows float64, isRoot, isMPP bool, costFlag uint64) float64 { aggFuncFactor := p.getAggFuncCostFactor(false) var cpuCost float64 sessVars := p.ctx.GetSessionVars() if isRoot { cpuCost = inputRows * sessVars.GetCPUFactor() * aggFuncFactor + } else if isMPP { + if p.ctx.GetSessionVars().CostModelVersion == modelVer2 { + // use the dedicated CPU factor for TiFlash on modelVer2 + cpuCost = inputRows * sessVars.GetTiFlashCPUFactor() * aggFuncFactor + } else { + cpuCost = inputRows * sessVars.GetCopCPUFactor() * aggFuncFactor + } } else { cpuCost = inputRows * sessVars.GetCopCPUFactor() * aggFuncFactor } @@ -916,7 +929,7 @@ func (p *PhysicalStreamAgg) GetPlanCost(taskType property.TaskType, costFlag uin return 0, err } p.planCost = childCost - p.planCost += p.GetCost(getCardinality(p.children[0], costFlag), taskType == property.RootTaskType, costFlag) + p.planCost += p.GetCost(getCardinality(p.children[0], costFlag), taskType == property.RootTaskType, taskType == property.MppTaskType, costFlag) p.planCostInit = true return p.planCost, nil } @@ -936,6 +949,13 @@ func (p *PhysicalHashAgg) GetCost(inputRows float64, isRoot, isMPP bool, costFla // Cost of additional goroutines. cpuCost += (con + 1) * sessVars.GetConcurrencyFactor() } + } else if isMPP { + if p.ctx.GetSessionVars().CostModelVersion == modelVer2 { + // use the dedicated CPU factor for TiFlash on modelVer2 + cpuCost = inputRows * sessVars.GetTiFlashCPUFactor() * aggFuncFactor + } else { + cpuCost = inputRows * sessVars.GetCopCPUFactor() * aggFuncFactor + } } else { cpuCost = inputRows * sessVars.GetCopCPUFactor() * aggFuncFactor } @@ -1144,6 +1164,9 @@ func (p *PhysicalExchangeReceiver) GetPlanCost(taskType property.TaskType, costF } func getOperatorActRows(operator PhysicalPlan) float64 { + if operator == nil { + return 0 + } runtimeInfo := operator.SCtx().GetSessionVars().StmtCtx.RuntimeStatsColl id := operator.ID() actRows := 0.0 diff --git a/planner/core/task.go b/planner/core/task.go index f197317eb836e..fd6cac675f4c6 100644 --- a/planner/core/task.go +++ b/planner/core/task.go @@ -1665,7 +1665,7 @@ func (p *PhysicalStreamAgg) attach2Task(tasks ...task) task { partialAgg.SetChildren(cop.indexPlan) cop.indexPlan = partialAgg } - cop.addCost(partialAgg.(*PhysicalStreamAgg).GetCost(inputRows, false, 0)) + cop.addCost(partialAgg.(*PhysicalStreamAgg).GetCost(inputRows, false, false, 0)) partialAgg.SetCost(cop.cost()) } t = cop.convertToRootTask(p.ctx) @@ -1678,7 +1678,7 @@ func (p *PhysicalStreamAgg) attach2Task(tasks ...task) task { } else { attachPlan2Task(p, t) } - t.addCost(final.GetCost(inputRows, true, 0)) + t.addCost(final.GetCost(inputRows, true, false, 0)) t.plan().SetCost(t.cost()) return t } From 121ce2a67082b4e316e10385c8cd7fd2276dcc58 Mon Sep 17 00:00:00 2001 From: Rain Li Date: Tue, 21 Jun 2022 23:40:36 +0800 Subject: [PATCH 2/4] *: upgrade go-proxyprotocol version (#35560) close pingcap/tidb#35587 --- DEPS.bzl | 4 ++-- go.mod | 2 +- go.sum | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/DEPS.bzl b/DEPS.bzl index 97d5e63b1776b..32f321378fa33 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -167,8 +167,8 @@ def go_deps(): name = "com_github_blacktear23_go_proxyprotocol", build_file_proto_mode = "disable_global", importpath = "github.com/blacktear23/go-proxyprotocol", - sum = "h1:rQlvB2AYWme2bIB18r/SipGiMEVJYE9U0z+MGoU/LtQ=", - version = "v0.0.0-20180807104634-af7a81e8dd0d", + sum = "h1:WmMmtZanGEfIHnJN9N3A4Pl6mM69D+GxEph2eOaCf7g=", + version = "v1.0.0", ) go_repository( name = "com_github_burntsushi_toml", diff --git a/go.mod b/go.mod index 78704e7a8ecd7..bfd038156bf93 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/Jeffail/gabs/v2 v2.5.1 github.com/Shopify/sarama v1.29.0 github.com/aws/aws-sdk-go v1.35.3 - github.com/blacktear23/go-proxyprotocol v0.0.0-20180807104634-af7a81e8dd0d + github.com/blacktear23/go-proxyprotocol v1.0.0 github.com/carlmjohnson/flagext v0.21.0 github.com/cheggaaa/pb/v3 v3.0.8 github.com/cheynewallace/tabby v1.1.1 diff --git a/go.sum b/go.sum index 66d46713ea35b..3a6e62f536235 100644 --- a/go.sum +++ b/go.sum @@ -119,8 +119,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= github.com/bketelsen/crypt v0.0.3-0.20200106085610-5cbc8cc4026c/go.mod h1:MKsuJmJgSg28kpZDP6UIiPt0e0Oz0kqKNGyRaWEPv84= -github.com/blacktear23/go-proxyprotocol v0.0.0-20180807104634-af7a81e8dd0d h1:rQlvB2AYWme2bIB18r/SipGiMEVJYE9U0z+MGoU/LtQ= -github.com/blacktear23/go-proxyprotocol v0.0.0-20180807104634-af7a81e8dd0d/go.mod h1:VKt7CNAQxpFpSDz3sXyj9hY/GbVsQCr0sB3w59nE7lU= +github.com/blacktear23/go-proxyprotocol v1.0.0 h1:WmMmtZanGEfIHnJN9N3A4Pl6mM69D+GxEph2eOaCf7g= +github.com/blacktear23/go-proxyprotocol v1.0.0/go.mod h1:fbqiWSHMxaW0KsJ3SHjpxOMbTpIaQSMRn1GRd+oPyEw= github.com/carlmjohnson/flagext v0.21.0 h1:/c4uK3ie786Z7caXLcIMvePNSSiH3bQVGDvmGLMme60= github.com/carlmjohnson/flagext v0.21.0/go.mod h1:Eenv0epIUAr4NuedNmkzI8WmBmjIxZC239XcKxYS2ac= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= From 3b34234ecf1e7a7b8ba11dc40931d07b2af547f2 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Tue, 21 Jun 2022 10:06:36 -0600 Subject: [PATCH 3/4] session: improve bootstrap code (#34755) close pingcap/tidb#34754 --- session/bootstrap.go | 83 +++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 40 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index 960ca9fa3a625..68e97c84d1dd2 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -1971,6 +1971,12 @@ func doDDLWorks(s Session) { mustExecute(s, CreateAdvisoryLocks) } +// inTestSuite checks if we are bootstrapping in the context of tests. +// There are some historical differences in behavior between tests and non-tests. +func inTestSuite() bool { + return flag.Lookup("test.v") != nil || flag.Lookup("check.v") != nil +} + // doDMLWorks executes DML statements in bootstrap stage. // All the statements run in a single transaction. // TODO: sanitize. @@ -1991,58 +1997,55 @@ func doDMLWorks(s Session) { ("%", "root", "", "mysql_native_password", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "Y", "N", "Y", "Y", "Y", "Y", "Y")`) } - // Init global system variables table. + // For GLOBAL scoped system variables, insert the initial value + // into the mysql.global_variables table. This is only run on initial + // bootstrap, and in some cases we will use a different default value + // for new installs versus existing installs. + values := make([]string, 0, len(variable.GetSysVars())) for k, v := range variable.GetSysVars() { - // Only global variables should be inserted. - if v.HasGlobalScope() { - vVal := v.Value - if v.Name == variable.TiDBTxnMode && config.GetGlobalConfig().Store == "tikv" { + if !v.HasGlobalScope() { + continue + } + vVal := v.Value + switch v.Name { + case variable.TiDBTxnMode: + if config.GetGlobalConfig().Store == "tikv" { vVal = "pessimistic" } - if v.Name == variable.TiDBRowFormatVersion { - vVal = strconv.Itoa(variable.DefTiDBRowFormatV2) - } - if v.Name == variable.TiDBPartitionPruneMode { - vVal = variable.DefTiDBPartitionPruneMode - if flag.Lookup("test.v") != nil || flag.Lookup("check.v") != nil || config.CheckTableBeforeDrop { - // enable Dynamic Prune by default in test case. - vVal = string(variable.Dynamic) - } - } - if v.Name == variable.TiDBMemOOMAction { - if flag.Lookup("test.v") != nil || flag.Lookup("check.v") != nil { - // Change the OOM action to log for the test suite. - vVal = variable.OOMActionLog - } - } - if v.Name == variable.TiDBEnableChangeMultiSchema { - vVal = variable.Off - if flag.Lookup("test.v") != nil || flag.Lookup("check.v") != nil { - // enable change multi schema in test case for compatibility with old cases. - vVal = variable.On - } - } - if v.Name == variable.TiDBEnableAsyncCommit && config.GetGlobalConfig().Store == "tikv" { + case variable.TiDBEnableAsyncCommit, variable.TiDBEnable1PC: + if config.GetGlobalConfig().Store == "tikv" { vVal = variable.On } - if v.Name == variable.TiDBEnable1PC && config.GetGlobalConfig().Store == "tikv" { - vVal = variable.On + case variable.TiDBPartitionPruneMode: + if inTestSuite() || config.CheckTableBeforeDrop { + vVal = string(variable.Dynamic) } - if v.Name == variable.TiDBEnableMutationChecker { + case variable.TiDBEnableChangeMultiSchema: + if inTestSuite() { vVal = variable.On } - if v.Name == variable.TiDBEnableAutoAnalyze { - if flag.Lookup("test.v") != nil || flag.Lookup("check.v") != nil { - vVal = variable.Off - } + case variable.TiDBMemOOMAction: + if inTestSuite() { + vVal = variable.OOMActionLog } - if v.Name == variable.TiDBTxnAssertionLevel { - vVal = variable.AssertionFastStr + case variable.TiDBEnableAutoAnalyze: + if inTestSuite() { + vVal = variable.Off } - value := fmt.Sprintf(`("%s", "%s")`, strings.ToLower(k), vVal) - values = append(values, value) + // For the following sysvars, we change the default + // FOR NEW INSTALLS ONLY. In most cases you don't want to do this. + // It is better to change the value in the Sysvar struct, so that + // all installs will have the same value. + case variable.TiDBRowFormatVersion: + vVal = strconv.Itoa(variable.DefTiDBRowFormatV2) + case variable.TiDBTxnAssertionLevel: + vVal = variable.AssertionFastStr + case variable.TiDBEnableMutationChecker: + vVal = variable.On } + value := fmt.Sprintf(`("%s", "%s")`, strings.ToLower(k), vVal) + values = append(values, value) } sql := fmt.Sprintf("INSERT HIGH_PRIORITY INTO %s.%s VALUES %s;", mysql.SystemDB, mysql.GlobalVariablesTable, strings.Join(values, ", ")) From ff1b6ff5b8ad46f15e23aeeec5f0e173c76e7108 Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Wed, 22 Jun 2022 01:18:36 +0800 Subject: [PATCH 4/4] planner: make some `show stmt` more fine-grained privilege check (#35493) close pingcap/tidb#35393 --- executor/showtest/show_test.go | 26 +++++++++++++++++++------- parser/parser.go | 6 +++--- parser/parser.y | 6 +++--- planner/core/planbuilder.go | 11 ++++++++--- plugin/integration_test.go | 2 ++ 5 files changed, 35 insertions(+), 16 deletions(-) diff --git a/executor/showtest/show_test.go b/executor/showtest/show_test.go index 641207d8b6a37..75e93d9655854 100644 --- a/executor/showtest/show_test.go +++ b/executor/showtest/show_test.go @@ -795,20 +795,32 @@ func TestShowStatsPrivilege(t *testing.T) { tk1 := testkit.NewTestKit(t, store) require.True(t, tk1.Session().Auth(&auth.UserIdentity{Username: "show_stats", Hostname: "%"}, nil, nil)) + e := "[planner:1142]SHOW command denied to user 'show_stats'@'%' for table" + err := tk1.ExecToErr("show stats_meta") + require.ErrorContains(t, err, e) + err = tk1.ExecToErr("SHOW STATS_BUCKETS") + require.ErrorContains(t, err, e) + err = tk1.ExecToErr("SHOW STATS_HISTOGRAMS") + require.ErrorContains(t, err, e) + eqErr := plannercore.ErrDBaccessDenied.GenWithStackByArgs("show_stats", "%", mysql.SystemDB) - _, err := tk1.Exec("show stats_meta") - require.EqualError(t, err, eqErr.Error()) - _, err = tk1.Exec("SHOW STATS_BUCKETS") - require.EqualError(t, err, eqErr.Error()) - _, err = tk1.Exec("SHOW STATS_HEALTHY") - require.EqualError(t, err, eqErr.Error()) - _, err = tk1.Exec("SHOW STATS_HISTOGRAMS") + err = tk1.ExecToErr("SHOW STATS_HEALTHY") require.EqualError(t, err, eqErr.Error()) tk.MustExec("grant select on mysql.* to show_stats") tk1.MustExec("show stats_meta") tk1.MustExec("SHOW STATS_BUCKETS") tk1.MustExec("SHOW STATS_HEALTHY") tk1.MustExec("SHOW STATS_HISTOGRAMS") + + tk.MustExec("create user a@'%' identified by '';") + require.True(t, tk1.Session().Auth(&auth.UserIdentity{Username: "a", Hostname: "%"}, nil, nil)) + tk.MustExec("grant select on mysql.stats_meta to a@'%';") + tk.MustExec("grant select on mysql.stats_buckets to a@'%';") + tk.MustExec("grant select on mysql.stats_histograms to a@'%';") + tk1.MustExec("show stats_meta") + tk1.MustExec("SHOW STATS_BUCKETS") + tk1.MustExec("SHOW STATS_HISTOGRAMS") + } func TestIssue18878(t *testing.T) { diff --git a/parser/parser.go b/parser/parser.go index cfd1786602b21..6263a9718444b 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -19010,11 +19010,11 @@ yynewstate: } case 1896: { - parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsMeta} + parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsMeta, Table: &ast.TableName{Name: model.NewCIStr("STATS_META"), Schema: model.NewCIStr(mysql.SystemDB)}} } case 1897: { - parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsHistograms} + parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsHistograms, Table: &ast.TableName{Name: model.NewCIStr("STATS_HISTOGRAMS"), Schema: model.NewCIStr(mysql.SystemDB)}} } case 1898: { @@ -19022,7 +19022,7 @@ yynewstate: } case 1899: { - parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsBuckets} + parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsBuckets, Table: &ast.TableName{Name: model.NewCIStr("STATS_BUCKETS"), Schema: model.NewCIStr(mysql.SystemDB)}} } case 1900: { diff --git a/parser/parser.y b/parser/parser.y index 1f175038467c2..465a2c69101aa 100644 --- a/parser/parser.y +++ b/parser/parser.y @@ -10821,11 +10821,11 @@ ShowTargetFilterable: } | "STATS_META" { - $$ = &ast.ShowStmt{Tp: ast.ShowStatsMeta} + $$ = &ast.ShowStmt{Tp: ast.ShowStatsMeta, Table: &ast.TableName{Name: model.NewCIStr("STATS_META"), Schema: model.NewCIStr(mysql.SystemDB)}} } | "STATS_HISTOGRAMS" { - $$ = &ast.ShowStmt{Tp: ast.ShowStatsHistograms} + $$ = &ast.ShowStmt{Tp: ast.ShowStatsHistograms, Table: &ast.TableName{Name: model.NewCIStr("STATS_HISTOGRAMS"), Schema: model.NewCIStr(mysql.SystemDB)}} } | "STATS_TOPN" { @@ -10833,7 +10833,7 @@ ShowTargetFilterable: } | "STATS_BUCKETS" { - $$ = &ast.ShowStmt{Tp: ast.ShowStatsBuckets} + $$ = &ast.ShowStmt{Tp: ast.ShowStatsBuckets, Table: &ast.TableName{Name: model.NewCIStr("STATS_BUCKETS"), Schema: model.NewCIStr(mysql.SystemDB)}} } | "STATS_HEALTHY" { diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 040573420a076..e4f60984a75b3 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -2986,13 +2986,18 @@ func (b *PlanBuilder) buildShow(ctx context.Context, show *ast.ShowStmt) (Plan, p.setSchemaAndNames(buildShowNextRowID()) b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, show.Table.Schema.L, show.Table.Name.L, "", ErrPrivilegeCheckFail) return p, nil - case ast.ShowStatsBuckets, ast.ShowStatsHistograms, ast.ShowStatsMeta, ast.ShowStatsExtended, ast.ShowStatsHealthy, ast.ShowStatsTopN, ast.ShowHistogramsInFlight, ast.ShowColumnStatsUsage: - user := b.ctx.GetSessionVars().User + case ast.ShowStatsExtended, ast.ShowStatsHealthy, ast.ShowStatsTopN, ast.ShowHistogramsInFlight, ast.ShowColumnStatsUsage: var err error - if user != nil { + if user := b.ctx.GetSessionVars().User; user != nil { err = ErrDBaccessDenied.GenWithStackByArgs(user.AuthUsername, user.AuthHostname, mysql.SystemDB) } b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, mysql.SystemDB, "", "", err) + case ast.ShowStatsBuckets, ast.ShowStatsHistograms, ast.ShowStatsMeta: + var err error + if user := b.ctx.GetSessionVars().User; user != nil { + err = ErrTableaccessDenied.GenWithStackByArgs("SHOW", user.AuthUsername, user.AuthHostname, show.Table.Name.L) + } + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, show.Table.Schema.L, show.Table.Name.L, "", err) case ast.ShowRegions: tableInfo, err := b.is.TableByName(show.Table.Schema, show.Table.Name) if err != nil { diff --git a/plugin/integration_test.go b/plugin/integration_test.go index 96bb5c3295a5f..1c62fa440da6d 100644 --- a/plugin/integration_test.go +++ b/plugin/integration_test.go @@ -485,11 +485,13 @@ func TestAuditLogNormal(t *testing.T) { sql: "show stats_histograms", stmtType: "Show", dbs: "mysql", + tables: "stats_histograms", }, { sql: "show stats_meta", stmtType: "Show", dbs: "mysql", + tables: "stats_meta", }, { sql: "show status",