Stop creating stoppage alerts once a group has reached a certain size.

Stop creating stoppage alerts once a group has reached a certain size.

Today when perf sheriffing I was faced with another group of 100+ stoppage alerts which are relatively slow to clear because, as-is, one can only triage groups of around 20 or fewer alerts at a time.

BUG=catapult:#1616

Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/906fc11c2f5b9dfb832c6b401ef2d3e7aa2655b9

[qyearsley, 2015-11-04 19:13:40]

Description was changed from

==========
Stop creating stoppage alerts once a group has reached a certain size.

BUG=#1616
==========

to

==========
Stop creating stoppage alerts once a group has reached a certain size.

Today when perf sheriffing I was faced with another group of 100+ stoppage
alerts which are relatively slow to clear because, as-is, one can only triage
groups of around 20 or fewer alerts at a time.

BUG=#1616
==========

[qyearsley, 2015-11-04 19:13:40]

qyearsley@chromium.org changed reviewers:
+ chrisphan@chromium.org, sullivan@chromium.org

[qyearsley, 2015-11-04 19:13:44]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[qyearsley, 2015-11-04 19:13:56]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-04 19:14:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1424393003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1424393003/20001

[commit-bot: I haz the power, 2015-11-04 19:27:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-04 19:27:35]

Dry run: This issue passed the CQ dry run.

[qyearsley, 2015-11-06 18:05:08]

Any thoughts on this CL?

If we can verify that it's OK to triage 100+ stoppage alerts at once (through
/file_bug or /associate_alerts or /edit_anomalies) then this CL would be
unnecessary. If all requests are done using POST and all puts are done with
put_multi then this probably shouldn't be an issue.

It actually looks like requests from the front-end to /edit_anomalies (to mark
alerts as ignored) are done using POST requests already, and puts are already
done with put_multi.

Next time a hundred stoppage alerts for one test build up we can verify whether
this is necessary?

[chrisphan, 2015-11-06 18:06:42]

lgtm

https://codereview.chromium.org/1424393003/diff/20001/dashboard/dashboard/mod...
File dashboard/dashboard/models/stoppage_alert.py (right):

https://codereview.chromium.org/1424393003/diff/20001/dashboard/dashboard/mod...
dashboard/dashboard/models/stoppage_alert.py:100: return None
Does this need to always return an entity?

[sullivan, 2015-11-06 18:09:13]

Will this continue to create alerts? It seems like once we have 20, we don't
want to create more. Especially in the case where the sheriff has triaged the
first 20, and then more start to trickle in, and it's not immediately clear that
the issue has already been logged.

[qyearsley, 2015-11-06 18:11:09]

https://codereview.chromium.org/1424393003/diff/20001/dashboard/dashboard/mod...
File dashboard/dashboard/models/stoppage_alert.py (right):

https://codereview.chromium.org/1424393003/diff/20001/dashboard/dashboard/mod...
dashboard/dashboard/models/stoppage_alert.py:100: return None
On 2015/11/06 18:06:42, chrisphan wrote:
> Does this need to always return an entity?

Nope, as is in this CL, CreateStoppageAlert decides whether or not an alert
should be created and returns None if there should be none created, and the
caller is then responsible for checking if it's None. (Docstring updated.) Does
this seem OK?

[sullivan, 2015-11-06 18:13:42]

On 2015/11/06 18:11:09, qyearsley wrote:
>
https://codereview.chromium.org/1424393003/diff/20001/dashboard/dashboard/mod...
> File dashboard/dashboard/models/stoppage_alert.py (right):
> 
>
https://codereview.chromium.org/1424393003/diff/20001/dashboard/dashboard/mod...
> dashboard/dashboard/models/stoppage_alert.py:100: return None
> On 2015/11/06 18:06:42, chrisphan wrote:
> > Does this need to always return an entity?
> 
> Nope, as is in this CL, CreateStoppageAlert decides whether or not an alert
> should be created and returns None if there should be none created, and the
> caller is then responsible for checking if it's None. (Docstring updated.)
Does
> this seem OK?

Yep, this seems best. lgtm

[qyearsley, 2015-11-06 18:22:08]

On 2015/11/06 18:09:13, sullivan wrote:
> Will this continue to create alerts? It seems like once we have 20, we don't
> want to create more. Especially in the case where the sheriff has triaged the
> first 20, and then more start to trickle in, and it's not immediately clear
that
> the issue has already been logged.

This should limit the number of alerts per group to 20 -- that is, if this
change is correct, then in the next few weeks we should never see any groups of
stoppage alerts listed on /alerts with more than 20.

[qyearsley, 2015-11-06 18:22:12]

The CQ bit was checked by qyearsley@chromium.org

[qyearsley, 2015-11-06 18:22:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrisphan@chromium.org
Link to the patchset: https://codereview.chromium.org/1424393003/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-11-06 18:22:24]

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

[commit-bot: I haz the power, 2015-11-06 18:33:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-06 18:33:50]

Try jobs failed on following builders:
  Catapult Presubmit on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Pr...)

[qyearsley, 2015-11-06 19:40:55]

Description was changed from

==========
Stop creating stoppage alerts once a group has reached a certain size.

Today when perf sheriffing I was faced with another group of 100+ stoppage
alerts which are relatively slow to clear because, as-is, one can only triage
groups of around 20 or fewer alerts at a time.

BUG=#1616
==========

to

==========
Stop creating stoppage alerts once a group has reached a certain size.

Today when perf sheriffing I was faced with another group of 100+ stoppage
alerts which are relatively slow to clear because, as-is, one can only triage
groups of around 20 or fewer alerts at a time.

BUG=catapult:#1616
==========

[qyearsley, 2015-11-06 19:41:00]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2015-11-06 19:41:08]

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

[commit-bot: I haz the power, 2015-11-06 19:42:11]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...