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

Issue 690105 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

don't reject CLs if tree is throttled at the end of a CQ run

Project Member Reported by akes...@chromium.org, Feb 8 2017

Issue description

Current behavior: 

If the tree was throttled *at the start* of a CQ run, then the CQ will not reject any changes if it fails (under the assumption that the tree was already broken).


Problems:

- Often, problems in the tree are first visible in the CQ itself. But throttling the tree in the middle of the CQ run once you notice that the tree is broken doesn't actually save any of the CLs in that run from rejection.

- Existing behavior is confusing and difficult to understand. To predict whether the current CQ run will reject CLs, would need to look back at historical tree status at time it started.


Suggestion:

- Add a tree status check to CommitQueueCompletion stage, just prior to handling of CL submission/rejection. Skip rejection if tree status was throttled at the beginning (saved in validation_pool tree_was_throttled, I believe) or at the end of the run.
 
Sorry, I don't completely understand.

Current behavior:
- accept all CLs in a CQ run if tree is already throttled when CQ starts
- run the CQ normally if tree gets throttled in the middle of a CQ run

Future behavior:
- treat the CLs in the CQ normally only if the tree is open during the entire duration of the CQ run
- accept all CLs otherwise

Alternative future behavior:
- replace "during the entire duration of the CQ run" above with "at the beginning and at the end of the CQ run" (so we don't care if the tree is throttled and unthrottled in the middle)


But either way, isn't this a problem in the sense that if we throttle the tree because of some intermittently bad CL, and this particular CQ run is successful, then we end up potentially merging changes on top of the bad CL?
reject = remove CQ +1 bit
accet = submit

(not reject) != accept
Oh I see... thanks.  So what do you call a CL that's neither accepted nor rejected?

I hate to nitpick on this, but "rejecting" and "not accepting" have a well-established equivalence in English.


> So what do you call a CL that's neither accepted nor rejected?
retried

Comment 5 by aut...@google.com, Feb 9 2017

Owner: nxia@chromium.org

Comment 6 by aut...@google.com, Feb 14 2017

Labels: -current-issue
#4 a better term for "neither accepted nor rejected" is IGNORED
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/93e6cb3b9c3d72ad4adcc2fa0e8370572ba0b649

commit 93e6cb3b9c3d72ad4adcc2fa0e8370572ba0b649
Author: Ningning Xia <nxia@chromium.org>
Date: Wed Apr 12 23:16:40 2017

Set TOT_sanity to False if tree isn't open at completion_stage.

If a tree is throttled in the middle of its run, treat TOT_sanity as
False and do not blame changes in failure handling.

BUG= chromium:690105 
TEST=unit_tests

Change-Id: I45c0032ae20b043960894e05f6ed8fd769dca953
Reviewed-on: https://chromium-review.googlesource.com/470906
Commit-Ready: Ningning Xia <nxia@chromium.org>
Tested-by: Ningning Xia <nxia@chromium.org>
Reviewed-by: Ningning Xia <nxia@chromium.org>

[modify] https://crrev.com/93e6cb3b9c3d72ad4adcc2fa0e8370572ba0b649/cbuildbot/stages/completion_stages_unittest.py
[modify] https://crrev.com/93e6cb3b9c3d72ad4adcc2fa0e8370572ba0b649/cbuildbot/stages/completion_stages.py
[modify] https://crrev.com/93e6cb3b9c3d72ad4adcc2fa0e8370572ba0b649/cbuildbot/tree_status.py

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/93e6cb3b9c3d72ad4adcc2fa0e8370572ba0b649

commit 93e6cb3b9c3d72ad4adcc2fa0e8370572ba0b649
Author: Ningning Xia <nxia@chromium.org>
Date: Wed Apr 12 23:16:40 2017

Set TOT_sanity to False if tree isn't open at completion_stage.

If a tree is throttled in the middle of its run, treat TOT_sanity as
False and do not blame changes in failure handling.

BUG= chromium:690105 
TEST=unit_tests

Change-Id: I45c0032ae20b043960894e05f6ed8fd769dca953
Reviewed-on: https://chromium-review.googlesource.com/470906
Commit-Ready: Ningning Xia <nxia@chromium.org>
Tested-by: Ningning Xia <nxia@chromium.org>
Reviewed-by: Ningning Xia <nxia@chromium.org>

[modify] https://crrev.com/93e6cb3b9c3d72ad4adcc2fa0e8370572ba0b649/cbuildbot/stages/completion_stages_unittest.py
[modify] https://crrev.com/93e6cb3b9c3d72ad4adcc2fa0e8370572ba0b649/cbuildbot/stages/completion_stages.py
[modify] https://crrev.com/93e6cb3b9c3d72ad4adcc2fa0e8370572ba0b649/cbuildbot/tree_status.py

Comment 10 by nxia@chromium.org, May 15 2017

Status: Fixed (was: Untriaged)
Labels: VerifyIn-61

Comment 12 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment