[tahoe-lafs-trac-stream] [tahoe-lafs] #1744: The documentation of the "slot_testv_and_readv_and_writev" method in interfaces.py requires clarification.

[tahoe-lafs]

#1744: The documentation of the "slot_testv_and_readv_and_writev" method in
interfaces.py requires clarification.
-------------------------------+----------------------------
     Reporter:  zancas         |      Owner:
         Type:  defect         |     Status:  new
     Priority:  normal         |  Milestone:  undecided
    Component:  documentation  |    Version:  1.9.1
   Resolution:                 |   Keywords:  usability docs
Launchpad Bug:                 |
-------------------------------+----------------------------
Description changed by zancas:

Old description:

> 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 an
> 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) Perform comparisons tests against the extant 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
> }}}

New description:

 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) Perform comparisons tests against the extant 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
 }}}

--

-- 
Ticket URL: <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1744#comment:2>
tahoe-lafs <https://tahoe-lafs.org>
secure decentralized storage