ZeroDivisionError in web/status.py #1166

Closed
opened 2010-08-10 08:34:23 +00:00 by davidsarah · 15 comments
davidsarah commented 2010-08-10 08:34:23 +00:00
Owner

(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.

(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.
tahoe-lafs added the
code-frontend-web
major
defect
1.8β
labels 2010-08-10 08:34:23 +00:00
tahoe-lafs added this to the undecided milestone 2010-08-10 08:34:23 +00:00
tahoe-lafs modified the milestone from undecided to soon 2010-08-10 08:38:45 +00:00

Attachment 1166-zerodiv.diff (1437 bytes) added

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

**Attachment** 1166-zerodiv.diff (1437 bytes) added avoid divide-by-zero. Windows, how I hate thee.

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.

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.

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").

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").

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.

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 commented 2010-08-12 01:28:56 +00:00
Author
Owner

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.)

Replying to [warner](/tahoe-lafs/trac-2024-07-25/issues/1166#issuecomment-79335): > 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.)

Is this important enough to fix in 1.8.0?

Is this important enough to fix in 1.8.0?
zooko modified the milestone from soon to 1.8.0 2010-08-14 07:04:28 +00:00
francois commented 2010-08-14 08:31:46 +00:00
Author
Owner

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

Looks like an easy ticket, I'll give it a go.
francois commented 2010-08-14 10:27:58 +00:00
Author
Owner

Attachment rate-computation-refactoring.dpatch (7496 bytes) added

**Attachment** rate-computation-refactoring.dpatch (7496 bytes) added
francois commented 2010-08-14 10:36:01 +00:00
Author
Owner

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.

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](/tahoe-lafs/trac-2024-07-25/issues/1166#issuecomment-79335): > 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`.

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.

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 commented 2010-08-15 14:50:49 +00:00
Author
Owner

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.

Replying to [warner](/tahoe-lafs/trac-2024-07-25/issues/1166#issuecomment-79341): > 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.
francois commented 2010-08-15 14:51:04 +00:00
Author
Owner

Attachment rate-computation-refactoring-2.dpatch (7633 bytes) added

**Attachment** rate-computation-refactoring-2.dpatch (7633 bytes) added
terrell commented 2010-08-21 21:19:39 +00:00
Author
Owner

looks good to me.

looks good to me.

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

terrell reviewed it so I'm removing the `review-needed` and adding `reviewed`.
francois@ctrlaltdel.ch commented 2010-08-21 23:33:20 +00:00
Author
Owner

In changeset:f026927f86584870:

web: refactor rate computation, fixes #1166
In changeset:f026927f86584870: ``` web: refactor rate computation, fixes #1166 ```
tahoe-lafs added the
fixed
label 2010-08-21 23:33:20 +00:00
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Reference: tahoe-lafs/trac-2024-07-25#1166
No description provided.