sync: Attempt to fix sync scheduler test flakiness

sync: Attempt to fix sync scheduler test flakiness

Increases the delay duration in several sync scheduler unit tests.
According to [1], the timer resolution on Windows may be under 1ms.  We
suspect the low timer resolution compbined with very short delays could
cause problems in some unit tests.  This will make the tests slower, but
hopefully it will make them more reliable, too.

Changes a timestmap comparison in nudge_tracker.cc from < to <=.  We
suspect that a lower timer resolution might allow two Time::Now() calls
that should be separated by a >1ms gap of time might still return the
same value in both calls.  If this did happen, it could be the cause of
misbehaving unit tests.

It is believed that these issues only affect tests.  The delay lengths
used in the real world should be much longer than 1ms.  The timer
resolution should not be an issue for delays of that length.

[1] http://www.chromium.org/developers/design-documents/threading

BUG=402212
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289731

[rlarocque, 2014-08-11 21:02:04]

Looks like the delay timers I helped pick for the SyncSchedulerTests were a bit
too aggressive.  I'm hoping that increasing them will fix the flakiness reported
in crbug.com/402212.

I'm going to wait a while to see if new data fits my hypothesis.  If all goes
well, I'll try to commit this patch tomorrow.

Please review.

[haitaol1, 2014-08-11 22:18:54]

lgtm

[rlarocque, 2014-08-12 21:25:55]

+jam: Did your patch to disable end up fixing the issue, as expected?  Would you
mind if I try to land this attempt at a more permanent fix?

By the way, how did you first notice that sync_unit_tests were affecting cycle
times in the first place?  Is it just a matter of looking for outliers in recent
trybot runs, or is there a better way to do it?

[jam, 2014-08-12 23:59:03]

On 2014/08/12 21:25:55, rlarocque wrote:
> +jam: Did your patch to disable end up fixing the issue, as expected?  Would
you
> mind if I try to land this attempt at a more permanent fix?

it didn't fix it (per graph below)

i.e.
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...

> 
> By the way, how did you first notice that sync_unit_tests were affecting cycle
> times in the first place?  Is it just a matter of looking for outliers in
recent
> trybot runs, or is there a better way to do it?

looking at the graphs on
http://build.chromium.org/p/tryserver.chromium.win/stats/win_chromium_rel_swa...

[rlarocque, 2014-08-13 00:07:51]

On 2014/08/12 23:59:03, jam wrote:
> On 2014/08/12 21:25:55, rlarocque wrote:
> > +jam: Did your patch to disable end up fixing the issue, as expected?  Would
> you
> > mind if I try to land this attempt at a more permanent fix?
> 
> it didn't fix it (per graph below)
> 
> i.e.
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
> 
> > 
> > By the way, how did you first notice that sync_unit_tests were affecting
cycle
> > times in the first place?  Is it just a matter of looking for outliers in
> recent
> > trybot runs, or is there a better way to do it?
> 
> looking at the graphs on
>
http://build.chromium.org/p/tryserver.chromium.win/stats/win_chromium_rel_swa...

Thanks.  This is actually good news, in a way.  

The results you linked to showed that SyncSchedulerTest.FailedRetry failed this
time.  My hypothesis (posted in the bug) could explain that.  Assuming my
hypothesis is correct, this CL should fix it.

Since the bots are currently still broken, it seems worthwhile to give this a
shot.  I'll send it to the CQ.  Feel free to abort if you have any concerns or
objections.

[rlarocque, 2014-08-13 00:21:08]

On 2014/08/13 00:07:51, rlarocque wrote:
> On 2014/08/12 23:59:03, jam wrote:
> > On 2014/08/12 21:25:55, rlarocque wrote:
> > > +jam: Did your patch to disable end up fixing the issue, as expected? 
Would
> > you
> > > mind if I try to land this attempt at a more permanent fix?
> > 
> > it didn't fix it (per graph below)
> > 
> > i.e.
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
> > 
> > > 
> > > By the way, how did you first notice that sync_unit_tests were affecting
> cycle
> > > times in the first place?  Is it just a matter of looking for outliers in
> > recent
> > > trybot runs, or is there a better way to do it?
> > 
> > looking at the graphs on
> >
>
http://build.chromium.org/p/tryserver.chromium.win/stats/win_chromium_rel_swa...
> 
> Thanks.  This is actually good news, in a way.  
> 
> The results you linked to showed that SyncSchedulerTest.FailedRetry failed
this
> time.  My hypothesis (posted in the bug) could explain that.  Assuming my
> hypothesis is correct, this CL should fix it.
> 
> Since the bots are currently still broken, it seems worthwhile to give this a
> shot.  I'll send it to the CQ.  Feel free to abort if you have any concerns or
> objections.

As discussed in https://codereview.chromium.org/468703002/, I'll rebase and try
again tomorrow.

[jabdelmalek, 2014-08-13 00:26:41]

On 2014/08/13 00:07:51, rlarocque wrote:
> On 2014/08/12 23:59:03, jam wrote:
> > On 2014/08/12 21:25:55, rlarocque wrote:
> > > +jam: Did your patch to disable end up fixing the issue, as expected? 
Would
> > you
> > > mind if I try to land this attempt at a more permanent fix?
> > 
> > it didn't fix it (per graph below)
> > 
> > i.e.
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
> > 
> > > 
> > > By the way, how did you first notice that sync_unit_tests were affecting
> cycle
> > > times in the first place?  Is it just a matter of looking for outliers in
> > recent
> > > trybot runs, or is there a better way to do it?
> > 
> > looking at the graphs on
> >
>
http://build.chromium.org/p/tryserver.chromium.win/stats/win_chromium_rel_swa...
> 
> Thanks.  This is actually good news, in a way.  
> 
> The results you linked to showed that SyncSchedulerTest.FailedRetry failed
this
> time.  My hypothesis (posted in the bug) could explain that.  Assuming my
> hypothesis is correct, this CL should fix it.
> 
> Since the bots are currently still broken, it seems worthwhile to give this a
> shot.  I'll send it to the CQ.  Feel free to abort if you have any concerns or
> objections.

since they were failing often before, please verify this by sending say 10 try
jobs on Windows to check that they're all fast. preferably do this in a slow
time when the trybots aren't overloaded :)

[rlarocque, 2014-08-14 18:01:37]

On 2014/08/13 00:26:41, jabdelmalek wrote:
> On 2014/08/13 00:07:51, rlarocque wrote:
> > On 2014/08/12 23:59:03, jam wrote:
> > > On 2014/08/12 21:25:55, rlarocque wrote:
> > > > +jam: Did your patch to disable end up fixing the issue, as expected? 
> Would
> > > you
> > > > mind if I try to land this attempt at a more permanent fix?
> > > 
> > > it didn't fix it (per graph below)
> > > 
> > > i.e.
> > >
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...
> > > 
> > > > 
> > > > By the way, how did you first notice that sync_unit_tests were affecting
> > cycle
> > > > times in the first place?  Is it just a matter of looking for outliers
in
> > > recent
> > > > trybot runs, or is there a better way to do it?
> > > 
> > > looking at the graphs on
> > >
> >
>
http://build.chromium.org/p/tryserver.chromium.win/stats/win_chromium_rel_swa...
> > 
> > Thanks.  This is actually good news, in a way.  
> > 
> > The results you linked to showed that SyncSchedulerTest.FailedRetry failed
> this
> > time.  My hypothesis (posted in the bug) could explain that.  Assuming my
> > hypothesis is correct, this CL should fix it.
> > 
> > Since the bots are currently still broken, it seems worthwhile to give this
a
> > shot.  I'll send it to the CQ.  Feel free to abort if you have any concerns
or
> > objections.
> 
> since they were failing often before, please verify this by sending say 10 try
> jobs on Windows to check that they're all fast. preferably do this in a slow
> time when the trybots aren't overloaded :)

I've got nine green trybots, and all finished sync_unit_tests in under 20s.  I'm
going to give this a shot.

By the way, it appears that jam's DISABLE patches have worked.  The run times
for sync_unit_tests are a lot less variable now.  If this CL re-breaks things,
it will be easy to see that this is the cause.

[rlarocque, 2014-08-14 18:14:28]

The CQ bit was checked by rlarocque@chromium.org

[commit-bot: I haz the power, 2014-08-14 18:17:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rlarocque@chromium.org/463563005/20001

[commit-bot: I haz the power, 2014-08-15 00:11:21]

Committed patchset #2 (20001) as 289731