<div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Mar 7, 2014 at 11:53 AM, Matthew Booth <span dir="ltr"><<a href="mailto:mbooth@redhat.com" target="_blank">mbooth@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">We need locking in the VMware driver. There are 2 questions:<br>
<br>
1. How much locking do we need?<br>
2. Do we need single-node or multi-node locking?<br>
<br>
I believe these are quite separate issues, so I'm going to try not to<br>
confuse them. I'm going to deal with the first question first.<br>
<br>
In reviewing the image cache ageing patch, I came across a race<br>
condition between cache ageing and spawn(). One example of the race is:<br>
<br>
Cache Ageing spawn()<br>
* Check timestamps<br>
* Delete timestamp<br>
* Check for image cache directory<br>
* Delete directory<br>
* Use image cache directory<br>
<br>
This will cause spawn() to explode. There are various permutations of<br>
this. For example, the following are all possible:<br>
<br>
* A simple failure of spawn() with no additional consequences.<br>
<br>
* Calling volumeops.attach_disk_to_vm() with a vmdk_path that doesn't<br>
exist. It's not 100% clear to me that ReconfigVM_Task will throw an<br>
error in this case, btw, which would probably be bad.<br>
<br>
* Leaving a partially deleted image cache directory which doesn't<br>
contain the base image. This would be really bad.<br>
<br>
The last comes about because recursive directory delete isn't atomic,<br>
and may partially succeed, which is a tricky problem. However, in<br>
discussion, Gary also pointed out that directory moves are not atomic<br>
(see MoveDatastoreFile_Task). This is positively nasty. We already knew<br>
that spawn() races with itself to create an image cache directory, and<br>
we've hit this problem in practise. We haven't fixed the race, but we do<br>
manage it. The management relies on the atomicity of a directory move.<br>
Unfortunately it isn't atomic, which presents the potential problem of<br>
spawn() attempting to use an incomplete image cache directory. We also<br>
have the problem of 2 spawns using a linked clone image racing to create<br>
the same resized copy.<br>
<br>
We could go through all of the above very carefully to assure ourselves<br>
that we've found all the possible failure paths, and that in every case<br>
the problems are manageable and documented. However, I would place a<br>
good bet that the above is far from a complete list, and we would have<br>
to revisit it in its entirety every time we touched any affected code.<br>
And that would be a lot of code.<br>
<br>
We need something to manage concurrent access to resources. In all of<br>
the above cases, if we make the rule that everything which uses an image<br>
cache directory must hold a lock on it whilst using it, all of the above<br>
problems go away. Reasoning about their correctness becomes the<br>
comparatively simple matter of ensuring that the lock is used correctly.<br>
Note that we need locking in both the single and multi node cases,<br>
because even single node is multi-threaded.<br>
<br>
The next question is whether that locking needs to be single node or<br>
multi node. Specifically, do we currently, or do we plan to, allow an<br>
architecture where multiple Nova nodes access the same datastore<br>
concurrently. If we do, then we need to find a distributed locking<br>
solution. Ideally this would use the datastore itself for lock<br>
mediation. Failing that, apparently this tool is used elsewhere within<br>
the project</blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">We do have such a case in nova (<a href="https://github.com/openstack/nova/blob/master/nova/virt/libvirt/imagebackend.py#L179">https://github.com/openstack/nova/blob/master/nova/virt/libvirt/imagebackend.py#L179</a>), which led to us restoring file locking in lockutils (<a href="https://review.openstack.org/#/c/77297/">https://review.openstack.org/#/c/77297/</a>).</div>
<div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">There is also work being done to create a library called "tooz" (<a href="http://git.openstack.org/cgit/stackforge/tooz">http://git.openstack.org/cgit/stackforge/tooz</a>) for abstracting some of the distributed coordination patterns to allow deployers to choose between platforms like zookeeper and etcd, just as we do with virt drivers and message queues. It would be good if any work in this area coordinated with the tooz team -- I anticipate the library being brought into Oslo in the future.</div>
<div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Doug</div><div class="gmail_default" style="font-size:small"><br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
:<br>
<br>
<a href="http://zookeeper.apache.org/doc/trunk/zookeeperOver.html" target="_blank">http://zookeeper.apache.org/doc/trunk/zookeeperOver.html</a><br>
<br>
That would be an added layer of architecture and deployment complexity,<br>
but if we need it, it's there.<br>
<br>
If we can confidently say that 2 Nova instances should never be<br>
accessing the same datastore (how about hot/warm/cold failover?), we can<br>
use Nova's internal synchronisation tools. This would simplify matters<br>
greatly!<br>
<br>
I think this is one of those areas which is going to improve both the<br>
quality of the driver, and the confidence of reviewers to merge changes.<br>
Right now it takes a lot of brain cycles to work through all the various<br>
paths of a race to work out if any of them are really bad, and it has to<br>
be repeated every time you touch the code. A little up-front effort will<br>
make a whole class of problems go away.<br>
<br>
Matt<br>
<span class=""><font color="#888888">--<br>
Matthew Booth, RHCA, RHCSS<br>
Red Hat Engineering, Virtualisation Team<br>
<br>
GPG ID: D33C3490<br>
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490<br>
<br>
_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</font></span></blockquote></div><br></div></div>