From d004d91883cc511430c424cda6c0183cdc5021e0 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Mon, 6 Dec 2021 10:27:24 -0500
Subject: [PATCH] add metric for add_lease operation
---
src/_zkapauthorizer/_storage_server.py | 26 +++++--
.../tests/test_storage_server.py | 68 +++++++++++++++++--
2 files changed, 85 insertions(+), 9 deletions(-)
diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py
index a0de104..50e1920 100644
--- a/src/_zkapauthorizer/_storage_server.py
+++ b/src/_zkapauthorizer/_storage_server.py
@@ -365,12 +365,27 @@ class ZKAPAuthorizerStorageServer(Referenceable):
passes,
self._signing_key,
)
- check_pass_quantity_for_lease(
+ allocated_sizes = check_pass_quantity_for_lease(
self._pass_value,
storage_index,
validation,
self._original,
)
+ # siiiigh. Tahoe doesn't guarantee that mutable shares in a single
+ # slot are all the same size. They *probably* are. The official
+ # (only) Tahoe client always makes them the same size. The storage
+ # protocol allows a client to make them different sizes though.
+ #
+ # So ... deal with that here. Attribute the ZKAPs being spent
+ # proportionally to the size of each share.
+ observe_spending_successes(
+ self._metric_spending_successes,
+ compute_spending_metrics(
+ self._pass_value,
+ allocated_sizes.values(),
+ ),
+ )
+
return self._original.remote_add_lease(storage_index, *a, **kw)
def remote_advise_corrupt_share(self, *a, **kw):
@@ -534,6 +549,7 @@ def check_pass_quantity(pass_value, validation, share_sizes):
def check_pass_quantity_for_lease(
pass_value, storage_index, validation, storage_server
):
+ # type: (int, bytes, _ValidationResult, ZKAPAuthorizerStorageServer) -> Dict[int, int]
"""
Check that the given number of passes is sufficient to add or renew a
lease for one period for the given storage index.
@@ -545,7 +561,8 @@ def check_pass_quantity_for_lease(
:raise MorePassesRequired: If the given number of passes is too few for
the share sizes at the given storage index.
- :return: ``None`` if the given number of passes is sufficient.
+ :return: A mapping from share number to share size on the server if the
+ number of passes given is sufficient.
"""
allocated_sizes = dict(
get_share_sizes(
@@ -553,8 +570,9 @@ def check_pass_quantity_for_lease(
storage_index,
list(get_all_share_numbers(storage_server, storage_index)),
),
- ).values()
- check_pass_quantity(pass_value, validation, allocated_sizes)
+ )
+ check_pass_quantity(pass_value, validation, allocated_sizes.values())
+ return allocated_sizes
def check_pass_quantity_for_write(pass_value, validation, sharenums, allocated_size):
diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py
index 49e6d60..e20da2a 100644
--- a/src/_zkapauthorizer/tests/test_storage_server.py
+++ b/src/_zkapauthorizer/tests/test_storage_server.py
@@ -25,9 +25,9 @@ from prometheus_client import CollectorRegistry, Histogram
from challenge_bypass_ristretto import RandomToken, random_signing_key
from foolscap.referenceable import LocalReferenceable
from hypothesis import given, note
-from hypothesis.strategies import integers, just, lists, one_of, tuples
-from testtools import TestCase
-from testtools.matchers import AfterPreprocessing, Equals, MatchesAll
+from hypothesis.strategies import integers, just, lists, one_of, tuples, dictionaries
+from testtools import TestCase, skip
+from testtools.matchers import AfterPreprocessing, Not, Equals, MatchesAll, MatchesPredicate, AllMatch
from twisted.internet.task import Clock
from twisted.python.runtime import platform
@@ -51,6 +51,7 @@ from .strategies import (
lease_cancel_secrets,
lease_renew_secrets,
sharenum_sets,
+ sharenums,
sizes,
slot_test_and_write_vectors_for_shares,
storage_indexes,
@@ -150,8 +151,9 @@ def read_bucket(storage_server, size):
if size <= upper_bound:
break
+ note("bucket_number {}".format(bucket_number))
buckets = storage_server._metric_spending_successes._buckets
- note(list((n, b.get()) for n, b in enumerate(buckets)))
+ note("bucket counters: {}".format(list((n, b.get()) for n, b in enumerate(buckets))))
return buckets[bucket_number].get()
@@ -676,7 +678,63 @@ class PassValidationTests(TestCase):
after_count = read_count(self.storage_server)
after_bucket = read_bucket(self.storage_server, size)
- note("bucket_number {}".format(bucket_number))
+ self.expectThat(
+ after_count - before_count,
+ Equals(expected),
+ "Unexpected histogram sum value",
+ )
+ self.assertThat(
+ after_bucket - before_bucket,
+ Equals(expected),
+ "Unexpected histogram bucket value",
+ )
+
+ @given(
+ storage_index=storage_indexes(),
+ renew_secret=lease_renew_secrets(),
+ cancel_secret=lease_cancel_secrets(),
+ sharenums=sharenum_sets(),
+ allocated_size=sizes(),
+ )
+ def test_add_lease_metrics(
+ self, storage_index, renew_secret, cancel_secret, sharenums, allocated_size,
+ ):
+ # Create some shares at a slot which will require lease renewal.
+ write_toy_shares(
+ self.anonymous_storage_server,
+ storage_index,
+ renew_secret,
+ cancel_secret,
+ sharenums,
+ allocated_size,
+ LocalReferenceable(None),
+ )
+
+ expected = required_passes(
+ self.storage_server._pass_value, [allocated_size] * len(sharenums)
+ )
+ valid_passes = make_passes(
+ self.signing_key,
+ add_lease_message(storage_index),
+ list(RandomToken.create() for i in range(expected)),
+ )
+
+ before_count = read_count(self.storage_server)
+ before_bucket = read_bucket(self.storage_server, allocated_size)
+
+ self.storage_server.doRemoteCall(
+ "add_lease",
+ (),
+ dict(
+ passes=valid_passes,
+ storage_index=storage_index,
+ renew_secret=renew_secret,
+ cancel_secret=cancel_secret,
+ ),
+ )
+
+ after_count = read_count(self.storage_server)
+ after_bucket = read_bucket(self.storage_server, allocated_size)
self.expectThat(
after_count - before_count,
--
GitLab