Skip to content
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

TiKV failover #77

Merged
merged 13 commits into from Sep 8, 2018
Merged

TiKV failover #77

merged 13 commits into from Sep 8, 2018

Conversation

weekface
Copy link
Contributor

@weekface weekface commented Sep 4, 2018

This PR fixes #58, adds TiKV failover feature to TiDB Operator.

@tennix @onlymellb @xiaojingchen @gregwebs PTAL

@weekface weekface mentioned this pull request Sep 4, 2018
4 tasks
@weekface
Copy link
Contributor Author

weekface commented Sep 5, 2018

/run-e2e-tests

xiaojingchen
xiaojingchen previously approved these changes Sep 7, 2018
@weekface
Copy link
Contributor Author

weekface commented Sep 7, 2018

/run-e2e-tests

TombStoneStores map[string]TiKVStore `json:"tombStoneStores,omitempty"`
Phase MemberPhase `json:"phase,omitempty"`
StatefulSet *apps.StatefulSetStatus `json:"statefulSet,omitempty"`
Stores map[string]TiKVStore `json:"stores,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Will Stores contain failure stores or tombstone stores?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stores contain failure stores, but don't contain tombstone stores.

tc.Status.TiKV.FailureStores = nil
}

func allTiKVStoressAreReady(tc *v1alpha1.TidbCluster) bool {
Copy link
Member

Choose a reason for hiding this comment

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

s/allTiKVStoress/AreReady/allTiKVStoresAreReady/

if tc.Status.TiKV.FailureStores != nil {
tkmm.tikvFailover.Recover(tc)
}
} else if int(tc.Spec.TiKV.Replicas) == int(tc.Status.TiKV.StatefulSet.Replicas) {
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary cast replicas to int


"time"

. "github.com/onsi/gomega"
Copy link
Member

Choose a reason for hiding this comment

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

Add revive instruction for dot-import.
You can choose whatever you like to follow revive linter suggests.
retool do revive -formatter friendly -config revive.toml pkg/...


metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"time"
Copy link
Member

Choose a reason for hiding this comment

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

Could you separate the standard library and external library?

pdClient.AddReaction(controller.GetConfigActionType, func(action *controller.Action) (interface{}, error) {
if test.getCfgErr {
return nil, fmt.Errorf("get config failed")
} else {
Copy link
Member

Choose a reason for hiding this comment

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

No need to add else block.

Copy link
Member

@tennix tennix left a comment

Choose a reason for hiding this comment

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

LGTM

@weekface
Copy link
Contributor Author

weekface commented Sep 7, 2018

/run-e2e-tests

@xiaojingchen xiaojingchen merged commit 76e4ccb into pingcap:master Sep 8, 2018
queenliuxx pushed a commit to queenliuxx/tidb-operator that referenced this pull request Dec 19, 2018
@weekface weekface deleted the tikv-failover branch June 21, 2019 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TiKV automatic failover
3 participants