Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/domain/infosync/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ go_test(
],
embed = [":infosync"],
flaky = True,
shard_count = 4,
shard_count = 5,
deps = [
"//pkg/config/kerneltype",
"//pkg/ddl/label",
Expand All @@ -81,6 +81,7 @@ go_test(
"//pkg/keyspace",
"//pkg/meta/model",
"//pkg/parser/ast",
"//pkg/sessionctx/vardef",
"//pkg/testkit/testsetup",
"@com_github_pingcap_failpoint//:failpoint",
"@com_github_pingcap_kvproto//pkg/keyspacepb",
Expand Down
46 changes: 46 additions & 0 deletions pkg/domain/infosync/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ const (
// ErrPrometheusAddrIsNotSet is the error that Prometheus address is not set in PD and etcd
var ErrPrometheusAddrIsNotSet = dbterror.ClassDomain.NewStd(errno.ErrPrometheusAddrIsNotSet)

func init() {
vardef.SetClusterReadOnlyChecker(getClusterReadOnlyStatus)
vardef.SetReadOnlyStatusReporter(updateServerReadOnlyStatus)
}

// InfoSyncer stores server info to etcd when the tidb-server starts and delete when tidb-server shuts down.
type InfoSyncer struct {
// `etcdClient` must be used when keyspace is not set, or when the logic to each etcd path needs to be separated by keyspace.
Expand Down Expand Up @@ -343,6 +348,47 @@ func GetAllServerInfo(ctx context.Context) (map[string]*serverinfo.ServerInfo, e
return is.svrInfoSyncer.GetAllServerInfo(ctx)
}

// UpdateServerReadOnlyStatus updates the local TiDB read-only status in the info syncer.
func UpdateServerReadOnlyStatus(ctx context.Context) error {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is race here. UpdateServerLabel and UpdateServerReadOnlyStatus both clone DynamicInfo, change one field group, then write the whole struct back.

Bug case: label update clones read_only=false; read-only update writes read_only=true; label update then writes its old snapshot with new labels, restoring read_only=false. After that, tidb_is_read_only may return OFF even though this TiDB is already read-only.

return updateServerReadOnlyStatus(ctx)
}

func updateServerReadOnlyStatus(ctx context.Context) error {
is, err := getGlobalInfoSyncer()
if err != nil {
return nil
}
return is.svrInfoSyncer.UpdateServerReadOnlyStatus(
ctx,
vardef.RestrictedReadOnly.Load(),
vardef.VarTiDBSuperReadOnly.Load(),
)
}

func getClusterReadOnlyStatus(ctx context.Context) (bool, error) {
is, err := getGlobalInfoSyncer()
if err != nil {
return vardef.LocalTiDBReadOnlyStatus(), nil
}
allServerInfo, err := is.svrInfoSyncer.GetAllServerInfo(ctx)
if err != nil {
return false, err
}
return clusterReadOnlyStatusFromServerInfo(allServerInfo), nil
}

func clusterReadOnlyStatusFromServerInfo(allServerInfo map[string]*serverinfo.ServerInfo) bool {
if len(allServerInfo) == 0 {
return false
}
for _, info := range allServerInfo {
if info == nil || !info.TiDBEffectiveReadOnly {
return false
}
}
return true
}

// UpdateServerLabel updates the server label for global info syncer.
func UpdateServerLabel(ctx context.Context, labels map[string]string) error {
is, err := getGlobalInfoSyncer()
Expand Down
57 changes: 57 additions & 0 deletions pkg/domain/infosync/info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/pingcap/tidb/pkg/keyspace"
"github.com/pingcap/tidb/pkg/meta/model"
"github.com/pingcap/tidb/pkg/parser/ast"
"github.com/pingcap/tidb/pkg/sessionctx/vardef"
"github.com/pingcap/tidb/pkg/testkit/testsetup"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
Expand Down Expand Up @@ -210,3 +211,59 @@ func TestInfoSyncerMarshal(t *testing.T) {
require.Equal(t, info.JSONServerID, decodeInfo.JSONServerID)
require.Equal(t, info.Labels, decodeInfo.Labels)
}

func TestClusterReadOnlyStatus(t *testing.T) {
ctx := context.Background()
_, err := GlobalInfoSyncerInit(ctx, "test-read-only", func() uint64 { return 1 }, nil, nil, nil, nil, keyspace.CodecV1, false, nil)
require.NoError(t, err)
t.Cleanup(func() {
vardef.RestrictedReadOnly.Store(false)
vardef.VarTiDBSuperReadOnly.Store(false)
require.NoError(t, UpdateServerReadOnlyStatus(context.Background()))
})

vardef.RestrictedReadOnly.Store(false)
vardef.VarTiDBSuperReadOnly.Store(false)
require.NoError(t, UpdateServerReadOnlyStatus(ctx))
on, err := vardef.GetClusterReadOnlyStatus(ctx)
require.NoError(t, err)
require.False(t, on)

vardef.VarTiDBSuperReadOnly.Store(true)
require.NoError(t, UpdateServerReadOnlyStatus(ctx))
on, err = vardef.GetClusterReadOnlyStatus(ctx)
require.NoError(t, err)
require.True(t, on)

for _, tt := range []struct {
name string
infos map[string]*serverinfo.ServerInfo
expect bool
}{
{
name: "all effective read-only",
infos: map[string]*serverinfo.ServerInfo{
"tidb-a": {DynamicInfo: serverinfo.DynamicInfo{TiDBSuperReadOnly: true, TiDBEffectiveReadOnly: true}},
"tidb-b": {DynamicInfo: serverinfo.DynamicInfo{TiDBRestrictedReadOnly: true, TiDBSuperReadOnly: true, TiDBEffectiveReadOnly: true}},
},
expect: true,
},
{
name: "one instance is writable",
infos: map[string]*serverinfo.ServerInfo{
"tidb-a": {DynamicInfo: serverinfo.DynamicInfo{TiDBSuperReadOnly: true, TiDBEffectiveReadOnly: true}},
"tidb-b": {DynamicInfo: serverinfo.DynamicInfo{}},
},
expect: false,
},
{
name: "no live instance",
infos: map[string]*serverinfo.ServerInfo{},
expect: false,
},
} {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.expect, clusterReadOnlyStatusFromServerInfo(tt.infos))
})
}
}
10 changes: 8 additions & 2 deletions pkg/domain/serverinfo/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,19 @@ func (i *StaticInfo) IsAssumed() bool {
// To update the dynamic server information, use `InfoSyncer.cloneDynamicServerInfo` to obtain a copy of the dynamic server info.
// After making modifications, use `InfoSyncer.setDynamicServerInfo` to update the dynamic server information.
type DynamicInfo struct {
Labels map[string]string `json:"labels"`
Labels map[string]string `json:"labels"`
TiDBRestrictedReadOnly bool `json:"tidb_restricted_read_only,omitempty"`
TiDBSuperReadOnly bool `json:"tidb_super_read_only,omitempty"`
TiDBEffectiveReadOnly bool `json:"tidb_effective_read_only,omitempty"`
}

// Clone the DynamicInfo.
func (d *DynamicInfo) Clone() *DynamicInfo {
return &DynamicInfo{
Labels: maps.Clone(d.Labels),
Labels: maps.Clone(d.Labels),
TiDBRestrictedReadOnly: d.TiDBRestrictedReadOnly,
TiDBSuperReadOnly: d.TiDBSuperReadOnly,
TiDBEffectiveReadOnly: d.TiDBEffectiveReadOnly,
}
}

Expand Down
41 changes: 39 additions & 2 deletions pkg/domain/serverinfo/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,39 @@ func (s *Syncer) UpdateServerLabel(ctx context.Context, labels map[string]string
return nil
}

// UpdateServerReadOnlyStatus updates the local server read-only status in etcd.
func (s *Syncer) UpdateServerReadOnlyStatus(ctx context.Context, restricted, super bool) error {
dynamicInfo := s.cloneDynamicServerInfo()
effective := restricted || super
if dynamicInfo.TiDBRestrictedReadOnly == restricted &&
dynamicInfo.TiDBSuperReadOnly == super &&
dynamicInfo.TiDBEffectiveReadOnly == effective {
return nil
}

dynamicInfo.TiDBRestrictedReadOnly = restricted
dynamicInfo.TiDBSuperReadOnly = super
dynamicInfo.TiDBEffectiveReadOnly = effective
if s.etcdCli == nil || s.session == nil {
s.setDynamicServerInfo(dynamicInfo)
return nil
}

info := s.GetLocalServerInfo().Clone()
info.DynamicInfo = *dynamicInfo
infoBuf, err := info.Marshal()
if err != nil {
return errors.Trace(err)
}
str := string(hack.String(infoBuf))
err = util.PutKVToEtcd(ctx, s.etcdCli, KeyOpDefaultRetryCnt, s.serverInfoPath, str, clientv3.WithLease(s.session.Lease()))
if err != nil {
return err
}
s.setDynamicServerInfo(dynamicInfo)
return nil
}

// cloneDynamicServerInfo returns a clone of the dynamic server info.
func (s *Syncer) cloneDynamicServerInfo() *DynamicInfo {
return s.info.Load().DynamicInfo.Clone()
Expand All @@ -204,8 +237,12 @@ func (s *Syncer) GetAllServerInfo(ctx context.Context) (map[string]*ServerInfo,
})
allInfo := make(map[string]*ServerInfo)
if s.etcdCli == nil {
info := s.info.Load()
allInfo[info.ID] = getServerInfo(info.ID, info.ServerIDGetter, "")
localInfo := s.info.Load()
info := getServerInfo(localInfo.ID, localInfo.ServerIDGetter, localInfo.AssumedKeyspace)
info.TiDBRestrictedReadOnly = localInfo.TiDBRestrictedReadOnly
info.TiDBSuperReadOnly = localInfo.TiDBSuperReadOnly
info.TiDBEffectiveReadOnly = localInfo.TiDBEffectiveReadOnly
allInfo[info.ID] = info
return allInfo, nil
}
allInfo, err := getInfo(ctx, s.etcdCli, ServerInformationPath, KeyOpDefaultRetryCnt, KeyOpDefaultTimeout, clientv3.WithPrefix())
Expand Down
29 changes: 29 additions & 0 deletions pkg/domain/serverinfo/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,32 @@ func TestAssumedServerInfoSyncer(t *testing.T) {
require.Equal(t, "ks1", info.AssumedKeyspace)
require.EqualValues(t, keyspace.System, info.Keyspace)
}

func TestGetAllServerInfoWithoutEtcd(t *testing.T) {
bak := config.GetGlobalConfig()
t.Cleanup(func() {
config.StoreGlobalConfig(bak)
})
config.UpdateGlobal(func(conf *config.Config) {
conf.Status.StatusPort = 10080
conf.Labels = map[string]string{"old": "label"}
})

syncer := NewSyncer("1", func() uint64 { return 1 }, nil, nil)
require.NoError(t, syncer.UpdateServerReadOnlyStatus(context.Background(), true, false))

config.UpdateGlobal(func(conf *config.Config) {
conf.Status.StatusPort = 12345
conf.Labels = map[string]string{"fresh": "label"}
})

infos, err := syncer.GetAllServerInfo(context.Background())
require.NoError(t, err)
info := infos["1"]
require.NotNil(t, info)
require.EqualValues(t, 12345, info.StatusPort)
require.Equal(t, map[string]string{"fresh": "label"}, info.Labels)
require.True(t, info.TiDBRestrictedReadOnly)
require.False(t, info.TiDBSuperReadOnly)
require.True(t, info.TiDBEffectiveReadOnly)
}
1 change: 1 addition & 0 deletions pkg/sessionctx/vardef/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
go_library(
name = "vardef",
srcs = [
"readonly.go",
"runtime.go",
"sysvar.go",
"tidb_vars.go",
Expand Down
64 changes: 64 additions & 0 deletions pkg/sessionctx/vardef/readonly.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright 2026 PingCAP, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package vardef

import (
"context"
"sync/atomic"
)

// ClusterReadOnlyChecker returns whether all live TiDB instances are effectively read-only.
type ClusterReadOnlyChecker func(context.Context) (bool, error)

// ReadOnlyStatusReporter publishes the local TiDB read-only status.
type ReadOnlyStatusReporter func(context.Context) error

var (
clusterReadOnlyChecker atomic.Value
readOnlyStatusReporter atomic.Value
)

// LocalTiDBReadOnlyStatus returns the local TiDB effective read-only status.
func LocalTiDBReadOnlyStatus() bool {
return RestrictedReadOnly.Load() || VarTiDBSuperReadOnly.Load()
}

// SetClusterReadOnlyChecker registers the checker used by the derived tidb_is_read_only variable.
func SetClusterReadOnlyChecker(checker ClusterReadOnlyChecker) {
clusterReadOnlyChecker.Store(checker)
}

// GetClusterReadOnlyStatus returns whether the whole TiDB cluster is effectively read-only.
func GetClusterReadOnlyStatus(ctx context.Context) (bool, error) {
checker, ok := clusterReadOnlyChecker.Load().(ClusterReadOnlyChecker)
if !ok || checker == nil {
return LocalTiDBReadOnlyStatus(), nil
}
return checker(ctx)
}

// SetReadOnlyStatusReporter registers a reporter for the local TiDB read-only status.
func SetReadOnlyStatusReporter(reporter ReadOnlyStatusReporter) {
readOnlyStatusReporter.Store(reporter)
}

// ReportReadOnlyStatus publishes the local TiDB read-only status when a reporter is available.
func ReportReadOnlyStatus(ctx context.Context) error {
reporter, ok := readOnlyStatusReporter.Load().(ReadOnlyStatusReporter)
if !ok || reporter == nil {
return nil
}
return reporter(ctx)
}
3 changes: 3 additions & 0 deletions pkg/sessionctx/vardef/tidb_vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,9 @@ const (
// TiDBSuperReadOnly is tidb's variant of mysql's super_read_only, which has some differences from mysql's super_read_only.
TiDBSuperReadOnly = "tidb_super_read_only"

// TiDBIsReadOnly is a read-only derived status that indicates whether all TiDB instances are effectively read-only.
TiDBIsReadOnly = "tidb_is_read_only"

// TiDBShardAllocateStep indicates the max size of continuous rowid shard in one transaction.
TiDBShardAllocateStep = "tidb_shard_allocate_step"
// TiDBEnableTelemetry indicates that whether usage data report to PingCAP is enabled.
Expand Down
19 changes: 17 additions & 2 deletions pkg/sessionctx/variable/sysvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ func withMinValue(minVal int64) execConcurrencySysVarOption {
return func(sv *SysVar) { sv.MinValue = minVal }
}

func reportTiDBReadOnlyStatus(ctx context.Context) {
if err := vardef.ReportReadOnlyStatus(ctx); err != nil {
logutil.BgLogger().Warn("update TiDB read-only status failed", zap.Error(err))
}
}
Comment on lines +84 to +88
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix the vardef API mismatch before merge.

Crossbuild is failing here because vardef.ReportReadOnlyStatus and vardef.GetClusterReadOnlyStatus are undefined. This PR will not compile until the plumbing layer exports these symbols or these call sites are updated to the actual API names.

Also applies to: 1014-1020

🧰 Tools
🪛 GitHub Check: Bazel Crossbuild (ubuntu-24.04-arm)

[failure] 85-85:
undefined: vardef.ReportReadOnlyStatus

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/sessionctx/variable/sysvar.go` around lines 84 - 88, The build is failing
because reportTiDBReadOnlyStatus calls vardef.ReportReadOnlyStatus and
vardef.GetClusterReadOnlyStatus which do not exist in the current plumbing;
update the call sites (e.g., in reportTiDBReadOnlyStatus and the other
occurrences around lines shown) to use the actual exported API names from the
plumbing layer or export these symbols from vardef: either change
vardef.ReportReadOnlyStatus -> the real function name provided by the plumbing
(and likewise for GetClusterReadOnlyStatus), or add matching
ReportReadOnlyStatus/GetClusterReadOnlyStatus wrappers in the vardef package
that forward to the plumbing implementation so the references in
reportTiDBReadOnlyStatus compile.


// newExecConcurrencySysVar creates a session/global SysVar for executor concurrency settings.
func newExecConcurrencySysVar(name string, defValue int, setter concurrencySetter, opts ...execConcurrencySysVarOption) *SysVar {
sv := &SysVar{
Expand Down Expand Up @@ -971,7 +977,7 @@ var defaultSysVars = []*SysVar{
tikvstore.TxnCommitBatchSize.Store(uint64(TidbOptInt64(val, int64(tikvstore.DefTxnCommitBatchSize))))
return nil
}},
{Scope: vardef.ScopeGlobal, Name: vardef.TiDBRestrictedReadOnly, Value: BoolToOnOff(vardef.DefTiDBRestrictedReadOnly), Type: vardef.TypeBool, SetGlobal: func(_ context.Context, s *SessionVars, val string) error {
{Scope: vardef.ScopeGlobal, Name: vardef.TiDBRestrictedReadOnly, Value: BoolToOnOff(vardef.DefTiDBRestrictedReadOnly), Type: vardef.TypeBool, SetGlobal: func(ctx context.Context, s *SessionVars, val string) error {
on := TiDBOptOn(val)
// For user initiated SET GLOBAL, also change the value of TiDBSuperReadOnly
if on && s.StmtCtx.StmtType == "Set" {
Expand All @@ -985,6 +991,7 @@ var defaultSysVars = []*SysVar{
}
}
vardef.RestrictedReadOnly.Store(on)
reportTiDBReadOnlyStatus(ctx)
return nil
}},
{Scope: vardef.ScopeGlobal, Name: vardef.TiDBSuperReadOnly, Value: BoolToOnOff(vardef.DefTiDBSuperReadOnly), Type: vardef.TypeBool, Validation: func(s *SessionVars, normalizedValue string, _ string, _ vardef.ScopeFlag) (string, error) {
Expand All @@ -999,10 +1006,18 @@ var defaultSysVars = []*SysVar{
}
}
return normalizedValue, nil
}, SetGlobal: func(_ context.Context, s *SessionVars, val string) error {
}, SetGlobal: func(ctx context.Context, s *SessionVars, val string) error {
vardef.VarTiDBSuperReadOnly.Store(TiDBOptOn(val))
reportTiDBReadOnlyStatus(ctx)
return nil
}},
{Scope: vardef.ScopeGlobal, Name: vardef.TiDBIsReadOnly, Value: vardef.Off, Type: vardef.TypeBool, ReadOnly: true, GetGlobal: func(ctx context.Context, s *SessionVars) (string, error) {
on, err := vardef.GetClusterReadOnlyStatus(ctx)
if err != nil {
return vardef.Off, err
}
return BoolToOnOff(on), nil
}},
{Scope: vardef.ScopeGlobal, Name: vardef.TiDBEnableGOGCTuner, Value: BoolToOnOff(vardef.DefTiDBEnableGOGCTuner), Type: vardef.TypeBool, SetGlobal: func(_ context.Context, s *SessionVars, val string) error {
on := TiDBOptOn(val)
gctuner.EnableGOGCTuner.Store(on)
Expand Down
Loading
Loading