add min lease renewal time to the config object as well

src/_zkapauthorizer/_plugin.py

@@ -18,7 +18,7 @@ Tahoe-LAFS.
 """
 
 import random
-from datetime import datetime, timedelta
+from datetime import datetime
 from functools import partial
 from weakref import WeakValueDictionary
 
@@ -27,13 +27,13 @@ from allmydata.client import _Client
 from allmydata.interfaces import IAnnounceableStorageServer, IFoolscapStoragePlugin
 from allmydata.node import MissingConfigEntry
 from challenge_bypass_ristretto import SigningKey
-from isodate import parse_duration
 from twisted.internet.defer import succeed
 from twisted.logger import Logger
 from twisted.python.filepath import FilePath
 from zope.interface import implementer
 
 from .api import ZKAPAuthorizerStorageClient, ZKAPAuthorizerStorageServer
+from .config import lease_maintenance_from_tahoe_config
 from .controller import get_redeemer
 from .lease_maintenance import (
     SERVICE_NAME,
@@ -230,20 +230,7 @@ def _create_maintenance_service(reactor, node_config, client_node):
         get_root_nodes=partial(get_root_nodes, client_node, node_config),
         storage_broker=client_node.get_storage_broker(),
         secret_holder=client_node._secret_holder,
-        # The greater the min lease remaining time, the more of each lease
-        # period is "wasted" by renewing the lease before it has expired.  The
-        # premise of ZKAPAuthorizer's use of leases is that if they expire,
-        # the storage server is free to reclaim the storage by forgetting
-        # about the share.  However, since we do not know of any
-        # ZKAPAuthorizer-enabled storage grids which will garbage collect
-        # shares when leases expire, we have no reason not to use a zero
-        # duration here - for now.
-        #
-        # In the long run, storage servers must run with garbage collection
-        # enabled.  Ideally, before that happens, we will have a system that
-        # doesn't involve trading of wasted lease time against reliability of
-        # leases being renewed before the shares are garbage collected.
-        min_lease_remaining=timedelta(seconds=0),
+        min_lease_remaining=maint_config.min_lease_remaining,
         progress=store.start_lease_maintenance,
         get_now=get_now,
     )
@@ -256,46 +243,10 @@ def _create_maintenance_service(reactor, node_config, client_node):
         reactor,
         last_run_path,
         random,
-        interval_mean=maint_config.crawl_interval_mean,
-        interval_range=maint_config.crawl_interval_range,
+        lease_maint_config=maint_config,
     )
 
 
-def lease_maintenance_from_tahoe_config(node_config):
-    # type: (_Config) -> LeaseMaintenanceConfig
-    """
-    Return a ``LeaseMaintenanceConfig`` representing the values from the given
-    configuration object.
-    """
-    return LeaseMaintenanceConfig(
-        crawl_interval_mean=_read_duration(node_config, u"lease.crawl-interval.mean"),
-        crawl_interval_range=_read_duration(node_config, u"lease.crawl-interval.range"),
-    )
-
-
-def _read_duration(cfg, option):
-    """
-    Read an ISO8601 "duration" from the ZKAPAuthorizer section of a Tahoe-LAFS
-    config.
-
-    :param cfg: The Tahoe-LAFS config object to consult.
-    :param option: The name of the option to read.
-
-    :return: ``None`` if the option is missing, otherwise the parsed duration
-        as a ``timedelta``.
-    """
-    # type: (_Config, str) -> Optional[timedelta]
-    section_name = u"storageclient.plugins.privatestorageio-zkapauthz-v1"
-    value_str = cfg.get_config(
-        section=section_name,
-        option=option,
-        default=None,
-    )
-    if value_str is None:
-        return None
-    return parse_duration(value_str)
-
-
 def get_root_nodes(client_node, node_config):
     try:
         rootcap = node_config.get_private_config(b"rootcap")

src/_zkapauthorizer/config.py

+# Copyright 2019 PrivateStorage.io, LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from datetime import timedelta
+from isodate import parse_duration
+
+from .lease_maintenance import (
+    LeaseMaintenanceConfig,
+)
+
+class _EmptyConfig(object):
+    """
+    Weakly pretend to be a Tahoe-LAFS configuration object with no
+    configuration.
+    """
+    def get_config(self, section, option, default=object(), boolean=False):
+        return default
+
+empty_config = _EmptyConfig()
+
+
+def lease_maintenance_from_tahoe_config(node_config):
+    # type: (_Config) -> LeaseMaintenanceConfig
+    """
+    Return a ``LeaseMaintenanceConfig`` representing the values from the given
+    configuration object.
+    """
+    return LeaseMaintenanceConfig(
+        crawl_interval_mean=_read_duration(
+            node_config,
+            u"lease.crawl-interval.mean",
+            timedelta(days=26),
+        ),
+        crawl_interval_range=_read_duration(
+            node_config,
+            u"lease.crawl-interval.range",
+            timedelta(days=4),
+        ),
+        # The greater the min lease remaining time, the more of each lease
+        # period is "wasted" by renewing the lease before it has expired.  The
+        # premise of ZKAPAuthorizer's use of leases is that if they expire,
+        # the storage server is free to reclaim the storage by forgetting
+        # about the share.  However, since we do not know of any
+        # ZKAPAuthorizer-enabled storage grids which will garbage collect
+        # shares when leases expire, we have no reason not to use a zero
+        # duration here - for now.
+        #
+        # In the long run, storage servers must run with garbage collection
+        # enabled.  Ideally, before that happens, we will have a system that
+        # doesn't involve trading of wasted lease time against reliability of
+        # leases being renewed before the shares are garbage collected.
+        #
+        # Also, since this is configuration, you can set it to something else
+        # if you want.
+        min_lease_remaining=_read_duration(
+            node_config,
+            u"lease.min-time-remaining",
+            timedelta(days=0),
+        ),
+    )
+
+
+def _read_duration(cfg, option, default):
+    """
+    Read an ISO8601 "duration" from the ZKAPAuthorizer section of a Tahoe-LAFS
+    config.
+
+    :param cfg: The Tahoe-LAFS config object to consult.
+    :param option: The name of the option to read.
+
+    :return: ``None`` if the option is missing, otherwise the parsed duration
+        as a ``timedelta``.
+    """
+    # type: (_Config, str) -> Optional[timedelta]
+    section_name = u"storageclient.plugins.privatestorageio-zkapauthz-v1"
+    value_str = cfg.get_config(
+        section=section_name,
+        option=option,
+        default=None,
+    )
+    if value_str is None:
+        return default
+    return parse_duration(value_str)

src/_zkapauthorizer/lease_maintenance.py

@@ -357,8 +357,7 @@ def lease_maintenance_service(
     reactor,
     last_run_path,
     random,
-    interval_mean=None,
-    interval_range=None,
+    lease_maint_config,
 ):
     """
     Get an ``IService`` which will maintain leases on ``root_node`` and any
@@ -377,19 +376,15 @@ def lease_maintenance_service(
     :param random: An object like ``random.Random`` which can be used as a
         source of scheduling delay.
 
-    :param timedelta interval_mean: The mean time between lease renewal checks.
-
-    :param timedelta interval_range: The range of the uniform distribution of
-        lease renewal checks (centered on ``interval_mean``).
+    :param lease_maint_config: Configuration for the tweakable lease
+        maintenance parameters.
 
     :param maintain_leases: A no-argument callable which performs a round of
         lease-maintenance.  The resulting service calls this periodically.
     """
-    if interval_mean is None:
-        interval_mean = timedelta(days=26)
-    if interval_range is None:
-        interval_range = timedelta(days=4)
-    halfrange = interval_range / 2
+    interval_mean = lease_maint_config.crawl_interval_mean
+    interval_range = lease_maint_config.crawl_interval_range
+    halfrange = lease_maint_config.crawl_interval_range / 2
 
     def sample_interval_distribution():
         return timedelta(
@@ -419,11 +414,8 @@ def lease_maintenance_service(
             timedelta(0),
         )
 
-    def get_config():
-        return LeaseMaintenanceConfig(
-            crawl_interval_mean=interval_mean,
-            crawl_interval_range=interval_range,
-        )
+    def get_lease_maint_config():
+        return lease_maint_config
 
     return _FuzzyTimerService(
         SERVICE_NAME,
@@ -437,7 +429,7 @@ def lease_maintenance_service(
         ),
         initial_interval,
         sample_interval_distribution,
-        get_config,
+        get_lease_maint_config,
         reactor,
     )
 
@@ -446,10 +438,19 @@ def lease_maintenance_service(
 class LeaseMaintenanceConfig(object):
     """
     Represent the configuration for a lease maintenance service.
+
+    :ivar crawl_interval_mean: The mean time between lease renewal checks.
+
+    :ivar crawl_interval_range: The range of the uniform distribution of lease
+        renewal checks (centered on ``interval_mean``).
+
+    :ivar min_lease_remaining: The minimum amount of time remaining to allow
+        on a lease without renewing it.
     """
 
     crawl_interval_mean = attr.ib()  # type: datetime.timedelta
     crawl_interval_range = attr.ib()  # type: datetime.timedelta
+    min_lease_remaining = attr.ib()  # type: datetime.timedelta
 
 
 def lease_maintenance_config_to_dict(lease_maint_config):
@@ -461,6 +462,9 @@ def lease_maintenance_config_to_dict(lease_maint_config):
         "lease.crawl-interval.range": duration_isoformat(
             lease_maint_config.crawl_interval_range,
         ),
+        "lease.min-time-remaining": duration_isoformat(
+            lease_maint_config.min_lease_remaining,
+        ),
     }
 
 
@@ -469,6 +473,7 @@ def lease_maintenance_config_from_dict(d):
     return LeaseMaintenanceConfig(
         crawl_interval_mean=parse_duration(d["lease.crawl-interval.mean"]),
         crawl_interval_range=parse_duration(d["lease.crawl-interval.range"]),
+        min_lease_remaining=parse_duration(d["lease.min-time-remaining"]),
     )
 
 

src/_zkapauthorizer/tests/strategies.py

@@ -379,6 +379,7 @@ def lease_maintenance_configurations():
         LeaseMaintenanceConfig,
         interval_means(),
         timedeltas(min_value=timedelta(0)),
+        timedeltas(min_value=timedelta(0)),
     )
 
 

src/_zkapauthorizer/tests/test_lease_maintenance.py

@@ -57,7 +57,9 @@ from twisted.python.filepath import FilePath
 from zope.interface import implementer
 
 from ..foolscap import ShareStat
+from ..config import lease_maintenance_from_tahoe_config, empty_config
 from ..lease_maintenance import (
+    LeaseMaintenanceConfig,
     MemoryMaintenanceObserver,
     NoopMaintenanceObserver,
     lease_maintenance_config_from_dict,
@@ -78,6 +80,7 @@ from .strategies import (
     storage_indexes,
 )
 
+default_lease_maint_config = lease_maintenance_from_tahoe_config(empty_config)
 
 def dummy_maintain_leases():
     pass
@@ -237,6 +240,7 @@ class LeaseMaintenanceServiceTests(TestCase):
             clock,
             FilePath(self.useFixture(TempDir()).join("last-run")),
             random,
+            lease_maint_config=default_lease_maint_config,
         )
         self.assertThat(
             service,
@@ -265,8 +269,11 @@ class LeaseMaintenanceServiceTests(TestCase):
             clock,
             FilePath(self.useFixture(TempDir()).join("last-run")),
             random,
-            mean,
-            range_,
+            LeaseMaintenanceConfig(
+                mean,
+                range_,
+                timedelta(0),
+            ),
         )
         service.startService()
         [maintenance_call] = clock.getDelayedCalls()
@@ -308,8 +315,11 @@ class LeaseMaintenanceServiceTests(TestCase):
             clock,
             last_run_path,
             random,
-            mean,
-            range_,
+            LeaseMaintenanceConfig(
+                mean,
+                range_,
+                timedelta(0),
+            ),
         )
         service.startService()
         [maintenance_call] = clock.getDelayedCalls()
@@ -355,6 +365,7 @@ class LeaseMaintenanceServiceTests(TestCase):
             clock,
             FilePath(self.useFixture(TempDir()).join("last-run")),
             random,
+            lease_maint_config=default_lease_maint_config,
         )
         service.startService()
         self.assertThat(
@@ -388,6 +399,7 @@ class LeaseMaintenanceServiceTests(TestCase):
             clock,
             FilePath(self.useFixture(TempDir()).join("last-run")),
             random,
+            lease_maint_config=default_lease_maint_config,
         )
         service.startService()
         [maintenance_call] = clock.getDelayedCalls()