New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 648290 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

CQ started unnecessary tryjob

Project Member Reported by jam@chromium.org, Sep 19 2016

Issue description

https://codereview.chromium.org/2345913002/#ps100001 already had a full set of green tryjobs. I had used "git cl try" before and the tryjobs were less than 24 hours old. Note that linux_android_rel_ng flaked twice, so I had manually picked it from the CQ UI. Then it turned green.

When I clicked CQ button, it decided to run linux_android_rel_ng again which is unnecessary.

Pawel: assigning to you as initial owner.
 
Cc: tandrii@chromium.org
Andrii do you know what may have happened there?

The two builds in question are below. Why is CQ launching the second one?

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/143584 (jam@ triggered just this builder manually with git cl try)

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/143782 (this is what CQ triggered despite above build succeeded)
Description: Show this description
For some reason CQ was considering linux_android_rel_ng not triggered at all during the second attempt:

master.tryserver.chromium.android:linux_android_rel_ng": "JobState(state=JOB_NOT_TRIGGERED, quota=0)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 29 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/82e171932d7e833248074404ee88b44fa30ab747

commit 82e171932d7e833248074404ee88b44fa30ab747
Author: phajdan <phajdan@google.com>
Date: Thu Sep 29 22:33:02 2016

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 29 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/82e171932d7e833248074404ee88b44fa30ab747

commit 82e171932d7e833248074404ee88b44fa30ab747
Author: phajdan <phajdan@google.com>
Date: Thu Sep 29 22:33:02 2016

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 4 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/993934927ac93f1ef14428668a33f21d0305c151

commit 993934927ac93f1ef14428668a33f21d0305c151
Author: phajdan <phajdan@google.com>
Date: Tue Oct 04 15:38:45 2016

Pawel managed to repro this on chromium/src, but I failed to repro it to infra/infra. So, trying to repro on chromium/src too.
OK, turns out the immediate cause of no repro for me was that I ran:
 git cl try -b linux_android_rel_ng -m tryserver.chromium.android    # good, CQ uses this.
while Pawel, and I think jam@, ran:
 git cl try -b linux_android_rel_ng       # bad, CQ doesn't notice this.

which results in roughly this diff in buildbucket properties:

AssertionError: {u'status': u'COMPLETED', 'parameters': {u'changes': [{u'revision': None, u'auth [truncated]... != {u'status': u'COMPLETED', 'parameters': {u'changes': [{u'revision': None, u'auth [truncated]...
  {u'bucket': u'master.tryserver.chromium.android',
   u'created_by': u'user:tandrii@chromium.org',
   'parameters': {u'builder_name': u'linux_android_rel_ng',
                  u'changes': [{u'author': {u'email': u'tandrii@chromium.org'},
                                u'revision': None}],
                  u'properties': {u'category': u'git_cl_try',
                                  u'issue': 2389803007,
-                                 u'master': u'master.tryserver.chromium.android',
?                                              -------

+                                 u'master': u'tryserver.chromium.android',
                                  u'patch_project': u'chromium',
                                  u'patch_storage': u'rietveld',
-                                 u'patchset': 1,
+                                 u'patchset': 20001,
?                                              ++++

                                  u'reason': u'issue-648290',
                                  u'rietveld': u'https://codereview.chromium.org'}},
   u'result': u'SUCCESS',
   u'status': u'COMPLETED',
-  u'tags': [u'bot_id:slave985-c4',
?                          ^^^

+  u'tags': [u'bot_id:slave603-c4',
?                          ^^^

             u'builder:linux_android_rel_ng',
-            u'buildset:patch/rietveld/codereview.chromium.org/2389803007/1',
+            u'buildset:patch/rietveld/codereview.chromium.org/2389803007/20001',
?                                                                         ++++

-            u'master:master.tryserver.chromium.android',
?                     -------

+            u'master:tryserver.chromium.android',
             u'user_agent:git_cl_try']}

of course buildset and patchset above are not important -> these are simply artifcats of repro. but I suspect the master tag is important here. It's important to note that bucket is set correctly in both cases.
btw, i made this diff with these two files (attached).
good_bad.py
572 bytes View Download
good.bad
5.0 KB Download
CL for immediate fix for this bug in CQ: https://chromereviews.googleplex.com/518017013

As for git cl, i don't know what to do. I'd completely remove 'master' from all properties and pave the way for LUCI: https://codereview.chromium.org/2397773002

See also related  issue 637031  to add -B --bucket flag to git cl try.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 5 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/17d89e94dd886171d6ef2cc51743bae4cf113e44

commit 17d89e94dd886171d6ef2cc51743bae4cf113e44
Author: tandrii <tandrii@google.com>
Date: Wed Oct 05 14:59:49 2016

and it seems my fix didn't help. I am not yet sure why. We have to go deeper...
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 5 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/9c8a2d27b50e0f1be5fe1db565d6bed27741cd29

commit 9c8a2d27b50e0f1be5fe1db565d6bed27741cd29
Author: tandrii <tandrii@google.com>
Date: Wed Oct 05 15:31:26 2016

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 5 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/29af2ee5a8240b97bd0e01cea34ecbf837f44d54

commit 29af2ee5a8240b97bd0e01cea34ecbf837f44d54
Author: tandrii <tandrii@google.com>
Date: Wed Oct 05 16:12:25 2016

Cc: phajdan.jr@chromium.org
Owner: tandrii@chromium.org
Status: Fixed (was: Assigned)
Well, so I think this is fixed, my comment #13 was wrong, it was my fault at testing. I've just re-tested in https://codereview.chromium.org/2389803007/ PS#7.

Furthermore, this CL adds an extra test to CQ to ensure no regressions https://chromereviews.googleplex.com/517497013/ 
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 6 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/86f30f7e1d4d0322ecfb1dc723a28bc7b6a171df

commit 86f30f7e1d4d0322ecfb1dc723a28bc7b6a171df
Author: tandrii <tandrii@google.com>
Date: Thu Oct 06 12:29:45 2016

Sign in to add a comment