web: refactor rate computation, fixes #1166

src/allmydata/test/test_web.py

@@ -87,6 +87,10 @@ def build_one_ds():
     ds.add_segment_request(2, now+4)
     ds.add_segment_request(3, now+5)
 
+    # simulate a segment which gets delivered faster than a system clock tick (ticket #1166)
+    ds.add_segment_request(4, now)
+    ds.add_segment_delivery(4, now, 0, 140, 0.5)
+
     e = ds.add_dyhb_sent("serverid_a", now)
     e.finished([1,2], now+1)
     e = ds.add_dyhb_sent("serverid_b", now+2) # left unfinished
@@ -3169,6 +3173,22 @@ class Util(ShouldFailMixin, testutil.ReallyEqualMixin, unittest.TestCase):
         self.failUnlessReallyEqual(common.abbreviate_time(0.000123), "123us")
         self.failUnlessReallyEqual(common.abbreviate_time(-123000), "-123000000000us")
 
+    def test_compute_rate(self):
+        self.failUnlessReallyEqual(common.compute_rate(None, None), None)
+        self.failUnlessReallyEqual(common.compute_rate(None, 1), None)
+        self.failUnlessReallyEqual(common.compute_rate(250000, None), None)
+        self.failUnlessReallyEqual(common.compute_rate(250000, 0), None)
+        self.failUnlessReallyEqual(common.compute_rate(250000, 10), 25000.0)
+        self.failUnlessReallyEqual(common.compute_rate(0, 10), 0.0)
+        self.shouldFail(AssertionError, "test_compute_rate", "",
+                        common.compute_rate, -100, 10)
+        self.shouldFail(AssertionError, "test_compute_rate", "",
+                        common.compute_rate, 100, -10)
+
+        # Sanity check
+        rate = common.compute_rate(10*1000*1000, 1)
+        self.failUnlessReallyEqual(common.abbreviate_rate(rate), "10.00MBps")
+
     def test_abbreviate_rate(self):
         self.failUnlessReallyEqual(common.abbreviate_rate(None), "")
         self.failUnlessReallyEqual(common.abbreviate_rate(1234000), "1.23MBps")

src/allmydata/web/common.py

@@ -90,6 +90,19 @@ def abbreviate_time(data):
         return "%.1fms" % (1000*s)
     return "%.0fus" % (1000000*s)
 
+def compute_rate(bytes, seconds):
+    if bytes is None:
+      return None
+
+    if seconds is None or seconds == 0:
+      return None
+
+    # negative values don't make sense here
+    assert bytes > -1
+    assert seconds > 0
+
+    return 1.0 * bytes / seconds
+
 def abbreviate_rate(data):
     # 21.8kBps, 554.4kBps 4.37MBps
     if data is None:

src/allmydata/web/status.py

@@ -5,7 +5,7 @@ from twisted.internet import defer
 from nevow import rend, inevow, tags as T
 from allmydata.util import base32, idlib
 from allmydata.web.common import getxmlfile, get_arg, \
-     abbreviate_time, abbreviate_rate, abbreviate_size, plural
+     abbreviate_time, abbreviate_rate, abbreviate_size, plural, compute_rate
 from allmydata.interfaces import IUploadStatus, IDownloadStatus, \
      IPublishStatus, IRetrieveStatus, IServermapUpdaterStatus
 
@@ -110,12 +110,7 @@ class UploadResultsRendererMixin(RateAndTimeMixin):
         def _convert(r):
             file_size = r.file_size
             time = r.timings.get(name)
-            if time is None:
+            return compute_rate(file_size, time)
-                return None
-            try:
-                return 1.0 * file_size / time
-            except ZeroDivisionError:
-                return None
         d.addCallback(_convert)
         return d
 
@@ -137,12 +132,10 @@ class UploadResultsRendererMixin(RateAndTimeMixin):
             file_size = r.file_size
             time1 = r.timings.get("cumulative_encoding")
             time2 = r.timings.get("cumulative_sending")
-            if (file_size is None or time1 is None or time2 is None):
+            if (time1 is None or time2 is None):
-                return None
-            try:
-                return 1.0 * file_size / (time1+time2)
-            except ZeroDivisionError:
                 return None
+            else:
+                return compute_rate(file_size, time1+time2)
         d.addCallback(_convert)
         return d
 
@@ -151,12 +144,7 @@ class UploadResultsRendererMixin(RateAndTimeMixin):
         def _convert(r):
             fetch_size = r.ciphertext_fetched
             time = r.timings.get("cumulative_fetch")
-            if (fetch_size is None or time is None):
+            return compute_rate(fetch_size, time)
-                return None
-            try:
-                return 1.0 * fetch_size / time
-            except ZeroDivisionError:
-                return None
         d.addCallback(_convert)
         return d
 
@@ -308,12 +296,7 @@ class DownloadResultsRendererMixin(RateAndTimeMixin):
         def _convert(r):
             file_size = r.file_size
             time = r.timings.get(name)
-            if time is None:
+            return compute_rate(file_size, time)
-                return None
-            try:
-                return 1.0 * file_size / time
-            except ZeroDivisionError:
-                return None
         d.addCallback(_convert)
         return d
 
@@ -433,7 +416,7 @@ class DownloadStatusPage(DownloadResultsRendererMixin, rend.Page):
             (start, length, requesttime, finishtime, bytes, decrypt, paused) = r_ev
             if finishtime is not None:
                 rtt = finishtime - requesttime - paused
-                speed = self.render_rate(None, 1.0 * bytes / rtt)
+                speed = self.render_rate(None, compute_rate(bytes, rtt))
                 rtt = self.render_time(None, rtt)
                 decrypt = self.render_time(None, decrypt)
                 paused = self.render_time(None, paused)
@@ -459,7 +442,7 @@ class DownloadStatusPage(DownloadResultsRendererMixin, rend.Page):
             elif etype == "delivery":
                 if reqtime[0] == segnum:
                     segtime = when - reqtime[1]
-                    speed = self.render_rate(None, 1.0 * seglen / segtime)
+                    speed = self.render_rate(None, compute_rate(seglen, segtime))
                     segtime = self.render_time(None, segtime)
                 else:
                     segtime, speed = "", ""
@@ -595,12 +578,7 @@ class RetrieveStatusPage(rend.Page, RateAndTimeMixin):
     def _get_rate(self, data, name):
         file_size = self.retrieve_status.get_size()
         time = self.retrieve_status.timings.get(name)
-        if time is None or file_size is None:
+        return compute_rate(file_size, time)
-            return None
-        try:
-            return 1.0 * file_size / time
-        except ZeroDivisionError:
-            return None
 
     def data_time_total(self, ctx, data):
         return self.retrieve_status.timings.get("total")
@@ -701,12 +679,7 @@ class PublishStatusPage(rend.Page, RateAndTimeMixin):
     def _get_rate(self, data, name):
         file_size = self.publish_status.get_size()
         time = self.publish_status.timings.get(name)
-        if time is None:
+        return compute_rate(file_size, time)
-            return None
-        try:
-            return 1.0 * file_size / time
-        except ZeroDivisionError:
-            return None
 
     def data_time_total(self, ctx, data):
         return self.publish_status.timings.get("total")