Skip to content

Commit

Permalink
Merge pull request #1127 from tahoe-lafs/3799-simplify-IStorageServer
Browse files Browse the repository at this point in the history 
Rip out unnecessary operator logic, and simplify IStorageServer

Fixes ticket:3799
  • Loading branch information
@itamarst
itamarst committed Sep 27, 2021
2 parents 310a32b + c740da4 commit eb5b6c5
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 180 deletions.
13 changes: 8 additions & 5 deletions docs/proposed/http-storage-node-protocol.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,6 @@ For example::
"test": [{
"offset": 3,
"size": 5,
"operator": "eq",
"specimen": "hello"
}, ...],
"write": [{
Expand All @@ -626,6 +625,9 @@ For example::
}
}

A test vector or read vector that read beyond the boundaries of existing data will return nothing for any bytes past the end.
As a result, if there is no data at all, an empty bytestring is returned no matter what the offset or length.

Reading
~~~~~~~

Expand Down Expand Up @@ -701,7 +703,10 @@ Immutable Data
Mutable Data
~~~~~~~~~~~~

1. Create mutable share number ``3`` with ``10`` bytes of data in slot ``BBBBBBBBBBBBBBBB``::
1. Create mutable share number ``3`` with ``10`` bytes of data in slot ``BBBBBBBBBBBBBBBB``.
The special test vector of size 1 but empty bytes will only pass
if there is no existing share,
otherwise it will read a byte which won't match `b""`::

POST /v1/mutable/BBBBBBBBBBBBBBBB/read-test-write
{
Expand All @@ -715,7 +720,6 @@ Mutable Data
"test": [{
"offset": 0,
"size": 1,
"operator": "eq",
"specimen": ""
}],
"write": [{
Expand Down Expand Up @@ -747,8 +751,7 @@ Mutable Data
3: {
"test": [{
"offset": 0,
"size": <checkstring size>,
"operator": "eq",
"size": <length of checkstring>,
"specimen": "<checkstring>"
}],
"write": [{
Expand Down
Empty file added newsfragments/3799.minor
View file
Empty file.
12 changes: 9 additions & 3 deletions src/allmydata/interfaces.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ def advise_corrupt_share(reason=bytes):

TestVector = ListOf(TupleOf(Offset, ReadSize, bytes, bytes))
# elements are (offset, length, operator, specimen)
# operator is one of "lt, le, eq, ne, ge, gt"
# nop always passes and is used to fetch data while writing.
# you should use length==len(specimen) for everything except nop
# operator must be b"eq", typically length==len(specimen), but one can ensure
# writes don't happen to empty shares by setting length to 1 and specimen to
# b"". The operator is still used for wire compatibility with old versions.
DataVector = ListOf(TupleOf(Offset, ShareData))
# (offset, data). This limits us to 30 writes of 1MiB each per call
TestAndWriteVectorsForShares = DictOf(int,
Expand Down Expand Up @@ -351,6 +351,12 @@ def slot_testv_and_readv_and_writev(
):
"""
:see: ``RIStorageServer.slot_testv_readv_and_writev``
While the interface mostly matches, test vectors are simplified.
Instead of a tuple ``(offset, read_size, operator, expected_data)`` in
the original, for this method you need only pass in
``(offset, read_size, expected_data)``, with the operator implicitly
being ``b"eq"``.
"""

def advise_corrupt_share(
Expand Down
16 changes: 9 additions & 7 deletions src/allmydata/mutable/layout.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ def set_checkstring(self, checkstring_or_seqnum,
salt)
else:
checkstring = checkstring_or_seqnum
self._testvs = [(0, len(checkstring), b"eq", checkstring)]
self._testvs = [(0, len(checkstring), checkstring)]


def get_checkstring(self):
Expand All @@ -318,7 +318,7 @@ def get_checkstring(self):
server.
"""
if self._testvs:
return self._testvs[0][3]
return self._testvs[0][2]
return b""


Expand Down Expand Up @@ -548,9 +548,9 @@ def finish_publishing(self):
if not self._testvs:
# Our caller has not provided us with another checkstring
# yet, so we assume that we are writing a new share, and set
# a test vector that will allow a new share to be written.
# a test vector that will only allow a new share to be written.
self._testvs = []
self._testvs.append(tuple([0, 1, b"eq", b""]))
self._testvs.append(tuple([0, 1, b""]))

tw_vectors = {}
tw_vectors[self.shnum] = (self._testvs, datavs, None)
Expand Down Expand Up @@ -889,7 +889,7 @@ def set_checkstring(self,
self._testvs = []
else:
self._testvs = []
self._testvs.append((0, len(checkstring), b"eq", checkstring))
self._testvs.append((0, len(checkstring), checkstring))


def __repr__(self):
Expand Down Expand Up @@ -1161,16 +1161,18 @@ def _write(self, datavs, on_failure=None, on_success=None):
"""I write the data vectors in datavs to the remote slot."""
tw_vectors = {}
if not self._testvs:
# Make sure we will only successfully write if the share didn't
# previously exist.
self._testvs = []
self._testvs.append(tuple([0, 1, b"eq", b""]))
self._testvs.append(tuple([0, 1, b""]))
if not self._written:
# Write a new checkstring to the share when we write it, so
# that we have something to check later.
new_checkstring = self.get_checkstring()
datavs.append((0, new_checkstring))
def _first_write():
self._written = True
self._testvs = [(0, len(new_checkstring), b"eq", new_checkstring)]
self._testvs = [(0, len(new_checkstring), new_checkstring)]
on_success = _first_write
tw_vectors[self.shnum] = (self._testvs, datavs, None)
d = self._storage_server.slot_testv_and_readv_and_writev(
Expand Down
17 changes: 3 additions & 14 deletions src/allmydata/storage/mutable.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,20 +434,9 @@ def writev(self, datav, new_length):
# self._change_container_size() here.

def testv_compare(a, op, b):
assert op in (b"lt", b"le", b"eq", b"ne", b"ge", b"gt")
if op == b"lt":
return a < b
if op == b"le":
return a <= b
if op == b"eq":
return a == b
if op == b"ne":
return a != b
if op == b"ge":
return a >= b
if op == b"gt":
return a > b
# never reached
assert op == b"eq"
return a == b


class EmptyShare(object):

Expand Down
10 changes: 9 additions & 1 deletion src/allmydata/storage_client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -994,11 +994,19 @@ def slot_testv_and_readv_and_writev(
tw_vectors,
r_vector,
):
# Match the wire protocol, which requires 4-tuples for test vectors.
wire_format_tw_vectors = {
key: (
[(start, length, b"eq", data) for (start, length, data) in value[0]],
value[1],
value[2],
) for (key, value) in tw_vectors.items()
}
return self._rref.callRemote(
"slot_testv_and_readv_and_writev",
storage_index,
secrets,
tw_vectors,
wire_format_tw_vectors,
r_vector,
)

Expand Down
2 changes: 1 addition & 1 deletion src/allmydata/test/mutable/util.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def slot_testv_and_readv_and_writev(self, storage_index, secrets,
readv = {}
for shnum, (testv, writev, new_length) in list(tw_vectors.items()):
for (offset, length, op, specimen) in testv:
assert op in (b"le", b"eq", b"ge")
assert op == b"eq"
# TODO: this isn't right, the read is controlled by read_vector,
# not by testv
readv[shnum] = [ specimen
Expand Down
149 changes: 0 additions & 149 deletions src/allmydata/test/test_storage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1074,23 +1074,6 @@ def test_allocate(self):
}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [data]})

# as should this one
answer = write(b"si1", secrets,
{0: ([(10, 5, b"lt", b"11111"),
],
[(0, b"x"*100)],
None),
},
[(10,5)],
)
self.failUnlessEqual(answer, (False,
{0: [b"11111"],
1: [b""],
2: [b""]},
))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [data]})


def test_operators(self):
# test operators, the data we're comparing is '11111' in all cases.
# test both fail+pass, reset data after each one.
Expand All @@ -1110,63 +1093,6 @@ def reset():

reset()

# lt
answer = write(b"si1", secrets, {0: ([(10, 5, b"lt", b"11110"),
],
[(0, b"x"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (False, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [data]})
self.failUnlessEqual(read(b"si1", [], [(0,100)]), {0: [data]})
reset()

answer = write(b"si1", secrets, {0: ([(10, 5, b"lt", b"11111"),
],
[(0, b"x"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (False, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [data]})
reset()

answer = write(b"si1", secrets, {0: ([(10, 5, b"lt", b"11112"),
],
[(0, b"y"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (True, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [b"y"*100]})
reset()

# le
answer = write(b"si1", secrets, {0: ([(10, 5, b"le", b"11110"),
],
[(0, b"x"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (False, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [data]})
reset()

answer = write(b"si1", secrets, {0: ([(10, 5, b"le", b"11111"),
],
[(0, b"y"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (True, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [b"y"*100]})
reset()

answer = write(b"si1", secrets, {0: ([(10, 5, b"le", b"11112"),
],
[(0, b"y"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (True, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [b"y"*100]})
reset()

# eq
answer = write(b"si1", secrets, {0: ([(10, 5, b"eq", b"11112"),
],
Expand All @@ -1186,81 +1112,6 @@ def reset():
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [b"y"*100]})
reset()

# ne
answer = write(b"si1", secrets, {0: ([(10, 5, b"ne", b"11111"),
],
[(0, b"x"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (False, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [data]})
reset()

answer = write(b"si1", secrets, {0: ([(10, 5, b"ne", b"11112"),
],
[(0, b"y"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (True, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [b"y"*100]})
reset()

# ge
answer = write(b"si1", secrets, {0: ([(10, 5, b"ge", b"11110"),
],
[(0, b"y"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (True, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [b"y"*100]})
reset()

answer = write(b"si1", secrets, {0: ([(10, 5, b"ge", b"11111"),
],
[(0, b"y"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (True, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [b"y"*100]})
reset()

answer = write(b"si1", secrets, {0: ([(10, 5, b"ge", b"11112"),
],
[(0, b"y"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (False, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [data]})
reset()

# gt
answer = write(b"si1", secrets, {0: ([(10, 5, b"gt", b"11110"),
],
[(0, b"y"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (True, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [b"y"*100]})
reset()

answer = write(b"si1", secrets, {0: ([(10, 5, b"gt", b"11111"),
],
[(0, b"x"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (False, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [data]})
reset()

answer = write(b"si1", secrets, {0: ([(10, 5, b"gt", b"11112"),
],
[(0, b"x"*100)],
None,
)}, [(10,5)])
self.failUnlessEqual(answer, (False, {0: [b"11111"]}))
self.failUnlessEqual(read(b"si1", [0], [(0,100)]), {0: [data]})
reset()

# finally, test some operators against empty shares
answer = write(b"si1", secrets, {1: ([(10, 5, b"eq", b"11112"),
],
Expand Down

0 comments on commit eb5b6c5

Please sign in to comment.