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

libkv should have a way to watch a directory that doesn't exist yet. #16

Open
spikecurtis opened this issue Jun 19, 2015 · 8 comments
Open

Comments

@spikecurtis
Copy link
Contributor

We need a way to watch a directory that doesn't exist yet.

  • This seems to work with the Consul backend.
  • On etcd, if the directory doesn't exist, it throws an error.
  • Haven't tested zk yet.

Basically, either all the backends need to handle watching directories that don't exist, or we need a way to create directories so the watcher can create, then watch. I'm guessing the latter is going to be easier.

/cc @mavenugo this problem is blocking libnetwork functioning correctly with etcd backend

@abronan
Copy link
Contributor

abronan commented Jun 19, 2015

I'm not sure this is a good pattern to watch over a directory that does not exist yet. The client code should make sure that the directory exists before watching it (or creating it if it does not exist). We have this problem in Swarm where we don't verify that the key exists before trying to watch it for the discovery (so we'll fix it in Swarm directly). A good way would be to do something like:

if !store.Exists(key) {
  store.Put(key, []byte(""))
}
store.Watch(key)

I'm not entirely sure that this is the responsibility of libkv to do this. (So even if consul works in this case, you should still make sure that the key exists no matter which one of the backend you are using)

@spikecurtis
Copy link
Contributor Author

@abronan It's fine with me that the client needs to ensure the directory exists before watching it. However, etcd makes a distinction between directories (which cannot have values) and other keys. I believe that in etcd, if you Put an empty string at a key, then try to create another key beneath it in the tree this will fail. Hence, my suggestion that we need a way to create directories. Perhaps a new WriteOption?

@abronan
Copy link
Contributor

abronan commented Jun 19, 2015

@spikecurtis You have a point!

We can adopt either your solution or the go-client has this method: SetDir, which transforms a key into a directory. So we can assume that you can create a directory foo/bar which first acts as a regular key with an empty value. Then if you try to put foo/bar/foo or tree watch foo/bar we test if bar is a key and if this is the case we call SetDir to transform it into a directory.

Thoughts?

@spikecurtis
Copy link
Contributor Author

@abronan I don't really like the idea of a WatchTree mutating the data. It violates the principle of least surprise.

Clearly, a well designed application should have a key structure that is unambiguous about which are and are not directories. However, it's easy to mess this up. Watches being "helpful" and overwriting existing keys (possibly throwing out any data on them) could be really difficult to debug.

@abronan
Copy link
Contributor

abronan commented Jun 19, 2015

@spikecurtis I see how this could turn bad even though the key structure is something that should be well thought out in the client code so if you do a Put and then a Watch on a key, the client code is aware that the current key is indeed a directory. If there is a mistaken WatchTree on a key that holds a valuable information, it is something that should probably be fixed in the client code.

We have the choice of either:

  • Follow the Consul model to manage keys: with no directories but just a key structure with prefixes separated by / (So no notions of Create or directories that couldn't hold values)
  • Follow the Zookeeper and Etcd model to manage keys and directories with separate operations for both notions

This is a choice and I'm probably slightly leaning towards the Consul model (what we are doing now with no CreateDir operations) because it's way easier to manage. Ultimately we wanted to avoid users of the library (also us as we use it in swarm) to think about directories or nodes creation at all, everything is just a key (even though you can still watch child keys under a prefix as in Consul).

It's just a matter of smoothing the experience on the three existing stores (and probably more in the future) so it's going to be nearly impossible to have an optimal solution for each store. We will need to make compromises for some backends unfortunately.

We can probably open a separate issue and talk about which model makes more sense for libkv as this is a huge deal on how we will handle more storage backends in the future.

@spikecurtis
Copy link
Contributor Author

@abronan if you want to open a separate issue, can you cc me on it?

For the record, I think Zookeeper works like Consul inasmuch as it doesn't distinguish between directories and "files". I.e. znodes can store values even if they are not a leaf in the tree.

I'm not sure we can offer a totally consistent API. If we follow what you call the Consul model, then

Put("/path/to/", "")
Put("/path/to/new/node", "Hello")

will succeed on Consul, but the second call will fail on etcd. Likewise

Put("/path/to/new/node", "Hello")
Put("/path", "World!")

will succeed on Consul, but will fail on etcd.

There will be similar inconsistencies if we follow the etcd model of explicitly creating directories, since etcd will fail later attempts to Put to them, but zookeeper and consul will succeed.

Basically, what I'm saying is that either way we go, application designers are going to have an inconsistent experience. Etcd is going to fail requests that would succeed in Consul. So, designers that want their applications to work with any backend are going to have to design the keyspace with etcd-style constraints, i.e. that you can only store data at leaves in your tree.

@abronan
Copy link
Contributor

abronan commented Jun 20, 2015

@spikecurtis Sure!

So far in swarm we only create values on Leaf keys/nodes (so we don't encounter the issue you mentioned). This is kind of a mix between Consul and etcd. Zookeeper is between the two with a notion of directory (node) in which you can still put a value.

This is mentioned nowhere so we need to spin up a generic documentation explaining what to expect from the library.

In your example, etcd would not fail if the call to Put transforms the key into a directory. I admit that this is bad because you lose the data in key: path/. This is a sacrifice to make in order to achieve "consistency" (double quotes here) across the three existing backends.

There are still room for improvement though as this is hard to abstract backend K/V stores with such different ways to handle keys/directories/values 😄

(PS: would you mind opening a separate issue with your example? I think this is a perfect example of what we need to take care of/improve in libkv)

@tomdee
Copy link

tomdee commented Jul 13, 2015

For anyone following this thread, @spikecurtis raised #20 to cover the example above (as requested by @abronan)

thegrumpylion pushed a commit to thegrumpylion/libkv that referenced this issue Apr 3, 2020
Return the modified key on WatchTree call for etcd v3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants