Skip to content

Commit

Permalink
owner: fix that get owner opValue before set this value when the etcd…
Browse files Browse the repository at this point in the history
…Client is nil (#45393)

close #45392
  • Loading branch information
zimulala authored Jul 18, 2023
1 parent aca4429 commit c0e7057
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 3 deletions.
3 changes: 2 additions & 1 deletion owner/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ go_test(
],
embed = [":owner"],
flaky = True,
shard_count = 4,
shard_count = 5,
deps = [
"//ddl",
"//infoschema",
"//kv",
"//parser/terror",
"//store/mockstore",
"//testkit",
"//testkit/testsetup",
"//util",
"//util/logutil",
Expand Down
30 changes: 30 additions & 0 deletions owner/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/pingcap/tidb/owner"
"github.com/pingcap/tidb/parser/terror"
"github.com/pingcap/tidb/store/mockstore"
"github.com/pingcap/tidb/testkit"
"github.com/pingcap/tidb/util/logutil"
"github.com/stretchr/testify/require"
clientv3 "go.etcd.io/etcd/client/v3"
Expand Down Expand Up @@ -176,6 +177,35 @@ func TestSetAndGetOwnerOpValue(t *testing.T) {
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/owner/MockDelOwnerKey"))
}

// TestGetOwnerOpValueBeforeSet tests get owner opValue before set this value when the etcdClient is nil.
func TestGetOwnerOpValueBeforeSet(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("integration.NewClusterV3 will create file contains a colon which is not allowed on Windows")
}
require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/owner/MockNotSetOwnerOp", `return(true)`))

_, dom := testkit.CreateMockStoreAndDomain(t)
ddl := dom.DDL()
require.NoError(t, ddl.OwnerManager().CampaignOwner())
isOwner := checkOwner(ddl, true)
require.True(t, isOwner)

// test set/get owner info
manager := ddl.OwnerManager()
ownerID, err := manager.GetOwnerID(context.Background())
require.NoError(t, err)
require.Equal(t, ddl.GetID(), ownerID)
op, err := owner.GetOwnerOpValue(context.Background(), nil, DDLOwnerKey, "log prefix")
require.NoError(t, err)
require.Equal(t, op, owner.OpNone)
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/owner/MockNotSetOwnerOp"))
err = manager.SetOwnerOpValue(context.Background(), owner.OpGetUpgradingState)
require.NoError(t, err)
op, err = owner.GetOwnerOpValue(context.Background(), nil, DDLOwnerKey, "log prefix")
require.NoError(t, err)
require.Equal(t, op, owner.OpGetUpgradingState)
}

func TestCluster(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("integration.NewClusterV3 will create file contains a colon which is not allowed on Windows")
Expand Down
14 changes: 12 additions & 2 deletions owner/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"github.com/pingcap/errors"
"github.com/pingcap/failpoint"
"github.com/pingcap/tidb/ddl/util"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/util/logutil"
Expand All @@ -44,13 +45,19 @@ type mockManager struct {
resignDone chan struct{}
}

var mockOwnerOpValue atomic.Pointer[OpType]

// NewMockManager creates a new mock Manager.
func NewMockManager(ctx context.Context, id string, store kv.Storage, ownerKey string) Manager {
cancelCtx, cancelFunc := context.WithCancel(ctx)
storeID := "mock_store_id"
if store != nil {
storeID = store.UUID()
}

// Make sure the mockOwnerOpValue is initialized before GetOwnerOpValue in bootstrap.
op := OpNone
mockOwnerOpValue.Store(&op)
return &mockManager{
id: id,
storeID: storeID,
Expand Down Expand Up @@ -106,9 +113,12 @@ func (m *mockManager) GetOwnerID(_ context.Context) (string, error) {
return "", errors.New("no owner")
}

var mockOwnerOpValue atomic.Pointer[OpType]

func (*mockManager) SetOwnerOpValue(_ context.Context, op OpType) error {
failpoint.Inject("MockNotSetOwnerOp", func(val failpoint.Value) {
if val.(bool) {
failpoint.Return(nil)
}
})
mockOwnerOpValue.Store(&op)
return nil
}
Expand Down

0 comments on commit c0e7057

Please sign in to comment.