[Findit] Adding spike detection for try job requests to data query script

[Findit] Adding spike detection for try job requests to data query script

BUG=629166
Committed: https://chromium.googlesource.com/infra/infra/+/5a5a514b1073399807989c3534152e69ff476358

[lijeffrey, 2016-07-18 19:56:24]

lijeffrey@chromium.org changed reviewers:
+ chanli@chromium.org, stgao@chromium.org

[lijeffrey, 2016-07-18 19:56:24]

ptal

[Sharu Jiang, 2016-07-18 22:15:30]

katesonia@chromium.org changed reviewers:
+ katesonia@chromium.org

[Sharu Jiang, 2016-07-18 22:15:31]

https://codereview.chromium.org/2160763002/diff/1/appengine/findit/util_scrip...
File appengine/findit/util_scripts/remote_queries/try_job_data_metrics.py
(right):

https://codereview.chromium.org/2160763002/diff/1/appengine/findit/util_scrip...
appengine/findit/util_scripts/remote_queries/try_job_data_metrics.py:135: for
point_being_examined in request_times[1:]:
I have one concern, in worst case, this approach would only detect half of the
spike load, it may be a problem.

[chanli, 2016-07-18 23:30:43]

On 2016/07/18 22:15:31, sharu jiang wrote:
>
https://codereview.chromium.org/2160763002/diff/1/appengine/findit/util_scrip...
> File appengine/findit/util_scripts/remote_queries/try_job_data_metrics.py
> (right):
> 
>
https://codereview.chromium.org/2160763002/diff/1/appengine/findit/util_scrip...
> appengine/findit/util_scripts/remote_queries/try_job_data_metrics.py:135: for
> point_being_examined in request_times[1:]:
> I have one concern, in worst case, this approach would only detect half of the
> spike load, it may be a problem.

lgtm

[lijeffrey, 2016-07-19 00:33:11]

https://codereview.chromium.org/2160763002/diff/1/appengine/findit/util_scrip...
File appengine/findit/util_scripts/remote_queries/try_job_data_metrics.py
(right):

https://codereview.chromium.org/2160763002/diff/1/appengine/findit/util_scrip...
appengine/findit/util_scripts/remote_queries/try_job_data_metrics.py:135: for
point_being_examined in request_times[1:]:
On 2016/07/18 22:15:31, sharu jiang wrote:
> I have one concern, in worst case, this approach would only detect half of the
> spike load, it may be a problem.

Yes, with this vanilla approach it is possible some spikes are improperly
interpreted. For example in a 30 minute window at minute 0 we have a request,
then at 28 and 29 we have 2 more requests. This is a spike of size 3. But at 30,
31, 32, 33, there are 4 more requests. This algorithm will detect 2 spikes, one
of size 3 (0, 28, 29), and one of size 4 (30, 31, 32, 33), when the real spike
is actually 28-33 of size 6. But this is expected to be a corner case. Since
this code is to estimate how many bots are needed, we are only considering
average spike size over a large period (3 months) of many spikes to minimize
compile try jobs being queued up under theoretical expected load.

[lijeffrey, 2016-07-19 18:19:29]

The CQ bit was checked by lijeffrey@chromium.org

[lijeffrey, 2016-07-19 18:19:29]

The patchset sent to the CQ was uploaded after l-g-t-m from chanli@chromium.org
Link to the patchset: https://codereview.chromium.org/2160763002/#ps20001
(title: "fixing bug if there are no request times")

[commit-bot: I haz the power, 2016-07-19 18:19:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-19 18:32:58]

Description was changed from

==========
[Findit] Adding spike detection for try job requests to data query script

BUG=629166
==========

to

==========
[Findit] Adding spike detection for try job requests to data query script

BUG=629166

Committed:
https://chromium.googlesource.com/infra/infra/+/5a5a514b1073399807989c3534152...
==========

[commit-bot: I haz the power, 2016-07-19 18:32:59]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/infra/infra/+/5a5a514b1073399807989c3534152...