Logic for deciding about if a CL might be at fault seems buggy |
||
Issue descriptionTake a look at these two CLs: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1056073/ https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1056074/ --- ...both CLs got picked up in the 10:13 am CQ run today. From the logs: You can follow along at http://cros-goldeneye/chromeos/healthmonitoring/buildDetails?buildbucketId=8945809517871216736. You can find the CL status and build information at https://chromeos-cl-viewer-ui.googleplex.com/cl_status/chromium-review.googlesource.com/1056074/1. or: You can find the CL status and build information at https://chromeos-cl-viewer-ui.googleplex.com/cl_status/chromium-review.googlesource.com/1056073/1. --- ...both CLs touch nearly the same files and are in the same kernel repository. --- ...both CLs touch kernel 4.14 only and none of the failing paladins were on boards using kernel 4.14 --- Despite the massive similarity between these two CLs and the fact that they are touching a kernel that almost no boards use (yet), one of the CLs was submitted and the other was denied. This seems like it points to a bug somewhere. Can anyone who knows this logic better dig deeper?
,
May 22 2018
Hi Doug, Looking through the logs and the code, it appears that the reason that one was picked up, and the other was not, is that it was associated with a successful slave execution of tests whereas 1056074 was associated with a failed execution as was marked as "not ignorable". 12:16:43: INFO: Build edgar-paladin failed with not ignorable failures, will not submit changes: CL:1056074 In both cases the CLs were picked up by the edgar-paladin build config as being relevant. The core reason for inclusion of one over the other is the previous execution; CL 1056073 passed a relevant edgar-paladin test whereas 1056074 failed in both attempts: https://paste.googleplex.com/4953640800354304 The logic states that some stages cannot be skipped, therefore 1056074 was kicked, but a previous execution is used to provide a pass for 1056073. Let me know if you have any further questions regarding this or if I missed anything in my research. Thanks, Mike
,
May 22 2018
I will mark this "working as intended" if you are satisfied with the results. Thanks, Mike
,
May 22 2018
@3: AH, I see. So it was because of the results of a previous CQ run that the first CL landed. I guess that makes sense. BTW: I thought that we also had rules such that a board not running a given kernel wouldn't prevent submission of a kernel change. AKA "edgar" doesn't use kernel 4.14, so it shouldn't block 4.14 changes. Did I dream that logic, did we lose it, or do we simply need to update it for kernel 4.14? Thanks much for your research!
,
May 23 2018
Hey Doug, I'm rather green as to the configuration of the relational configuration. I'll have to do some additional digging as the only rules I'm seeing do not go to the kernel level. Not sure if it exists and I'm not seeing it, existed and was removed, or otherwise. -- Mike
,
May 23 2018
@5: OK, I dug a little myself and I think I was remembering it wrong. It's only pre-cq that has this magic (pre-cq-configs in COMMIT-QUEUE.ini), not the CQ. I guess something like this could be a feature request for the CQ eventually, but it's not there now. I guess we can close this bug. Do you think it's worth opening up a feature request for something like this for the main commit queue?
,
May 23 2018
I think it is worth opening a feature request; with the load the build infrastructure is under, avoiding builds that are deemed unnecessary will provide benefits in freeing up resources. Thanks Doug. -- Mike
,
May 23 2018
,
May 23 2018
Feature request in bug #845941 |
||
►
Sign in to add a comment |
||
Comment 1 by mikenichols@chromium.org
, May 22 2018