Mailing List Archive: Zope: Dev

time.sleep in a boolean check in zope.publisher.http.HTTPRequest.supportsRetry?

Index | Next | Previous | View Threaded

gary.poster at gmail

Aug 17, 2009, 10:43 AM

Post #1 of 3 (483 views)
Permalink

time.sleep in a boolean check in zope.publisher.http.HTTPRequest.supportsRetry?

Two teams here at Canonical just encountered the STAGGER_RETRIES
behavior in http://svn.zope.org/zope.publisher/trunk/src/zope/publisher/http.py?rev=101538&view=auto
. I don't see anything in tests or comments to explain it. Our
guess is that it tries to put some breathing room around retries so
that the chance of a conflict error might be reduced.

In one of our tests setting STAGGER_RETRIES to False reduced a test
run from almost 9 minutes to about 1 minute (see https://bugs.edge.launchpad.net/launchpad-foundations/+bug/401586)
. We have papered this over in our test suite to no ill effect,
giving speed advantages. We wonder if we should remove the behavior
entirely, even in production.

1) Why should the time.sleep go into supportsRetry rather than retry?
it seems really odd to have it in the method that returns a boolean,
rather than the one that does the work.

2) Can someone give some background for this code? Can they give
examples of it actually helping anything?

We'd like to improve this, minimally by adding some explanatory
comments, and maybe by changing, moving, or removing this code.

(If anyone tries to do an "annotate" on this, you'll see Jim checked
this in back at the dawns of time in rev 8532. On IRC, he didn't
recognize this code on a quick look, so he thinks someone else might
be more familiar with this.)

Gary
_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

jim at zope

Aug 20, 2009, 10:50 AM

Post #2 of 3 (436 views)
Permalink

Re: time.sleep in a boolean check in zope.publisher.http.HTTPRequest.supportsRetry? [In reply to]

On Mon, Aug 17, 2009 at 1:43 PM, Gary Poster<gary.poster [at] gmail> wrote:
> Two teams here at Canonical just encountered the STAGGER_RETRIES
> behavior in http://svn.zope.org/zope.publisher/trunk/src/zope/publisher/http.py?rev=101538&view=auto
> . I don't see anything in tests or comments to explain it. Our
> guess is that it tries to put some breathing room around retries so
> that the chance of a conflict error might be reduced.

Yup, although I think it's misguided in this case. With conflicts,
there's always a winner, so it makes sense to try again right away.

>
> In one of our tests setting STAGGER_RETRIES to False reduced a test
> run from almost 9 minutes to about 1 minute (see https://bugs.edge.launchpad.net/launchpad-foundations/+bug/401586)
> . We have papered this over in our test suite to no ill effect,
> giving speed advantages. We wonder if we should remove the behavior
> entirely, even in production.

I think so.

> 1) Why should the time.sleep go into supportsRetry rather than retry?
> it seems really odd to have it in the method that returns a boolean,
> rather than the one that does the work.

Yup.

> 2) Can someone give some background for this code? Can they give
> examples of it actually helping anything?

I doubt it.

> We'd like to improve this, minimally by adding some explanatory
> comments, and maybe by changing, moving, or removing this code.

Let's just remove it.

Jim

--
Jim Fulton
_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

gary.poster at gmail

Aug 20, 2009, 12:51 PM

Post #3 of 3 (434 views)
Permalink

Re: time.sleep in a boolean check in zope.publisher.http.HTTPRequest.supportsRetry? [In reply to]

On Aug 20, 2009, at 1:50 PM, Jim Fulton wrote:

> On Mon, Aug 17, 2009 at 1:43 PM, Gary Poster<gary.poster [at] gmail>
> wrote:
>> Two teams here at Canonical just encountered the STAGGER_RETRIES
>> behavior in http://svn.zope.org/zope.publisher/trunk/src/zope/publisher/http.py?rev=101538&view=auto
>> . I don't see anything in tests or comments to explain it. Our
>> guess is that it tries to put some breathing room around retries so
>> that the chance of a conflict error might be reduced.
>
> Yup, although I think it's misguided in this case. With conflicts,
> there's always a winner, so it makes sense to try again right away.
>
>>
>> In one of our tests setting STAGGER_RETRIES to False reduced a test
>> run from almost 9 minutes to about 1 minute (see https://bugs.edge.launchpad.net/launchpad-foundations/+bug/401586)
>> . We have papered this over in our test suite to no ill effect,
>> giving speed advantages. We wonder if we should remove the behavior
>> entirely, even in production.
>
> I think so.
>
>> 1) Why should the time.sleep go into supportsRetry rather than retry?
>> it seems really odd to have it in the method that returns a boolean,
>> rather than the one that does the work.
>
> Yup.
>
>> 2) Can someone give some background for this code? Can they give
>> examples of it actually helping anything?
>
> I doubt it.
>
>> We'd like to improve this, minimally by adding some explanatory
>> comments, and maybe by changing, moving, or removing this code.
>
> Let's just remove it.

Cool, I'll do it tonight or tomorrow (have to run right now). Thank
you very much!

Gary

_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads