Move flaky run processing into a taskqueue

Move flaky run processing into a taskqueue

This will ensure that there is no transaction competition for updating Flake
entities because the issue-updates task queue has a limit of 1 concurrent task.
As a result, there will be fewer transaction errors and fewer identical issues
created or identically updated.

R=tandrii@chromium.org
BUG=583457
Committed: https://chromium.googlesource.com/infra/infra/+/13cec698ca86f128403697715929440871fe74b1

[Sergiy Byelozyorov, 2016-02-02 21:24:35]

https://codereview.chromium.org/1660043002/diff/1/appengine/chromium_try_flak...
File appengine/chromium_try_flakes/handlers/flake_issues.py (right):

https://codereview.chromium.org/1660043002/diff/1/appengine/chromium_try_flak...
appengine/chromium_try_flakes/handlers/flake_issues.py:343: class
CreateFlakyRun(webapp2.RequestHandler):
Most of the methods here are copied with minor modifications from
status/cq_status.py. Same goes for tests in handlers/test/flake_issues_test.py,
which has copied and slightly modified tests from status/test/cq_stats_test.py.

[Sergiy Byelozyorov, 2016-02-02 21:27:27]

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
File appengine/chromium_try_flakes/status/util.py (right):

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
appengine/chromium_try_flakes/status/util.py:5: """Utility functions for
handling flakes."""
This is copied from stats/cq_status.py. If kept there, I would need to import
stats.cq_status from handlers.flake_issues, but since stats.cq_status already
also imports handlers.flake_issues, this would create a loop. I've also tried
moving it to handlers/flake_issues.py, but since it's also used in
handlers/index.py, decided to move these functions to a new file.

[Sergiy Byelozyorov, 2016-02-02 21:29:29]

Description was changed from

==========
Move flaky run processing into a taskqueue

This will ensure that there is no transaction competition for updating Flake
entities because the issue-updates task queue has a limit of 1 concurrent task.
As a result, there will be fewer transaction errors and fewer identical issues
created or identically updated.

R=tandrii@chromium.org
BUG=545408
==========

to

==========
Move flaky run processing into a taskqueue

This will ensure that there is no transaction competition for updating Flake
entities because the issue-updates task queue has a limit of 1 concurrent task.
As a result, there will be fewer transaction errors and fewer identical issues
created or identically updated.

R=tandrii@chromium.org
BUG=583457
==========

[tandrii(chromium), 2016-02-02 21:54:18]

LGTM % disclaimer for lack of knowledge on AE transactions.

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
File appengine/chromium_try_flakes/handlers/flake_issues.py (right):

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
appengine/chromium_try_flakes/handlers/flake_issues.py:346: # transaction.
i do not fully understand why, though I believe you. I'd certainly recommend you
get review of someone with more AE experience to review this bit.

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
File appengine/chromium_try_flakes/status/util.py (right):

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
appengine/chromium_try_flakes/status/util.py:5: """Utility functions for
handling flakes."""
On 2016/02/02 21:27:27, Sergiy Byelozyorov wrote:
> This is copied from stats/cq_status.py. If kept there, I would need to import
> stats.cq_status from handlers.flake_issues, but since stats.cq_status already
> also imports handlers.flake_issues, this would create a loop. I've also tried
> moving it to handlers/flake_issues.py, but since it's also used in
> handlers/index.py, decided to move these functions to a new file.

yeah, new file is LGTM.

[tandrii(chromium), 2016-02-02 21:59:28]

and if I may, I'd recommend that you upload first PS moving the methods around
wihtout modifications, and then another PS with actual modifications, so it's
easier to review.

[Sergiy Byelozyorov, 2016-02-03 08:59:22]

On 2016/02/02 21:59:28, tandrii(chromium) wrote:
> and if I may, I'd recommend that you upload first PS moving the methods around
> wihtout modifications, and then another PS with actual modifications, so it's
> easier to review.

Yeah, I know this pain and yet create it for others. I'll try to create a better
review next time.

[Paweł Hajdan Jr., 2016-02-03 09:09:51]

phajdan.jr@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[Paweł Hajdan Jr., 2016-02-03 09:09:51]

LGTM

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
File appengine/chromium_try_flakes/handlers/flake_issues.py (right):

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
appengine/chromium_try_flakes/handlers/flake_issues.py:346: # transaction.
On 2016/02/02 at 21:54:18, tandrii(chromium) wrote:
> i do not fully understand why, though I believe you. I'd certainly recommend
you get review of someone with more AE experience to review this bit.

Yes, AE has limits on transactions, not all of which even used to be documented.

This looks fine to me so far.

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
appengine/chromium_try_flakes/handlers/flake_issues.py:366: # TODO(jam): It's
unfortunate to have to parse this html. Can we get it from
I know this is pre-existing, but IMHO important in general (not in scope for
this patch of course).

Consider that with move to DM we'd no longer have buildbot JSON/HTML anyway.

Having some independent mechanism to avoid this parsing would be very useful.
It'd also help improve more precise communication between the recipe and this
app - currently there seem to be lots of "fixups" that wouldn't necessarily be
needed otherwise.

[Sergiy Byelozyorov, 2016-02-03 09:15:56]

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
File appengine/chromium_try_flakes/handlers/flake_issues.py (right):

https://codereview.chromium.org/1660043002/diff/20001/appengine/chromium_try_...
appengine/chromium_try_flakes/handlers/flake_issues.py:366: # TODO(jam): It's
unfortunate to have to parse this html. Can we get it from
On 2016/02/03 09:09:51, Paweł Hajdan Jr. wrote:
> I know this is pre-existing, but IMHO important in general (not in scope for
> this patch of course).
> 
> Consider that with move to DM we'd no longer have buildbot JSON/HTML anyway.
> 
> Having some independent mechanism to avoid this parsing would be very useful.
> It'd also help improve more precise communication between the recipe and this
> app - currently there seem to be lots of "fixups" that wouldn't necessarily be
> needed otherwise.

I was actually planning to move to Buildbot JSON + GTest JSON output very soon.
I am not sure what DM will offer, but we will have to fix it one way or another.

[Sergiy Byelozyorov, 2016-02-03 09:15:59]

The CQ bit was checked by sergiyb@chromium.org

[commit-bot: I haz the power, 2016-02-03 09:16:02]

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

[commit-bot: I haz the power, 2016-02-03 09:18:40]

Description was changed from

==========
Move flaky run processing into a taskqueue

This will ensure that there is no transaction competition for updating Flake
entities because the issue-updates task queue has a limit of 1 concurrent task.
As a result, there will be fewer transaction errors and fewer identical issues
created or identically updated.

R=tandrii@chromium.org
BUG=583457
==========

to

==========
Move flaky run processing into a taskqueue

This will ensure that there is no transaction competition for updating Flake
entities because the issue-updates task queue has a limit of 1 concurrent task.
As a result, there will be fewer transaction errors and fewer identical issues
created or identically updated.

R=tandrii@chromium.org
BUG=583457

Committed:
https://chromium.googlesource.com/infra/infra/+/13cec698ca86f1284036977159294...
==========

[commit-bot: I haz the power, 2016-02-03 09:18:41]

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