dirnode 'delete' which hits UCWE will appear to fail, but actually succeeds #550

New Issue

warner · 2008-12-05T07:27:18Z

warner commented

2008-12-05 07:27:18 +00:00

The top-level problem seen in #546 occurred when an object was removed from a
directory. This involves a mutable file modify() call, using a
"Deleter" function as the modifier callback. In this case, the first attempt
to modify the file resulted in an UncoordinatedWriteError (which was
probably inappropriate, see #546 and #547). This causes a retry, which
downloads the file a second time, applies the modifier callback again, and
then (if the file's contents have changed) re-publishes the result.

In this case, the second application of the modifier callback failed, because
in fact the first publish succeeded: it placed 10 shares of the new
(post-delete) version of the file. The UCWE was only triggered because of a
lurking 11th share, which is not enough to affect the results of a
download_best_version(). So when the Deleter was called a second time, the
object it sought to delete was already gone.

However, Deleter comes with a must_exist=True default argument, which means
that the Deleter will throw an exception if the object to be deleted was not
already present. This exception does not cause a retry (only UCWE does
that), so it gets returned to the caller (in this case, a webapi DELETE
operation returned a "404 No Such Object" error, even though the object was
present before the DELETE and correctly missing afterwards).

This must_exist=True test conflicts with the try-until-success design of
modify(): the first attempt can apply this requirement, but subsequent
retries must not, else they will appear to fail even though they have
actually succeeded.

The fix is to either remove this default (making it always must_exist=False),
or change the modify() logic to pass an extra is_retry=bool
argument to the modifier callback, to allow it to change its behavior on
non-initial calls. For Deleter, the logic would be to not assert
must_exist=True if is_retry=True.

The top-level problem seen in #546 occurred when an object was removed from a directory. This involves a mutable file `modify()` call, using a "Deleter" function as the modifier callback. In this case, the first attempt to modify the file resulted in an UncoordinatedWriteError (which was probably inappropriate, see #546 and #547). This causes a retry, which downloads the file a second time, applies the modifier callback again, and then (if the file's contents have changed) re-publishes the result. In this case, the second application of the modifier callback failed, because in fact the first publish succeeded: it placed 10 shares of the new (post-delete) version of the file. The UCWE was only triggered because of a lurking 11th share, which is not enough to affect the results of a download_best_version(). So when the Deleter was called a second time, the object it sought to delete was already gone. However, Deleter comes with a must_exist=True default argument, which means that the Deleter will throw an exception if the object to be deleted was not already present. This exception does *not* cause a retry (only UCWE does that), so it gets returned to the caller (in this case, a webapi DELETE operation returned a "404 No Such Object" error, even though the object was present before the DELETE and correctly missing afterwards). This must_exist=True test conflicts with the try-until-success design of `modify()`: the first attempt can apply this requirement, but subsequent retries must not, else they will appear to fail even though they have actually succeeded. The fix is to either remove this default (making it always must_exist=False), or change the `modify()` logic to pass an extra `is_retry=bool` argument to the modifier callback, to allow it to change its behavior on non-initial calls. For Deleter, the logic would be to not assert must_exist=True if is_retry=True.

warner added the

labels 2008-12-05 07:27:18 +00:00

warner added this to the undecided milestone 2008-12-05 07:27:18 +00:00

warner commented

2008-12-06 04:11:21 +00:00

changeset:fb9af2c7a00d7bec and changeset:7a0afb59a4aff9a2 fix this, with an extra 'first_time' argument to the modifier callback. I left Adder alone, and only changed Deleter.