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

server: add api member delete #245

Merged
merged 4 commits into from
Aug 5, 2016
Merged

Conversation

overvenus
Copy link
Member

resp, err := s.hc.Get(strings.Join(parts, ""))
c.Assert(err, IsNil)
buf, err := ioutil.ReadAll(resp.Body)
c.Assert(err, IsNil)
Copy link
Contributor

Choose a reason for hiding this comment

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

close resp.Body?

@siddontang
Copy link
Contributor

In etcd, if we removed a member with ID, we can't add this member with same ID again.
so I think we can't add PD with same name again. Do you test it?

cfgs, _, clean := mustNewCluster(c, 3)
defer clean()

someone := rand.Intn(len(cfgs))
Copy link
Member

Choose a reason for hiding this comment

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

s/someone/target/

@overvenus
Copy link
Member Author

Cluster: pd1, pd2, pd3

If pd2 was removed, a new pd named pd2 can be joined, as long as there is no data in it's data-dir.

@siddontang
Copy link
Contributor

if has data, can join?


for _, t := range table {
parts := []string{"http://", t.addr, "/api/v1/members", "/" + t.name}
req, err := http.NewRequest(http.MethodDelete, strings.Join(parts, ""), nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

can't compile in go 1.5 with http.MethodDelete, see travis CI.

@overvenus
Copy link
Member Author

Unfortunately, we can's use -join flag, if we use it, pd2 will shutdown immediately, and other pd servers keep dialing pd2.

Howerver, -initial-cluster will not mess up cluster instead pd2 says aborting publish because server is stopped.

@siddontang
Copy link
Contributor

so we should check data dir when join and must ensure the joining peer is clear and new.

@overvenus
Copy link
Member Author

Yes, I guess, but it's another story, maybe I should tweak join flag in another PR, let's focus on delete api 😄

@siddontang
Copy link
Contributor

yes, I will open another issue for you.


// step 1. get etcd id
var id uint64
name := (mux.Vars(r))["name"]
Copy link
Contributor

Choose a reason for hiding this comment

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

check empty or missing name here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, it is fine, only DELETE /api/v1/members/xxx will be routed to here.

@siddontang
Copy link
Contributor

LGTM

@qiuyesuifeng
Copy link
Contributor

@huachaohuang PTAL

@huachaohuang
Copy link
Contributor

LGTM

@overvenus overvenus merged commit 5dee1df into tikv:master Aug 5, 2016
@overvenus overvenus mentioned this pull request Aug 5, 2016
rleungx pushed a commit to rleungx/pd that referenced this pull request Jan 25, 2024
…ikv#245)

Co-authored-by: Yongbo Jiang <cabinfeveroier@gmail.com>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
close tikv#7496
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.

5 participants