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=1015... . 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
On Mon, Aug 17, 2009 at 1:43 PM, Gary Poster<gary.poster@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=1015... . 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
On Aug 20, 2009, at 1:50 PM, Jim Fulton wrote:
On Mon, Aug 17, 2009 at 1:43 PM, Gary Poster<gary.poster@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=1015... . 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
participants (2)
-
Gary Poster -
Jim Fulton