The documentation of the "slot_testv_and_readv_and_writev" method in interfaces.py requires clarification.

[zancas]

There are several ambiguities, or errors, in the old version [ or my understanding thereof ;-) ].

First: It wasn't clear that the read operation occurs regardless of the outcome of the test operation.

Second: It ambiguously states that the write vectors are applied to "those shares" (in fact [iiuc] the write vector shares may be overlapping, or even disjoint, from the existing shares).

It _is_ correct that the shares which may be written will only be written in the case that a call to:

SOMESHARE.check_testv(testv)  

returns "True" for all shares in the test_and_write_vectors, but the SOMESHARE will be EmptyShare if the share to be written is not already on the server.

Here's how I changed it in a local git repo:

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       modified:   interfaces.py
#
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#       modified:   mutable/layout.py
#
diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index a73ce86..b15863b 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -159,10 +159,13 @@ class RIStorageServer(RemoteInterface):
                                         tw_vectors=TestAndWriteVectorsForShares,
                                         r_vector=ReadVector,
                                         ):
-        """General-purpose test-and-set operation for mutable slots. Perform
-        a bunch of comparisons against the existing shares. If they all pass,
-        then apply a bunch of write vectors to those shares. Then use the
-        read vectors to extract data from all the shares and return the data.
+        """
+        General-purpose atomic test-read-and-set operation for mutable slots.
+        (1) For submitted shnums compare against extant-and-empty shares. 
+        (2) Use the read vectors to extract "old data" from all extant shares.
+        (3) If all tests in (1) passed, then write tw_vectors shares. 
+        (4) Return "whether-the-tests-passed" and the "old data", which does not 
+        include any modifications made by the writes.

         This method is, um, large. The goal is to allow clients to update all
         the shares associated with a mutable file in a single round trip.
@@ -232,9 +235,11 @@ class RIStorageServer(RemoteInterface):
         than the size of the data after applying all write vectors.

         The read vector is used to extract data from all known shares,
-        *before* any writes have been applied. The same vector is used for
-        all shares. This captures the state that was tested by the test
-        vector.
+        *before* any writes have been applied. The same read vector is used 
+        for all shares. This captures the state that was tested by the test
+        vector, for extant shares. Of course, the extracted data contains no
+        information about shares written to new shares that did not previously
+        exist.

         This method returns two values: a boolean and a dict. The boolean is
         True if the write vectors were applied, False if not. The dict is

[davidsarah]

+1, but I think the sentence "Of course, the extracted data contains no information about shares written to new shares that did not previously exist." is redundant, since it's implied by "old data" and "extant shares".

Please also apply the following diff:

@@ -204,7 +204,9 @@
 
         The write vector will be applied to the given share, expanding it if
         necessary. A write vector applied to a share number that did not
-        exist previously will cause that share to be created.
+        exist previously will cause that share to be created. Write vectors
+        must not overlap (if they do, this will either cause an error or
+        apply them in an unspecified order).
 
         In Tahoe-LAFS v1.8.3 or later (except 1.9.0a1), if you send a write
         vector whose offset is beyond the end of the current data, the space

[david-sarah@…]

In daa24bce8b61e25b:

(The changeset message doesn't reference this ticket)

[david-sarah@…]

In 5509/1.9.2:

Clarify documentation of RIStorageServer.slot_testv_and_readv_and_writev. fixes #1744

[david-sarah <david-sarah@…>]

In daa24bce8b61e25b:

Clarify documentation of RIStorageServer.slot_testv_and_readv_and_writev. fixes #1744

[david-sarah@…]

In 5858/cloud-backend:

Clarify documentation of RIStorageServer.slot_testv_and_readv_and_writev. fixes #1744