ZeroDivisionError in web/status.py

[davidsarah]

​http://tahoe-lafs.org/buildbot/builders/FreeStorm%20WinXP-x86%20py2.6/builds/252/steps/test/logs/stdio

[ERROR]: allmydata.test.test_system.SystemTest.test_filesystem

Traceback (most recent call last):
  File "c:\python26\lib\site-packages\nevow-0.10.0-py2.6.egg\nevow\flat\twist.py", line 24, in _drive
    next = iterable.next()
  File "c:\python26\lib\site-packages\nevow-0.10.0-py2.6.egg\nevow\flat\ten.py", line 83, in iterflatten
    for item in gen:
  File "c:\python26\lib\site-packages\nevow-0.10.0-py2.6.egg\nevow\flat\flatstan.py", line 103, in TagSerializer
    yield serialize(toBeRenderedBy, context)
  File "c:\python26\lib\site-packages\nevow-0.10.0-py2.6.egg\nevow\flat\ten.py", line 70, in serialize
    return partialflatten(context, obj)
  File "c:\python26\lib\site-packages\nevow-0.10.0-py2.6.egg\nevow\flat\ten.py", line 61, in partialflatten
    return flattener(obj, context)
  File "c:\python26\lib\site-packages\nevow-0.10.0-py2.6.egg\nevow\flat\flatstan.py", line 264, in DirectiveSerializer
    return serialize(renderer, context)
  File "c:\python26\lib\site-packages\nevow-0.10.0-py2.6.egg\nevow\flat\ten.py", line 70, in serialize
    return partialflatten(context, obj)
  File "c:\python26\lib\site-packages\nevow-0.10.0-py2.6.egg\nevow\flat\ten.py", line 61, in partialflatten
    return flattener(obj, context)
  File "c:\python26\lib\site-packages\nevow-0.10.0-py2.6.egg\nevow\flat\flatstan.py", line 247, in MethodSerializer
    return FunctionSerializer(original, context, nocontext)
  File "c:\python26\lib\site-packages\nevow-0.10.0-py2.6.egg\nevow\flat\flatstan.py", line 236, in FunctionSerializer
    result = original(context, data)
  File "c:\buildbot_tahoe\freestorm_winxp-x86_py2.6\build\src\allmydata\web\status.py", line 462, in render_events
    speed = self.render_rate(None, 1.0 * seglen / segtime)
exceptions.ZeroDivisionError: float division

Looks like segtime was zero because something took less time than the clock granularity.

[warner]

avoid divide-by-zero. Windows, how I hate thee.

[zooko]

Oh come on, you can't blame Windows! "I will tick my clock faster than you do anything you care about." is not part of the contract that we choose to rely on. Also note that even after attachment:1166-zerodiv.diff​, when rtt > 0 but rtt <= ϵ then the speed measurement will be wildly inaccurate.

[warner]

ok, ok.

Needs tests anyways: test_web should acquire a second DownloadStatus instance in its FakeHistory object (with zero durations), and Web.test_status should make sure it renders without exception.

I don't know what to suggest about very small durations and very large speed measurements. Since the duration is clearly visible next to the speed, I believe users will figure it out for themselves ("huh, 18 terabytes per second? oh, look, and the whole thing completed in one microsecond. maybe that's not very accurate").

[warner]

Maybe we should change the common abbreviate_rate function to take (bytes, seconds) arguments and have it do the division (and zero-avoidance) in one central place.

[davidsarah]

Replying to warner:

Maybe we should change the common abbreviate_rate function to take (bytes, seconds) arguments and have it do the division (and zero-avoidance) in one central place.

+1.

(Brian and Zooko are both right: we shouldn't rely on the clock ticking fast enough, but Windows' clock ticks stupidly slowly.)

[zooko]

Is this important enough to fix in 1.8.0?

[francois]

Looks like an easy ticket, I'll give it a go.

[francois]

The attached patch contains a test case for this specific bug and a complete refactoring of rate computation on the status page.

Replying to warner:

Maybe we should change the common abbreviate_rate function to take (bytes, seconds) arguments and have it do the division (and zero-avoidance) in one central place.

This is implemented in a new function compute_rate. It sounds cleaner to me, because abbreviate_rate is called (via render_rate) from Nevow templates which only pass a single data argument. Moreover, this allows the function to be used more often in web/status.py.

[warner]

wait, what's that 0.1 doing in there? If it said 1.0, and if the tests were updated to match, I'd be +1 on this patch.

[francois]

Replying to warner:

wait, what's that 0.1 doing in there? If it said 1.0, and if the tests were updated to match, I'd be +1 on this patch.

Oh crap! Can't believe how I missed that. Thanks for spotting it!

This is probably an indication that tests were not thorough enough, so I added a new human understandable one.

[terrell]

looks good to me.

[zooko]

terrell reviewed it so I'm removing the review-needed and adding reviewed.

[francois@…]

In f026927f86584870:

web: refactor rate computation, fixes #1166