From 52cb25070142f6648c759996cdafe992ae37d94a Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 27 Aug 2021 16:42:23 +0000
Subject: [PATCH 01/95] This is the handler we need to create.

---
 src/allmydata/web/status.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py
index 158d897f9..8c78b1156 100644
--- a/src/allmydata/web/status.py
+++ b/src/allmydata/web/status.py
@@ -1551,6 +1551,21 @@ class Statistics(MultiFormatResource):
         req.setHeader("content-type", "text/plain")
         return json.dumps(stats, indent=1) + "\n"
 
+    @render_exception
+    def render_OPENMETRICS(self, req):
+        req.setHeader("content-type", "application/openmetrics-text")
+        return "3. occurence. This should be the one.\n"
+
+    # @render_exception
+    # def render_OPENMETRICS(self, req):
+    #     req.setHeader("content-type", "text/plain")
+    #     if self._helper:
+    #         stats = self._helper.get_stats()
+    #         import pprint
+    #         return pprint.PrettyPrinter().pprint(stats) + "\n"
+    #     return "uh oh\n"
+
+
 class StatisticsElement(Element):
 
     loader = XMLFile(FilePath(__file__).sibling("statistics.xhtml"))

From 4000116c244aaa6dda0c4cb87f7fc9bf7d332cb8 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 3 Sep 2021 13:42:09 +0000
Subject: [PATCH 02/95] Newsfragment for OpenMetrics endpoint

---
 newsfragments/3786.feature | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 newsfragments/3786.feature

diff --git a/newsfragments/3786.feature b/newsfragments/3786.feature
new file mode 100644
index 000000000..82ce3f974
--- /dev/null
+++ b/newsfragments/3786.feature
@@ -0,0 +1 @@
+tahoe-lafs now provides its statistics also in OpenMetrics format (for Prometheus et. al.) on `/statistics?t=openmetrics`.

From 8a64f50b79cefb8d76ce6111b51290c255099caf Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 3 Sep 2021 14:40:34 +0000
Subject: [PATCH 03/95] WIP - Could be wronger

---
 src/allmydata/web/status.py | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py
index 8c78b1156..741f93061 100644
--- a/src/allmydata/web/status.py
+++ b/src/allmydata/web/status.py
@@ -14,6 +14,7 @@ from past.builtins import long
 
 import itertools
 import hashlib
+import re
 from twisted.internet import defer
 from twisted.python.filepath import FilePath
 from twisted.web.resource import Resource
@@ -1553,18 +1554,22 @@ class Statistics(MultiFormatResource):
 
     @render_exception
     def render_OPENMETRICS(self, req):
-        req.setHeader("content-type", "application/openmetrics-text")
-        return "3. occurence. This should be the one.\n"
+        def mangle_name(name):
+            return re.sub(
+                "_(\d\d)_(\d)_percentile",
+                '{quantile="0.\g<1>\g<2>"}',
+                name.replace(".", "_")
+            )
 
-    # @render_exception
-    # def render_OPENMETRICS(self, req):
-    #     req.setHeader("content-type", "text/plain")
-    #     if self._helper:
-    #         stats = self._helper.get_stats()
-    #         import pprint
-    #         return pprint.PrettyPrinter().pprint(stats) + "\n"
-    #     return "uh oh\n"
+        req.setHeader(
+            "content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8"
+        )
 
+        stats = self._provider.get_stats()
+        return (str({mangle_name(k): v for k, v in stats['counters'].items()})
+                + str({mangle_name(k): v for k, v in stats['stats'].items()})
+                + "\n"
+        )
 
 class StatisticsElement(Element):
 

From 2dbb9434b0d10b086e6d5367087aa1a4cdd77be0 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 8 Sep 2021 14:54:57 +0000
Subject: [PATCH 04/95] OpenMetrics endpoint WIP

---
 src/allmydata/web/status.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py
index 741f93061..7d3fdd06e 100644
--- a/src/allmydata/web/status.py
+++ b/src/allmydata/web/status.py
@@ -1554,6 +1554,10 @@ class Statistics(MultiFormatResource):
 
     @render_exception
     def render_OPENMETRICS(self, req):
+        req.setHeader("content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8")
+        stats = self._provider.get_stats()
+        ret = u""
+
         def mangle_name(name):
             return re.sub(
                 "_(\d\d)_(\d)_percentile",
@@ -1561,15 +1565,13 @@ class Statistics(MultiFormatResource):
                 name.replace(".", "_")
             )
 
-        req.setHeader(
-            "content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8"
-        )
+        for (k, v) in sorted(stats['counters'].items()):
+            ret += u"tahoe_counters_%s %s\n" % (mangle_name(k), v)
 
-        stats = self._provider.get_stats()
-        return (str({mangle_name(k): v for k, v in stats['counters'].items()})
-                + str({mangle_name(k): v for k, v in stats['stats'].items()})
-                + "\n"
-        )
+        for (k, v) in sorted(stats['stats'].items()):
+            ret += u"tahoe_stats_%s %s\n" % (mangle_name(k), v)
+
+        return ret
 
 class StatisticsElement(Element):
 

From ca865e60db6d94bba6abb749a2db196a77b25d36 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 8 Sep 2021 15:08:25 +0000
Subject: [PATCH 05/95] OpenMetrics endpoint

---
 src/allmydata/web/status.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py
index 7d3fdd06e..4be935a54 100644
--- a/src/allmydata/web/status.py
+++ b/src/allmydata/web/status.py
@@ -1565,12 +1565,13 @@ class Statistics(MultiFormatResource):
                 name.replace(".", "_")
             )
 
+        def mangle_value(val):
+            return str(val) if val is not None else "NaN"
+
         for (k, v) in sorted(stats['counters'].items()):
-            ret += u"tahoe_counters_%s %s\n" % (mangle_name(k), v)
-
+            ret += u"tahoe_counters_%s %s\n" % (mangle_name(k), mangle_value(v))
         for (k, v) in sorted(stats['stats'].items()):
-            ret += u"tahoe_stats_%s %s\n" % (mangle_name(k), v)
-
+            ret += u"tahoe_stats_%s %s\n" % (mangle_name(k), mangle_value(v))
         return ret
 
 class StatisticsElement(Element):

From 4674bccde799be427c20dcfa8cc133126b620b99 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Thu, 9 Sep 2021 13:54:03 +0000
Subject: [PATCH 06/95] OpenMetrics: add trailing EOF marker

---
 src/allmydata/web/status.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py
index 4be935a54..3e0e1a3ee 100644
--- a/src/allmydata/web/status.py
+++ b/src/allmydata/web/status.py
@@ -1572,6 +1572,9 @@ class Statistics(MultiFormatResource):
             ret += u"tahoe_counters_%s %s\n" % (mangle_name(k), mangle_value(v))
         for (k, v) in sorted(stats['stats'].items()):
             ret += u"tahoe_stats_%s %s\n" % (mangle_name(k), mangle_value(v))
+
+        ret += u"# EOF\n"
+
         return ret
 
 class StatisticsElement(Element):

From d05e373d4236f7e5a106120da11882b64abd7ded Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Thu, 9 Sep 2021 13:57:59 +0000
Subject: [PATCH 07/95] OpenMetrics: All strings are unicode.

---
 src/allmydata/web/status.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py
index 3e0e1a3ee..6d36861ad 100644
--- a/src/allmydata/web/status.py
+++ b/src/allmydata/web/status.py
@@ -1560,13 +1560,13 @@ class Statistics(MultiFormatResource):
 
         def mangle_name(name):
             return re.sub(
-                "_(\d\d)_(\d)_percentile",
-                '{quantile="0.\g<1>\g<2>"}',
-                name.replace(".", "_")
+                u"_(\d\d)_(\d)_percentile",
+                u'{quantile="0.\g<1>\g<2>"}',
+                name.replace(u".", u"_")
             )
 
         def mangle_value(val):
-            return str(val) if val is not None else "NaN"
+            return str(val) if val is not None else u"NaN"
 
         for (k, v) in sorted(stats['counters'].items()):
             ret += u"tahoe_counters_%s %s\n" % (mangle_name(k), mangle_value(v))

From 74965b271c026fe35436207a3d358ff9c40b32dc Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Thu, 9 Sep 2021 22:36:30 +0000
Subject: [PATCH 08/95] typo

---
 newsfragments/3786.feature | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/newsfragments/3786.feature b/newsfragments/3786.feature
index 82ce3f974..ecbfc0372 100644
--- a/newsfragments/3786.feature
+++ b/newsfragments/3786.feature
@@ -1 +1 @@
-tahoe-lafs now provides its statistics also in OpenMetrics format (for Prometheus et. al.) on `/statistics?t=openmetrics`.
+tahoe-lafs now provides its statistics also in OpenMetrics format (for Prometheus et. al.) at `/statistics?t=openmetrics`.

From 30771149fc7467fd7a61076b1572bcfddaa9a218 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Thu, 9 Sep 2021 23:31:39 +0000
Subject: [PATCH 09/95] Openmetrics: Add test case scaffold

---
 src/allmydata/test/test_openmetrics.py | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 src/allmydata/test/test_openmetrics.py

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
new file mode 100644
index 000000000..ad11f3468
--- /dev/null
+++ b/src/allmydata/test/test_openmetrics.py
@@ -0,0 +1,11 @@
+from twisted.trial import unittest
+
+class FakeStatsProvider(object):
+    def get_stats(self):
+        stats = {'stats': {}, 'counters': {}}
+        return stats
+
+class OpenMetrics(unittest.TestCase):
+    def test_spec_compliance(self):
+        self.assertEqual('1', '2')
+

From fca1482b35b2360d07ec700c0a22d20162bc5acb Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 10 Sep 2021 00:10:11 +0000
Subject: [PATCH 10/95] OpenMetrics Tests WIP

---
 src/allmydata/test/test_openmetrics.py | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index ad11f3468..2211c3561 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -1,4 +1,6 @@
+import mock
 from twisted.trial import unittest
+from allmydata.web.status import Statistics
 
 class FakeStatsProvider(object):
     def get_stats(self):
@@ -6,6 +8,18 @@ class FakeStatsProvider(object):
         return stats
 
 class OpenMetrics(unittest.TestCase):
-    def test_spec_compliance(self):
-        self.assertEqual('1', '2')
+    def test_header(self):
+        req = mock.Mock()
+        stats = mock.Mock()
+        stats._provider = FakeStatsProvider()
+        metrics = Statistics.render_OPENMETRICS(stats, req)
+        req.setHeader.assert_called_with("content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8")
+
+    def test_spec_compliance(self):
+        req = mock.Mock()
+        stats = mock.Mock()
+        stats._provider = FakeStatsProvider()
+        metrics = Statistics.render_OPENMETRICS(stats, req)
+        # TODO test that output adheres to spec
+        # TODO add more realistic stats, incl. missing (None) values
 

From d04157d18a0fbe9a1832f94bf352b04e931039e1 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 10 Sep 2021 13:00:15 +0000
Subject: [PATCH 11/95] OpenMetrics test: Add parser to check against spec

---
 src/allmydata/test/test_openmetrics.py | 18 +++++++++++-------
 tox.ini                                |  2 ++
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 2211c3561..cd5c5d407 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -1,4 +1,5 @@
 import mock
+from prometheus_client.openmetrics import parser
 from twisted.trial import unittest
 from allmydata.web.status import Statistics
 
@@ -8,18 +9,21 @@ class FakeStatsProvider(object):
         return stats
 
 class OpenMetrics(unittest.TestCase):
-    def test_header(self):
+    def test_spec_compliance(self):
+        """
+        Does our output adhere to the OpenMetrics spec?
+        https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md
+        """
         req = mock.Mock()
         stats = mock.Mock()
         stats._provider = FakeStatsProvider()
         metrics = Statistics.render_OPENMETRICS(stats, req)
+
+        # "The content type MUST be..."
         req.setHeader.assert_called_with("content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8")
 
-    def test_spec_compliance(self):
-        req = mock.Mock()
-        stats = mock.Mock()
-        stats._provider = FakeStatsProvider()
-        metrics = Statistics.render_OPENMETRICS(stats, req)
-        # TODO test that output adheres to spec
+        # The parser throws if it can't parse.
+        # Wrap in a list() to drain the generator.
+        families = list(parser.text_string_to_metric_families(metrics))
         # TODO add more realistic stats, incl. missing (None) values
 
diff --git a/tox.ini b/tox.ini
index 9b0f71038..0e8e58ea6 100644
--- a/tox.ini
+++ b/tox.ini
@@ -52,6 +52,8 @@ deps =
      certifi
      # VCS hooks support
      py36,!coverage: pre-commit
+     # Does our OpenMetrics endpoint adhere to the spec:
+     prometheus-client==0.11.0
 
 # We add usedevelop=False because testing against a true installation gives
 # more useful results.

From 6c18983f7b182fe696dd466dab517312692c1648 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 10 Sep 2021 13:13:13 +0000
Subject: [PATCH 12/95] OpenMetrics test: Use realistic input data

---
 src/allmydata/test/test_openmetrics.py | 92 +++++++++++++++++++++++++-
 1 file changed, 89 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index cd5c5d407..c7e26213a 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -5,7 +5,95 @@ from allmydata.web.status import Statistics
 
 class FakeStatsProvider(object):
     def get_stats(self):
-        stats = {'stats': {}, 'counters': {}}
+        # Parsed into a dict from a running tahoe's /statistics?t=json
+        stats = {'stats': {
+           'storage_server.latencies.get.99_9_percentile': None,
+           'storage_server.latencies.close.10_0_percentile': 0.00021910667419433594,
+           'storage_server.latencies.read.01_0_percentile': 2.8848648071289062e-05,
+           'storage_server.latencies.writev.99_9_percentile': None,
+           'storage_server.latencies.read.99_9_percentile': None,
+           'storage_server.latencies.allocate.99_0_percentile': 0.000988006591796875,
+           'storage_server.latencies.writev.mean': 0.00045332245070571654,
+           'storage_server.latencies.close.99_9_percentile': None,
+           'cpu_monitor.15min_avg': 0.00017592000079223033,
+           'storage_server.disk_free_for_root': 103289454592,
+           'storage_server.latencies.get.99_0_percentile': 0.000347137451171875,
+           'storage_server.latencies.get.mean': 0.00021158285060171353,
+           'storage_server.latencies.read.90_0_percentile': 8.893013000488281e-05,
+           'storage_server.latencies.write.01_0_percentile': 3.600120544433594e-05,
+           'storage_server.latencies.write.99_9_percentile': 0.00017690658569335938,
+           'storage_server.latencies.close.90_0_percentile': 0.00033211708068847656,
+           'storage_server.disk_total': 103497859072,
+           'storage_server.latencies.close.95_0_percentile': 0.0003509521484375,
+           'storage_server.latencies.readv.samplesize': 1000,
+           'storage_server.disk_free_for_nonroot': 103289454592,
+           'storage_server.latencies.close.mean': 0.0002715024480059103,
+           'storage_server.latencies.writev.95_0_percentile': 0.0007410049438476562,
+           'storage_server.latencies.readv.90_0_percentile': 0.0003781318664550781,
+           'storage_server.latencies.readv.99_0_percentile': 0.0004050731658935547,
+           'storage_server.latencies.allocate.mean': 0.0007128627429454784,
+           'storage_server.latencies.close.samplesize': 326,
+           'storage_server.latencies.get.50_0_percentile': 0.0001819133758544922,
+           'storage_server.latencies.write.50_0_percentile': 4.482269287109375e-05,
+           'storage_server.latencies.readv.01_0_percentile': 0.0002970695495605469,
+           'storage_server.latencies.get.10_0_percentile': 0.00015687942504882812,
+           'storage_server.latencies.allocate.90_0_percentile': 0.0008189678192138672,
+           'storage_server.latencies.get.samplesize': 472,
+           'storage_server.total_bucket_count': 393,
+           'storage_server.latencies.read.mean': 5.936201880959903e-05,
+           'storage_server.latencies.allocate.01_0_percentile': 0.0004208087921142578,
+           'storage_server.latencies.allocate.99_9_percentile': None,
+           'storage_server.latencies.readv.mean': 0.00034061360359191893,
+           'storage_server.disk_used': 208404480,
+           'storage_server.latencies.allocate.50_0_percentile': 0.0007410049438476562,
+           'storage_server.latencies.read.99_0_percentile': 0.00011992454528808594,
+           'node.uptime': 3805759.8545179367,
+           'storage_server.latencies.writev.10_0_percentile': 0.00035190582275390625,
+           'storage_server.latencies.writev.90_0_percentile': 0.0006821155548095703,
+           'storage_server.latencies.close.01_0_percentile': 0.00021505355834960938,
+           'storage_server.latencies.close.50_0_percentile': 0.0002579689025878906,
+           'cpu_monitor.1min_avg': 0.0002130000000003444,
+           'storage_server.latencies.writev.50_0_percentile': 0.0004138946533203125,
+           'storage_server.latencies.read.95_0_percentile': 9.107589721679688e-05,
+           'storage_server.latencies.readv.95_0_percentile': 0.0003859996795654297,
+           'storage_server.latencies.write.10_0_percentile': 3.719329833984375e-05,
+           'storage_server.accepting_immutable_shares': 1,
+           'storage_server.latencies.writev.samplesize': 309,
+           'storage_server.latencies.get.95_0_percentile': 0.0003190040588378906,
+           'storage_server.latencies.readv.10_0_percentile': 0.00032210350036621094,
+           'storage_server.latencies.get.90_0_percentile': 0.0002999305725097656,
+           'storage_server.latencies.get.01_0_percentile': 0.0001239776611328125,
+           'cpu_monitor.total': 641.4941180000001,
+           'storage_server.latencies.write.samplesize': 1000,
+           'storage_server.latencies.write.95_0_percentile': 9.489059448242188e-05,
+           'storage_server.latencies.read.50_0_percentile': 6.890296936035156e-05,
+           'storage_server.latencies.writev.01_0_percentile': 0.00033211708068847656,
+           'storage_server.latencies.read.10_0_percentile': 3.0994415283203125e-05,
+           'storage_server.latencies.allocate.10_0_percentile': 0.0004949569702148438,
+           'storage_server.reserved_space': 0,
+           'storage_server.disk_avail': 103289454592,
+           'storage_server.latencies.write.99_0_percentile': 0.00011301040649414062,
+           'storage_server.latencies.write.90_0_percentile': 9.083747863769531e-05,
+           'cpu_monitor.5min_avg': 0.0002370666691157502,
+           'storage_server.latencies.write.mean': 5.8008909225463864e-05,
+           'storage_server.latencies.readv.50_0_percentile': 0.00033020973205566406,
+           'storage_server.latencies.close.99_0_percentile': 0.0004038810729980469,
+           'storage_server.allocated': 0,
+           'storage_server.latencies.writev.99_0_percentile': 0.0007710456848144531,
+           'storage_server.latencies.readv.99_9_percentile': 0.0004780292510986328,
+           'storage_server.latencies.read.samplesize': 170,
+           'storage_server.latencies.allocate.samplesize': 406,
+           'storage_server.latencies.allocate.95_0_percentile': 0.0008411407470703125
+         }, 'counters': {
+           'storage_server.writev': 309,
+           'storage_server.bytes_added': 197836146,
+           'storage_server.close': 326,
+           'storage_server.readv': 14299,
+           'storage_server.allocate': 406,
+           'storage_server.read': 170,
+           'storage_server.write': 3775,
+           'storage_server.get': 472}
+         }
         return stats
 
 class OpenMetrics(unittest.TestCase):
@@ -25,5 +113,3 @@ class OpenMetrics(unittest.TestCase):
         # The parser throws if it can't parse.
         # Wrap in a list() to drain the generator.
         families = list(parser.text_string_to_metric_families(metrics))
-        # TODO add more realistic stats, incl. missing (None) values
-

From 339e1747e7b2636a49c4155f9786e79fdb37de3f Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 10 Sep 2021 13:15:56 +0000
Subject: [PATCH 13/95] clean up

---
 src/allmydata/test/test_openmetrics.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index c7e26213a..9ef7c9c8e 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -108,8 +108,10 @@ class OpenMetrics(unittest.TestCase):
         metrics = Statistics.render_OPENMETRICS(stats, req)
 
         # "The content type MUST be..."
-        req.setHeader.assert_called_with("content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8")
+        req.setHeader.assert_called_with(
+            "content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8"
+        )
 
-        # The parser throws if it can't parse.
-        # Wrap in a list() to drain the generator.
+        # The parser throws if it does not like its input.
+        # Wrapped in a list() to drain the generator.
         families = list(parser.text_string_to_metric_families(metrics))

From ad84f5df2b9310df2fac25c5dc989c4044af491b Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 10 Sep 2021 13:21:06 +0000
Subject: [PATCH 14/95] newline at the end.

---
 src/allmydata/test/test_openmetrics.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 9ef7c9c8e..fdb645b42 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -115,3 +115,4 @@ class OpenMetrics(unittest.TestCase):
         # The parser throws if it does not like its input.
         # Wrapped in a list() to drain the generator.
         families = list(parser.text_string_to_metric_families(metrics))
+

From 570f15284ab77c8314b4181b9ef8de52d54375fa Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Fri, 10 Sep 2021 09:44:49 -0400
Subject: [PATCH 15/95] More tests for IStorageServer.get_buckets().

---
 newsfragments/3795.minor                  |  0
 src/allmydata/test/test_istorageserver.py | 88 +++++++++++++++++++++++
 2 files changed, 88 insertions(+)
 create mode 100644 newsfragments/3795.minor

diff --git a/newsfragments/3795.minor b/newsfragments/3795.minor
new file mode 100644
index 000000000..e69de29bb
diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 75fbe42e2..74d4fb033 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -255,6 +255,94 @@ class IStorageServerImmutableAPIsTestsMixin(object):
             (yield buckets[3].callRemote("read", 0, 1024)), b"5" * 24 + b"6" * 1000
         )
 
+    @inlineCallbacks
+    def test_get_buckets_skips_unfinished_buckets(self):
+        """
+        Buckets that are not fully written are not returned by
+        ``IStorageServer.get_buckets()`` implementations.
+        """
+        si = new_storage_index()
+        (_, allocated) = yield self.storage_server.allocate_buckets(
+            si,
+            new_secret(),
+            new_secret(),
+            set(range(5)),
+            10,
+            Referenceable(),
+        )
+
+        # Bucket 1 is fully written
+        yield allocated[1].callRemote("write", 0, b"1" * 10)
+        yield allocated[1].callRemote("close")
+
+        # Bucket 2 is partially written
+        yield allocated[2].callRemote("write", 0, b"1" * 5)
+
+        buckets = yield self.storage_server.get_buckets(si)
+        self.assertEqual(set(buckets.keys()), {1})
+
+    @inlineCallbacks
+    def test_read_bucket_at_offset(self):
+        """
+        Given a read bucket returned from ``IStorageServer.get_buckets()``, it
+        is possible to read at different offsets and lengths, with reads past
+        the end resulting in empty bytes.
+        """
+        length = 256 * 17
+
+        si = new_storage_index()
+        (_, allocated) = yield self.storage_server.allocate_buckets(
+            si,
+            new_secret(),
+            new_secret(),
+            set(range(1)),
+            length,
+            Referenceable(),
+        )
+
+        total_data = _randbytes(256) * 17
+        yield allocated[0].callRemote("write", 0, total_data)
+        yield allocated[0].callRemote("close")
+
+        buckets = yield self.storage_server.get_buckets(si)
+        bucket = buckets[0]
+        for start, to_read in [
+            (0, 250),  # fraction
+            (0, length),  # whole thing
+            (100, 1024),  # offset fraction
+            (length + 1, 100),  # completely out of bounds
+            (length - 100, 200),  # partially out of bounds
+        ]:
+            data = yield bucket.callRemote("read", start, to_read)
+            self.assertEqual(
+                data,
+                total_data[start : start + to_read],
+                "Didn't match for start {}, length {}".format(start, to_read),
+            )
+
+    @inlineCallbacks
+    def test_bucket_advise_corrupt_share(self):
+        """
+        Calling ``advise_corrupt_share()`` on a bucket returned by
+        ``IStorageServer.get_buckets()`` does not result in error (other
+        behavior is opaque at this level of abstraction).
+        """
+        si = new_storage_index()
+        (_, allocated) = yield self.storage_server.allocate_buckets(
+            si,
+            new_secret(),
+            new_secret(),
+            set(range(1)),
+            10,
+            Referenceable(),
+        )
+
+        yield allocated[0].callRemote("write", 0, b"0123456789")
+        yield allocated[0].callRemote("close")
+
+        buckets = yield self.storage_server.get_buckets(si)
+        yield buckets[0].callRemote("advise_corrupt_share", b"OH NO")
+
 
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""

From bb626890ed1cf7e82a334fce9851dc7161cba36d Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 14 Sep 2021 08:57:32 -0400
Subject: [PATCH 16/95] Match review comment suggestions from previous PR.

---
 src/allmydata/test/test_istorageserver.py | 48 +++++++++++------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 7f2c9b7ff..78de0ae3d 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -268,14 +268,14 @@ class IStorageServerImmutableAPIsTestsMixin(object):
         Buckets that are not fully written are not returned by
         ``IStorageServer.get_buckets()`` implementations.
         """
-        si = new_storage_index()
+        storage_index = new_storage_index()
         (_, allocated) = yield self.storage_server.allocate_buckets(
-            si,
-            new_secret(),
-            new_secret(),
-            set(range(5)),
-            10,
-            Referenceable(),
+            storage_index,
+            renew_secret=new_secret(),
+            cancel_secret=new_secret(),
+            sharenums=set(range(5)),
+            allocated_size=10,
+            canary=Referenceable(),
         )
 
         # Bucket 1 is fully written
@@ -285,7 +285,7 @@ class IStorageServerImmutableAPIsTestsMixin(object):
         # Bucket 2 is partially written
         yield allocated[2].callRemote("write", 0, b"1" * 5)
 
-        buckets = yield self.storage_server.get_buckets(si)
+        buckets = yield self.storage_server.get_buckets(storage_index)
         self.assertEqual(set(buckets.keys()), {1})
 
     @inlineCallbacks
@@ -297,21 +297,21 @@ class IStorageServerImmutableAPIsTestsMixin(object):
         """
         length = 256 * 17
 
-        si = new_storage_index()
+        storage_index = new_storage_index()
         (_, allocated) = yield self.storage_server.allocate_buckets(
-            si,
-            new_secret(),
-            new_secret(),
-            set(range(1)),
-            length,
-            Referenceable(),
+            storage_index,
+            renew_secret=new_secret(),
+            cancel_secret=new_secret(),
+            sharenums=set(range(1)),
+            allocated_size=length,
+            canary=Referenceable(),
         )
 
         total_data = _randbytes(256) * 17
         yield allocated[0].callRemote("write", 0, total_data)
         yield allocated[0].callRemote("close")
 
-        buckets = yield self.storage_server.get_buckets(si)
+        buckets = yield self.storage_server.get_buckets(storage_index)
         bucket = buckets[0]
         for start, to_read in [
             (0, 250),  # fraction
@@ -334,20 +334,20 @@ class IStorageServerImmutableAPIsTestsMixin(object):
         ``IStorageServer.get_buckets()`` does not result in error (other
         behavior is opaque at this level of abstraction).
         """
-        si = new_storage_index()
+        storage_index = new_storage_index()
         (_, allocated) = yield self.storage_server.allocate_buckets(
-            si,
-            new_secret(),
-            new_secret(),
-            set(range(1)),
-            10,
-            Referenceable(),
+            storage_index,
+            renew_secret=new_secret(),
+            cancel_secret=new_secret(),
+            sharenums=set(range(1)),
+            allocated_size=10,
+            canary=Referenceable(),
         )
 
         yield allocated[0].callRemote("write", 0, b"0123456789")
         yield allocated[0].callRemote("close")
 
-        buckets = yield self.storage_server.get_buckets(si)
+        buckets = yield self.storage_server.get_buckets(storage_index)
         yield buckets[0].callRemote("advise_corrupt_share", b"OH NO")
 
 

From 0aee960ea8a4a66d5b6174fdb8f2dccd4cb9954c Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 14 Sep 2021 11:26:19 -0400
Subject: [PATCH 17/95] News file.

---
 newsfragments/3797.minor | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 newsfragments/3797.minor

diff --git a/newsfragments/3797.minor b/newsfragments/3797.minor
new file mode 100644
index 000000000..e69de29bb

From d207c468553a5c76ff431fa725e7543cd1eae66c Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 14 Sep 2021 11:26:23 -0400
Subject: [PATCH 18/95] First mutable test.

---
 src/allmydata/test/test_istorageserver.py | 62 +++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 1c1383a2d..8de2be7a0 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -263,6 +263,62 @@ class IStorageServerImmutableAPIsTestsMixin(object):
         )
 
 
+class IStorageServerMutableAPIsTestsMixin(object):
+    """
+    Tests for ``IStorageServer``'s mutable APIs.
+
+    ``self.storage_server`` is expected to provide ``IStorageServer``.
+
+    ``STARAW`` is short for ``slot_testv_and_readv_and_writev``.
+    """
+
+    # slot_testv_and_readv_and_writev
+    # TODO it's possible to write and then in separate call read
+    # TODO reads happen before (re)writes
+    # TODO write prevented if tests fail
+    # TODO reads beyond the edge
+    # TODO wrong write enabled prevents writes
+    # TODO write prevented if test data against empty share
+    # TODO writes can create additional shares if only some exist
+    # TODO later writes overwrite
+
+    def new_secrets(self):
+        """Return a 3-tuple of secrets for STARAW calls."""
+        return (new_secret(), new_secret(), new_secret())
+
+    def staraw(self, *args, **kwargs):
+        """Like ``slot_testv_and_readv_and_writev``, but less typing."""
+        return self.storage_server.slot_testv_and_readv_and_writev(*args, **kwargs)
+
+    @inlineCallbacks
+    def test_STARAW_reads(self):
+        """
+        When data is written with
+        ``IStorageServer.slot_testv_and_readv_and_writev``, it can then be read
+        by a separate call using that API.
+        """
+        secrets = self.new_secrets()
+        storage_index = new_storage_index()
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([], [(0, b"abcdefg")], 7),
+                1: ([], [(0, b"0123456")], 7),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+        (_, reads) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={},
+            r_vector=[(0, 7)],
+        )
+        self.assertEqual(reads, {0: [b"abcdefg"], 1: [b"0123456"]})
+
+
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""
 
@@ -293,3 +349,9 @@ class FoolscapImmutableAPIsTests(
     _FoolscapMixin, IStorageServerImmutableAPIsTestsMixin, AsyncTestCase
 ):
     """Foolscap-specific tests for immutable ``IStorageServer`` APIs."""
+
+
+class FoolscapMutableAPIsTests(
+    _FoolscapMixin, IStorageServerMutableAPIsTestsMixin, AsyncTestCase
+):
+    """Foolscap-specific tests for immutable ``IStorageServer`` APIs."""

From 5b704ff12d9a22263750f02890eb4b42b53f7c37 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 14 Sep 2021 11:36:12 -0400
Subject: [PATCH 19/95] Another mutable test.

---
 src/allmydata/test/test_istorageserver.py | 46 +++++++++++++++++++++--
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 8de2be7a0..780bb68c4 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -273,10 +273,10 @@ class IStorageServerMutableAPIsTestsMixin(object):
     """
 
     # slot_testv_and_readv_and_writev
-    # TODO it's possible to write and then in separate call read
-    # TODO reads happen before (re)writes
+    # DONE it's possible to write and then in separate call read
+    # DONE reads happen before (re)writes
     # TODO write prevented if tests fail
-    # TODO reads beyond the edge
+    # TODO partial reads, reads beyond the edge
     # TODO wrong write enabled prevents writes
     # TODO write prevented if test data against empty share
     # TODO writes can create additional shares if only some exist
@@ -318,6 +318,46 @@ class IStorageServerMutableAPIsTestsMixin(object):
         )
         self.assertEqual(reads, {0: [b"abcdefg"], 1: [b"0123456"]})
 
+    @inlineCallbacks
+    def test_SATRAW_reads_happen_before_writes_in_single_query(self):
+        """
+        If a ``IStorageServer.slot_testv_and_readv_and_writev`` command
+        contains both reads and writes, the read returns results that precede
+        the write.
+        """
+        secrets = self.new_secrets()
+        storage_index = new_storage_index()
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([], [(0, b"abcdefg")], 7),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+        # Read and write in same command; read happens before write:
+        (written, reads) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([], [(0, b"X" * 7)], 7),
+            },
+            r_vector=[(0, 7)],
+        )
+        self.assertEqual(written, True)
+        self.assertEqual(reads, {0: [b"abcdefg"]})
+
+        # The write is available in next read:
+        (_, reads) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={},
+            r_vector=[(0, 7)],
+        )
+        self.assertEqual(reads, {0: [b"X" * 7]})
+
 
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""

From aa8001edf21dfb9a2a272f404998666894293b83 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 14 Sep 2021 12:30:45 -0400
Subject: [PATCH 20/95] Another test.

---
 src/allmydata/test/test_istorageserver.py | 58 ++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 780bb68c4..ddb855c4f 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -275,7 +275,8 @@ class IStorageServerMutableAPIsTestsMixin(object):
     # slot_testv_and_readv_and_writev
     # DONE it's possible to write and then in separate call read
     # DONE reads happen before (re)writes
-    # TODO write prevented if tests fail
+    # DONE write happens if test succeeds
+    # DONE write prevented if tests fail
     # TODO partial reads, reads beyond the edge
     # TODO wrong write enabled prevents writes
     # TODO write prevented if test data against empty share
@@ -358,6 +359,61 @@ class IStorageServerMutableAPIsTestsMixin(object):
         )
         self.assertEqual(reads, {0: [b"X" * 7]})
 
+    @inlineCallbacks
+    def test_SATRAW_writen_happens_only_if_test_matches(self):
+        """
+        If a ``IStorageServer.slot_testv_and_readv_and_writev`` includes both a
+        test and a write, the write succeeds if the test matches, and fails if
+        the test does not match.
+        """
+        secrets = self.new_secrets()
+        storage_index = new_storage_index()
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([], [(0, b"1" * 7)], 7),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+        # Test matches, so write happens:
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([(0, 7, b"eq", b"1" * 7)], [(0, b"2" * 7)], 7),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+        (_, reads) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={},
+            r_vector=[(0, 7)],
+        )
+        self.assertEqual(reads, {0: [b"2" * 7]})
+
+        # Test does not match, so write does not happen:
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([(0, 7, b"eq", b"1" * 7)], [(0, b"3" * 7)], 7),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, False)
+        (_, reads) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={},
+            r_vector=[(0, 7)],
+        )
+        self.assertEqual(reads, {0: [b"2" * 7]})
+
 
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""

From 7b97ecfb7c27513f89129df0687da9df12e33b9f Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 14 Sep 2021 12:47:03 -0400
Subject: [PATCH 21/95] More tests.

---
 src/allmydata/test/test_istorageserver.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index ddb855c4f..796d6a9c6 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -277,8 +277,10 @@ class IStorageServerMutableAPIsTestsMixin(object):
     # DONE reads happen before (re)writes
     # DONE write happens if test succeeds
     # DONE write prevented if tests fail
-    # TODO partial reads, reads beyond the edge
-    # TODO wrong write enabled prevents writes
+    # TODO multiple test vectors
+    # TODO multiple writes
+    # TODO multiple reads
+    # TODO wrong write enabler secret prevents writes
     # TODO write prevented if test data against empty share
     # TODO writes can create additional shares if only some exist
     # TODO later writes overwrite
@@ -292,7 +294,7 @@ class IStorageServerMutableAPIsTestsMixin(object):
         return self.storage_server.slot_testv_and_readv_and_writev(*args, **kwargs)
 
     @inlineCallbacks
-    def test_STARAW_reads(self):
+    def test_STARAW_reads_after_write(self):
         """
         When data is written with
         ``IStorageServer.slot_testv_and_readv_and_writev``, it can then be read

From 98e566fc44c5fde0b0e408e8ada65593762b33fd Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 14 Sep 2021 12:51:32 -0400
Subject: [PATCH 22/95] Expand testing scope.

---
 src/allmydata/test/test_istorageserver.py | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 796d6a9c6..28b285151 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -277,9 +277,9 @@ class IStorageServerMutableAPIsTestsMixin(object):
     # DONE reads happen before (re)writes
     # DONE write happens if test succeeds
     # DONE write prevented if tests fail
-    # TODO multiple test vectors
-    # TODO multiple writes
-    # TODO multiple reads
+    # DONE multiple test vectors
+    # DONE multiple writes
+    # DONE multiple reads
     # TODO wrong write enabler secret prevents writes
     # TODO write prevented if test data against empty share
     # TODO writes can create additional shares if only some exist
@@ -307,7 +307,7 @@ class IStorageServerMutableAPIsTestsMixin(object):
             secrets,
             tw_vectors={
                 0: ([], [(0, b"abcdefg")], 7),
-                1: ([], [(0, b"0123456")], 7),
+                1: ([], [(0, b"0123"), (4, b"456")], 7),
             },
             r_vector=[],
         )
@@ -317,9 +317,14 @@ class IStorageServerMutableAPIsTestsMixin(object):
             storage_index,
             secrets,
             tw_vectors={},
-            r_vector=[(0, 7)],
+            # Whole thing, partial, going beyond the edge, completely outside
+            # range:
+            r_vector=[(0, 7), (2, 3), (6, 8), (100, 10)],
+        )
+        self.assertEqual(
+            reads,
+            {0: [b"abcdefg", b"cde", b"g", b""], 1: [b"0123456", b"234", b"6", b""]},
         )
-        self.assertEqual(reads, {0: [b"abcdefg"], 1: [b"0123456"]})
 
     @inlineCallbacks
     def test_SATRAW_reads_happen_before_writes_in_single_query(self):
@@ -385,7 +390,11 @@ class IStorageServerMutableAPIsTestsMixin(object):
             storage_index,
             secrets,
             tw_vectors={
-                0: ([(0, 7, b"eq", b"1" * 7)], [(0, b"2" * 7)], 7),
+                0: (
+                    [(0, 3, b"eq", b"1" * 3), (3, 4, b"eq", b"1" * 4)],
+                    [(0, b"2" * 7)],
+                    7,
+                ),
             },
             r_vector=[],
         )

From 241f4c841b33884adf301e2a7440d3948b990112 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 14 Sep 2021 13:00:29 -0400
Subject: [PATCH 23/95] Another test.

---
 src/allmydata/test/test_istorageserver.py | 42 +++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 28b285151..c299841f9 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -24,7 +24,7 @@ from testtools import skipIf
 
 from twisted.internet.defer import inlineCallbacks
 
-from foolscap.api import Referenceable
+from foolscap.api import Referenceable, RemoteException
 
 from allmydata.interfaces import IStorageServer
 from .common_system import SystemTestMixin
@@ -280,7 +280,7 @@ class IStorageServerMutableAPIsTestsMixin(object):
     # DONE multiple test vectors
     # DONE multiple writes
     # DONE multiple reads
-    # TODO wrong write enabler secret prevents writes
+    # DONE wrong write enabler secret prevents writes
     # TODO write prevented if test data against empty share
     # TODO writes can create additional shares if only some exist
     # TODO later writes overwrite
@@ -425,6 +425,44 @@ class IStorageServerMutableAPIsTestsMixin(object):
         )
         self.assertEqual(reads, {0: [b"2" * 7]})
 
+    @inlineCallbacks
+    def test_STARAW_write_enabler_must_match(self):
+        """
+        If the write enabler secret passed to
+        ``IStorageServer.slot_testv_and_readv_and_writev`` doesn't match
+        previous writes, the write fails.
+        """
+        secrets = self.new_secrets()
+        storage_index = new_storage_index()
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([], [(0, b"1" * 7)], 7),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+        # Write enabler secret does not match, so write does not happen:
+        bad_secrets = (new_secret(),) + secrets[1:]
+        with self.assertRaises(RemoteException):
+            yield self.staraw(
+                storage_index,
+                bad_secrets,
+                tw_vectors={
+                    0: ([], [(0, b"2" * 7)], 7),
+                },
+                r_vector=[],
+            )
+        (_, reads) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={},
+            r_vector=[(0, 7)],
+        )
+        self.assertEqual(reads, {0: [b"1" * 7]})
+
 
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""

From 88a2e7a4fb993fb28a14a6cfe1ac1025a998bb93 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 15 Sep 2021 10:09:55 +0000
Subject: [PATCH 24/95] OpenMetrics test suite: Get rid of status mock

---
 src/allmydata/test/test_openmetrics.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index fdb645b42..57ed989e0 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -96,6 +96,10 @@ class FakeStatsProvider(object):
          }
         return stats
 
+class FakeStats():
+    def __init__(self):
+        self._provider = FakeStatsProvider()
+
 class OpenMetrics(unittest.TestCase):
     def test_spec_compliance(self):
         """
@@ -103,8 +107,7 @@ class OpenMetrics(unittest.TestCase):
         https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md
         """
         req = mock.Mock()
-        stats = mock.Mock()
-        stats._provider = FakeStatsProvider()
+        stats = FakeStats()
         metrics = Statistics.render_OPENMETRICS(stats, req)
 
         # "The content type MUST be..."

From 57a3c1168e98fbce9b6b10d8a20100f4384a1620 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 15 Sep 2021 11:03:31 +0000
Subject: [PATCH 25/95] OpenMetrics: Use list of strings instead of string
 concatenation

---
 src/allmydata/web/status.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py
index 6d36861ad..2542ca75f 100644
--- a/src/allmydata/web/status.py
+++ b/src/allmydata/web/status.py
@@ -1556,7 +1556,7 @@ class Statistics(MultiFormatResource):
     def render_OPENMETRICS(self, req):
         req.setHeader("content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8")
         stats = self._provider.get_stats()
-        ret = u""
+        ret = []
 
         def mangle_name(name):
             return re.sub(
@@ -1569,13 +1569,13 @@ class Statistics(MultiFormatResource):
             return str(val) if val is not None else u"NaN"
 
         for (k, v) in sorted(stats['counters'].items()):
-            ret += u"tahoe_counters_%s %s\n" % (mangle_name(k), mangle_value(v))
+            ret.append(u"tahoe_counters_%s %s" % (mangle_name(k), mangle_value(v)))
         for (k, v) in sorted(stats['stats'].items()):
-            ret += u"tahoe_stats_%s %s\n" % (mangle_name(k), mangle_value(v))
+            ret.append(u"tahoe_stats_%s %s" % (mangle_name(k), mangle_value(v)))
 
-        ret += u"# EOF\n"
+        ret.append(u"# EOF")
 
-        return ret
+        return u"\n".join(ret)
 
 class StatisticsElement(Element):
 

From d864cab5b0861a7c4a4caec294bafac884f318a6 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 15 Sep 2021 11:14:52 +0000
Subject: [PATCH 26/95] OpenMetrics: Add test dep to nix packaging

---
 nix/tahoe-lafs.nix | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nix/tahoe-lafs.nix b/nix/tahoe-lafs.nix
index 35b29f1cc..17b2f4463 100644
--- a/nix/tahoe-lafs.nix
+++ b/nix/tahoe-lafs.nix
@@ -107,6 +107,7 @@ EOF
     beautifulsoup4
     html5lib
     tenacity
+    prometheus_client
   ];
 
   checkPhase = ''

From c66ae302c8412365c0f3848298351a1d69d975d8 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 15 Sep 2021 11:26:30 +0000
Subject: [PATCH 27/95] OpenMetrics: Extra newline at the end

---
 src/allmydata/web/status.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py
index 2542ca75f..7b16a60b2 100644
--- a/src/allmydata/web/status.py
+++ b/src/allmydata/web/status.py
@@ -1573,7 +1573,7 @@ class Statistics(MultiFormatResource):
         for (k, v) in sorted(stats['stats'].items()):
             ret.append(u"tahoe_stats_%s %s" % (mangle_name(k), mangle_value(v)))
 
-        ret.append(u"# EOF")
+        ret.append(u"# EOF\n")
 
         return u"\n".join(ret)
 

From cbe5ea1115e3e77e86d69a4dc2b29e508724e18e Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 15 Sep 2021 11:28:39 +0000
Subject: [PATCH 28/95] OpenMetrics: Add docstring

---
 src/allmydata/web/status.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py
index 7b16a60b2..65647f491 100644
--- a/src/allmydata/web/status.py
+++ b/src/allmydata/web/status.py
@@ -1554,6 +1554,12 @@ class Statistics(MultiFormatResource):
 
     @render_exception
     def render_OPENMETRICS(self, req):
+        """
+        Render our stats in `OpenMetrics <https://openmetrics.io/>` format.
+        For example Prometheus and Victoriametrics can parse this.
+        Point the scraper to ``/statistics?t=openmetrics`` (instead of the
+        default ``/metrics``).
+        """
         req.setHeader("content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8")
         stats = self._provider.get_stats()
         ret = []

From 21c471ed8113ce29fd1b6c6dc02e2acf124d550a Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 15 Sep 2021 11:39:32 +0000
Subject: [PATCH 29/95] OpenMetrics test: Add hopefully more stable URIs to
 OpenMetrics spec info

---
 src/allmydata/test/test_openmetrics.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 57ed989e0..7753960d4 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -103,8 +103,9 @@ class FakeStats():
 class OpenMetrics(unittest.TestCase):
     def test_spec_compliance(self):
         """
-        Does our output adhere to the OpenMetrics spec?
-        https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md
+        Does our output adhere to the `OpenMetrics <https://openmetrics.io/>` spec?
+        https://github.com/OpenObservability/OpenMetrics/
+        https://prometheus.io/docs/instrumenting/exposition_formats/
         """
         req = mock.Mock()
         stats = FakeStats()

From 6bcff5472b9aca6dd09e585e80bb937832d254e7 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 15 Sep 2021 11:50:20 +0000
Subject: [PATCH 30/95] OpenMetrics test suite: Add a check to see whether our
 stats were parsed at all.

---
 src/allmydata/test/test_openmetrics.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 7753960d4..a9a0e5712 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -120,3 +120,7 @@ class OpenMetrics(unittest.TestCase):
         # Wrapped in a list() to drain the generator.
         families = list(parser.text_string_to_metric_families(metrics))
 
+        # Has the parser parsed our data?
+        # Just check the last item.
+        self.assertEqual(families[-1].name, u"tahoe_stats_storage_server_total_bucket_count")
+

From 383ab4729a7c362f25526097c3f6c66e9451ef2c Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 15 Sep 2021 11:53:48 +0000
Subject: [PATCH 31/95] OpenMetrics tests: Tryfix resolve TypeError on CI

Was:
> TypeError: unbound method render_OPENMETRICS() must be called with Statistics instance as first argument (got FakeStats instance instead)
---
 src/allmydata/test/test_openmetrics.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index a9a0e5712..9c840714c 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -96,7 +96,7 @@ class FakeStatsProvider(object):
          }
         return stats
 
-class FakeStats():
+class FakeStats(Statistics):
     def __init__(self):
         self._provider = FakeStatsProvider()
 

From d210062dd7e943c7038bb312709a21c2196d5f86 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 15 Sep 2021 09:47:16 -0400
Subject: [PATCH 32/95] Another test for STARAW.

---
 src/allmydata/test/test_istorageserver.py | 52 +++++++++++++++++------
 1 file changed, 38 insertions(+), 14 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index c299841f9..8145bef44 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -271,20 +271,6 @@ class IStorageServerMutableAPIsTestsMixin(object):
 
     ``STARAW`` is short for ``slot_testv_and_readv_and_writev``.
     """
-
-    # slot_testv_and_readv_and_writev
-    # DONE it's possible to write and then in separate call read
-    # DONE reads happen before (re)writes
-    # DONE write happens if test succeeds
-    # DONE write prevented if tests fail
-    # DONE multiple test vectors
-    # DONE multiple writes
-    # DONE multiple reads
-    # DONE wrong write enabler secret prevents writes
-    # TODO write prevented if test data against empty share
-    # TODO writes can create additional shares if only some exist
-    # TODO later writes overwrite
-
     def new_secrets(self):
         """Return a 3-tuple of secrets for STARAW calls."""
         return (new_secret(), new_secret(), new_secret())
@@ -463,6 +449,44 @@ class IStorageServerMutableAPIsTestsMixin(object):
         )
         self.assertEqual(reads, {0: [b"1" * 7]})
 
+    @inlineCallbacks
+    def test_STARAW_zero_new_length_deletes(self):
+        """
+        A zero new length passed to
+        ``IStorageServer.slot_testv_and_readv_and_writev`` deletes the share.
+        """
+        secrets = self.new_secrets()
+        storage_index = new_storage_index()
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([], [(0, b"1" * 7)], 7),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+        # Write with new length of 0:
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([], [(0, b"1" * 7)], 0),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+        # It's gone!
+        (_, reads) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={},
+            r_vector=[(0, 7)],
+        )
+        self.assertEqual(reads, {})
+
 
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""

From 863343298060a9bfca1cc95bda0f4ca88325be1c Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 15 Sep 2021 10:33:51 -0400
Subject: [PATCH 33/95] Switch IStorageServer interface to be slightly
 different than RIStorageServer.

---
 src/allmydata/interfaces.py     |  4 ++++
 src/allmydata/mutable/layout.py | 12 ++++++------
 src/allmydata/storage_client.py | 10 +++++++++-
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index 1c64bce8a..9000787e8 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -351,6 +351,10 @@ class IStorageServer(Interface):
     ):
         """
         :see: ``RIStorageServer.slot_testv_readv_and_writev``
+
+        While the interface mostly matches, test vectors are simplified.
+        Instead of a tuple ``(start, offset, operator, data)`` they are just
+        ``(start, data)`` with operator implicitly being ``b"eq"``.
         """
 
     def advise_corrupt_share(
diff --git a/src/allmydata/mutable/layout.py b/src/allmydata/mutable/layout.py
index ce51a8833..50b60e30c 100644
--- a/src/allmydata/mutable/layout.py
+++ b/src/allmydata/mutable/layout.py
@@ -309,7 +309,7 @@ class SDMFSlotWriteProxy(object):
                                       salt)
         else:
             checkstring = checkstring_or_seqnum
-        self._testvs = [(0, len(checkstring), b"eq", checkstring)]
+        self._testvs = [(0, checkstring)]
 
 
     def get_checkstring(self):
@@ -318,7 +318,7 @@ class SDMFSlotWriteProxy(object):
         server.
         """
         if self._testvs:
-            return self._testvs[0][3]
+            return self._testvs[0][1]
         return b""
 
 
@@ -550,7 +550,7 @@ class SDMFSlotWriteProxy(object):
             # 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.
             self._testvs = []
-            self._testvs.append(tuple([0, 1, b"eq", b""]))
+            self._testvs.append(tuple([0, b""]))
 
         tw_vectors = {}
         tw_vectors[self.shnum] = (self._testvs, datavs, None)
@@ -889,7 +889,7 @@ class MDMFSlotWriteProxy(object):
             self._testvs = []
         else:
             self._testvs = []
-            self._testvs.append((0, len(checkstring), b"eq", checkstring))
+            self._testvs.append((0, checkstring))
 
 
     def __repr__(self):
@@ -1162,7 +1162,7 @@ class MDMFSlotWriteProxy(object):
         tw_vectors = {}
         if not self._testvs:
             self._testvs = []
-            self._testvs.append(tuple([0, 1, b"eq", b""]))
+            self._testvs.append(tuple([0, b""]))
         if not self._written:
             # Write a new checkstring to the share when we write it, so
             # that we have something to check later.
@@ -1170,7 +1170,7 @@ class MDMFSlotWriteProxy(object):
             datavs.append((0, new_checkstring))
             def _first_write():
                 self._written = True
-                self._testvs = [(0, len(new_checkstring), b"eq", new_checkstring)]
+                self._testvs = [(0, new_checkstring)]
             on_success = _first_write
         tw_vectors[self.shnum] = (self._testvs, datavs, None)
         d = self._storage_server.slot_testv_and_readv_and_writev(
diff --git a/src/allmydata/storage_client.py b/src/allmydata/storage_client.py
index e9dc8c84c..ec877779d 100644
--- a/src/allmydata/storage_client.py
+++ b/src/allmydata/storage_client.py
@@ -994,11 +994,19 @@ class _StorageServer(object):
             tw_vectors,
             r_vector,
     ):
+        # Match the wire protocol, which requires 4-tuples for test vectors.
+        wire_format_tw_vectors = {
+            key: (
+                [(start, len(data), b"eq", data) for (start, 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,
         )
 

From f109afa3b130ab9c89209f794e8309cf1b5ab917 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 15 Sep 2021 10:51:43 -0400
Subject: [PATCH 34/95] This is unnecessary, empty vector list is fine too.

---
 src/allmydata/mutable/layout.py | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/src/allmydata/mutable/layout.py b/src/allmydata/mutable/layout.py
index 50b60e30c..35f932b89 100644
--- a/src/allmydata/mutable/layout.py
+++ b/src/allmydata/mutable/layout.py
@@ -545,13 +545,6 @@ class SDMFSlotWriteProxy(object):
         # in its entirely.
         datavs = [(0, final_share)]
 
-        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.
-            self._testvs = []
-            self._testvs.append(tuple([0, b""]))
-
         tw_vectors = {}
         tw_vectors[self.shnum] = (self._testvs, datavs, None)
         return self._storage_server.slot_testv_and_readv_and_writev(
@@ -888,8 +881,7 @@ class MDMFSlotWriteProxy(object):
             # empty string means.
             self._testvs = []
         else:
-            self._testvs = []
-            self._testvs.append((0, checkstring))
+            self._testvs = [(0, checkstring)]
 
 
     def __repr__(self):
@@ -1160,9 +1152,6 @@ class MDMFSlotWriteProxy(object):
     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:
-            self._testvs = []
-            self._testvs.append(tuple([0, b""]))
         if not self._written:
             # Write a new checkstring to the share when we write it, so
             # that we have something to check later.

From 911a5e2ed1dfb6ffc593a6d402d98f3783d2397f Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 15 Sep 2021 11:07:02 -0400
Subject: [PATCH 35/95] Rip out server-side usage of operators other than eq,
 because nothing ever used them.

---
 src/allmydata/interfaces.py        |   5 +-
 src/allmydata/storage/mutable.py   |  17 +---
 src/allmydata/test/mutable/util.py |   2 +-
 src/allmydata/test/test_storage.py | 149 -----------------------------
 4 files changed, 6 insertions(+), 167 deletions(-)

diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index 9000787e8..dcd442624 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -91,9 +91,8 @@ class RIBucketReader(RemoteInterface):
 
 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", you should use length==len(specimen).
+# (These are only 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,
diff --git a/src/allmydata/storage/mutable.py b/src/allmydata/storage/mutable.py
index a44a2e18d..2ef0c3215 100644
--- a/src/allmydata/storage/mutable.py
+++ b/src/allmydata/storage/mutable.py
@@ -434,20 +434,9 @@ class MutableShareFile(object):
                     # 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):
 
diff --git a/src/allmydata/test/mutable/util.py b/src/allmydata/test/mutable/util.py
index 7e3bd3ec7..dac61a6e3 100644
--- a/src/allmydata/test/mutable/util.py
+++ b/src/allmydata/test/mutable/util.py
@@ -149,7 +149,7 @@ class FakeStorageServer(object):
         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
diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py
index bd0ab80f3..42a3ed0a1 100644
--- a/src/allmydata/test/test_storage.py
+++ b/src/allmydata/test/test_storage.py
@@ -1074,23 +1074,6 @@ class MutableServer(unittest.TestCase):
                                        }))
         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.
@@ -1110,63 +1093,6 @@ class MutableServer(unittest.TestCase):
 
         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"),
                                              ],
@@ -1186,81 +1112,6 @@ class MutableServer(unittest.TestCase):
         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"),
                                              ],

From 24c483aeda8c32233daee90baf02b0e378d4f464 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 15 Sep 2021 11:07:23 -0400
Subject: [PATCH 36/95] Update to simplified test vector support.

---
 docs/proposed/http-storage-node-protocol.rst | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst
index a84d62176..febcb250e 100644
--- a/docs/proposed/http-storage-node-protocol.rst
+++ b/docs/proposed/http-storage-node-protocol.rst
@@ -599,8 +599,6 @@ For example::
            0: {
                "test": [{
                    "offset": 3,
-                   "size": 5,
-                   "operator": "eq",
                    "specimen": "hello"
                }, ...],
                "write": [{
@@ -712,12 +710,7 @@ Mutable Data
          },
          "test-write-vectors": {
              3: {
-                 "test": [{
-                     "offset": 0,
-                     "size": 1,
-                     "operator": "eq",
-                     "specimen": ""
-                 }],
+                 "test": [],
                  "write": [{
                      "offset": 0,
                      "data": "xxxxxxxxxx"
@@ -747,8 +740,6 @@ Mutable Data
              3: {
                  "test": [{
                      "offset": 0,
-                     "size": <checkstring size>,
-                     "operator": "eq",
                      "specimen": "<checkstring>"
                  }],
                  "write": [{

From d9151b643a2cea518c294cd0c6c19b22b39ebfac Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 15 Sep 2021 11:07:54 -0400
Subject: [PATCH 37/95] News file.

---
 newsfragments/3799.minor | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 newsfragments/3799.minor

diff --git a/newsfragments/3799.minor b/newsfragments/3799.minor
new file mode 100644
index 000000000..e69de29bb

From b0e1cf924d44b271314efac94106247afdc7be10 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Wed, 15 Sep 2021 15:14:29 +0000
Subject: [PATCH 38/95] OpenMetrics test: White space only: Format JSON fixture
 to be easier on the eyes

---
 src/allmydata/test/test_openmetrics.py | 177 +++++++++++++------------
 1 file changed, 89 insertions(+), 88 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 9c840714c..7d9bd6429 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -6,94 +6,95 @@ from allmydata.web.status import Statistics
 class FakeStatsProvider(object):
     def get_stats(self):
         # Parsed into a dict from a running tahoe's /statistics?t=json
-        stats = {'stats': {
-           'storage_server.latencies.get.99_9_percentile': None,
-           'storage_server.latencies.close.10_0_percentile': 0.00021910667419433594,
-           'storage_server.latencies.read.01_0_percentile': 2.8848648071289062e-05,
-           'storage_server.latencies.writev.99_9_percentile': None,
-           'storage_server.latencies.read.99_9_percentile': None,
-           'storage_server.latencies.allocate.99_0_percentile': 0.000988006591796875,
-           'storage_server.latencies.writev.mean': 0.00045332245070571654,
-           'storage_server.latencies.close.99_9_percentile': None,
-           'cpu_monitor.15min_avg': 0.00017592000079223033,
-           'storage_server.disk_free_for_root': 103289454592,
-           'storage_server.latencies.get.99_0_percentile': 0.000347137451171875,
-           'storage_server.latencies.get.mean': 0.00021158285060171353,
-           'storage_server.latencies.read.90_0_percentile': 8.893013000488281e-05,
-           'storage_server.latencies.write.01_0_percentile': 3.600120544433594e-05,
-           'storage_server.latencies.write.99_9_percentile': 0.00017690658569335938,
-           'storage_server.latencies.close.90_0_percentile': 0.00033211708068847656,
-           'storage_server.disk_total': 103497859072,
-           'storage_server.latencies.close.95_0_percentile': 0.0003509521484375,
-           'storage_server.latencies.readv.samplesize': 1000,
-           'storage_server.disk_free_for_nonroot': 103289454592,
-           'storage_server.latencies.close.mean': 0.0002715024480059103,
-           'storage_server.latencies.writev.95_0_percentile': 0.0007410049438476562,
-           'storage_server.latencies.readv.90_0_percentile': 0.0003781318664550781,
-           'storage_server.latencies.readv.99_0_percentile': 0.0004050731658935547,
-           'storage_server.latencies.allocate.mean': 0.0007128627429454784,
-           'storage_server.latencies.close.samplesize': 326,
-           'storage_server.latencies.get.50_0_percentile': 0.0001819133758544922,
-           'storage_server.latencies.write.50_0_percentile': 4.482269287109375e-05,
-           'storage_server.latencies.readv.01_0_percentile': 0.0002970695495605469,
-           'storage_server.latencies.get.10_0_percentile': 0.00015687942504882812,
-           'storage_server.latencies.allocate.90_0_percentile': 0.0008189678192138672,
-           'storage_server.latencies.get.samplesize': 472,
-           'storage_server.total_bucket_count': 393,
-           'storage_server.latencies.read.mean': 5.936201880959903e-05,
-           'storage_server.latencies.allocate.01_0_percentile': 0.0004208087921142578,
-           'storage_server.latencies.allocate.99_9_percentile': None,
-           'storage_server.latencies.readv.mean': 0.00034061360359191893,
-           'storage_server.disk_used': 208404480,
-           'storage_server.latencies.allocate.50_0_percentile': 0.0007410049438476562,
-           'storage_server.latencies.read.99_0_percentile': 0.00011992454528808594,
-           'node.uptime': 3805759.8545179367,
-           'storage_server.latencies.writev.10_0_percentile': 0.00035190582275390625,
-           'storage_server.latencies.writev.90_0_percentile': 0.0006821155548095703,
-           'storage_server.latencies.close.01_0_percentile': 0.00021505355834960938,
-           'storage_server.latencies.close.50_0_percentile': 0.0002579689025878906,
-           'cpu_monitor.1min_avg': 0.0002130000000003444,
-           'storage_server.latencies.writev.50_0_percentile': 0.0004138946533203125,
-           'storage_server.latencies.read.95_0_percentile': 9.107589721679688e-05,
-           'storage_server.latencies.readv.95_0_percentile': 0.0003859996795654297,
-           'storage_server.latencies.write.10_0_percentile': 3.719329833984375e-05,
-           'storage_server.accepting_immutable_shares': 1,
-           'storage_server.latencies.writev.samplesize': 309,
-           'storage_server.latencies.get.95_0_percentile': 0.0003190040588378906,
-           'storage_server.latencies.readv.10_0_percentile': 0.00032210350036621094,
-           'storage_server.latencies.get.90_0_percentile': 0.0002999305725097656,
-           'storage_server.latencies.get.01_0_percentile': 0.0001239776611328125,
-           'cpu_monitor.total': 641.4941180000001,
-           'storage_server.latencies.write.samplesize': 1000,
-           'storage_server.latencies.write.95_0_percentile': 9.489059448242188e-05,
-           'storage_server.latencies.read.50_0_percentile': 6.890296936035156e-05,
-           'storage_server.latencies.writev.01_0_percentile': 0.00033211708068847656,
-           'storage_server.latencies.read.10_0_percentile': 3.0994415283203125e-05,
-           'storage_server.latencies.allocate.10_0_percentile': 0.0004949569702148438,
-           'storage_server.reserved_space': 0,
-           'storage_server.disk_avail': 103289454592,
-           'storage_server.latencies.write.99_0_percentile': 0.00011301040649414062,
-           'storage_server.latencies.write.90_0_percentile': 9.083747863769531e-05,
-           'cpu_monitor.5min_avg': 0.0002370666691157502,
-           'storage_server.latencies.write.mean': 5.8008909225463864e-05,
-           'storage_server.latencies.readv.50_0_percentile': 0.00033020973205566406,
-           'storage_server.latencies.close.99_0_percentile': 0.0004038810729980469,
-           'storage_server.allocated': 0,
-           'storage_server.latencies.writev.99_0_percentile': 0.0007710456848144531,
-           'storage_server.latencies.readv.99_9_percentile': 0.0004780292510986328,
-           'storage_server.latencies.read.samplesize': 170,
-           'storage_server.latencies.allocate.samplesize': 406,
-           'storage_server.latencies.allocate.95_0_percentile': 0.0008411407470703125
-         }, 'counters': {
-           'storage_server.writev': 309,
-           'storage_server.bytes_added': 197836146,
-           'storage_server.close': 326,
-           'storage_server.readv': 14299,
-           'storage_server.allocate': 406,
-           'storage_server.read': 170,
-           'storage_server.write': 3775,
-           'storage_server.get': 472}
-         }
+        stats = {
+          'stats': {
+            'storage_server.latencies.get.99_9_percentile': None,
+            'storage_server.latencies.close.10_0_percentile': 0.00021910667419433594,
+            'storage_server.latencies.read.01_0_percentile': 2.8848648071289062e-05,
+            'storage_server.latencies.writev.99_9_percentile': None,
+            'storage_server.latencies.read.99_9_percentile': None,
+            'storage_server.latencies.allocate.99_0_percentile': 0.000988006591796875,
+            'storage_server.latencies.writev.mean': 0.00045332245070571654,
+            'storage_server.latencies.close.99_9_percentile': None,
+            'cpu_monitor.15min_avg': 0.00017592000079223033,
+            'storage_server.disk_free_for_root': 103289454592,
+            'storage_server.latencies.get.99_0_percentile': 0.000347137451171875,
+            'storage_server.latencies.get.mean': 0.00021158285060171353,
+            'storage_server.latencies.read.90_0_percentile': 8.893013000488281e-05,
+            'storage_server.latencies.write.01_0_percentile': 3.600120544433594e-05,
+            'storage_server.latencies.write.99_9_percentile': 0.00017690658569335938,
+            'storage_server.latencies.close.90_0_percentile': 0.00033211708068847656,
+            'storage_server.disk_total': 103497859072,
+            'storage_server.latencies.close.95_0_percentile': 0.0003509521484375,
+            'storage_server.latencies.readv.samplesize': 1000,
+            'storage_server.disk_free_for_nonroot': 103289454592,
+            'storage_server.latencies.close.mean': 0.0002715024480059103,
+            'storage_server.latencies.writev.95_0_percentile': 0.0007410049438476562,
+            'storage_server.latencies.readv.90_0_percentile': 0.0003781318664550781,
+            'storage_server.latencies.readv.99_0_percentile': 0.0004050731658935547,
+            'storage_server.latencies.allocate.mean': 0.0007128627429454784,
+            'storage_server.latencies.close.samplesize': 326,
+            'storage_server.latencies.get.50_0_percentile': 0.0001819133758544922,
+            'storage_server.latencies.write.50_0_percentile': 4.482269287109375e-05,
+            'storage_server.latencies.readv.01_0_percentile': 0.0002970695495605469,
+            'storage_server.latencies.get.10_0_percentile': 0.00015687942504882812,
+            'storage_server.latencies.allocate.90_0_percentile': 0.0008189678192138672,
+            'storage_server.latencies.get.samplesize': 472,
+            'storage_server.total_bucket_count': 393,
+            'storage_server.latencies.read.mean': 5.936201880959903e-05,
+            'storage_server.latencies.allocate.01_0_percentile': 0.0004208087921142578,
+            'storage_server.latencies.allocate.99_9_percentile': None,
+            'storage_server.latencies.readv.mean': 0.00034061360359191893,
+            'storage_server.disk_used': 208404480,
+            'storage_server.latencies.allocate.50_0_percentile': 0.0007410049438476562,
+            'storage_server.latencies.read.99_0_percentile': 0.00011992454528808594,
+            'node.uptime': 3805759.8545179367,
+            'storage_server.latencies.writev.10_0_percentile': 0.00035190582275390625,
+            'storage_server.latencies.writev.90_0_percentile': 0.0006821155548095703,
+            'storage_server.latencies.close.01_0_percentile': 0.00021505355834960938,
+            'storage_server.latencies.close.50_0_percentile': 0.0002579689025878906,
+            'cpu_monitor.1min_avg': 0.0002130000000003444,
+            'storage_server.latencies.writev.50_0_percentile': 0.0004138946533203125,
+            'storage_server.latencies.read.95_0_percentile': 9.107589721679688e-05,
+            'storage_server.latencies.readv.95_0_percentile': 0.0003859996795654297,
+            'storage_server.latencies.write.10_0_percentile': 3.719329833984375e-05,
+            'storage_server.accepting_immutable_shares': 1,
+            'storage_server.latencies.writev.samplesize': 309,
+            'storage_server.latencies.get.95_0_percentile': 0.0003190040588378906,
+            'storage_server.latencies.readv.10_0_percentile': 0.00032210350036621094,
+            'storage_server.latencies.get.90_0_percentile': 0.0002999305725097656,
+            'storage_server.latencies.get.01_0_percentile': 0.0001239776611328125,
+            'cpu_monitor.total': 641.4941180000001,
+            'storage_server.latencies.write.samplesize': 1000,
+            'storage_server.latencies.write.95_0_percentile': 9.489059448242188e-05,
+            'storage_server.latencies.read.50_0_percentile': 6.890296936035156e-05,
+            'storage_server.latencies.writev.01_0_percentile': 0.00033211708068847656,
+            'storage_server.latencies.read.10_0_percentile': 3.0994415283203125e-05,
+            'storage_server.latencies.allocate.10_0_percentile': 0.0004949569702148438,
+            'storage_server.reserved_space': 0,
+            'storage_server.disk_avail': 103289454592,
+            'storage_server.latencies.write.99_0_percentile': 0.00011301040649414062,
+            'storage_server.latencies.write.90_0_percentile': 9.083747863769531e-05,
+            'cpu_monitor.5min_avg': 0.0002370666691157502,
+            'storage_server.latencies.write.mean': 5.8008909225463864e-05,
+            'storage_server.latencies.readv.50_0_percentile': 0.00033020973205566406,
+            'storage_server.latencies.close.99_0_percentile': 0.0004038810729980469,
+            'storage_server.allocated': 0,
+            'storage_server.latencies.writev.99_0_percentile': 0.0007710456848144531,
+            'storage_server.latencies.readv.99_9_percentile': 0.0004780292510986328,
+            'storage_server.latencies.read.samplesize': 170,
+            'storage_server.latencies.allocate.samplesize': 406,
+            'storage_server.latencies.allocate.95_0_percentile': 0.0008411407470703125},
+          'counters': {
+            'storage_server.writev': 309,
+            'storage_server.bytes_added': 197836146,
+            'storage_server.close': 326,
+            'storage_server.readv': 14299,
+            'storage_server.allocate': 406,
+            'storage_server.read': 170,
+            'storage_server.write': 3775,
+            'storage_server.get': 472}
+          }
         return stats
 
 class FakeStats(Statistics):

From 1d2073b8f8bd883a364ba4b18e3c6cfeb8e68fd7 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 15 Sep 2021 16:19:24 -0400
Subject: [PATCH 39/95] Revert "This is unnecessary, empty vector list is fine
 too."

This reverts commit f109afa3b130ab9c89209f794e8309cf1b5ab917.
---
 src/allmydata/mutable/layout.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/allmydata/mutable/layout.py b/src/allmydata/mutable/layout.py
index 35f932b89..50b60e30c 100644
--- a/src/allmydata/mutable/layout.py
+++ b/src/allmydata/mutable/layout.py
@@ -545,6 +545,13 @@ class SDMFSlotWriteProxy(object):
         # in its entirely.
         datavs = [(0, final_share)]
 
+        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.
+            self._testvs = []
+            self._testvs.append(tuple([0, b""]))
+
         tw_vectors = {}
         tw_vectors[self.shnum] = (self._testvs, datavs, None)
         return self._storage_server.slot_testv_and_readv_and_writev(
@@ -881,7 +888,8 @@ class MDMFSlotWriteProxy(object):
             # empty string means.
             self._testvs = []
         else:
-            self._testvs = [(0, checkstring)]
+            self._testvs = []
+            self._testvs.append((0, checkstring))
 
 
     def __repr__(self):
@@ -1152,6 +1160,9 @@ class MDMFSlotWriteProxy(object):
     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:
+            self._testvs = []
+            self._testvs.append(tuple([0, b""]))
         if not self._written:
             # Write a new checkstring to the share when we write it, so
             # that we have something to check later.

From e11e5dfbe6c5a8556d5d29e187f4a8cb0e899931 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 15 Sep 2021 16:31:54 -0400
Subject: [PATCH 40/95] Revert removal of length in IStorageServer.

---
 docs/proposed/http-storage-node-protocol.rst | 13 +++++++++++--
 src/allmydata/interfaces.py                  |  7 ++++---
 src/allmydata/mutable/layout.py              | 16 +++++++++-------
 src/allmydata/storage_client.py              |  2 +-
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst
index febcb250e..35811cad0 100644
--- a/docs/proposed/http-storage-node-protocol.rst
+++ b/docs/proposed/http-storage-node-protocol.rst
@@ -599,6 +599,7 @@ For example::
            0: {
                "test": [{
                    "offset": 3,
+                   "size": 5,
                    "specimen": "hello"
                }, ...],
                "write": [{
@@ -699,7 +700,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
      {
@@ -710,7 +714,11 @@ Mutable Data
          },
          "test-write-vectors": {
              3: {
-                 "test": [],
+                 "test": [{
+                     "offset": 0,
+                     "size": 1,
+                     "specimen": ""
+                 }],
                  "write": [{
                      "offset": 0,
                      "data": "xxxxxxxxxx"
@@ -740,6 +748,7 @@ Mutable Data
              3: {
                  "test": [{
                      "offset": 0,
+                     "size": <length of checkstring>,
                      "specimen": "<checkstring>"
                  }],
                  "write": [{
diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index dcd442624..0283b443a 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -91,8 +91,9 @@ class RIBucketReader(RemoteInterface):
 
 TestVector = ListOf(TupleOf(Offset, ReadSize, bytes, bytes))
 # elements are (offset, length, operator, specimen)
-# operator must be b"eq", you should use length==len(specimen).
-# (These are only used for wire compatibility with old versions).
+# 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,
@@ -353,7 +354,7 @@ class IStorageServer(Interface):
 
         While the interface mostly matches, test vectors are simplified.
         Instead of a tuple ``(start, offset, operator, data)`` they are just
-        ``(start, data)`` with operator implicitly being ``b"eq"``.
+        ``(start, data_length, data)`` with operator implicitly being ``b"eq"``.
         """
 
     def advise_corrupt_share(
diff --git a/src/allmydata/mutable/layout.py b/src/allmydata/mutable/layout.py
index 50b60e30c..8bb2f3083 100644
--- a/src/allmydata/mutable/layout.py
+++ b/src/allmydata/mutable/layout.py
@@ -309,7 +309,7 @@ class SDMFSlotWriteProxy(object):
                                       salt)
         else:
             checkstring = checkstring_or_seqnum
-        self._testvs = [(0, checkstring)]
+        self._testvs = [(0, len(checkstring), checkstring)]
 
 
     def get_checkstring(self):
@@ -318,7 +318,7 @@ class SDMFSlotWriteProxy(object):
         server.
         """
         if self._testvs:
-            return self._testvs[0][1]
+            return self._testvs[0][2]
         return b""
 
 
@@ -548,9 +548,9 @@ class SDMFSlotWriteProxy(object):
         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, b""]))
+            self._testvs.append(tuple([0, 1, b""]))
 
         tw_vectors = {}
         tw_vectors[self.shnum] = (self._testvs, datavs, None)
@@ -889,7 +889,7 @@ class MDMFSlotWriteProxy(object):
             self._testvs = []
         else:
             self._testvs = []
-            self._testvs.append((0, checkstring))
+            self._testvs.append((0, len(checkstring), checkstring))
 
 
     def __repr__(self):
@@ -1161,8 +1161,10 @@ class MDMFSlotWriteProxy(object):
         """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, 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.
@@ -1170,7 +1172,7 @@ class MDMFSlotWriteProxy(object):
             datavs.append((0, new_checkstring))
             def _first_write():
                 self._written = True
-                self._testvs = [(0, 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(
diff --git a/src/allmydata/storage_client.py b/src/allmydata/storage_client.py
index ec877779d..ac6c107d5 100644
--- a/src/allmydata/storage_client.py
+++ b/src/allmydata/storage_client.py
@@ -997,7 +997,7 @@ class _StorageServer(object):
         # Match the wire protocol, which requires 4-tuples for test vectors.
         wire_format_tw_vectors = {
             key: (
-                [(start, len(data), b"eq", data) for (start, data) in value[0]],
+                [(start, length, b"eq", data) for (start, length, data) in value[0]],
                 value[1],
                 value[2],
             ) for (key, value) in tw_vectors.items()

From 5825b8bd42328618dfdc9f7a2aa02dd761fe2887 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Thu, 16 Sep 2021 15:58:04 +0000
Subject: [PATCH 41/95] OpenMetrics: rework test suite with exarkun

---
 src/allmydata/test/test_openmetrics.py | 90 ++++++++++++++++++++------
 1 file changed, 72 insertions(+), 18 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 7d9bd6429..a0822d594 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -1,7 +1,25 @@
-import mock
 from prometheus_client.openmetrics import parser
-from twisted.trial import unittest
+
+from treq.testing import RequestTraversalAgent
+
+from twisted.web.http import OK
+from twisted.web.client import readBody    
+from twisted.web.resource import Resource
+
+from testtools.twistedsupport import succeeded
+from testtools.matchers import (
+    Always,
+    AfterPreprocessing,
+    Equals,
+    MatchesAll,
+    MatchesStructure,
+    MatchesPredicate,
+    )
+from testtools.content import text_content
+
 from allmydata.web.status import Statistics
+from allmydata.test.common import SyncTestCase
+
 
 class FakeStatsProvider(object):
     def get_stats(self):
@@ -97,31 +115,67 @@ class FakeStatsProvider(object):
           }
         return stats
 
-class FakeStats(Statistics):
-    def __init__(self):
-        self._provider = FakeStatsProvider()
+class HackItResource(Resource):
+    def getChildWithDefault(self, path, request):
+        request.fields = None
+        return Resource.getChildWithDefault(self, path, request)
 
-class OpenMetrics(unittest.TestCase):
+
+class OpenMetrics(SyncTestCase):
     def test_spec_compliance(self):
         """
         Does our output adhere to the `OpenMetrics <https://openmetrics.io/>` spec?
         https://github.com/OpenObservability/OpenMetrics/
         https://prometheus.io/docs/instrumenting/exposition_formats/
         """
-        req = mock.Mock()
-        stats = FakeStats()
-        metrics = Statistics.render_OPENMETRICS(stats, req)
+        root = HackItResource()
+        root.putChild(b"", Statistics(FakeStatsProvider())) 
+        rta = RequestTraversalAgent(root)
+        d = rta.request(b"GET", b"http://localhost/?t=openmetrics")
+        self.assertThat(d, succeeded(matches_stats(self)))
 
-        # "The content type MUST be..."
-        req.setHeader.assert_called_with(
-            "content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8"
+def matches_stats(testcase):
+    def add_detail(testcase):
+        def predicate(body):
+            testcase.addDetail("body", text_content(body))
+            return True
+        return predicate
+
+    return MatchesAll(
+        MatchesStructure(
+            code=Equals(OK),
+            # "The content type MUST be..."
+            headers=has_header("content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8"),
+        ),
+        AfterPreprocessing(
+            readBodyText,
+            succeeded(MatchesAll(
+                MatchesPredicate(add_detail(testcase), "%s dummy"),
+                parses_as_openmetrics(),
+            ))
         )
+    )
 
-        # The parser throws if it does not like its input.
-        # Wrapped in a list() to drain the generator.
-        families = list(parser.text_string_to_metric_families(metrics))
+def readBodyText(response):
+    d = readBody(response)
+    d.addCallback(lambda body: body.decode("utf-8"))
+    return d
+
+def has_header(name, value):
+    return AfterPreprocessing(
+        lambda headers: headers.getRawHeaders(name),
+        Equals([value]),
+    )
+
+def parses_as_openmetrics():
+    # The parser throws if it does not like its input.
+    # Wrapped in a list() to drain the generator. 
+    return AfterPreprocessing(
+        lambda body: list(parser.text_string_to_metric_families(body)),
+        AfterPreprocessing(
+            lambda families: families[-1].name, 
+            Equals(u"tahoe_stats_storage_server_total_bucket_count"),
+        ),
+    )
 
-        # Has the parser parsed our data?
-        # Just check the last item.
-        self.assertEqual(families[-1].name, u"tahoe_stats_storage_server_total_bucket_count")
 

From a2378d0e704add3741aa269adc37ac3f781ac36a Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 17 Sep 2021 12:04:12 +0000
Subject: [PATCH 42/95] OpenMetrics test suite: Make CI happy: No old style
 objects

---
 src/allmydata/test/test_openmetrics.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index a0822d594..43704c4a3 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -115,7 +115,7 @@ class FakeStatsProvider(object):
           }
         return stats
 
-class HackItResource(Resource):
+class HackItResource(Resource, object):
     def getChildWithDefault(self, path, request):
         request.fields = None
         return Resource.getChildWithDefault(self, path, request)

From fb0335cc17c709d5663f7ea2c31ccb98b6d6ddf1 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 17 Sep 2021 12:16:37 +0000
Subject: [PATCH 43/95] OpenMetrics test suite: More clean up

The Linter complains:
> 'testtools.matchers.Always' imported but unused
---
 src/allmydata/test/test_openmetrics.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 43704c4a3..91692af33 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -8,7 +8,6 @@ from twisted.web.resource import Resource
 
 from testtools.twistedsupport import succeeded
 from testtools.matchers import (
-    Always,
     AfterPreprocessing,
     Equals,
     MatchesAll,

From e5e0d71ef518c73e3fe2a0bc860e1c7428ba91a9 Mon Sep 17 00:00:00 2001
From: Florian Sesser <florian@private.storage>
Date: Fri, 17 Sep 2021 13:20:59 +0000
Subject: [PATCH 44/95] OpenMetrics test suite: More clean ups

trailing whitespace
---
 src/allmydata/test/test_openmetrics.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 91692af33..9677de4a9 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -3,7 +3,7 @@ from prometheus_client.openmetrics import parser
 from treq.testing import RequestTraversalAgent
 
 from twisted.web.http import OK
-from twisted.web.client import readBody    
+from twisted.web.client import readBody
 from twisted.web.resource import Resource
 
 from testtools.twistedsupport import succeeded
@@ -128,7 +128,7 @@ class OpenMetrics(SyncTestCase):
         https://prometheus.io/docs/instrumenting/exposition_formats/
         """
         root = HackItResource()
-        root.putChild(b"", Statistics(FakeStatsProvider())) 
+        root.putChild(b"", Statistics(FakeStatsProvider()))
         rta = RequestTraversalAgent(root)
         d = rta.request(b"GET", b"http://localhost/?t=openmetrics")
         self.assertThat(d, succeeded(matches_stats(self)))
@@ -168,11 +168,11 @@ def has_header(name, value):
 
 def parses_as_openmetrics():
     # The parser throws if it does not like its input.
-    # Wrapped in a list() to drain the generator. 
+    # Wrapped in a list() to drain the generator.
     return AfterPreprocessing(
         lambda body: list(parser.text_string_to_metric_families(body)),
         AfterPreprocessing(
-            lambda families: families[-1].name, 
+            lambda families: families[-1].name,
             Equals(u"tahoe_stats_storage_server_total_bucket_count"),
         ),
     )

From 5e26f25b3714167a628e93ae76a529b074fb4c69 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 23 Sep 2021 07:41:43 -0400
Subject: [PATCH 45/95] It's ported to Python 3!

---
 src/allmydata/test/test_openmetrics.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 9677de4a9..350d6abbd 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -1,3 +1,18 @@
+"""
+Tests for ``/statistics?t=openmetrics``.
+
+Ported to Python 3.
+"""
+
+from __future__ import print_function
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import unicode_literals
+
+from future.utils import PY2
+if PY2:
+    from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
+
 from prometheus_client.openmetrics import parser
 
 from treq.testing import RequestTraversalAgent

From f8c07bfd11edfee9b62b8bb13cbe46f643267322 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 23 Sep 2021 07:42:59 -0400
Subject: [PATCH 46/95] add some docstrings

---
 src/allmydata/test/test_openmetrics.py | 37 ++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 350d6abbd..34dcd266b 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -36,6 +36,10 @@ from allmydata.test.common import SyncTestCase
 
 
 class FakeStatsProvider(object):
+    """
+    A stats provider that hands backed a canned collection of performance
+    statistics.
+    """
     def get_stats(self):
         # Parsed into a dict from a running tahoe's /statistics?t=json
         stats = {
@@ -130,12 +134,21 @@ class FakeStatsProvider(object):
         return stats
 
 class HackItResource(Resource, object):
+    """
+    A bridge between ``RequestTraversalAgent`` and ``MultiFormatResource``
+    (used by ``Statistics``).  ``MultiFormatResource`` expects the request
+    object to have a ``fields`` attribute but Twisted's ``IRequest`` has no
+    such attribute.  Create it here.
+    """
     def getChildWithDefault(self, path, request):
         request.fields = None
         return Resource.getChildWithDefault(self, path, request)
 
 
 class OpenMetrics(SyncTestCase):
+    """
+    Tests for ``/statistics?t=openmetrics``.
+    """
     def test_spec_compliance(self):
         """
         Does our output adhere to the `OpenMetrics <https://openmetrics.io/>` spec?
@@ -171,17 +184,41 @@ def matches_stats(testcase):
     )
 
 def readBodyText(response):
+    """
+    Read the response body and decode it using UTF-8.
+
+    :param twisted.web.iweb.IResponse response: The response from which to
+        read the body.
+
+    :return: A ``Deferred`` that fires with the ``str`` body.
+    """
     d = readBody(response)
     d.addCallback(lambda body: body.decode("utf-8"))
     return d
 
 def has_header(name, value):
+    """
+    Create a matcher that matches a response object that includes the given
+    name / value pair.
+
+    :param str name: The name of the item in the HTTP header to match.
+    :param str value: The value of the item in the HTTP header to match by equality.
+
+    :return: A matcher.
+    """
     return AfterPreprocessing(
         lambda headers: headers.getRawHeaders(name),
         Equals([value]),
     )
 
 def parses_as_openmetrics():
+    """
+    Create a matcher that matches a ``str`` string that can be parsed as an
+    OpenMetrics response and includes a certain well-known value expected by
+    the tests.
+
+    :return: A matcher.
+    """
     # The parser throws if it does not like its input.
     # Wrapped in a list() to drain the generator.
     return AfterPreprocessing(

From 4d8164773c404373f4aeea37f2969680fa02c265 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 23 Sep 2021 07:43:18 -0400
Subject: [PATCH 47/95] factor helper function out to top-level

---
 src/allmydata/test/test_openmetrics.py | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 34dcd266b..a92098c88 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -162,11 +162,6 @@ class OpenMetrics(SyncTestCase):
         self.assertThat(d, succeeded(matches_stats(self)))
 
 def matches_stats(testcase):
-    def add_detail(testcase):
-        def predicate(body):
-            testcase.addDetail("body", text_content(body))
-            return True
-        return predicate
 
     return MatchesAll(
         MatchesStructure(
@@ -177,12 +172,26 @@ def matches_stats(testcase):
         AfterPreprocessing(
             readBodyText,
             succeeded(MatchesAll(
-                MatchesPredicate(add_detail(testcase), "%s dummy"),
+                MatchesPredicate(add_detail(testcase, u"response body"), u"%s dummy"),
                 parses_as_openmetrics(),
             ))
         )
     )
 
+def add_detail(testcase, name):
+    """
+    Create a matcher that always matches and as a side-effect adds the matched
+    value as detail to the testcase.
+
+    :param testtools.TestCase testcase: The case to which to add the detail.
+
+    :return: A matcher.
+    """
+    def predicate(value):
+        testcase.addDetail(name, text_content(value))
+        return True
+    return predicate
+
 def readBodyText(response):
     """
     Read the response body and decode it using UTF-8.

From cbb96bd57a2fa3fa5a0a11662ea4890a5ad9bc41 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 23 Sep 2021 07:43:33 -0400
Subject: [PATCH 48/95] one more docstring

---
 src/allmydata/test/test_openmetrics.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index a92098c88..385bf32a8 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -162,7 +162,20 @@ class OpenMetrics(SyncTestCase):
         self.assertThat(d, succeeded(matches_stats(self)))
 
 def matches_stats(testcase):
+    """
+    Create a matcher that matches a response that confirms to the OpenMetrics
+    specification.
 
+    * The ``Content-Type`` is **application/openmetrics-text; version=1.0.0; charset=utf-8**.
+    * The status is **OK**.
+    * The body can be parsed by an OpenMetrics parser.
+    * The metric families in the body are grouped and sorted.
+    * At least one of the expected families appears in the body.
+
+    :param testtools.TestCase testcase: The case to which to add detail about the matching process.
+
+    :return: A matcher.
+    """
     return MatchesAll(
         MatchesStructure(
             code=Equals(OK),

From f66a8ab1369f197163832a940c25efdb44f3dcde Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 23 Sep 2021 07:43:37 -0400
Subject: [PATCH 49/95] formatting and explicit unicode string literals

---
 src/allmydata/test/test_openmetrics.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 385bf32a8..d74958ffd 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -180,7 +180,10 @@ def matches_stats(testcase):
         MatchesStructure(
             code=Equals(OK),
             # "The content type MUST be..."
-            headers=has_header("content-type", "application/openmetrics-text; version=1.0.0; charset=utf-8"),
+            headers=has_header(
+                u"content-type",
+                u"application/openmetrics-text; version=1.0.0; charset=utf-8",
+            ),
         ),
         AfterPreprocessing(
             readBodyText,

From 4b6d00221e289a94fa9603dc0d1050605c0411ab Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 23 Sep 2021 07:48:19 -0400
Subject: [PATCH 50/95] protect this crazy line from black

---
 src/allmydata/test/test_openmetrics.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index d74958ffd..71433c11a 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -10,8 +10,11 @@ from __future__ import division
 from __future__ import unicode_literals
 
 from future.utils import PY2
+
 if PY2:
+    # fmt: off
     from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
+    # fmt: on
 
 from prometheus_client.openmetrics import parser
 

From 2f60ab300ba724bd67c0ca0825af089eecf13e36 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 23 Sep 2021 07:48:27 -0400
Subject: [PATCH 51/95] black formatting

---
 src/allmydata/test/test_openmetrics.py | 211 +++++++++++++------------
 1 file changed, 112 insertions(+), 99 deletions(-)

diff --git a/src/allmydata/test/test_openmetrics.py b/src/allmydata/test/test_openmetrics.py
index 71433c11a..66cbc7dec 100644
--- a/src/allmydata/test/test_openmetrics.py
+++ b/src/allmydata/test/test_openmetrics.py
@@ -31,7 +31,7 @@ from testtools.matchers import (
     MatchesAll,
     MatchesStructure,
     MatchesPredicate,
-    )
+)
 from testtools.content import text_content
 
 from allmydata.web.status import Statistics
@@ -43,99 +43,103 @@ class FakeStatsProvider(object):
     A stats provider that hands backed a canned collection of performance
     statistics.
     """
+
     def get_stats(self):
         # Parsed into a dict from a running tahoe's /statistics?t=json
         stats = {
-          'stats': {
-            'storage_server.latencies.get.99_9_percentile': None,
-            'storage_server.latencies.close.10_0_percentile': 0.00021910667419433594,
-            'storage_server.latencies.read.01_0_percentile': 2.8848648071289062e-05,
-            'storage_server.latencies.writev.99_9_percentile': None,
-            'storage_server.latencies.read.99_9_percentile': None,
-            'storage_server.latencies.allocate.99_0_percentile': 0.000988006591796875,
-            'storage_server.latencies.writev.mean': 0.00045332245070571654,
-            'storage_server.latencies.close.99_9_percentile': None,
-            'cpu_monitor.15min_avg': 0.00017592000079223033,
-            'storage_server.disk_free_for_root': 103289454592,
-            'storage_server.latencies.get.99_0_percentile': 0.000347137451171875,
-            'storage_server.latencies.get.mean': 0.00021158285060171353,
-            'storage_server.latencies.read.90_0_percentile': 8.893013000488281e-05,
-            'storage_server.latencies.write.01_0_percentile': 3.600120544433594e-05,
-            'storage_server.latencies.write.99_9_percentile': 0.00017690658569335938,
-            'storage_server.latencies.close.90_0_percentile': 0.00033211708068847656,
-            'storage_server.disk_total': 103497859072,
-            'storage_server.latencies.close.95_0_percentile': 0.0003509521484375,
-            'storage_server.latencies.readv.samplesize': 1000,
-            'storage_server.disk_free_for_nonroot': 103289454592,
-            'storage_server.latencies.close.mean': 0.0002715024480059103,
-            'storage_server.latencies.writev.95_0_percentile': 0.0007410049438476562,
-            'storage_server.latencies.readv.90_0_percentile': 0.0003781318664550781,
-            'storage_server.latencies.readv.99_0_percentile': 0.0004050731658935547,
-            'storage_server.latencies.allocate.mean': 0.0007128627429454784,
-            'storage_server.latencies.close.samplesize': 326,
-            'storage_server.latencies.get.50_0_percentile': 0.0001819133758544922,
-            'storage_server.latencies.write.50_0_percentile': 4.482269287109375e-05,
-            'storage_server.latencies.readv.01_0_percentile': 0.0002970695495605469,
-            'storage_server.latencies.get.10_0_percentile': 0.00015687942504882812,
-            'storage_server.latencies.allocate.90_0_percentile': 0.0008189678192138672,
-            'storage_server.latencies.get.samplesize': 472,
-            'storage_server.total_bucket_count': 393,
-            'storage_server.latencies.read.mean': 5.936201880959903e-05,
-            'storage_server.latencies.allocate.01_0_percentile': 0.0004208087921142578,
-            'storage_server.latencies.allocate.99_9_percentile': None,
-            'storage_server.latencies.readv.mean': 0.00034061360359191893,
-            'storage_server.disk_used': 208404480,
-            'storage_server.latencies.allocate.50_0_percentile': 0.0007410049438476562,
-            'storage_server.latencies.read.99_0_percentile': 0.00011992454528808594,
-            'node.uptime': 3805759.8545179367,
-            'storage_server.latencies.writev.10_0_percentile': 0.00035190582275390625,
-            'storage_server.latencies.writev.90_0_percentile': 0.0006821155548095703,
-            'storage_server.latencies.close.01_0_percentile': 0.00021505355834960938,
-            'storage_server.latencies.close.50_0_percentile': 0.0002579689025878906,
-            'cpu_monitor.1min_avg': 0.0002130000000003444,
-            'storage_server.latencies.writev.50_0_percentile': 0.0004138946533203125,
-            'storage_server.latencies.read.95_0_percentile': 9.107589721679688e-05,
-            'storage_server.latencies.readv.95_0_percentile': 0.0003859996795654297,
-            'storage_server.latencies.write.10_0_percentile': 3.719329833984375e-05,
-            'storage_server.accepting_immutable_shares': 1,
-            'storage_server.latencies.writev.samplesize': 309,
-            'storage_server.latencies.get.95_0_percentile': 0.0003190040588378906,
-            'storage_server.latencies.readv.10_0_percentile': 0.00032210350036621094,
-            'storage_server.latencies.get.90_0_percentile': 0.0002999305725097656,
-            'storage_server.latencies.get.01_0_percentile': 0.0001239776611328125,
-            'cpu_monitor.total': 641.4941180000001,
-            'storage_server.latencies.write.samplesize': 1000,
-            'storage_server.latencies.write.95_0_percentile': 9.489059448242188e-05,
-            'storage_server.latencies.read.50_0_percentile': 6.890296936035156e-05,
-            'storage_server.latencies.writev.01_0_percentile': 0.00033211708068847656,
-            'storage_server.latencies.read.10_0_percentile': 3.0994415283203125e-05,
-            'storage_server.latencies.allocate.10_0_percentile': 0.0004949569702148438,
-            'storage_server.reserved_space': 0,
-            'storage_server.disk_avail': 103289454592,
-            'storage_server.latencies.write.99_0_percentile': 0.00011301040649414062,
-            'storage_server.latencies.write.90_0_percentile': 9.083747863769531e-05,
-            'cpu_monitor.5min_avg': 0.0002370666691157502,
-            'storage_server.latencies.write.mean': 5.8008909225463864e-05,
-            'storage_server.latencies.readv.50_0_percentile': 0.00033020973205566406,
-            'storage_server.latencies.close.99_0_percentile': 0.0004038810729980469,
-            'storage_server.allocated': 0,
-            'storage_server.latencies.writev.99_0_percentile': 0.0007710456848144531,
-            'storage_server.latencies.readv.99_9_percentile': 0.0004780292510986328,
-            'storage_server.latencies.read.samplesize': 170,
-            'storage_server.latencies.allocate.samplesize': 406,
-            'storage_server.latencies.allocate.95_0_percentile': 0.0008411407470703125},
-          'counters': {
-            'storage_server.writev': 309,
-            'storage_server.bytes_added': 197836146,
-            'storage_server.close': 326,
-            'storage_server.readv': 14299,
-            'storage_server.allocate': 406,
-            'storage_server.read': 170,
-            'storage_server.write': 3775,
-            'storage_server.get': 472}
-          }
+            "stats": {
+                "storage_server.latencies.get.99_9_percentile": None,
+                "storage_server.latencies.close.10_0_percentile": 0.00021910667419433594,
+                "storage_server.latencies.read.01_0_percentile": 2.8848648071289062e-05,
+                "storage_server.latencies.writev.99_9_percentile": None,
+                "storage_server.latencies.read.99_9_percentile": None,
+                "storage_server.latencies.allocate.99_0_percentile": 0.000988006591796875,
+                "storage_server.latencies.writev.mean": 0.00045332245070571654,
+                "storage_server.latencies.close.99_9_percentile": None,
+                "cpu_monitor.15min_avg": 0.00017592000079223033,
+                "storage_server.disk_free_for_root": 103289454592,
+                "storage_server.latencies.get.99_0_percentile": 0.000347137451171875,
+                "storage_server.latencies.get.mean": 0.00021158285060171353,
+                "storage_server.latencies.read.90_0_percentile": 8.893013000488281e-05,
+                "storage_server.latencies.write.01_0_percentile": 3.600120544433594e-05,
+                "storage_server.latencies.write.99_9_percentile": 0.00017690658569335938,
+                "storage_server.latencies.close.90_0_percentile": 0.00033211708068847656,
+                "storage_server.disk_total": 103497859072,
+                "storage_server.latencies.close.95_0_percentile": 0.0003509521484375,
+                "storage_server.latencies.readv.samplesize": 1000,
+                "storage_server.disk_free_for_nonroot": 103289454592,
+                "storage_server.latencies.close.mean": 0.0002715024480059103,
+                "storage_server.latencies.writev.95_0_percentile": 0.0007410049438476562,
+                "storage_server.latencies.readv.90_0_percentile": 0.0003781318664550781,
+                "storage_server.latencies.readv.99_0_percentile": 0.0004050731658935547,
+                "storage_server.latencies.allocate.mean": 0.0007128627429454784,
+                "storage_server.latencies.close.samplesize": 326,
+                "storage_server.latencies.get.50_0_percentile": 0.0001819133758544922,
+                "storage_server.latencies.write.50_0_percentile": 4.482269287109375e-05,
+                "storage_server.latencies.readv.01_0_percentile": 0.0002970695495605469,
+                "storage_server.latencies.get.10_0_percentile": 0.00015687942504882812,
+                "storage_server.latencies.allocate.90_0_percentile": 0.0008189678192138672,
+                "storage_server.latencies.get.samplesize": 472,
+                "storage_server.total_bucket_count": 393,
+                "storage_server.latencies.read.mean": 5.936201880959903e-05,
+                "storage_server.latencies.allocate.01_0_percentile": 0.0004208087921142578,
+                "storage_server.latencies.allocate.99_9_percentile": None,
+                "storage_server.latencies.readv.mean": 0.00034061360359191893,
+                "storage_server.disk_used": 208404480,
+                "storage_server.latencies.allocate.50_0_percentile": 0.0007410049438476562,
+                "storage_server.latencies.read.99_0_percentile": 0.00011992454528808594,
+                "node.uptime": 3805759.8545179367,
+                "storage_server.latencies.writev.10_0_percentile": 0.00035190582275390625,
+                "storage_server.latencies.writev.90_0_percentile": 0.0006821155548095703,
+                "storage_server.latencies.close.01_0_percentile": 0.00021505355834960938,
+                "storage_server.latencies.close.50_0_percentile": 0.0002579689025878906,
+                "cpu_monitor.1min_avg": 0.0002130000000003444,
+                "storage_server.latencies.writev.50_0_percentile": 0.0004138946533203125,
+                "storage_server.latencies.read.95_0_percentile": 9.107589721679688e-05,
+                "storage_server.latencies.readv.95_0_percentile": 0.0003859996795654297,
+                "storage_server.latencies.write.10_0_percentile": 3.719329833984375e-05,
+                "storage_server.accepting_immutable_shares": 1,
+                "storage_server.latencies.writev.samplesize": 309,
+                "storage_server.latencies.get.95_0_percentile": 0.0003190040588378906,
+                "storage_server.latencies.readv.10_0_percentile": 0.00032210350036621094,
+                "storage_server.latencies.get.90_0_percentile": 0.0002999305725097656,
+                "storage_server.latencies.get.01_0_percentile": 0.0001239776611328125,
+                "cpu_monitor.total": 641.4941180000001,
+                "storage_server.latencies.write.samplesize": 1000,
+                "storage_server.latencies.write.95_0_percentile": 9.489059448242188e-05,
+                "storage_server.latencies.read.50_0_percentile": 6.890296936035156e-05,
+                "storage_server.latencies.writev.01_0_percentile": 0.00033211708068847656,
+                "storage_server.latencies.read.10_0_percentile": 3.0994415283203125e-05,
+                "storage_server.latencies.allocate.10_0_percentile": 0.0004949569702148438,
+                "storage_server.reserved_space": 0,
+                "storage_server.disk_avail": 103289454592,
+                "storage_server.latencies.write.99_0_percentile": 0.00011301040649414062,
+                "storage_server.latencies.write.90_0_percentile": 9.083747863769531e-05,
+                "cpu_monitor.5min_avg": 0.0002370666691157502,
+                "storage_server.latencies.write.mean": 5.8008909225463864e-05,
+                "storage_server.latencies.readv.50_0_percentile": 0.00033020973205566406,
+                "storage_server.latencies.close.99_0_percentile": 0.0004038810729980469,
+                "storage_server.allocated": 0,
+                "storage_server.latencies.writev.99_0_percentile": 0.0007710456848144531,
+                "storage_server.latencies.readv.99_9_percentile": 0.0004780292510986328,
+                "storage_server.latencies.read.samplesize": 170,
+                "storage_server.latencies.allocate.samplesize": 406,
+                "storage_server.latencies.allocate.95_0_percentile": 0.0008411407470703125,
+            },
+            "counters": {
+                "storage_server.writev": 309,
+                "storage_server.bytes_added": 197836146,
+                "storage_server.close": 326,
+                "storage_server.readv": 14299,
+                "storage_server.allocate": 406,
+                "storage_server.read": 170,
+                "storage_server.write": 3775,
+                "storage_server.get": 472,
+            },
+        }
         return stats
 
+
 class HackItResource(Resource, object):
     """
     A bridge between ``RequestTraversalAgent`` and ``MultiFormatResource``
@@ -143,6 +147,7 @@ class HackItResource(Resource, object):
     object to have a ``fields`` attribute but Twisted's ``IRequest`` has no
     such attribute.  Create it here.
     """
+
     def getChildWithDefault(self, path, request):
         request.fields = None
         return Resource.getChildWithDefault(self, path, request)
@@ -152,6 +157,7 @@ class OpenMetrics(SyncTestCase):
     """
     Tests for ``/statistics?t=openmetrics``.
     """
+
     def test_spec_compliance(self):
         """
         Does our output adhere to the `OpenMetrics <https://openmetrics.io/>` spec?
@@ -164,6 +170,7 @@ class OpenMetrics(SyncTestCase):
         d = rta.request(b"GET", b"http://localhost/?t=openmetrics")
         self.assertThat(d, succeeded(matches_stats(self)))
 
+
 def matches_stats(testcase):
     """
     Create a matcher that matches a response that confirms to the OpenMetrics
@@ -184,19 +191,22 @@ def matches_stats(testcase):
             code=Equals(OK),
             # "The content type MUST be..."
             headers=has_header(
-                u"content-type",
-                u"application/openmetrics-text; version=1.0.0; charset=utf-8",
+                "content-type",
+                "application/openmetrics-text; version=1.0.0; charset=utf-8",
             ),
         ),
         AfterPreprocessing(
             readBodyText,
-            succeeded(MatchesAll(
-                MatchesPredicate(add_detail(testcase, u"response body"), u"%s dummy"),
-                parses_as_openmetrics(),
-            ))
-        )
+            succeeded(
+                MatchesAll(
+                    MatchesPredicate(add_detail(testcase, "response body"), "%s dummy"),
+                    parses_as_openmetrics(),
+                )
+            ),
+        ),
     )
 
+
 def add_detail(testcase, name):
     """
     Create a matcher that always matches and as a side-effect adds the matched
@@ -206,11 +216,14 @@ def add_detail(testcase, name):
 
     :return: A matcher.
     """
+
     def predicate(value):
         testcase.addDetail(name, text_content(value))
         return True
+
     return predicate
 
+
 def readBodyText(response):
     """
     Read the response body and decode it using UTF-8.
@@ -224,6 +237,7 @@ def readBodyText(response):
     d.addCallback(lambda body: body.decode("utf-8"))
     return d
 
+
 def has_header(name, value):
     """
     Create a matcher that matches a response object that includes the given
@@ -239,6 +253,7 @@ def has_header(name, value):
         Equals([value]),
     )
 
+
 def parses_as_openmetrics():
     """
     Create a matcher that matches a ``str`` string that can be parsed as an
@@ -253,8 +268,6 @@ def parses_as_openmetrics():
         lambda body: list(parser.text_string_to_metric_families(body)),
         AfterPreprocessing(
             lambda families: families[-1].name,
-            Equals(u"tahoe_stats_storage_server_total_bucket_count"),
+            Equals("tahoe_stats_storage_server_total_bucket_count"),
         ),
     )
-
-

From 7183d53c23e3126032a59b5617259af240ead552 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 23 Sep 2021 07:58:02 -0400
Subject: [PATCH 52/95] put test dependency in the setuptools test extra

---
 setup.py | 2 ++
 tox.ini  | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/setup.py b/setup.py
index 3433e93f4..27407a403 100644
--- a/setup.py
+++ b/setup.py
@@ -404,6 +404,8 @@ setup(name="tahoe-lafs", # also set in __init__.py
               "tenacity",
               "paramiko",
               "pytest-timeout",
+              # Does our OpenMetrics endpoint adhere to the spec:
+              "prometheus-client == 0.11.0",
           ] + tor_requires + i2p_requires,
           "tor": tor_requires,
           "i2p": i2p_requires,
diff --git a/tox.ini b/tox.ini
index 0e8e58ea6..9b0f71038 100644
--- a/tox.ini
+++ b/tox.ini
@@ -52,8 +52,6 @@ deps =
      certifi
      # VCS hooks support
      py36,!coverage: pre-commit
-     # Does our OpenMetrics endpoint adhere to the spec:
-     prometheus-client==0.11.0
 
 # We add usedevelop=False because testing against a true installation gives
 # more useful results.

From ec6dfb8297f7903911f4ecfdd2278e720709af09 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Thu, 23 Sep 2021 14:20:34 -0400
Subject: [PATCH 53/95] Re-enable test.

---
 src/allmydata/test/test_istorageserver.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 328000489..a63189204 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -248,12 +248,11 @@ class IStorageServerImmutableAPIsTestsMixin(object):
             (yield buckets[2].callRemote("read", 0, 1024)), b"3" * 512 + b"4" * 512
         )
 
-    @skipIf(True, "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3801")
     def test_overlapping_writes(self):
         """
-        The policy for overlapping writes is TBD:
-        https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3801
+        Overlapping writes in immutable uploads fail with ``OverlappingWriteError``.
         """
+        1/0
 
 
 class _FoolscapMixin(SystemTestMixin):

From 1ff4e61e417a5f73f89ed46148dd3e8808a471ed Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Fri, 24 Sep 2021 10:33:26 -0400
Subject: [PATCH 54/95] Low-level tests for conflicting and non-conflicting
 writes.

---
 src/allmydata/interfaces.py        |  8 +++
 src/allmydata/storage/common.py    |  5 +-
 src/allmydata/test/test_storage.py | 96 +++++++++++++++++++++++++++++-
 3 files changed, 104 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index 1c64bce8a..8dffa9e7e 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -53,6 +53,14 @@ LeaseRenewSecret = Hash # used to protect lease renewal requests
 LeaseCancelSecret = Hash # was used to protect lease cancellation requests
 
 
+class DataTooLargeError(Exception):
+    """The write went past the expected size of the bucket."""
+
+
+class ConflictingWriteError(Exception):
+    """Two writes happened to same immutable with different data."""
+
+
 class RIBucketWriter(RemoteInterface):
     """ Objects of this kind live on the server side. """
     def write(offset=Offset, data=ShareData):
diff --git a/src/allmydata/storage/common.py b/src/allmydata/storage/common.py
index cb6116e5b..d72bb3fbc 100644
--- a/src/allmydata/storage/common.py
+++ b/src/allmydata/storage/common.py
@@ -13,8 +13,9 @@ if PY2:
 import os.path
 from allmydata.util import base32
 
-class DataTooLargeError(Exception):
-    pass
+# Backwards compatibility.
+from allmydata.interfaces import DataTooLargeError
+
 class UnknownMutableContainerVersionError(Exception):
     pass
 class UnknownImmutableContainerVersionError(Exception):
diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py
index bd0ab80f3..61494eebd 100644
--- a/src/allmydata/test/test_storage.py
+++ b/src/allmydata/test/test_storage.py
@@ -8,7 +8,7 @@ from __future__ import division
 from __future__ import print_function
 from __future__ import unicode_literals
 
-from future.utils import native_str, PY2, bytes_to_native_str
+from future.utils import native_str, PY2, bytes_to_native_str, bchr
 if PY2:
     from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
 from six import ensure_str
@@ -20,12 +20,15 @@ import stat
 import struct
 import shutil
 import gc
+from uuid import uuid4
 
 from twisted.trial import unittest
 
 from twisted.internet import defer
 from twisted.internet.task import Clock
 
+from hypothesis import given, strategies
+
 import itertools
 from allmydata import interfaces
 from allmydata.util import fileutil, hashutil, base32
@@ -33,7 +36,7 @@ from allmydata.storage.server import StorageServer, DEFAULT_RENEWAL_TIME
 from allmydata.storage.shares import get_share_file
 from allmydata.storage.mutable import MutableShareFile
 from allmydata.storage.immutable import BucketWriter, BucketReader, ShareFile
-from allmydata.storage.common import DataTooLargeError, storage_index_to_dir, \
+from allmydata.storage.common import storage_index_to_dir, \
      UnknownMutableContainerVersionError, UnknownImmutableContainerVersionError, \
      si_b2a, si_a2b
 from allmydata.storage.lease import LeaseInfo
@@ -47,7 +50,9 @@ from allmydata.mutable.layout import MDMFSlotWriteProxy, MDMFSlotReadProxy, \
                                      SIGNATURE_SIZE, \
                                      VERIFICATION_KEY_SIZE, \
                                      SHARE_HASH_CHAIN_SIZE
-from allmydata.interfaces import BadWriteEnablerError
+from allmydata.interfaces import (
+    BadWriteEnablerError, DataTooLargeError, ConflictingWriteError,
+)
 from allmydata.test.no_network import NoNetworkServer
 from allmydata.storage_client import (
     _StorageServer,
@@ -147,6 +152,91 @@ class Bucket(unittest.TestCase):
         self.failUnlessEqual(br.remote_read(25, 25), b"b"*25)
         self.failUnlessEqual(br.remote_read(50, 7), b"c"*7)
 
+    def test_write_past_size_errors(self):
+        """Writing beyond the size of the bucket throws an exception."""
+        for (i, (offset, length)) in enumerate([(0, 201), (10, 191), (202, 34)]):
+            incoming, final = self.make_workdir(
+                "test_write_past_size_errors-{}".format(i)
+            )
+            bw = BucketWriter(self, incoming, final, 200, self.make_lease(),
+                              FakeCanary())
+            with self.assertRaises(DataTooLargeError):
+                bw.remote_write(offset, b"a" * length)
+
+    @given(
+        maybe_overlapping_offset=strategies.integers(min_value=0, max_value=98),
+        maybe_overlapping_length=strategies.integers(min_value=1, max_value=100),
+    )
+    def test_overlapping_writes_ok_if_matching(
+            self, maybe_overlapping_offset, maybe_overlapping_length
+    ):
+        """
+        Writes that overlap with previous writes are OK when the content is the
+        same.
+        """
+        length = 100
+        expected_data = b"".join(bchr(i) for i in range(100))
+        incoming, final = self.make_workdir("overlapping_writes_{}".format(uuid4()))
+        bw = BucketWriter(
+            self, incoming, final, length, self.make_lease(),
+            FakeCanary()
+        )
+        # Three writes: 10-19, 30-39, 50-59. This allows for a bunch of holes.
+        bw.remote_write(10, expected_data[10:20])
+        bw.remote_write(30, expected_data[30:40])
+        bw.remote_write(50, expected_data[50:60])
+        # Then, an overlapping write but with matching data:
+        bw.remote_write(
+            maybe_overlapping_offset,
+            expected_data[
+                maybe_overlapping_offset:maybe_overlapping_offset + maybe_overlapping_length
+            ]
+        )
+        # Now fill in the holes:
+        bw.remote_write(0, expected_data[0:10])
+        bw.remote_write(20, expected_data[20:30])
+        bw.remote_write(40, expected_data[40:50])
+        bw.remote_write(60, expected_data[60:])
+        bw.remote_close()
+
+        br = BucketReader(self, bw.finalhome)
+        self.assertEqual(br.remote_read(0, length), expected_data)
+
+
+    @given(
+        maybe_overlapping_offset=strategies.integers(min_value=0, max_value=98),
+        maybe_overlapping_length=strategies.integers(min_value=1, max_value=100),
+    )
+    def test_overlapping_writes_not_ok_if_different(
+            self, maybe_overlapping_offset, maybe_overlapping_length
+    ):
+        """
+        Writes that overlap with previous writes fail with an exception if the
+        contents don't match.
+        """
+        length = 100
+        incoming, final = self.make_workdir("overlapping_writes_{}".format(uuid4()))
+        bw = BucketWriter(
+            self, incoming, final, length, self.make_lease(),
+            FakeCanary()
+        )
+        # Three writes: 10-19, 30-39, 50-59. This allows for a bunch of holes.
+        bw.remote_write(10, b"1" * 10)
+        bw.remote_write(30, b"1" * 10)
+        bw.remote_write(50, b"1" * 10)
+        # Then, write something that might overlap with some of them, but
+        # conflicts. Then fill in holes left by first three writes. Conflict is
+        # inevitable.
+        with self.assertRaises(ConflictingWriteError):
+            bw.remote_write(
+                maybe_overlapping_offset,
+                b'X' * min(maybe_overlapping_length, length - maybe_overlapping_offset),
+            )
+            bw.remote_write(0, b"1" * 10)
+            bw.remote_write(20, b"1" * 10)
+            bw.remote_write(40, b"1" * 10)
+            bw.remote_write(60, b"1" * 40)
+
     def test_read_past_end_of_share_data(self):
         # test vector for immutable files (hard-coded contents of an immutable share
         # file):

From cae989e8dec1ba76db46d2a4745aa3c3e65a7ee5 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Fri, 24 Sep 2021 11:54:03 -0400
Subject: [PATCH 55/95] News file.

---
 newsfragments/3801.bugfix | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 newsfragments/3801.bugfix

diff --git a/newsfragments/3801.bugfix b/newsfragments/3801.bugfix
new file mode 100644
index 000000000..504b3999d
--- /dev/null
+++ b/newsfragments/3801.bugfix
@@ -0,0 +1 @@
+When uploading an immutable, overlapping writes that include conflicting data are rejected. In practice, this likely didn't happen in real-world usage.
\ No newline at end of file

From 6ef3811112ac5323af70aa8b3741c0075817f9fe Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Fri, 24 Sep 2021 11:54:08 -0400
Subject: [PATCH 56/95] Prevent conflicting overlapping writes.

---
 nix/tahoe-lafs.nix                 |  2 +-
 setup.py                           |  3 +++
 src/allmydata/storage/immutable.py | 24 +++++++++++++++++++++---
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/nix/tahoe-lafs.nix b/nix/tahoe-lafs.nix
index 35b29f1cc..e08a2ab46 100644
--- a/nix/tahoe-lafs.nix
+++ b/nix/tahoe-lafs.nix
@@ -97,7 +97,7 @@ EOF
     setuptoolsTrial pyasn1 zope_interface
     service-identity pyyaml magic-wormhole treq
     eliot autobahn cryptography netifaces setuptools
-    future pyutil distro configparser
+    future pyutil distro configparser collections-extended
   ];
 
   checkInputs = with python.pkgs; [
diff --git a/setup.py b/setup.py
index e1d711ccf..8241898fe 100644
--- a/setup.py
+++ b/setup.py
@@ -137,6 +137,9 @@ install_requires = [
 
     # Backported configparser for Python 2:
     "configparser ; python_version < '3.0'",
+
+    # For the RangeMap datastructure.
+    "collections-extended",
 ]
 
 setup_requires = [
diff --git a/src/allmydata/storage/immutable.py b/src/allmydata/storage/immutable.py
index 4b60d79f1..9e3a9622a 100644
--- a/src/allmydata/storage/immutable.py
+++ b/src/allmydata/storage/immutable.py
@@ -13,16 +13,20 @@ if PY2:
 
 import os, stat, struct, time
 
+from collections_extended import RangeMap
+
 from foolscap.api import Referenceable
 
 from zope.interface import implementer
-from allmydata.interfaces import RIBucketWriter, RIBucketReader
+from allmydata.interfaces import (
+    RIBucketWriter, RIBucketReader, ConflictingWriteError,
+    DataTooLargeError,
+)
 from allmydata.util import base32, fileutil, log
 from allmydata.util.assertutil import precondition
 from allmydata.util.hashutil import timing_safe_compare
 from allmydata.storage.lease import LeaseInfo
-from allmydata.storage.common import UnknownImmutableContainerVersionError, \
-     DataTooLargeError
+from allmydata.storage.common import UnknownImmutableContainerVersionError
 
 # each share file (in storage/shares/$SI/$SHNUM) contains lease information
 # and share data. The share data is accessed by RIBucketWriter.write and
@@ -217,6 +221,7 @@ class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78
         # also, add our lease to the file now, so that other ones can be
         # added by simultaneous uploaders
         self._sharefile.add_lease(lease_info)
+        self._already_written = RangeMap()
 
     def allocated_size(self):
         return self._max_size
@@ -226,7 +231,20 @@ class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78
         precondition(not self.closed)
         if self.throw_out_all_data:
             return
+
+        # Make sure we're not conflicting with existing data:
+        end = offset + len(data)
+        for (chunk_start, chunk_stop, _) in self._already_written.ranges(offset, end):
+            chunk_len = chunk_stop - chunk_start
+            actual_chunk = self._sharefile.read_share_data(chunk_start, chunk_len)
+            writing_chunk = data[chunk_start - offset:chunk_stop - offset]
+            if actual_chunk != writing_chunk:
+                raise ConflictingWriteError(
+                    "Chunk {}-{} doesn't match already written data.".format(chunk_start, chunk_stop)
+                )
         self._sharefile.write_share_data(offset, data)
+
+        self._already_written.set(True, offset, end)
         self.ss.add_latency("write", time.time() - start)
         self.ss.count("write")
 

From c1f8e9f8c763011cd73f1bb7c499fffe1ccf7171 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Fri, 24 Sep 2021 12:02:30 -0400
Subject: [PATCH 57/95] IStorageServer test for overlapping writes.

---
 src/allmydata/test/test_istorageserver.py | 26 ++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index a63189204..ef93dadf5 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -24,7 +24,7 @@ from testtools import skipIf
 
 from twisted.internet.defer import inlineCallbacks
 
-from foolscap.api import Referenceable
+from foolscap.api import Referenceable, RemoteException
 
 from allmydata.interfaces import IStorageServer
 from .common_system import SystemTestMixin
@@ -248,11 +248,31 @@ class IStorageServerImmutableAPIsTestsMixin(object):
             (yield buckets[2].callRemote("read", 0, 1024)), b"3" * 512 + b"4" * 512
         )
 
+    @inlineCallbacks
     def test_overlapping_writes(self):
         """
-        Overlapping writes in immutable uploads fail with ``OverlappingWriteError``.
+        Overlapping, non-identical writes in immutable uploads fail.
         """
-        1/0
+        storage_index, renew_secret, cancel_secret = (
+            new_storage_index(),
+            new_secret(),
+            new_secret(),
+        )
+        (_, allocated) = yield self.storage_server.allocate_buckets(
+            storage_index,
+            renew_secret,
+            cancel_secret,
+            sharenums={0},
+            allocated_size=30,
+            canary=Referenceable(),
+        )
+
+        yield allocated[0].callRemote("write", 0, b"1" * 10)
+        # Overlapping write that matches:
+        yield allocated[0].callRemote("write", 5, b"1" * 20)
+        # Overlapping write that doesn't match:
+        with self.assertRaises(RemoteException):
+            yield allocated[0].callRemote("write", 20, b"2" * 10)
 
 
 class _FoolscapMixin(SystemTestMixin):

From 0b1082fc04ffa56a808bddb7fa519665a6135e66 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Fri, 24 Sep 2021 12:04:12 -0400
Subject: [PATCH 58/95] Fix lint.

---
 src/allmydata/storage/common.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/allmydata/storage/common.py b/src/allmydata/storage/common.py
index d72bb3fbc..e5563647f 100644
--- a/src/allmydata/storage/common.py
+++ b/src/allmydata/storage/common.py
@@ -14,7 +14,7 @@ import os.path
 from allmydata.util import base32
 
 # Backwards compatibility.
-from allmydata.interfaces import DataTooLargeError
+from allmydata.interfaces import DataTooLargeError  # noqa: F401
 
 class UnknownMutableContainerVersionError(Exception):
     pass

From 96acb1419977bd64898beb08e93a3c6e58505e8f Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Fri, 24 Sep 2021 13:48:07 -0400
Subject: [PATCH 59/95] Document impact of semantic changes on HTTP protocol.

---
 docs/proposed/http-storage-node-protocol.rst | 28 +++++++++++---------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst
index a84d62176..bf01bcf12 100644
--- a/docs/proposed/http-storage-node-protocol.rst
+++ b/docs/proposed/http-storage-node-protocol.rst
@@ -497,22 +497,26 @@ If any one of these requests fails then at most 128KiB of upload work needs to b
 
 The server must recognize when all of the data has been received and mark the share as complete
 (which it can do because it was informed of the size when the storage index was initialized).
-Clients should upload chunks in re-assembly order.
 
 * When a chunk that does not complete the share is successfully uploaded the response is ``OK``.
 * When the chunk that completes the share is successfully uploaded the response is ``CREATED``.
-* If the *Content-Range* for a request covers part of the share that has already been uploaded the response is ``CONFLICT``.
-  The response body indicates the range of share data that has yet to be uploaded.
-  That is::
+* If the *Content-Range* for a request covers part of the share that has already,
+  and the data does not match already written data,
+  the response is ``CONFLICT``.
+  At this point the only thing to do is abort the upload and start from scratch (see below).
 
-    { "required":
-      [ { "begin": <byte position, inclusive>
-        , "end":   <byte position, exclusive>
-        }
-      ,
-      ...
-      ]
-    }
+``PUT /v1/immutable/:storage_index/:share_number/abort``
+!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+
+This cancels an *in-progress* upload.
+
+The response code:
+
+* When the upload is still in progress and therefore the abort has succeeded,
+  the response is ``OK``.
+  Future uploads can start from scratch with no pre-existing upload state stored on the server.
+* If the uploaded has already finished, the response is 405 (Method Not Allowed)
+  and no change is made.
 
 
 ``POST /v1/immutable/:storage_index/:share_number/corrupt``

From b1b64c787ebd84d12df754d7455c46a0062adc04 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 27 Sep 2021 15:00:52 -0400
Subject: [PATCH 60/95] Add more randomness.

---
 src/allmydata/test/test_istorageserver.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index e35fbf29d..355a07841 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -300,7 +300,7 @@ class IStorageServerImmutableAPIsTestsMixin(object):
             canary=Referenceable(),
         )
 
-        total_data = _randbytes(256) * 17
+        total_data = _randbytes(256 * 17)
         yield allocated[0].callRemote("write", 0, total_data)
         yield allocated[0].callRemote("close")
 

From da937cef9e74f67c0c6abe388ae8f14075276059 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 27 Sep 2021 15:04:04 -0400
Subject: [PATCH 61/95] Correct docstring.

---
 src/allmydata/interfaces.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index 0283b443a..7f7564550 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -353,8 +353,10 @@ class IStorageServer(Interface):
         :see: ``RIStorageServer.slot_testv_readv_and_writev``
 
         While the interface mostly matches, test vectors are simplified.
-        Instead of a tuple ``(start, offset, operator, data)`` they are just
-        ``(start, data_length, data)`` with operator implicitly being ``b"eq"``.
+        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(

From c740da4acde9f70a6a765235eb1dd061d730dcc2 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 27 Sep 2021 15:09:24 -0400
Subject: [PATCH 62/95] Explain what happens in reads past the end.

---
 docs/proposed/http-storage-node-protocol.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst
index 35811cad0..9d419657b 100644
--- a/docs/proposed/http-storage-node-protocol.rst
+++ b/docs/proposed/http-storage-node-protocol.rst
@@ -625,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
 ~~~~~~~
 

From 38e449aceb611661e899b4dec0babf166ddd5f09 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 27 Sep 2021 16:44:43 -0400
Subject: [PATCH 63/95] Add collections-extended.

---
 nix/collections-extended.nix | 19 +++++++++++++++++++
 nix/overlays.nix             |  3 +++
 2 files changed, 22 insertions(+)
 create mode 100644 nix/collections-extended.nix

diff --git a/nix/collections-extended.nix b/nix/collections-extended.nix
new file mode 100644
index 000000000..3f1ad165a
--- /dev/null
+++ b/nix/collections-extended.nix
@@ -0,0 +1,19 @@
+{ lib, buildPythonPackage, fetchPypi }:
+buildPythonPackage rec {
+  pname = "collections-extended";
+  version = "1.0.3";
+
+  src = fetchPypi {
+    inherit pname version;
+    sha256 = "0lb69x23asd68n0dgw6lzxfclavrp2764xsnh45jm97njdplznkw";
+  };
+
+  # Tests aren't in tarball, for 1.0.3 at least.
+  doCheck = false;
+
+  meta = with lib; {
+    homepage = https://github.com/mlenzen/collections-extended;
+    description = "Extra Python Collections - bags (multisets), setlists (unique list / indexed set), RangeMap and IndexedDict";
+    license = licenses.asl20;
+  };
+}
diff --git a/nix/overlays.nix b/nix/overlays.nix
index 2bf58575e..aae808131 100644
--- a/nix/overlays.nix
+++ b/nix/overlays.nix
@@ -18,6 +18,9 @@ self: super: {
 
       # Need a newer version of Twisted, too.
       twisted = python-super.callPackage ./twisted.nix { };
+
+      # collections-extended is not part of nixpkgs at this time.
+      collections-extended = python-super.callPackage ./collections-extended.nix
     };
   };
 }

From 4e6438352ad95fe332bce67f817aa49d91a20001 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 27 Sep 2021 16:45:01 -0400
Subject: [PATCH 64/95] Extend to end.

---
 docs/proposed/http-storage-node-protocol.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst
index bf01bcf12..d91687960 100644
--- a/docs/proposed/http-storage-node-protocol.rst
+++ b/docs/proposed/http-storage-node-protocol.rst
@@ -506,7 +506,7 @@ The server must recognize when all of the data has been received and mark the sh
   At this point the only thing to do is abort the upload and start from scratch (see below).
 
 ``PUT /v1/immutable/:storage_index/:share_number/abort``
-!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
 This cancels an *in-progress* upload.
 

From 60cb3c08836603016f4fb9e59e075cb34a92f1cb Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 27 Sep 2021 16:52:25 -0400
Subject: [PATCH 65/95] Restore range result.

---
 docs/proposed/http-storage-node-protocol.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst
index d91687960..2984b5c6d 100644
--- a/docs/proposed/http-storage-node-protocol.rst
+++ b/docs/proposed/http-storage-node-protocol.rst
@@ -499,6 +499,18 @@ The server must recognize when all of the data has been received and mark the sh
 (which it can do because it was informed of the size when the storage index was initialized).
 
 * When a chunk that does not complete the share is successfully uploaded the response is ``OK``.
+  The response body indicates the range of share data that has yet to be uploaded.
+  That is::
+
+    { "required":
+      [ { "begin": <byte position, inclusive>
+        , "end":   <byte position, exclusive>
+        }
+      ,
+      ...
+      ]
+    }
+
 * When the chunk that completes the share is successfully uploaded the response is ``CREATED``.
 * If the *Content-Range* for a request covers part of the share that has already,
   and the data does not match already written data,

From de1a7d7fcee4a400c791f28f1afb32493bb3e43e Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 27 Sep 2021 16:56:24 -0400
Subject: [PATCH 66/95] A more explicit test for successful overlapping.

---
 src/allmydata/test/test_istorageserver.py | 39 ++++++++++++++++++++---
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index ef93dadf5..9e9a41d35 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -249,9 +249,10 @@ class IStorageServerImmutableAPIsTestsMixin(object):
         )
 
     @inlineCallbacks
-    def test_overlapping_writes(self):
+    def test_non_matching_overlapping_writes(self):
         """
-        Overlapping, non-identical writes in immutable uploads fail.
+        When doing overlapping writes in immutable uploads, non-matching writes
+        fail.
         """
         storage_index, renew_secret, cancel_secret = (
             new_storage_index(),
@@ -267,13 +268,41 @@ class IStorageServerImmutableAPIsTestsMixin(object):
             canary=Referenceable(),
         )
 
-        yield allocated[0].callRemote("write", 0, b"1" * 10)
-        # Overlapping write that matches:
-        yield allocated[0].callRemote("write", 5, b"1" * 20)
+        yield allocated[0].callRemote("write", 0, b"1" * 25)
         # Overlapping write that doesn't match:
         with self.assertRaises(RemoteException):
             yield allocated[0].callRemote("write", 20, b"2" * 10)
 
+    @inlineCallbacks
+    def test_matching_overlapping_writes(self):
+        """
+        When doing overlapping writes in immutable uploads, matching writes
+        succeed.
+        """
+        storage_index, renew_secret, cancel_secret = (
+            new_storage_index(),
+            new_secret(),
+            new_secret(),
+        )
+        (_, allocated) = yield self.storage_server.allocate_buckets(
+            storage_index,
+            renew_secret,
+            cancel_secret,
+            sharenums={0},
+            allocated_size=25,
+            canary=Referenceable(),
+        )
+
+        yield allocated[0].callRemote("write", 0, b"1" * 10)
+        # Overlapping write that matches:
+        yield allocated[0].callRemote("write", 5, b"1" * 20)
+        yield allocated[0].callRemote("close")
+
+        buckets = yield self.storage_server.get_buckets(storage_index)
+        self.assertEqual(set(buckets.keys()), {0})
+
+        self.assertEqual((yield buckets[0].callRemote("read", 0, 25)), b"1" * 25)
+
 
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""

From f66f3e64ada92a37c15d7d2e41c7a37cc78c2c41 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 27 Sep 2021 16:58:18 -0400
Subject: [PATCH 67/95] Fix syntax.

---
 nix/overlays.nix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nix/overlays.nix b/nix/overlays.nix
index aae808131..14a47ca5a 100644
--- a/nix/overlays.nix
+++ b/nix/overlays.nix
@@ -20,7 +20,7 @@ self: super: {
       twisted = python-super.callPackage ./twisted.nix { };
 
       # collections-extended is not part of nixpkgs at this time.
-      collections-extended = python-super.callPackage ./collections-extended.nix
+      collections-extended = python-super.callPackage ./collections-extended.nix { };
     };
   };
 }

From 9f80435b41b246c7675a5586292d17931e392791 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 27 Sep 2021 17:04:22 -0400
Subject: [PATCH 68/95] Update to new interface.

---
 src/allmydata/test/test_istorageserver.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 98c096b3a..2aad5786c 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -458,7 +458,7 @@ class IStorageServerMutableAPIsTestsMixin(object):
             secrets,
             tw_vectors={
                 0: (
-                    [(0, 3, b"eq", b"1" * 3), (3, 4, b"eq", b"1" * 4)],
+                    [(0, 3, b"1" * 3), (3, 4, b"1" * 4)],
                     [(0, b"2" * 7)],
                     7,
                 ),
@@ -479,7 +479,7 @@ class IStorageServerMutableAPIsTestsMixin(object):
             storage_index,
             secrets,
             tw_vectors={
-                0: ([(0, 7, b"eq", b"1" * 7)], [(0, b"3" * 7)], 7),
+                0: ([(0, 7, b"1" * 7)], [(0, b"3" * 7)], 7),
             },
             r_vector=[],
         )

From 914ca567758bad81ae91844a525a143d975fe638 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 27 Sep 2021 17:05:03 -0400
Subject: [PATCH 69/95] TODOs.

---
 src/allmydata/test/test_istorageserver.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 2aad5786c..615a1c24f 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -568,7 +568,12 @@ class IStorageServerMutableAPIsTestsMixin(object):
         )
         self.assertEqual(reads, {})
 
+    # TODO detection of empty/missing shares with length 1 reads that expect
+    # empty bytestring.
 
+    # TODO what else?
+
+    
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""
 

From c668e342c294a1c479f2f95ad3b020861f7c94a3 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 28 Sep 2021 09:51:40 -0400
Subject: [PATCH 70/95] More STARAW tests.

---
 src/allmydata/test/test_istorageserver.py | 71 +++++++++++++++++++++--
 1 file changed, 65 insertions(+), 6 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 615a1c24f..c606afcc1 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -352,6 +352,7 @@ class IStorageServerMutableAPIsTestsMixin(object):
 
     ``STARAW`` is short for ``slot_testv_and_readv_and_writev``.
     """
+
     def new_secrets(self):
         """Return a 3-tuple of secrets for STARAW calls."""
         return (new_secret(), new_secret(), new_secret())
@@ -434,7 +435,7 @@ class IStorageServerMutableAPIsTestsMixin(object):
         self.assertEqual(reads, {0: [b"X" * 7]})
 
     @inlineCallbacks
-    def test_SATRAW_writen_happens_only_if_test_matches(self):
+    def test_SATRAW_writes_happens_only_if_test_matches(self):
         """
         If a ``IStorageServer.slot_testv_and_readv_and_writev`` includes both a
         test and a write, the write succeeds if the test matches, and fails if
@@ -492,6 +493,69 @@ class IStorageServerMutableAPIsTestsMixin(object):
         )
         self.assertEqual(reads, {0: [b"2" * 7]})
 
+    @inlineCallbacks
+    def test_SATRAW_tests_past_end_of_data(self):
+        """
+        If a ``IStorageServer.slot_testv_and_readv_and_writev`` includes a test
+        vector that reads past the end of the data, the result is limited to
+        actual available data.
+        """
+        secrets = self.new_secrets()
+        storage_index = new_storage_index()
+
+        # Since there is no data on server, the test vector will return empty
+        # string, which matches expected result, so write will succeed.
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([(0, 10, b"")], [(0, b"1" * 7)], 7),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+        # Now the test vector is a 10-read off of a 7-byte value, but expected
+        # value is still 7 bytes, so the write will again succeed.
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([(0, 10, b"1" * 7)], [(0, b"2" * 7)], 7),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+    @inlineCallbacks
+    def test_SATRAW_reads_past_end_of_data(self):
+        """
+        If a ``IStorageServer.slot_testv_and_readv_and_writev`` reads past the
+        end of the data, the result is limited to actual available data.
+        """
+        secrets = self.new_secrets()
+        storage_index = new_storage_index()
+
+        # Write some data
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([], [(0, b"12345")], 5),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+        # Reads past end.
+        (_, reads) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={},
+            r_vector=[(0, 100), (2, 50)],
+        )
+        self.assertEqual(reads, {0: [b"12345", b"345"]})
+
     @inlineCallbacks
     def test_STARAW_write_enabler_must_match(self):
         """
@@ -568,12 +632,7 @@ class IStorageServerMutableAPIsTestsMixin(object):
         )
         self.assertEqual(reads, {})
 
-    # TODO detection of empty/missing shares with length 1 reads that expect
-    # empty bytestring.
 
-    # TODO what else?
-
-    
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""
 

From 8e5928a39e6f0e982871a27d30a34a614fff9e38 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 28 Sep 2021 10:03:49 -0400
Subject: [PATCH 71/95] News file.

---
 newsfragments/3805.minor | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 newsfragments/3805.minor

diff --git a/newsfragments/3805.minor b/newsfragments/3805.minor
new file mode 100644
index 000000000..e69de29bb

From 7e232f602a97592dc62ffa1033a84fcf6090cc75 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 28 Sep 2021 10:34:57 -0400
Subject: [PATCH 72/95] Switch to PATCH.

---
 docs/proposed/http-storage-node-protocol.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst
index 9d419657b..56f383e2c 100644
--- a/docs/proposed/http-storage-node-protocol.rst
+++ b/docs/proposed/http-storage-node-protocol.rst
@@ -482,7 +482,7 @@ The response includes ``already-have`` and ``allocated`` for two reasons:
   This might be because a server has become unavailable and a remaining server needs to store more shares for the upload.
   It could also just be that the client's preferred servers have changed.
 
-``PUT /v1/immutable/:storage_index/:share_number``
+``PATCH /v1/immutable/:storage_index/:share_number``
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
 Write data for the indicated share.
@@ -668,19 +668,19 @@ Immutable Data
 
 #. Upload the content for immutable share ``7``::
 
-     PUT /v1/immutable/AAAAAAAAAAAAAAAA/7
+     PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7
      Content-Range: bytes 0-15/48
      <first 16 bytes of share data>
 
      200 OK
 
-     PUT /v1/immutable/AAAAAAAAAAAAAAAA/7
+     PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7
      Content-Range: bytes 16-31/48
      <second 16 bytes of share data>
 
      200 OK
 
-     PUT /v1/immutable/AAAAAAAAAAAAAAAA/7
+     PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7
      Content-Range: bytes 32-47/48
      <final 16 bytes of share data>
 

From ed290caeeca024c73bb15982709e4caf28a10014 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 28 Sep 2021 10:35:10 -0400
Subject: [PATCH 73/95] News file.

---
 newsfragments/3806.minor | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 newsfragments/3806.minor

diff --git a/newsfragments/3806.minor b/newsfragments/3806.minor
new file mode 100644
index 000000000..e69de29bb

From bb85a9d2cf046dcd3d49312e29ffe4cd5ae6a672 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 28 Sep 2021 10:37:16 -0400
Subject: [PATCH 74/95] !!

---
 docs/proposed/http-storage-node-protocol.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst
index 56f383e2c..e3bd48128 100644
--- a/docs/proposed/http-storage-node-protocol.rst
+++ b/docs/proposed/http-storage-node-protocol.rst
@@ -483,7 +483,7 @@ The response includes ``already-have`` and ``allocated`` for two reasons:
   It could also just be that the client's preferred servers have changed.
 
 ``PATCH /v1/immutable/:storage_index/:share_number``
-!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
 Write data for the indicated share.
 The share number must belong to the storage index.

From 9970559019451c6a5bf68d1f4d06baeac430092e Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 28 Sep 2021 10:46:21 -0400
Subject: [PATCH 75/95] Tests for slot_readv.

---
 src/allmydata/test/test_istorageserver.py | 62 +++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index c606afcc1..931cf6ed6 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -632,6 +632,68 @@ class IStorageServerMutableAPIsTestsMixin(object):
         )
         self.assertEqual(reads, {})
 
+    @inlineCallbacks
+    def test_slot_readv(self):
+        """
+        Data written with ``IStorageServer.slot_testv_and_readv_and_writev()``
+        can be read using ``IStorageServer.slot_readv()``.  Reads can't go past
+        the end of the data.
+        """
+        secrets = self.new_secrets()
+        storage_index = new_storage_index()
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([], [(0, b"abcdefg")], 7),
+                1: ([], [(0, b"0123"), (4, b"456")], 7),
+                2: ([], [(0, b"0123")], 4),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+        reads = yield self.storage_server.slot_readv(
+            storage_index,
+            shares=[0, 1],
+            # Whole thing, partial, going beyond the edge, completely outside
+            # range:
+            readv=[(0, 7), (2, 3), (6, 8), (100, 10)],
+        )
+        self.assertEqual(
+            reads,
+            {0: [b"abcdefg", b"cde", b"g", b""], 1: [b"0123456", b"234", b"6", b""]},
+        )
+
+    @inlineCallbacks
+    def test_slot_readv_no_shares(self):
+        """
+        With no shares given, ``IStorageServer.slot_readv()`` reads from all shares.
+        """
+        secrets = self.new_secrets()
+        storage_index = new_storage_index()
+        (written, _) = yield self.staraw(
+            storage_index,
+            secrets,
+            tw_vectors={
+                0: ([], [(0, b"abcdefg")], 7),
+                1: ([], [(0, b"0123456")], 7),
+                2: ([], [(0, b"9876543")], 7),
+            },
+            r_vector=[],
+        )
+        self.assertEqual(written, True)
+
+        reads = yield self.storage_server.slot_readv(
+            storage_index,
+            shares=[],
+            readv=[(0, 7)],
+        )
+        self.assertEqual(
+            reads,
+            {0: [b"abcdefg"], 1: [b"0123456"], 2: [b"9876543"]},
+        )
+
 
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""

From a02d5f4c9cb81e46c461dda417be327b2de604a9 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 28 Sep 2021 13:02:01 -0400
Subject: [PATCH 76/95] Just stick to current behavior.

---
 src/allmydata/test/test_istorageserver.py | 44 +++--------------------
 1 file changed, 4 insertions(+), 40 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 7c6090980..be327770e 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -98,13 +98,10 @@ class IStorageServerImmutableAPIsTestsMixin(object):
         # We validate the bucket objects' interface in a later test.
 
     @inlineCallbacks
-    @skipIf(True, "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793")
     def test_allocate_buckets_repeat(self):
         """
-        allocate_buckets() with the same storage index returns the same result,
-        because the shares have not been written to.
-
-        This fails due to https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793
+        allocate_buckets() with the same storage index does not return
+        work-in-progress buckets, but will add any newly added buckets.
         """
         storage_index, renew_secret, cancel_secret = (
             new_storage_index(),
@@ -115,7 +112,7 @@ class IStorageServerImmutableAPIsTestsMixin(object):
             storage_index,
             renew_secret,
             cancel_secret,
-            sharenums=set(range(5)),
+            sharenums=set(range(4)),
             allocated_size=1024,
             canary=Referenceable(),
         )
@@ -128,40 +125,7 @@ class IStorageServerImmutableAPIsTestsMixin(object):
             Referenceable(),
         )
         self.assertEqual(already_got, already_got2)
-        self.assertEqual(set(allocated.keys()), set(allocated2.keys()))
-
-    @skipIf(True, "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793")
-    @inlineCallbacks
-    def test_allocate_buckets_more_sharenums(self):
-        """
-        allocate_buckets() with the same storage index but more sharenums
-        acknowledges the extra shares don't exist.
-
-        Fails due to https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793
-        """
-        storage_index, renew_secret, cancel_secret = (
-            new_storage_index(),
-            new_secret(),
-            new_secret(),
-        )
-        yield self.storage_server.allocate_buckets(
-            storage_index,
-            renew_secret,
-            cancel_secret,
-            sharenums=set(range(5)),
-            allocated_size=1024,
-            canary=Referenceable(),
-        )
-        (already_got2, allocated2) = yield self.storage_server.allocate_buckets(
-            storage_index,
-            renew_secret,
-            cancel_secret,
-            sharenums=set(range(7)),
-            allocated_size=1024,
-            canary=Referenceable(),
-        )
-        self.assertEqual(already_got2, set())  # none were fully written
-        self.assertEqual(set(allocated2.keys()), set(range(7)))
+        self.assertEqual(set(allocated2.keys()), {4})
 
     @inlineCallbacks
     def test_written_shares_are_allocated(self):

From 2b1502eff601a5c43b2d7f2fb9ad6de4959a4fc5 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 28 Sep 2021 13:02:24 -0400
Subject: [PATCH 77/95] News file.

---
 newsfragments/3793.minor | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 newsfragments/3793.minor

diff --git a/newsfragments/3793.minor b/newsfragments/3793.minor
new file mode 100644
index 000000000..e69de29bb

From e64c397fc5d22052c6268e14704041ed0c85f7f6 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Tue, 28 Sep 2021 13:51:31 -0400
Subject: [PATCH 78/95] WIP disconnection test.

---
 src/allmydata/test/test_istorageserver.py | 75 +++++++++++++++++++++--
 1 file changed, 69 insertions(+), 6 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index be327770e..daf264286 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -20,9 +20,9 @@ if PY2:
 
 from random import Random
 
-from testtools import skipIf
-
 from twisted.internet.defer import inlineCallbacks
+from twisted.internet import reactor
+from twisted.internet.task import deferLater
 
 from foolscap.api import Referenceable, RemoteException
 
@@ -77,6 +77,10 @@ class IStorageServerImmutableAPIsTestsMixin(object):
     Tests for ``IStorageServer``'s immutable APIs.
 
     ``self.storage_server`` is expected to provide ``IStorageServer``.
+
+    ``self.disconnect()`` should disconnect and then reconnect, creating a new
+    ``self.storage_server``.  Some implementations may wish to skip tests using
+    this; HTTP has no notion of disconnection.
     """
 
     @inlineCallbacks
@@ -100,7 +104,7 @@ class IStorageServerImmutableAPIsTestsMixin(object):
     @inlineCallbacks
     def test_allocate_buckets_repeat(self):
         """
-        allocate_buckets() with the same storage index does not return
+        ``IStorageServer.allocate_buckets()`` with the same storage index does not return
         work-in-progress buckets, but will add any newly added buckets.
         """
         storage_index, renew_secret, cancel_secret = (
@@ -127,6 +131,45 @@ class IStorageServerImmutableAPIsTestsMixin(object):
         self.assertEqual(already_got, already_got2)
         self.assertEqual(set(allocated2.keys()), {4})
 
+    @inlineCallbacks
+    def test_disconnection(self):
+        """
+        If we disconnect in the middle of writing to a bucket, all data is
+        wiped, and it's even possible to write different data to the bucket
+        (don't do that though, mostly it's just a good way to test that the
+        data really was wiped).
+        """
+        storage_index, renew_secret, cancel_secret = (
+            new_storage_index(),
+            new_secret(),
+            new_secret(),
+        )
+        (_, allocated) = yield self.storage_server.allocate_buckets(
+            storage_index,
+            renew_secret,
+            cancel_secret,
+            sharenums={0},
+            allocated_size=1024,
+            canary=Referenceable(),
+        )
+
+        # Bucket 1 is fully written in one go.
+        yield allocated[0].callRemote("write", 0, b"1" * 1024)
+
+        # Disconnect:
+        yield self.disconnect()
+
+        # Write different data with no complaint:
+        (_, allocated) = yield self.storage_server.allocate_buckets(
+            storage_index,
+            renew_secret,
+            cancel_secret,
+            sharenums={0},
+            allocated_size=1024,
+            canary=Referenceable(),
+        )
+        yield allocated[0].callRemote("write", 0, b"2" * 1024)
+
     @inlineCallbacks
     def test_written_shares_are_allocated(self):
         """
@@ -359,15 +402,16 @@ class IStorageServerImmutableAPIsTestsMixin(object):
 class _FoolscapMixin(SystemTestMixin):
     """Run tests on Foolscap version of ``IStorageServer."""
 
+    def _get_native_server(self):
+        return next(iter(self.clients[0].storage_broker.get_known_servers()))
+
     @inlineCallbacks
     def setUp(self):
         AsyncTestCase.setUp(self)
         self.basedir = "test_istorageserver/" + self.id()
         yield SystemTestMixin.setUp(self)
         yield self.set_up_nodes(1)
-        self.storage_server = next(
-            iter(self.clients[0].storage_broker.get_known_servers())
-        ).get_storage_server()
+        self.storage_server = self._get_native_server().get_storage_server()
         self.assertTrue(IStorageServer.providedBy(self.storage_server))
 
     @inlineCallbacks
@@ -375,6 +419,25 @@ class _FoolscapMixin(SystemTestMixin):
         AsyncTestCase.tearDown(self)
         yield SystemTestMixin.tearDown(self)
 
+    @inlineCallbacks
+    def disconnect(self):
+        """
+        Disconnect and then reconnect with a new ``IStorageServer``.
+        """
+        current = self.storage_server
+        self._get_native_server()._rref.tracker.broker.transport.loseConnection()
+        for i in range(100000):
+            yield deferLater(reactor, 0.001)
+            import pdb
+
+            pdb.set_trace()
+            new = self._get_native_server().get_storage_server()
+            if new is not None and new is not current:
+                self.storage_server = new
+                return
+
+        raise RuntimeError("Failed to reconnect")
+
 
 class FoolscapSharedAPIsTests(
     _FoolscapMixin, IStorageServerSharedAPIsTestsMixin, AsyncTestCase

From 51e8b5e197e07ec2247c75212e0df342bc3a091d Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 29 Sep 2021 11:17:33 -0400
Subject: [PATCH 79/95] Disconnection test works now.

---
 src/allmydata/test/test_istorageserver.py | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index daf264286..25b814237 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -135,9 +135,10 @@ class IStorageServerImmutableAPIsTestsMixin(object):
     def test_disconnection(self):
         """
         If we disconnect in the middle of writing to a bucket, all data is
-        wiped, and it's even possible to write different data to the bucket
-        (don't do that though, mostly it's just a good way to test that the
-        data really was wiped).
+        wiped, and it's even possible to write different data to the bucket.
+
+        (In the real world one shouldn't do that, but writing different data is
+        a good way to test that the original data really was wiped.)
         """
         storage_index, renew_secret, cancel_secret = (
             new_storage_index(),
@@ -425,18 +426,9 @@ class _FoolscapMixin(SystemTestMixin):
         Disconnect and then reconnect with a new ``IStorageServer``.
         """
         current = self.storage_server
-        self._get_native_server()._rref.tracker.broker.transport.loseConnection()
-        for i in range(100000):
-            yield deferLater(reactor, 0.001)
-            import pdb
-
-            pdb.set_trace()
-            new = self._get_native_server().get_storage_server()
-            if new is not None and new is not current:
-                self.storage_server = new
-                return
-
-        raise RuntimeError("Failed to reconnect")
+        yield self.bounce_client(0)
+        self.storage_server = self._get_native_server().get_storage_server()
+        assert self.storage_server is not current
 
 
 class FoolscapSharedAPIsTests(

From a4153b71256db2dd4094e2e21324dd2b8c9cfcab Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 29 Sep 2021 11:56:04 -0400
Subject: [PATCH 80/95] Implementation plan.

---
 src/allmydata/storage/immutable.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/allmydata/storage/immutable.py b/src/allmydata/storage/immutable.py
index 9e3a9622a..c2b190e01 100644
--- a/src/allmydata/storage/immutable.py
+++ b/src/allmydata/storage/immutable.py
@@ -204,6 +204,16 @@ class ShareFile(object):
             self.unlink()
         return space_freed
 
+# TODOs
+# Batch 1:
+# - bucketwriter dict in the server, to persist them + TEST of persistence
+# - aborting bucketwriter removes it from server persistent + TEST
+# - get rid of disconnect notification (probably no test, rely on existing?)
+# - add bucketwriter cancellation to remote_allocate_buckets() (probably rely on existing tests)
+# Batch 2:
+# - scheduled events for aborting bucketwriter + TEST
+# - bucketwriter writes delay cancellation + TEST
+
 
 @implementer(RIBucketWriter)
 class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78

From 8fb6afee1bc2ff5509695c5d53b2d5dd3d72ed08 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 29 Sep 2021 13:42:17 -0400
Subject: [PATCH 81/95] Refactor BucketWriters such that disconnection can be
 limited Foolscap.

---
 src/allmydata/storage/immutable.py | 14 +++-----
 src/allmydata/storage/server.py    | 54 ++++++++++++++++++++++++------
 src/allmydata/test/common_util.py  |  5 +++
 src/allmydata/test/test_storage.py | 34 ++++++-------------
 4 files changed, 65 insertions(+), 42 deletions(-)

diff --git a/src/allmydata/storage/immutable.py b/src/allmydata/storage/immutable.py
index c2b190e01..d3b6ce875 100644
--- a/src/allmydata/storage/immutable.py
+++ b/src/allmydata/storage/immutable.py
@@ -208,8 +208,9 @@ class ShareFile(object):
 # Batch 1:
 # - bucketwriter dict in the server, to persist them + TEST of persistence
 # - aborting bucketwriter removes it from server persistent + TEST
-# - get rid of disconnect notification (probably no test, rely on existing?)
-# - add bucketwriter cancellation to remote_allocate_buckets() (probably rely on existing tests)
+# - disconnect still aborts _for Foolscap only_
+# - existing in-use buckets are not returned _for Foolscap only_
+# - this implies splitting remote_allocate_buckets into generic and Foolscap-y parts
 # Batch 2:
 # - scheduled events for aborting bucketwriter + TEST
 # - bucketwriter writes delay cancellation + TEST
@@ -218,13 +219,11 @@ class ShareFile(object):
 @implementer(RIBucketWriter)
 class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78
 
-    def __init__(self, ss, incominghome, finalhome, max_size, lease_info, canary):
+    def __init__(self, ss, incominghome, finalhome, max_size, lease_info):
         self.ss = ss
         self.incominghome = incominghome
         self.finalhome = finalhome
         self._max_size = max_size # don't allow the client to write more than this
-        self._canary = canary
-        self._disconnect_marker = canary.notifyOnDisconnect(self._disconnected)
         self.closed = False
         self.throw_out_all_data = False
         self._sharefile = ShareFile(incominghome, create=True, max_size=max_size)
@@ -290,22 +289,19 @@ class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78
             pass
         self._sharefile = None
         self.closed = True
-        self._canary.dontNotifyOnDisconnect(self._disconnect_marker)
 
         filelen = os.stat(self.finalhome)[stat.ST_SIZE]
         self.ss.bucket_writer_closed(self, filelen)
         self.ss.add_latency("close", time.time() - start)
         self.ss.count("close")
 
-    def _disconnected(self):
+    def disconnected(self):
         if not self.closed:
             self._abort()
 
     def remote_abort(self):
         log.msg("storage: aborting sharefile %s" % self.incominghome,
                 facility="tahoe.storage", level=log.UNUSUAL)
-        if not self.closed:
-            self._canary.dontNotifyOnDisconnect(self._disconnect_marker)
         self._abort()
         self.ss.count("abort")
 
diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py
index f4996756e..92b3d2a1b 100644
--- a/src/allmydata/storage/server.py
+++ b/src/allmydata/storage/server.py
@@ -11,13 +11,14 @@ if PY2:
     # Omit open() to get native behavior where open("w") always accepts native
     # strings. Omit bytes so we don't leak future's custom bytes.
     from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, pow, round, super, dict, list, object, range, str, max, min  # noqa: F401
-
+else:
+    from typing import Dict
 
 import os, re, struct, time
-import weakref
 import six
 
 from foolscap.api import Referenceable
+from foolscap.ipb import IRemoteReference
 from twisted.application import service
 
 from zope.interface import implementer
@@ -89,7 +90,6 @@ class StorageServer(service.MultiService, Referenceable):
         self.incomingdir = os.path.join(sharedir, 'incoming')
         self._clean_incomplete()
         fileutil.make_dirs(self.incomingdir)
-        self._active_writers = weakref.WeakKeyDictionary()
         log.msg("StorageServer created", facility="tahoe.storage")
 
         if reserved_space:
@@ -121,6 +121,15 @@ class StorageServer(service.MultiService, Referenceable):
         self.lease_checker.setServiceParent(self)
         self._get_current_time = get_current_time
 
+        # Currently being-written Bucketwriters. TODO Can probably refactor so
+        # active_writers is unnecessary, do as second pass.
+        # Map BucketWriter -> (storage_index, share_num)
+        self._active_writers = {}  # type: Dict[BucketWriter, (bytes,int)]
+        # Map (storage_index, share_num) -> BucketWriter:
+        self._bucket_writers = {}  # type: Dict[(bytes, int),BucketWriter]
+        # Canaries and disconnect markers for BucketWriters created via Foolscap:
+        self._bucket_writer_disconnect_markers = {}  # type: Dict[BucketWriter,(IRemoteReference, object)]
+
     def __repr__(self):
         return "<StorageServer %s>" % (idlib.shortnodeid_b2a(self.my_nodeid),)
 
@@ -263,10 +272,14 @@ class StorageServer(service.MultiService, Referenceable):
                     }
         return version
 
-    def remote_allocate_buckets(self, storage_index,
-                                renew_secret, cancel_secret,
-                                sharenums, allocated_size,
-                                canary, owner_num=0):
+    def allocate_buckets(self, storage_index,
+                         renew_secret, cancel_secret,
+                         sharenums, allocated_size,
+                         include_in_progress,
+                         owner_num=0):
+        """
+        Generic bucket allocation API.
+        """
         # owner_num is not for clients to set, but rather it should be
         # curried into the PersonalStorageServer instance that is dedicated
         # to a particular owner.
@@ -315,6 +328,7 @@ class StorageServer(service.MultiService, Referenceable):
                 # great! we already have it. easy.
                 pass
             elif os.path.exists(incominghome):
+                # TODO use include_in_progress
                 # Note that we don't create BucketWriters for shnums that
                 # have a partial share (in incoming/), so if a second upload
                 # occurs while the first is still in progress, the second
@@ -323,11 +337,12 @@ class StorageServer(service.MultiService, Referenceable):
             elif (not limited) or (remaining_space >= max_space_per_bucket):
                 # ok! we need to create the new share file.
                 bw = BucketWriter(self, incominghome, finalhome,
-                                  max_space_per_bucket, lease_info, canary)
+                                  max_space_per_bucket, lease_info)
                 if self.no_storage:
                     bw.throw_out_all_data = True
                 bucketwriters[shnum] = bw
-                self._active_writers[bw] = 1
+                self._active_writers[bw] = (storage_index, shnum)
+                self._bucket_writers[(storage_index, shnum)] = bw
                 if limited:
                     remaining_space -= max_space_per_bucket
             else:
@@ -340,6 +355,21 @@ class StorageServer(service.MultiService, Referenceable):
         self.add_latency("allocate", self._get_current_time() - start)
         return alreadygot, bucketwriters
 
+    def remote_allocate_buckets(self, storage_index,
+                                renew_secret, cancel_secret,
+                                sharenums, allocated_size,
+                                canary, owner_num=0):
+        """Foolscap-specific ``allocate_buckets()`` API."""
+        alreadygot, bucketwriters = self.allocate_buckets(
+            storage_index, renew_secret, cancel_secret, sharenums, allocated_size,
+            include_in_progress=False, owner_num=owner_num,
+        )
+        # Abort BucketWriters if disconnection happens.
+        for bw in bucketwriters.values():
+            disconnect_marker = canary.notifyOnDisconnect(bw.disconnected)
+            self._bucket_writer_disconnect_markers[bw] = (canary, disconnect_marker)
+        return alreadygot, bucketwriters
+
     def _iter_share_files(self, storage_index):
         for shnum, filename in self._get_bucket_shares(storage_index):
             with open(filename, 'rb') as f:
@@ -383,7 +413,11 @@ class StorageServer(service.MultiService, Referenceable):
     def bucket_writer_closed(self, bw, consumed_size):
         if self.stats_provider:
             self.stats_provider.count('storage_server.bytes_added', consumed_size)
-        del self._active_writers[bw]
+        storage_index, shnum = self._active_writers.pop(bw)
+        del self._bucket_writers[(storage_index, shnum)]
+        if bw in self._bucket_writer_disconnect_markers:
+            canary, disconnect_marker = self._bucket_writer_disconnect_markers.pop(bw)
+            canary.dontNotifyOnDisconnect(disconnect_marker)
 
     def _get_bucket_shares(self, storage_index):
         """Return a list of (shnum, pathname) tuples for files that hold
diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py
index b5229ca11..de8d774b3 100644
--- a/src/allmydata/test/common_util.py
+++ b/src/allmydata/test/common_util.py
@@ -314,6 +314,11 @@ class FakeCanary(object):
     def getPeer(self):
         return "<fake>"
 
+    # For use by tests:
+    def disconnected(self):
+        for (f, args, kwargs) in list(self.disconnectors.values()):
+            f(*args, **kwargs)
+
 
 class ShouldFailMixin(object):
 
diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py
index 60173cf75..3b36f293f 100644
--- a/src/allmydata/test/test_storage.py
+++ b/src/allmydata/test/test_storage.py
@@ -129,8 +129,7 @@ class Bucket(unittest.TestCase):
 
     def test_create(self):
         incoming, final = self.make_workdir("test_create")
-        bw = BucketWriter(self, incoming, final, 200, self.make_lease(),
-                          FakeCanary())
+        bw = BucketWriter(self, incoming, final, 200, self.make_lease())
         bw.remote_write(0, b"a"*25)
         bw.remote_write(25, b"b"*25)
         bw.remote_write(50, b"c"*25)
@@ -139,8 +138,7 @@ class Bucket(unittest.TestCase):
 
     def test_readwrite(self):
         incoming, final = self.make_workdir("test_readwrite")
-        bw = BucketWriter(self, incoming, final, 200, self.make_lease(),
-                          FakeCanary())
+        bw = BucketWriter(self, incoming, final, 200, self.make_lease())
         bw.remote_write(0, b"a"*25)
         bw.remote_write(25, b"b"*25)
         bw.remote_write(50, b"c"*7) # last block may be short
@@ -158,8 +156,7 @@ class Bucket(unittest.TestCase):
             incoming, final = self.make_workdir(
                 "test_write_past_size_errors-{}".format(i)
             )
-            bw = BucketWriter(self, incoming, final, 200, self.make_lease(),
-                              FakeCanary())
+            bw = BucketWriter(self, incoming, final, 200, self.make_lease())
             with self.assertRaises(DataTooLargeError):
                 bw.remote_write(offset, b"a" * length)
 
@@ -179,7 +176,6 @@ class Bucket(unittest.TestCase):
         incoming, final = self.make_workdir("overlapping_writes_{}".format(uuid4()))
         bw = BucketWriter(
             self, incoming, final, length, self.make_lease(),
-            FakeCanary()
         )
         # Three writes: 10-19, 30-39, 50-59. This allows for a bunch of holes.
         bw.remote_write(10, expected_data[10:20])
@@ -218,7 +214,6 @@ class Bucket(unittest.TestCase):
         incoming, final = self.make_workdir("overlapping_writes_{}".format(uuid4()))
         bw = BucketWriter(
             self, incoming, final, length, self.make_lease(),
-            FakeCanary()
         )
         # Three writes: 10-19, 30-39, 50-59. This allows for a bunch of holes.
         bw.remote_write(10, b"1" * 10)
@@ -318,8 +313,7 @@ class BucketProxy(unittest.TestCase):
         final = os.path.join(basedir, "bucket")
         fileutil.make_dirs(basedir)
         fileutil.make_dirs(os.path.join(basedir, "tmp"))
-        bw = BucketWriter(self, incoming, final, size, self.make_lease(),
-                          FakeCanary())
+        bw = BucketWriter(self, incoming, final, size, self.make_lease())
         rb = RemoteBucket(bw)
         return bw, rb, final
 
@@ -669,7 +663,7 @@ class Server(unittest.TestCase):
         # the size we request.
         OVERHEAD = 3*4
         LEASE_SIZE = 4+32+32+4
-        canary = FakeCanary(True)
+        canary = FakeCanary()
         already, writers = self.allocate(ss, b"vid1", [0,1,2], 1000, canary)
         self.failUnlessEqual(len(writers), 3)
         # now the StorageServer should have 3000 bytes provisionally
@@ -677,16 +671,14 @@ class Server(unittest.TestCase):
         self.failUnlessEqual(len(ss._active_writers), 3)
 
         # allocating 1001-byte shares only leaves room for one
-        already2, writers2 = self.allocate(ss, b"vid2", [0,1,2], 1001, canary)
+        canary2 = FakeCanary()
+        already2, writers2 = self.allocate(ss, b"vid2", [0,1,2], 1001, canary2)
         self.failUnlessEqual(len(writers2), 1)
         self.failUnlessEqual(len(ss._active_writers), 4)
 
         # we abandon the first set, so their provisional allocation should be
         # returned
-
-        del already
-        del writers
-        gc.collect()
+        canary.disconnected()
 
         self.failUnlessEqual(len(ss._active_writers), 1)
         # now we have a provisional allocation of 1001 bytes
@@ -697,9 +689,6 @@ class Server(unittest.TestCase):
         for bw in writers2.values():
             bw.remote_write(0, b"a"*25)
             bw.remote_close()
-        del already2
-        del writers2
-        del bw
         self.failUnlessEqual(len(ss._active_writers), 0)
 
         # this also changes the amount reported as available by call_get_disk_stats
@@ -707,13 +696,12 @@ class Server(unittest.TestCase):
 
         # now there should be ALLOCATED=1001+12+72=1085 bytes allocated, and
         # 5000-1085=3915 free, therefore we can fit 39 100byte shares
-        already3, writers3 = self.allocate(ss, b"vid3", list(range(100)), 100, canary)
+        canary3 = FakeCanary()
+        already3, writers3 = self.allocate(ss, b"vid3", list(range(100)), 100, canary3)
         self.failUnlessEqual(len(writers3), 39)
         self.failUnlessEqual(len(ss._active_writers), 39)
 
-        del already3
-        del writers3
-        gc.collect()
+        canary3.disconnected()
 
         self.failUnlessEqual(len(ss._active_writers), 0)
         ss.disownServiceParent()

From 58d7e2f62785eaa698107528a06469e86f1cbf05 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 29 Sep 2021 13:58:53 -0400
Subject: [PATCH 82/95] Simplify implementation.

---
 src/allmydata/storage/immutable.py | 11 -----------
 src/allmydata/storage/server.py    | 28 +++++++++++++---------------
 src/allmydata/test/test_storage.py | 12 ++++++------
 3 files changed, 19 insertions(+), 32 deletions(-)

diff --git a/src/allmydata/storage/immutable.py b/src/allmydata/storage/immutable.py
index d3b6ce875..b8b18f140 100644
--- a/src/allmydata/storage/immutable.py
+++ b/src/allmydata/storage/immutable.py
@@ -204,17 +204,6 @@ class ShareFile(object):
             self.unlink()
         return space_freed
 
-# TODOs
-# Batch 1:
-# - bucketwriter dict in the server, to persist them + TEST of persistence
-# - aborting bucketwriter removes it from server persistent + TEST
-# - disconnect still aborts _for Foolscap only_
-# - existing in-use buckets are not returned _for Foolscap only_
-# - this implies splitting remote_allocate_buckets into generic and Foolscap-y parts
-# Batch 2:
-# - scheduled events for aborting bucketwriter + TEST
-# - bucketwriter writes delay cancellation + TEST
-
 
 @implementer(RIBucketWriter)
 class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78
diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py
index 92b3d2a1b..2a4b9a54d 100644
--- a/src/allmydata/storage/server.py
+++ b/src/allmydata/storage/server.py
@@ -121,12 +121,14 @@ class StorageServer(service.MultiService, Referenceable):
         self.lease_checker.setServiceParent(self)
         self._get_current_time = get_current_time
 
-        # Currently being-written Bucketwriters. TODO Can probably refactor so
-        # active_writers is unnecessary, do as second pass.
-        # Map BucketWriter -> (storage_index, share_num)
-        self._active_writers = {}  # type: Dict[BucketWriter, (bytes,int)]
-        # Map (storage_index, share_num) -> BucketWriter:
-        self._bucket_writers = {}  # type: Dict[(bytes, int),BucketWriter]
+        # Currently being-written Bucketwriters. For Foolscap, lifetime is tied
+        # to connection: when disconnection happens, the BucketWriters are
+        # removed. For HTTP, this makes no sense, so there will be
+        # timeout-based cleanup; see
+        # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3807.
+
+        # Map in-progress filesystem path -> BucketWriter:
+        self._bucket_writers = {}  # type: Dict[str,BucketWriter]
         # Canaries and disconnect markers for BucketWriters created via Foolscap:
         self._bucket_writer_disconnect_markers = {}  # type: Dict[BucketWriter,(IRemoteReference, object)]
 
@@ -247,7 +249,7 @@ class StorageServer(service.MultiService, Referenceable):
 
     def allocated_size(self):
         space = 0
-        for bw in self._active_writers:
+        for bw in self._bucket_writers.values():
             space += bw.allocated_size()
         return space
 
@@ -275,7 +277,6 @@ class StorageServer(service.MultiService, Referenceable):
     def allocate_buckets(self, storage_index,
                          renew_secret, cancel_secret,
                          sharenums, allocated_size,
-                         include_in_progress,
                          owner_num=0):
         """
         Generic bucket allocation API.
@@ -328,8 +329,7 @@ class StorageServer(service.MultiService, Referenceable):
                 # great! we already have it. easy.
                 pass
             elif os.path.exists(incominghome):
-                # TODO use include_in_progress
-                # Note that we don't create BucketWriters for shnums that
+                # For Foolscap we don't create BucketWriters for shnums that
                 # have a partial share (in incoming/), so if a second upload
                 # occurs while the first is still in progress, the second
                 # uploader will use different storage servers.
@@ -341,8 +341,7 @@ class StorageServer(service.MultiService, Referenceable):
                 if self.no_storage:
                     bw.throw_out_all_data = True
                 bucketwriters[shnum] = bw
-                self._active_writers[bw] = (storage_index, shnum)
-                self._bucket_writers[(storage_index, shnum)] = bw
+                self._bucket_writers[incominghome] = bw
                 if limited:
                     remaining_space -= max_space_per_bucket
             else:
@@ -362,7 +361,7 @@ class StorageServer(service.MultiService, Referenceable):
         """Foolscap-specific ``allocate_buckets()`` API."""
         alreadygot, bucketwriters = self.allocate_buckets(
             storage_index, renew_secret, cancel_secret, sharenums, allocated_size,
-            include_in_progress=False, owner_num=owner_num,
+            owner_num=owner_num,
         )
         # Abort BucketWriters if disconnection happens.
         for bw in bucketwriters.values():
@@ -413,8 +412,7 @@ class StorageServer(service.MultiService, Referenceable):
     def bucket_writer_closed(self, bw, consumed_size):
         if self.stats_provider:
             self.stats_provider.count('storage_server.bytes_added', consumed_size)
-        storage_index, shnum = self._active_writers.pop(bw)
-        del self._bucket_writers[(storage_index, shnum)]
+        del self._bucket_writers[bw.incominghome]
         if bw in self._bucket_writer_disconnect_markers:
             canary, disconnect_marker = self._bucket_writer_disconnect_markers.pop(bw)
             canary.dontNotifyOnDisconnect(disconnect_marker)
diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py
index 3b36f293f..640280bc3 100644
--- a/src/allmydata/test/test_storage.py
+++ b/src/allmydata/test/test_storage.py
@@ -668,19 +668,19 @@ class Server(unittest.TestCase):
         self.failUnlessEqual(len(writers), 3)
         # now the StorageServer should have 3000 bytes provisionally
         # allocated, allowing only 2000 more to be claimed
-        self.failUnlessEqual(len(ss._active_writers), 3)
+        self.failUnlessEqual(len(ss._bucket_writers), 3)
 
         # allocating 1001-byte shares only leaves room for one
         canary2 = FakeCanary()
         already2, writers2 = self.allocate(ss, b"vid2", [0,1,2], 1001, canary2)
         self.failUnlessEqual(len(writers2), 1)
-        self.failUnlessEqual(len(ss._active_writers), 4)
+        self.failUnlessEqual(len(ss._bucket_writers), 4)
 
         # we abandon the first set, so their provisional allocation should be
         # returned
         canary.disconnected()
 
-        self.failUnlessEqual(len(ss._active_writers), 1)
+        self.failUnlessEqual(len(ss._bucket_writers), 1)
         # now we have a provisional allocation of 1001 bytes
 
         # and we close the second set, so their provisional allocation should
@@ -689,7 +689,7 @@ class Server(unittest.TestCase):
         for bw in writers2.values():
             bw.remote_write(0, b"a"*25)
             bw.remote_close()
-        self.failUnlessEqual(len(ss._active_writers), 0)
+        self.failUnlessEqual(len(ss._bucket_writers), 0)
 
         # this also changes the amount reported as available by call_get_disk_stats
         allocated = 1001 + OVERHEAD + LEASE_SIZE
@@ -699,11 +699,11 @@ class Server(unittest.TestCase):
         canary3 = FakeCanary()
         already3, writers3 = self.allocate(ss, b"vid3", list(range(100)), 100, canary3)
         self.failUnlessEqual(len(writers3), 39)
-        self.failUnlessEqual(len(ss._active_writers), 39)
+        self.failUnlessEqual(len(ss._bucket_writers), 39)
 
         canary3.disconnected()
 
-        self.failUnlessEqual(len(ss._active_writers), 0)
+        self.failUnlessEqual(len(ss._bucket_writers), 0)
         ss.disownServiceParent()
         del ss
 

From f8604e239406278dc7082ddfd730524b035f3800 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 29 Sep 2021 14:00:11 -0400
Subject: [PATCH 83/95] Fix flakes.

---
 src/allmydata/test/test_istorageserver.py | 2 --
 src/allmydata/test/test_storage.py        | 1 -
 2 files changed, 3 deletions(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 25b814237..9ad6a8224 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -21,8 +21,6 @@ if PY2:
 from random import Random
 
 from twisted.internet.defer import inlineCallbacks
-from twisted.internet import reactor
-from twisted.internet.task import deferLater
 
 from foolscap.api import Referenceable, RemoteException
 
diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py
index 640280bc3..d18960a1e 100644
--- a/src/allmydata/test/test_storage.py
+++ b/src/allmydata/test/test_storage.py
@@ -19,7 +19,6 @@ import platform
 import stat
 import struct
 import shutil
-import gc
 from uuid import uuid4
 
 from twisted.trial import unittest

From 016d6b4530af011c5ff470846d82d85a941848c3 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 29 Sep 2021 14:10:14 -0400
Subject: [PATCH 84/95] Fix spurious type checking error.

---
 src/allmydata/storage/server.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py
index 2a4b9a54d..041783a4e 100644
--- a/src/allmydata/storage/server.py
+++ b/src/allmydata/storage/server.py
@@ -274,10 +274,10 @@ class StorageServer(service.MultiService, Referenceable):
                     }
         return version
 
-    def allocate_buckets(self, storage_index,
-                         renew_secret, cancel_secret,
-                         sharenums, allocated_size,
-                         owner_num=0):
+    def _allocate_buckets(self, storage_index,
+                          renew_secret, cancel_secret,
+                          sharenums, allocated_size,
+                          owner_num=0):
         """
         Generic bucket allocation API.
         """
@@ -359,7 +359,7 @@ class StorageServer(service.MultiService, Referenceable):
                                 sharenums, allocated_size,
                                 canary, owner_num=0):
         """Foolscap-specific ``allocate_buckets()`` API."""
-        alreadygot, bucketwriters = self.allocate_buckets(
+        alreadygot, bucketwriters = self._allocate_buckets(
             storage_index, renew_secret, cancel_secret, sharenums, allocated_size,
             owner_num=owner_num,
         )

From 23fd11be43b2978e07d9be2c0685be0469f47149 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 29 Sep 2021 14:13:18 -0400
Subject: [PATCH 85/95] Expand explanation.

---
 src/allmydata/test/test_istorageserver.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 9ad6a8224..29ce272e2 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -137,6 +137,10 @@ class IStorageServerImmutableAPIsTestsMixin(object):
 
         (In the real world one shouldn't do that, but writing different data is
         a good way to test that the original data really was wiped.)
+
+        HTTP protocol should skip this test, since disconnection is meaningless
+        concept; this is more about testing implicit contract the Foolscap
+        implementation depends on doesn't change as we refactor things.
         """
         storage_index, renew_secret, cancel_secret = (
             new_storage_index(),

From 1f6daf02eb05d48f4ddce8f3443706473dec061b Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Wed, 29 Sep 2021 15:15:56 -0400
Subject: [PATCH 86/95] news fragment

---
 newsfragments/3808.installation | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 newsfragments/3808.installation

diff --git a/newsfragments/3808.installation b/newsfragments/3808.installation
new file mode 100644
index 000000000..157f08a0c
--- /dev/null
+++ b/newsfragments/3808.installation
@@ -0,0 +1 @@
+Tahoe-LAFS now supports running on NixOS 21.05 with Python 3.

From fc01835a56ee260970887eb999ed4d4dae9c7b4d Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Wed, 29 Sep 2021 15:16:01 -0400
Subject: [PATCH 87/95] ci configuration

---
 .circleci/config.yml | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 62d1bd752..ea3d50de5 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -42,6 +42,9 @@ workflows:
       - "nixos-19-09":
           {}
 
+      - "nixos-21-05":
+          {}
+
       # Test against PyPy 2.7
       - "pypy27-buster":
           {}
@@ -438,8 +441,7 @@ jobs:
         image: "tahoelafsci/fedora:29-py"
         user: "nobody"
 
-
-  nixos-19-09:
+  nixos-19-09: &NIXOS
     docker:
       # Run in a highly Nix-capable environment.
       - <<: *DOCKERHUB_AUTH
@@ -465,6 +467,15 @@ jobs:
             # them in parallel.
             nix-build --cores 3 --max-jobs 2 nix/
 
+  nixos-21-05:
+    <<: *NIXOS
+
+    environment:
+      # Note this doesn't look more similar to the 19.09 NIX_PATH URL because
+      # there was some internal shuffling by the NixOS project about how they
+      # publish stable revisions.
+      NIX_PATH: "nixpkgs=https://github.com/NixOS/nixpkgs/archive/refs/heads/nixos-21.05-small.tar.gz"
+
   typechecks:
     docker:
       - <<: *DOCKERHUB_AUTH

From 49ee4b8acfff3820f80a01d0fb8d2eab60f0a1d5 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Wed, 29 Sep 2021 15:27:17 -0400
Subject: [PATCH 88/95] callPackage not directly available from python-self in
 newer nixpkgs

---
 nix/overlays.nix | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/nix/overlays.nix b/nix/overlays.nix
index 14a47ca5a..f5ef72cc0 100644
--- a/nix/overlays.nix
+++ b/nix/overlays.nix
@@ -2,25 +2,25 @@ self: super: {
   python27 = super.python27.override {
     packageOverrides = python-self: python-super: {
       # eliot is not part of nixpkgs at all at this time.
-      eliot = python-self.callPackage ./eliot.nix { };
+      eliot = python-self.pythonPackages.callPackage ./eliot.nix { };
 
       # NixOS autobahn package has trollius as a dependency, although
       # it is optional. Trollius is unmaintained and fails on CI.
-      autobahn = python-super.callPackage ./autobahn.nix { };
+      autobahn = python-super.pythonPackages.callPackage ./autobahn.nix { };
 
       # Porting to Python 3 is greatly aided by the future package.  A
       # slightly newer version than appears in nixos 19.09 is helpful.
-      future = python-super.callPackage ./future.nix { };
+      future = python-super.pythonPackages.callPackage ./future.nix { };
 
       # Need version of pyutil that supports Python 3. The version in 19.09
       # is too old.
-      pyutil = python-super.callPackage ./pyutil.nix { };
+      pyutil = python-super.pythonPackages.callPackage ./pyutil.nix { };
 
       # Need a newer version of Twisted, too.
-      twisted = python-super.callPackage ./twisted.nix { };
+      twisted = python-super.pythonPackages.callPackage ./twisted.nix { };
 
       # collections-extended is not part of nixpkgs at this time.
-      collections-extended = python-super.callPackage ./collections-extended.nix { };
+      collections-extended = python-super.pythonPackages.callPackage ./collections-extended.nix { };
     };
   };
 }

From 5a3028bdabf1a4674da8576d78f0026fb4f37517 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Wed, 29 Sep 2021 15:46:18 -0400
Subject: [PATCH 89/95] add a python3 expression

most deps are in nixpkgs now but we still need an overlay for th very very
recent collections-extended dependency
---
 .circleci/config.yml | 6 ++++--
 nix/overlays.nix     | 7 +++++++
 nix/py3.nix          | 7 +++++++
 3 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 nix/py3.nix

diff --git a/.circleci/config.yml b/.circleci/config.yml
index ea3d50de5..dd877ca7f 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -449,6 +449,7 @@ jobs:
 
     environment:
       NIX_PATH: "nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/nixos-19.09-small.tar.gz"
+      SOURCE: "nix/"
 
     steps:
       - "checkout"
@@ -465,7 +466,7 @@ jobs:
             # build a couple simple little dependencies that don't take
             # advantage of multiple cores and we get a little speedup by doing
             # them in parallel.
-            nix-build --cores 3 --max-jobs 2 nix/
+            nix-build --cores 3 --max-jobs 2 "$SOURCE"
 
   nixos-21-05:
     <<: *NIXOS
@@ -474,7 +475,8 @@ jobs:
       # Note this doesn't look more similar to the 19.09 NIX_PATH URL because
       # there was some internal shuffling by the NixOS project about how they
       # publish stable revisions.
-      NIX_PATH: "nixpkgs=https://github.com/NixOS/nixpkgs/archive/refs/heads/nixos-21.05-small.tar.gz"
+      NIX_PATH: "nixpkgs=https://github.com/NixOS/nixpkgs/archive/refs/d32b07e6df276d78e3640eb43882b80c9b2b3459.tar.gz"
+      SOURCE: "nix/py3.nix"
 
   typechecks:
     docker:
diff --git a/nix/overlays.nix b/nix/overlays.nix
index f5ef72cc0..fbd0ce3bb 100644
--- a/nix/overlays.nix
+++ b/nix/overlays.nix
@@ -23,4 +23,11 @@ self: super: {
       collections-extended = python-super.pythonPackages.callPackage ./collections-extended.nix { };
     };
   };
+
+  python39 = super.python39.override {
+    packageOverrides = python-self: python-super: {
+      # collections-extended is not part of nixpkgs at this time.
+      collections-extended = python-super.pythonPackages.callPackage ./collections-extended.nix { };
+    };
+  };
 }
diff --git a/nix/py3.nix b/nix/py3.nix
new file mode 100644
index 000000000..34ede49dd
--- /dev/null
+++ b/nix/py3.nix
@@ -0,0 +1,7 @@
+# This is the main entrypoint for the Tahoe-LAFS derivation.
+{ pkgs ? import <nixpkgs> { } }:
+# Add our Python packages to nixpkgs to simplify the expression for the
+# Tahoe-LAFS derivation.
+let pkgs' = pkgs.extend (import ./overlays.nix);
+# Evaluate the expression for our Tahoe-LAFS derivation.
+in pkgs'.python39.pkgs.callPackage ./tahoe-lafs.nix { }

From 49df402f0762b34c88b01d183b9d217da117cc79 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Wed, 29 Sep 2021 15:48:33 -0400
Subject: [PATCH 90/95] maybe this is the right url

---
 .circleci/config.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index dd877ca7f..2fc8e88e7 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -475,7 +475,7 @@ jobs:
       # Note this doesn't look more similar to the 19.09 NIX_PATH URL because
       # there was some internal shuffling by the NixOS project about how they
       # publish stable revisions.
-      NIX_PATH: "nixpkgs=https://github.com/NixOS/nixpkgs/archive/refs/d32b07e6df276d78e3640eb43882b80c9b2b3459.tar.gz"
+      NIX_PATH: "nixpkgs=https://github.com/NixOS/nixpkgs/archive/d32b07e6df276d78e3640eb43882b80c9b2b3459.tar.gz"
       SOURCE: "nix/py3.nix"
 
   typechecks:

From add34efffbb4c1bc0193cfaae9ea6c28c5c93762 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 4 Oct 2021 10:58:42 -0400
Subject: [PATCH 91/95] News file.

---
 newsfragments/3810.minor | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 newsfragments/3810.minor

diff --git a/newsfragments/3810.minor b/newsfragments/3810.minor
new file mode 100644
index 000000000..e69de29bb

From 2b83edc5b30f910eaafaf6831f26b3d8b71b27a9 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Mon, 4 Oct 2021 11:00:16 -0400
Subject: [PATCH 92/95] Use macos-10.15 for Python 2.7.

---
 .github/workflows/ci.yml | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index e161ec243..45b2986a3 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -28,7 +28,7 @@ jobs:
           - 3.9
         include:
           # On macOS don't bother with 3.6-3.8, just to get faster builds.
-          - os: macos-latest
+          - os: macos-10.15
             python-version: 2.7
           - os: macos-latest
             python-version: 3.9
@@ -168,7 +168,7 @@ jobs:
           - 3.9
         include:
           # On macOS don't bother with 3.6, just to get faster builds.
-          - os: macos-latest
+          - os: macos-10.15
             python-version: 2.7
           - os: macos-latest
             python-version: 3.9
@@ -183,7 +183,7 @@ jobs:
       # We have to use an older version of Tor for running integration
       # tests on macOS.
       - name: Install Tor [macOS, ${{ matrix.python-version }} ]
-        if: ${{ matrix.os == 'macos-latest' }}
+        if: ${{ contains(matrix.os, 'macos') }}
         run: |
           brew extract --version 0.4.5.8 tor homebrew/cask
           brew install tor@0.4.5.8
@@ -247,7 +247,7 @@ jobs:
       fail-fast: false
       matrix:
         os:
-          - macos-latest
+          - macos-10.15
           - windows-latest
           - ubuntu-latest
         python-version:

From aef581628f356ae2a8302b1d3b18eb1b72468901 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 6 Oct 2021 15:10:58 -0400
Subject: [PATCH 93/95] Add discussion.

---
 docs/proposed/http-storage-node-protocol.rst | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst
index e3bd48128..bb5dc28ad 100644
--- a/docs/proposed/http-storage-node-protocol.rst
+++ b/docs/proposed/http-storage-node-protocol.rst
@@ -515,6 +515,24 @@ Clients should upload chunks in re-assembly order.
     }
 
 
+Discussion
+``````````
+
+``PUT`` verbs are only supposed to be used to replace the whole resource,
+thus the use of ``PATCH``.
+From RFC 7231::
+
+   An origin server that allows PUT on a given target resource MUST send
+   a 400 (Bad Request) response to a PUT request that contains a
+   Content-Range header field (Section 4.2 of [RFC7233]), since the
+   payload is likely to be partial content that has been mistakenly PUT
+   as a full representation.  Partial content updates are possible by
+   targeting a separately identified resource with state that overlaps a
+   portion of the larger resource, or by using a different method that
+   has been specifically defined for partial updates (for example, the
+   PATCH method defined in [RFC5789]).
+
+
 ``POST /v1/immutable/:storage_index/:share_number/corrupt``
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 

From 82cbce6b7e258a651ff2e8de580fc10f5b0e9468 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 6 Oct 2021 15:12:22 -0400
Subject: [PATCH 94/95] Better explanation.

---
 src/allmydata/test/test_istorageserver.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py
index 931cf6ed6..26d976d00 100644
--- a/src/allmydata/test/test_istorageserver.py
+++ b/src/allmydata/test/test_istorageserver.py
@@ -647,7 +647,9 @@ class IStorageServerMutableAPIsTestsMixin(object):
             tw_vectors={
                 0: ([], [(0, b"abcdefg")], 7),
                 1: ([], [(0, b"0123"), (4, b"456")], 7),
-                2: ([], [(0, b"0123")], 4),
+                # This will never get read from, just here to show we only read
+                # from shares explicitly requested by slot_readv:
+                2: ([], [(0, b"XYZW")], 4),
             },
             r_vector=[],
         )

From bf176144c5f0fc99fa25f7c250927a3fe9d43b50 Mon Sep 17 00:00:00 2001
From: Itamar Turner-Trauring <itamar@itamarst.org>
Date: Wed, 6 Oct 2021 15:18:00 -0400
Subject: [PATCH 95/95] Handle double-disconnect, should it happen by mistake.

---
 src/allmydata/test/common_util.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py
index de8d774b3..d2d20916d 100644
--- a/src/allmydata/test/common_util.py
+++ b/src/allmydata/test/common_util.py
@@ -314,10 +314,15 @@ class FakeCanary(object):
     def getPeer(self):
         return "<fake>"
 
-    # For use by tests:
     def disconnected(self):
-        for (f, args, kwargs) in list(self.disconnectors.values()):
-            f(*args, **kwargs)
+        """Disconnect the canary, to be called by test code.
+
+        Can only  happen once.
+        """
+        if self.disconnectors is not None:
+            for (f, args, kwargs) in list(self.disconnectors.values()):
+                f(*args, **kwargs)
+            self.disconnectors = None
 
 
 class ShouldFailMixin(object):