Chromium Code Reviews Chromium Code Reviews
Issue
2670003006:
Revert of Dashboard - Remove pre-hook logic from alert.py. (Closed)
DescriptionRevert of Dashboard - Remove pre-hook logic from alert.py. (patchset #2 id:240001 of https://codereview.chromium.org/2650683007/ )
Reason for revert:
Fixing flakey test.
Original issue's description:
> Previously alert.py has some pre-hook logic to update associated groups. This involved several sync gets + query, with some potential puts and deletes. The newer implementation strips the pre-hook out in favour of heavily batching async calls when alerts are updated.
>
> So ran through StackDriver re-triaging a bug with 53 alerts (did the new one first so that any memcache boosts would benefit the old one not the new one), gathered these timings:
>
> Demo: https://dev-simonhatch-66cc9519-dot-chromeperf.appspot.com/
>
> Old:
>
> @479 ms
> https://monorail-prod.appspot.com/_ah/api/monorail/v1/projects/chromium/issues?alt=json&sendEmail=true
>
> <Modify Alerts>
>
> @10,430 ms
> https://monorail-prod.appspot.com/_ah/api/monorail/v1/projects/chromium/issues/685469/comments?alt=json&sendEmail=true
>
> @11,354 ms
> https://cr-buildbucket.appspot.com/api/discovery/v1/apis/buildbucket/v1/rest?userIp=2620%3A0%3A1000%3A1409%3A4c33%3Ac669%3A2634%3Af3bd
>
>
> New:
>
> @770 ms
> https://monorail-prod.appspot.com/_ah/api/monorail/v1/projects/chromium/issues?alt=json&sendEmail=true
>
> <Modify Alerts>
>
> @2,072 ms
> https://monorail-prod.appspot.com/_ah/api/monorail/v1/projects/chromium/issues/685468/comments?alt=json&sendEmail=true
>
> @3,121 ms
> https://cr-buildbucket.appspot.com/api/discovery/v1/apis/buildbucket/v1/rest?userIp=2620%3A0%3A1000%3A1409%3A4c33%3Ac669%3A2634%3Af3bd
>
>
> This would put the old implementation at about 9951 ms vs 1302 ms in this particular run if I'm understanding these traces correctly.
>
> Could probably make some of these endpoints faster by returning futures from alert_group.ModifyAlertsAndAssociatedGroups() and weaving in with the surrounding functions (like in /file_bug we could theoretically async the issue_tracker stuff).
>
> Also might want to file an issue after to refactor some of this Alert/AlertGroup code. Seems a bit brittle that you have these associated groups that need to be updated whenever attributes on an alert are modified.
>
> BUG=catapult:#2771
>
> Review-Url: https://codereview.chromium.org/2650683007
> Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/20c5e0b31c76cfbcf8ee0f629fe4c3069367f3c4
TBR=sullivan@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=catapult:#2771
Review-Url: https://codereview.chromium.org/2670003006
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/4cc1200e5d9563db853de9a82ff7dc176aa95484
Patch Set 1 #
Messages
Total messages: 6 (3 generated)
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
