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

api: support set store state. #757

Merged
merged 3 commits into from
Sep 18, 2017
Merged

api: support set store state. #757

merged 3 commits into from
Sep 18, 2017

Conversation

disksing
Copy link
Contributor

Sometime we want to recover store from wrong state. Sometime we want to cancel removing a store.

// NewSetStoreStateCommand returns a state subcommand of storeCmd.
func NewSetStoreStateCommand() *cobra.Command {
d := &cobra.Command{
Use: "state <store_id> [Up|Offline|Tombstone]",
Copy link
Contributor

Choose a reason for hiding this comment

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

can we set the tombstone state directly? Seem it is dangerous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already have API to let user to set a store's state to 'Tombstone' (from any state)... See: https://github.com/pingcap/pd/blob/73fa4c4f2c394817fbb511afb06a5933b845167e/server/cluster.go#L486-L490

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it.

Copy link
Contributor

Choose a reason for hiding this comment

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

But can we transfer a tombstone store to up again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can. The 'tombstone' is just a flag in pd-server that prevent data been transferred to the store, and make the store ignored by any balancers.

@siddontang
Copy link
Contributor

/run-integration-test

Copy link
Contributor

@nolouch nolouch left a comment

Choose a reason for hiding this comment

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

rest LGTM

fmt.Println("store_id should be a number")
return
}
prefix := fmt.Sprintf(storePrefix+"/state?state=%s", args[0], args[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this way more like GET

@siddontang
Copy link
Contributor

LGTM
PTAL @huachaohuang

@disksing disksing changed the title api, pdctl: support set store state. api: support set store state. Sep 18, 2017
@huachaohuang
Copy link
Contributor

LGTM

@disksing
Copy link
Contributor Author

/run-all-test

@disksing disksing merged commit 53c32f0 into master Sep 18, 2017
@disksing disksing deleted the disksing/store-state branch September 18, 2017 06:42
@cofyc
Copy link

cofyc commented Jul 24, 2019

I saw the original commit to add state set command in pd-ctrl, why it was removed?

@disksing
Copy link
Contributor Author

@cofyc It's a dangerous feature. We only want experts to use it.

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.

None yet

5 participants