search

coreos / fleet

fleet ties together systemd and etcd into a distributed init system
Apache License 2.0
2.43k stars 302 forks source link

api: compare units on update operations #851

Open jonboulle opened 10 years ago

jonboulle commented 10 years ago

unitsResource.set() currently makes no effort to check whether a unit being updated is consistent with the unit found in the Registry. We should be a little more atomic/safe by at least doing a basic check.

bcwaldon commented 10 years ago

This is something that was originally intended to be available in the API, but it got in the way while I was reworking things and it was dropped. I'm more than happy to see hash (maybe options, actually) comparison in mutating operations: both PUT and DELETE.

bcwaldon commented 10 years ago

@jonboulle We've talked about this before, but I totally forgot until I started implementing this feature. We currently don't lock or serialize access to the Registry, which means we don't have a way to atomically update units. I'm leaning towards not implementing this feature until we can guarantee the necessary atomicity. What do you think?

bcwaldon commented 10 years ago

The other side of the argument, however, is that we already have inherently racy operations. For example, when a PUT request is made, the server first checks if the underlying unit exists. If it does not exist, the unit options are used to create the unit file before the target-state key is set. If the unit does exist, the unit options are discarded, and the target-state key is set immediately. If the unit file is destroyed due to a simultaneous request after the server checks for unit existence but before it sets the target-state key, the Registry will end up in a bad state.

jonboulle commented 10 years ago

@bcwaldon Correct, even if we implement this at the API level we will still be racy (until we have multi-key requests, booyah!). However, this change would dramatically minimise the window for raciness with certain user requests, at least.