Changed backoff to be exponential. This will help to reduce apply_issue flakiness.

rietveld.py

 # coding: utf-8
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 """Defines class Rietveld to easily access a rietveld instance.
 
 Security implications:
 
 The following hypothesis are made:
 - Rietveld enforces:
@@ skipping 416 matching lines @@
             raise
           if (not 'Name or service not known' in e.reason and
               not 'EOF occurred in violation of protocol' in e.reason):
             # Usually internal GAE flakiness.
             raise
         except ssl.SSLError, e:
           if retry >= (maxtries - 1):
             raise
           if not 'timed out' in str(e):
             raise
-        # If reaching this line, loop again. Uses a small backoff.
-        time.sleep(1+maxtries*2)
+        # If reaching this line, loop again. Uses an exponential backoff.
+        time.sleep(1+5**retry)

[M-A Ruel, 2014/06/11 13:27:41]

5**3 = 125. That's a hell of a long time.

I don't see why it would sleep more than 10 seconds. It's better to fail that be
sleeping for several minutes.

What's the current failure rate?

[Sergiy Byelozyorov, 2014/06/11 15:46:42]

I guess what I didn't consider is interactive usage. Is this used in any
commands which can be invoked by the users interactively?
In fact, it is quite low: about 0.23% on tryserver.chromium.

[M-A Ruel, 2014/06/11 17:52:48]

Oh, I'm concerned about try jobs what would get stuck in a loop too.
That's surprisingly high, especially that the code is already trying 5 times.
What I'm concerned is that these .23% failures could be:
- failed to apply the patch at all. (?)
- the DB is corrupted, so retrying wouldn't help.

This CL only helps when the transient failure is lasting more than 16 seconds
(previous total sleep time) but less than 160s (new sleep time).

I'd recommend to try more often than sleeping more. It's GAE, it can sustain the
load.

[Sergiy Byelozyorov, 2014/06/12 09:43:50]

Why is this an issue? We only wait extra few minutes and this only happens 0.23%
of the time.
These are all urllib2.URLError errors when connecting to the GAE.
Previously it was 55 seconds (11 seconds each time as maxtries was used instead
of retry). New time would be 786s (2s + 6s + 26s + 126s + 626s), each longer
delay with decreasing probability.
 
We can try that too. This CL is an experiment anyway. We do not know what's
really happening. I'll increase the number of tries and reduce the delay.

[M-A Ruel, 2014/06/12 12:18:07]

Yes but I don't know if you specifically looked at url errors or a raw
apply_issue to calculate the number.
It only sleeps for 0, 1, 2, and 3. It doesn't sleep for try #4.
Please do this instead. It's fine to hammer the server. There's a few things you
may want to optimize for:
- Make it work reliably even when the server is transiently stable.
- Make it fail fast to save costs.
- Make it succeed fast to save devs time.
- Reduce the number of requests to save costs.
- Increase the number of requests to get faster turn around.

It's clear to me here that you should aim to optimize is to save devs time. As
such, sleeping is a net cost with no benefit.

     finally:
       upload.ErrorExit = old_error_exit
 
   # DEPRECATED.
   Send = get
 
 
 class OAuthRpcServer(object):
   def __init__(self,
                host,
@@ skipping 268 matching lines @@
   def trigger_try_jobs(  # pylint:disable=R0201
       self, issue, patchset, reason, clobber, revision, builders_and_tests,
       master=None):
     logging.info('ReadOnlyRietveld: triggering try jobs %r for issue %d' %
         (builders_and_tests, issue))
 
   def trigger_distributed_try_jobs(  # pylint:disable=R0201
       self, issue, patchset, reason, clobber, revision, masters):
     logging.info('ReadOnlyRietveld: triggering try jobs %r for issue %d' %
         (masters, issue))