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

Support ephemeral nodes #52

Closed
pitr opened this issue Mar 5, 2013 · 14 comments
Closed

Support ephemeral nodes #52

pitr opened this issue Mar 5, 2013 · 14 comments
Assignees
Labels

Comments

@pitr
Copy link

pitr commented Mar 5, 2013

No description provided.

@dgryski
Copy link

dgryski commented Mar 6, 2013

ActiveState's fork ( https://github.com/ActiveState/doozerd ) has support for ephemeral nodes.

@pitr
Copy link
Author

pitr commented Mar 6, 2013

awesome. let's keep this issue open until it's merged and documented

@bketelsen
Copy link
Member

Patches GLADLY reviewed.

@mreiferson
Copy link
Contributor

I poked around ActiveState's fork and, to be honest, I'm not terribly happy with the implementation chosen.

I think any node should be able to be created as ephemeral (not just some specific hard coded path requiring special case handling in both get and set code).

I think this would produce the most flexible implementation but I acknowledge that it might mean more breaking changes to the API.

I'm curious to hear other's thoughts before I take a swing at this.

@pitr
Copy link
Author

pitr commented Mar 8, 2013

@mreiferson I also looked at the code and I agree. It's obvious that ActiveState did what anyone else would have done in their place - quickly get the feature to work. But for it to be useful to everyone, this needs to be done properly and thoughtfully. Before we hit major version 1 I think it's perfectly acceptable to break API, considering how important this feature is.

@mreiferson
Copy link
Contributor

@pitr Yes, absolutely, it's always a matter of tradeoffs 😄

@srid
Copy link
Member

srid commented Mar 10, 2013

It's obvious that ActiveState did what anyone else would have done in their place - quickly get the feature to work.

that's exactly what happened (the commit itself was titled "limited ephemeral node support").

I think any node should be able to be created as ephemeral (not just some specific hard coded path requiring special case handling in both get and set code).

+1

But for it to be useful to everyone, this needs to be done properly and thoughtfully. Before we hit major version 1 I think it's perfectly acceptable to break API, considering how important this feature is.

as for the API, how about having SET take an extra argument (t.req.CreateMode = EPHEMERAL)?

as for the implementation, i'm thinking that the list of ephemeral paths can be stored in the conn{} struct; to be deleted during disconnection.

@mreiferson
Copy link
Contributor

@srid I was thinking the same thing (your suggested implementation)

Do you want to take a stab at this? I'm working on cleaning up logging and other misc odds and ends so I might not get to this for a bit.

@srid
Copy link
Member

srid commented Mar 11, 2013

sure, i'll take a look.

@ghost ghost assigned srid Mar 11, 2013
@pitr
Copy link
Author

pitr commented Mar 11, 2013

should directories have ability to be ephemeral?

@mreiferson
Copy link
Contributor

@pitr at the moment you only ever indirectly create directories (SET works on "file" paths), so my vote would be no.

@bketelsen
Copy link
Member

Agreed. And there's no real need for data-owning directories in the use cases I've seen.

@cespare
Copy link

cespare commented Mar 16, 2013

I've certainly seen Zookeeper use cases where ephemeral znodes with other nodes underneath them (ephemeral directories in Doozer) are useful. For instance, a server might come online, open up an ephemeral node called $HOSTNAME to indicate its existence and keep status information in files underneath that.

But since you don't really deal with directories directly in Doozer, I see how that wouldn't really make sense here.

@bketelsen
Copy link
Member

Sorry my comment is unclear. I meant data-owning in the sense that all nodes in zk can have data associated, but they can't in doozer. A path like /myHostname/ can't have data attached, but an easy work around is to apply children paths like /myHostname/ipAddress with the data you want to store.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants