On Dec 19, 2005, at 6:07 PM, Rob Ross wrote:
I don't consider rmdirent to be a case that we need to highly
optimize, so if it helps to lookup the name and do the getattr
beforehand, that's cool by me. I'm sure somewhere out there there
is a rmdir test that we'll do a little worse on, but whatever.
You'll still have to getattr again after the rmdirent to make sure
that the directory is still empty though, in case someone sneaks in
between the first getattr and the rmdirent and creates something.
I think I can just check if the directory is not empty. If the
getattr tells me it is empty, I just do the remove, and if someone
else has added an entry in the meantime, I'll get the ENOTOEMPTY from
the server at that point, so I can go add the entry back in to the
parent (which is what we do now).
The race that's an issue seems to be if the getattr tells me the
directory is not empty, in which case I just return ENOTEMPTY back to
the caller, but in the meantime someone else could have removed all
the entries (in which case the remove should have succeeded). I'm
not sure that's much of an issue for our users though.
Right now our caching scheme is under-the-covers so to speak, where
we always continue with the operation if the cache misses. It seems
like we could make a decision based on a cache hit. In other words,
if I know the name and attribute are cached, I can save time by
checking if the directory is not empty. If its not cached, I can
just go down the rmdirent path like we do now. Doing lookup/getattr
will add an extra roundtrip in most cases (where the directory is
empty) if things aren't being cached. Overkill?
-sam
rob
Sam Lang wrote:
On Dec 19, 2005, at 3:51 PM, Julian Martin Kunkel wrote:
Hi,
1. rmdirent(object name,parent handle) => object handle
2. getattr(object handle) => attr
3. remove(object handle) => ENOTEMPTY
4. crdirent(object_handle)
Ahh, you are right. I didn't realize that the attributes
weren't read
until after removing the directory entry. With that being the
case it
would have to come after step 2) as you say, although that isn't as
helpful as if we had the information earlier.
I don't think this would be helpful because it won't change
something. If a
client breaks after step 1 the directory object is gone, even if
it has
objects inside.
Currently you check whether the directory is empty directly after
step 2.
It would save the remove roundtrip (step 3 from the list above),
since we first check the attr and discover the directory isn't empty.
From my opinion the problem is that you don't check the
directory state before
you remove in step 1 the directory entry.
Yes of course you can't do this because you get the object handle
while
rmdirent.
A idea would be to first lookup the object name to get the object
handle and
then getattr (handle) and then look if it's empty or not (like it is
currently done).
I think this adds a roundtrip, doesn't it? The rmdirent acts as a
lookup in the current code, so we get a two-for-one there. The
only reason I can think of for doing this would be that the name
cache would prevent the over-the-wire lookup, and then the attr
cache would return the attributes, so the is-empty check could
potentially be done (given cache hits) without any over-the-wire
operations. Right now we bypass the name cache. Not sure what's
most optimal here.
-sam
If it's empty do rmdirent and then remove object. The
mechanism with the ENOTEMPTY and crdirent can be keept in case
somebody
creates a object in that directory while another client checked
the number of
directory entries with getattr.
Julian
_______________________________________________
PVFS2-developers mailing list
PVFS2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
_______________________________________________
PVFS2-developers mailing list
PVFS2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
_______________________________________________
PVFS2-developers mailing list
PVFS2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
