Retry RPC deadlines when fetching GAE settings.

Retry RPC deadlines when fetching GAE settings.

Should eliminate rare panics during initial settings fetch.

R=dnj@chromium.org
BUG=590594
Committed: https://github.com/luci/luci-go/commit/af6c7eaa57f2d353a135df9b61c6401144bb5c9e

[Vadim Sh., 2016-03-01 00:08:44]

Patchset #1 (id:1) has been deleted

[Vadim Sh., 2016-03-01 00:28:16]

vadimsh@chromium.org changed reviewers:
+ iannucci@chromium.org

[Vadim Sh., 2016-03-01 00:28:17]

Robbie, this is the test with discussed in
https://github.com/luci/luci-go/issues/11. PTAL.

[iannucci, 2016-03-01 00:44:21]

https://codereview.chromium.org/1748933002/diff/40001/common/lazyslot/lazyslo...
File common/lazyslot/lazyslot.go (right):

https://codereview.chromium.org/1748933002/diff/40001/common/lazyslot/lazyslo...
common/lazyslot/lazyslot.go:49: RetryCallback retry.Callback // called before
retries, useful in tests
Is this still used?

[Vadim Sh., 2016-03-01 01:24:05]

rebased, ptal

https://codereview.chromium.org/1748933002/diff/40001/common/lazyslot/lazyslo...
File common/lazyslot/lazyslot.go (right):

https://codereview.chromium.org/1748933002/diff/40001/common/lazyslot/lazyslo...
common/lazyslot/lazyslot.go:49: RetryCallback retry.Callback // called before
retries, useful in tests
On 2016/03/01 00:44:21, iannucci wrote:
> Is this still used?

No, but it can be useful in general (e.g. to plug in logging for retries), so I
kept it.

[dnj, 2016-03-01 02:21:40]

https://codereview.chromium.org/1748933002/diff/60001/server/settings/setting...
File server/settings/settings.go (right):

https://codereview.chromium.org/1748933002/diff/60001/server/settings/setting...
server/settings/settings.go:124: c, _ = clock.WithTimeout(c, 2*time.Second) //
trigger a retry after 2 sec RPC timeout
WDYT about moving the retries into the Fetcher instead of lazySlot? It keeps
lazySlot high-level ("I call this function!") and stops you from having to pass
parameters in and (effectively) back out.

[Vadim Sh., 2016-03-02 22:52:04]

PTAL

https://codereview.chromium.org/1748933002/diff/60001/server/settings/setting...
File server/settings/settings.go (right):

https://codereview.chromium.org/1748933002/diff/60001/server/settings/setting...
server/settings/settings.go:124: c, _ = clock.WithTimeout(c, 2*time.Second) //
trigger a retry after 2 sec RPC timeout
On 2016/03/01 02:21:40, dnj wrote:
> WDYT about moving the retries into the Fetcher instead of lazySlot? It keeps
> lazySlot high-level ("I call this function!") and stops you from having to
pass
> parameters in and (effectively) back out.

Done.

[dnj, 2016-03-02 23:04:25]

lgtm

[Vadim Sh., 2016-03-02 23:09:57]

The CQ bit was checked by vadimsh@chromium.org

[commit-bot: I haz the power, 2016-03-02 23:10:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1748933002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1748933002/80001

[commit-bot: I haz the power, 2016-03-02 23:18:58]

Description was changed from

==========
Retry RPC deadlines when fetching GAE settings.

Should eliminate rare panics during initial settings fetch.

R=dnj@chromium.org
BUG=590594
==========

to

==========
Retry RPC deadlines when fetching GAE settings.

Should eliminate rare panics during initial settings fetch.

R=dnj@chromium.org
BUG=590594

Committed:
https://github.com/luci/luci-go/commit/af6c7eaa57f2d353a135df9b61c6401144bb5c9e
==========

[commit-bot: I haz the power, 2016-03-02 23:19:02]

Committed patchset #4 (id:80001) as
https://github.com/luci/luci-go/commit/af6c7eaa57f2d353a135df9b61c6401144bb5c9e