tahoe-lafs/trac-2024-07-25

ZeroDivisionError in web/status.py #1166

New issue

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

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

warner commented

2010-08-11 07:16:37 +00:00

Author

Owner

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.

1166-zerodiv.diff

1.4 KiB

zooko commented

2010-08-11 07:26:23 +00:00

Author

Owner

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.

warner commented

2010-08-11 15:26:51 +00:00

Author

Owner

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

warner commented

2010-08-11 18:32:03 +00:00

Author

Owner

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-120992): > 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 commented

2010-08-14 07:04:28 +00:00

Author

Owner

Is this important enough to fix in 1.8.0?

tahoe-lafs 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.

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

rate-computation-refactoring.dpatch

7.3 KiB

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-120992): > 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 commented

2010-08-14 18:27:33 +00:00

Author

Owner

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-120998): > 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

rate-computation-refactoring-2.dpatch

7.5 KiB

terrell commented

2010-08-21 21:19:39 +00:00

Author

Owner

looks good to me.

zooko commented

2010-08-21 22:29:56 +00:00

Author

Owner

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

francois@ctrlaltdel.ch closed this issue

2010-08-21 23:33:20 +00:00

No labels

LeastAuthority.com automation

code-frontend-ftp-sftp

code-frontend-magic-folder

somebody else's problem

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: tahoe-lafs/trac-2024-07-25#1166

No description provided.