[Zope-dev] time.sleep in a boolean check in zope.publisher.http.HTTPRequest.supportsRetry?

[Jim Fulton]

On Mon, Aug 17, 2009 at 1:43 PM, Gary Poster<gary.poster at gmail.com> 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