Skip to content

Comments

fix: ProtectedEphemeralSequential in the wrong zk path#849

Merged
peterbourgon merged 1 commit intogo-kit:masterfrom
RedDragonet:patch-1
Apr 7, 2019
Merged

fix: ProtectedEphemeralSequential in the wrong zk path#849
peterbourgon merged 1 commit intogo-kit:masterfrom
RedDragonet:patch-1

Conversation

@RedDragonet
Copy link
Contributor

register := zk.NewRegistrar(client,zk.Service{
	Path: "/services/hello",
	Name: "abc",
	Data: []byte("http://127.0.0.1:8080"),
},logger)

It will be created in zk

[zk: localhost:2181(CONNECTED) 161] ls /services/hello
[_c_c83db041ac654566228b72cbd541bcb5-abc0000000006, abc]

/services/hello/_c_c83db041ac654566228b72cbd541bcb5 is correct
/services/hello/abc is empty node

//will get 0 instance
zk.NewInstancer(client,"/services/hello/abc",logger)
//will get 2 instances, one of them is <nil>
zk.NewInstancer(client,"/services/hello",logger)

In my understanding, CreateProtectedEphemeralSequential function will be created under /services/hello/abc

So is this a bug? Or there's something wrong with my usage?

```golang
register := zk.NewRegistrar(client,zk.Service{
	Path: "/services/hello",
	Name: "abc",
	Data: []byte("http://127.0.0.1:8080"),
},logger)
```
> It will be created in zk
```sh
[zk: localhost:2181(CONNECTED) 161] ls /services/hello
[_c_c83db041ac654566228b72cbd541bcb5-abc0000000006, abc]
```
**/services/hello/_c_c83db041ac654566228b72cbd541bcb5** is correct
**/services/hello/abc** is empty node

```golang
//will get 0 instance
zk.NewInstancer(client,"/services/hello/abc",logger)
//will get 2 instances, one of them is <nil>
zk.NewInstancer(client,"/services/hello",logger)
```
In my understanding, [CreateProtectedEphemeralSequential](https://github.com/go-kit/kit/blob/master/sd/zk/client.go#L241) function will be created under `/services/hello/abc`

So is this a bug? Or there's something wrong with my usage?
@peterbourgon
Copy link
Member

@basvanbeek Opinions?

@RedDragonet
Copy link
Contributor Author

@peterbourgon Any suggestions?

@peterbourgon
Copy link
Member

I don't have enough knowledge about ZooKeeper to give any opinion here.

@peterbourgon
Copy link
Member

I guess it's fine?

@peterbourgon peterbourgon merged commit 8c84d88 into go-kit:master Apr 7, 2019
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.

2 participants