From 8f8c43319dd616fc7cb690f43803bcd2f481345d Mon Sep 17 00:00:00 2001 From: SeaRise Date: Wed, 13 Sep 2023 13:07:38 +0800 Subject: [PATCH] sessionctx: change variable `tidb_enable_tiflash_pipeline_model` to global level (#46709) close pingcap/tidb#45618 --- distsql/distsql.go | 2 +- sessionctx/variable/session.go | 6 ------ sessionctx/variable/sysvar.go | 6 ++++-- sessionctx/variable/sysvar_test.go | 22 ++++++++++++++++++++++ sessionctx/variable/tidb_vars.go | 10 +++++----- sessionctx/variable/variable_test.go | 7 +++++++ 6 files changed, 39 insertions(+), 14 deletions(-) diff --git a/distsql/distsql.go b/distsql/distsql.go index 67ba478a440c1..21ee51dea01df 100644 --- a/distsql/distsql.go +++ b/distsql/distsql.go @@ -129,7 +129,7 @@ func SetTiFlashConfVarsInContext(ctx context.Context, sctx sessionctx.Context) c if sctx.GetSessionVars().TiFlashMaxBytesBeforeExternalSort != -1 { ctx = metadata.AppendToOutgoingContext(ctx, variable.TiDBMaxBytesBeforeTiFlashExternalSort, strconv.FormatInt(sctx.GetSessionVars().TiFlashMaxBytesBeforeExternalSort, 10)) } - if sctx.GetSessionVars().TiFlashEnablePipelineMode { + if variable.TiFlashEnablePipelineMode.Load() { ctx = metadata.AppendToOutgoingContext(ctx, variable.TiDBEnableTiFlashPipelineMode, "1") } else { ctx = metadata.AppendToOutgoingContext(ctx, variable.TiDBEnableTiFlashPipelineMode, "0") diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 61dcaafaa357f..6d2834bc0e3b4 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -892,11 +892,6 @@ type SessionVars struct { // TiFlashQuerySpillRatio is the percentage threshold to trigger auto spill in TiFlash if TiFlashMaxQueryMemoryPerNode is set TiFlashQuerySpillRatio float64 - // TiFlashEnablePipelineMode means if we should use pipeline model to execute query or not in tiflash. - // Default value is `true`, means never use pipeline model in tiflash. - // Value set to `true` means try to execute query with pipeline model in tiflash. - TiFlashEnablePipelineMode bool - // TiDBAllowAutoRandExplicitInsert indicates whether explicit insertion on auto_random column is allowed. AllowAutoRandExplicitInsert bool @@ -2057,7 +2052,6 @@ func NewSessionVars(hctx HookContext) *SessionVars { vars.TiFlashMaxBytesBeforeExternalSort = DefTiFlashMaxBytesBeforeExternalSort vars.TiFlashMaxQueryMemoryPerNode = DefTiFlashMemQuotaQueryPerNode vars.TiFlashQuerySpillRatio = DefTiFlashQuerySpillRatio - vars.TiFlashEnablePipelineMode = DefTiDBEnableTiFlashPipelineMode vars.MPPStoreFailTTL = DefTiDBMPPStoreFailTTL vars.DiskTracker = disk.NewTracker(memory.LabelForSession, -1) vars.MemTracker = memory.NewTracker(memory.LabelForSession, vars.MemQuotaQuery) diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index cc184fecd4e70..c07d440c40187 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -192,9 +192,11 @@ var defaultSysVars = []*SysVar{ s.TiFlashQuerySpillRatio = tidbOptFloat64(val, DefTiFlashQuerySpillRatio) return nil }}, - {Scope: ScopeGlobal | ScopeSession, Name: TiDBEnableTiFlashPipelineMode, Type: TypeBool, Value: BoolToOnOff(DefTiDBEnableTiFlashPipelineMode), SetSession: func(s *SessionVars, val string) error { - s.TiFlashEnablePipelineMode = TiDBOptOn(val) + {Scope: ScopeGlobal, Name: TiDBEnableTiFlashPipelineMode, Type: TypeBool, Value: BoolToOnOff(DefTiDBEnableTiFlashPipelineMode), SetGlobal: func(ctx context.Context, vars *SessionVars, s string) error { + TiFlashEnablePipelineMode.Store(TiDBOptOn(s)) return nil + }, GetGlobal: func(ctx context.Context, vars *SessionVars) (string, error) { + return BoolToOnOff(TiFlashEnablePipelineMode.Load()), nil }}, {Scope: ScopeSession, Name: TiDBSnapshot, Value: "", skipInit: true, SetSession: func(s *SessionVars, val string) error { err := setSnapshotTS(s, val) diff --git a/sessionctx/variable/sysvar_test.go b/sessionctx/variable/sysvar_test.go index 0ab7e59cda32a..b4938cf1e2630 100644 --- a/sessionctx/variable/sysvar_test.go +++ b/sessionctx/variable/sysvar_test.go @@ -1360,6 +1360,28 @@ func TestTiDBTiFlashReplicaRead(t *testing.T) { require.Equal(t, DefTiFlashReplicaRead, val) } +func TestSetEnableTiFlashPipeline(t *testing.T) { + vars := NewSessionVars(nil) + mock := NewMockGlobalAccessor4Tests() + mock.SessionVars = vars + vars.GlobalVarsAccessor = mock + enablePipeline := GetSysVar(TiDBEnableTiFlashPipelineMode) + // Check default value + require.Equal(t, "ON", enablePipeline.Value) + + err := mock.SetGlobalSysVar(context.Background(), TiDBEnableTiFlashPipelineMode, "OFF") + require.NoError(t, err) + val, err := mock.GetGlobalSysVar(TiDBEnableTiFlashPipelineMode) + require.NoError(t, err) + require.Equal(t, "OFF", val) + + err = mock.SetGlobalSysVar(context.Background(), TiDBEnableTiFlashPipelineMode, "ON") + require.NoError(t, err) + val, err = mock.GetGlobalSysVar(TiDBEnableTiFlashPipelineMode) + require.NoError(t, err) + require.Equal(t, "ON", val) +} + func TestSetTiDBCloudStorageURI(t *testing.T) { vars := NewSessionVars(nil) mock := NewMockGlobalAccessor4Tests() diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index 5f65b3b5b06c5..1be1d5e9e5a42 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -416,11 +416,6 @@ const ( // TiFlashQuerySpillRatio is the threshold that TiFlash will trigger auto spill when the memory usage is above this percentage TiFlashQuerySpillRatio = "tiflash_query_spill_ratio" - // TiDBEnableTiFlashPipelineMode means if we should use pipeline model to execute query or not in tiflash. - // Default value is `true`, means never use pipeline model in tiflash. - // Value set to `true` means try to execute query with pipeline model in tiflash. - TiDBEnableTiFlashPipelineMode = "tidb_enable_tiflash_pipeline_model" - // TiDBMPPStoreFailTTL is the unavailable time when a store is detected failed. During that time, tidb will not send any task to // TiFlash even though the failed TiFlash node has been recovered. TiDBMPPStoreFailTTL = "tidb_mpp_store_fail_ttl" @@ -1101,6 +1096,10 @@ const ( TiDBServiceScope = "tidb_service_scope" // TiDBSchemaVersionCacheLimit defines the capacity size of domain infoSchema cache. TiDBSchemaVersionCacheLimit = "tidb_schema_version_cache_limit" + // TiDBEnableTiFlashPipelineMode means if we should use pipeline model to execute query or not in tiflash. + // Default value is `true`, means never use pipeline model in tiflash. + // Value set to `true` means try to execute query with pipeline model in tiflash. + TiDBEnableTiFlashPipelineMode = "tiflash_enable_pipeline_model" ) // TiDB intentional limits @@ -1504,6 +1503,7 @@ var ( EnableResourceControl = atomic.NewBool(false) EnableCheckConstraint = atomic.NewBool(DefTiDBEnableCheckConstraint) SkipMissingPartitionStats = atomic.NewBool(DefTiDBSkipMissingPartitionStats) + TiFlashEnablePipelineMode = atomic.NewBool(DefTiDBEnableTiFlashPipelineMode) ServiceScope = atomic.NewString("") SchemaVersionCacheLimit = atomic.NewInt64(DefTiDBSchemaVersionCacheLimit) CloudStorageURI = atomic.NewString("") diff --git a/sessionctx/variable/variable_test.go b/sessionctx/variable/variable_test.go index 9ed0c76a13622..3b059775321b4 100644 --- a/sessionctx/variable/variable_test.go +++ b/sessionctx/variable/variable_test.go @@ -676,6 +676,13 @@ func TestSkipSysvarCache(t *testing.T) { require.False(t, GetSysVar(TiDBEnableAsyncCommit).SkipSysvarCache()) } +func TestTiFlashEnablePipeline(t *testing.T) { + require.True(t, GetSysVar(TiDBEnableTiFlashPipelineMode).HasGlobalScope()) + require.False(t, GetSysVar(TiDBEnableTiFlashPipelineMode).HasSessionScope()) + require.False(t, GetSysVar(TiDBEnableTiFlashPipelineMode).HasInstanceScope()) + require.False(t, GetSysVar(TiDBEnableTiFlashPipelineMode).HasNoneScope()) +} + func TestTimeValidationWithTimezone(t *testing.T) { sv := SysVar{Scope: ScopeSession, Name: "mynewsysvar", Value: "23:59 +0000", Type: TypeTime} vars := NewSessionVars(nil)