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

Issue 854162 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Nov 12
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: ----



Sign in to add a comment

rendering.mobile Dashboard Upload failing on chromium.perf/Android Nexus5X Perf

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Jun 19 2018

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of sullivan@google.com

Ethan, this failed at least 12 times. Can you take a look?

rendering.mobile Dashboard Upload failing on chromium.perf/Android Nexus5X Perf

Builders failed on: 
- Android Nexus5X Perf: 
  https://ci.chromium.org/buildbot/chromium.perf/Android%20Nexus5X%20Perf

Example build:
https://ci.chromium.org/buildbot/chromium.perf/Android%20Nexus5X%20Perf/1912
Log:
https://logs.chromium.org/v/?s=chrome%2Fbb%2Fchromium.perf%2FAndroid_Nexus5X_Perf%2F1912%2F%2B%2Frecipes%2Fsteps%2Frendering.mobile_Dashboard_Upload%2F0%2Fstdout


Sending result 2 of 2 to dashboard.
Error uploading histogram data: HTTP Response 401: Unauthorize

 
Owner: eakuefner@chromium.org
How long are the oauth tokens good for? That upload step is taking 12 minutes, probably a good chunk of which is in add_reserved_diagnostics.
Cc: eyaich@chromium.org
Status: Assigned (was: Available)
vadimsh@ told me once that they're valid for about 5 minutes, so the token is probably expiring.

Last week, benjhayden@, simonhatch@, and I looked at the performance of add_reserved_diagnostics and realized that histogram merging is a pain point. In particular, it's currently implemented in JS and we're having to JSON.parse the entire HistogramSet. We could eliminate that overhead entirely which would probably help here.

A more robust fix here would be to decouple the merging step from the uploading step, or maybe to (as we originally had thought about doing), pass the service account credentials directly into the merge script so that it can generate a token. eyaich@ had talked about doing this but we ended up generating the token in the recipe instead.

Comment 4 by eyaich@chromium.org, Jun 26 2018

Interestingly we currently generate the token before we start any uploads and use the same one for each upload.  The upload takes about an hour so if it truly is no longer valid after 5 minutes I feel like we would be seeing a lot more errors.

We could change it to re-generate the token before every upload.  In my mutli-processing work that would actually be easier on the merge side.

I wonder if this is what is also happening in  crbug.com/856612 
Cc: nednguyen@chromium.org
 Issue 856612  has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/681ba107f87635d29521442b150e84dcc1e2a05f

commit 681ba107f87635d29521442b150e84dcc1e2a05f
Author: Emily Hanley <eyaich@google.com>
Date: Thu Jun 28 18:50:17 2018

Parallelizing perf dashboard uploads

This change also update the oauth token to generating
ne per run to one per benchmark

Bug:713357, 854162 
Change-Id: If06e71c53fe8083f82307584a6e92104e33b2f65


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:obbs_fyi

Change-Id: If06e71c53fe8083f82307584a6e92104e33b2f65
Reviewed-on: https://chromium-review.googlesource.com/1114690
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#571212}
[modify] https://crrev.com/681ba107f87635d29521442b150e84dcc1e2a05f/tools/perf/core/oauth_api.py
[modify] https://crrev.com/681ba107f87635d29521442b150e84dcc1e2a05f/tools/perf/core/results_dashboard.py
[modify] https://crrev.com/681ba107f87635d29521442b150e84dcc1e2a05f/tools/perf/process_perf_results.py

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cce5450a6d372adba5fbaa2a46e2065382e4b503

commit cce5450a6d372adba5fbaa2a46e2065382e4b503
Author: Ned Nguyen <nednguyen@google.com>
Date: Fri Jun 29 11:34:34 2018

Revert "Parallelizing perf dashboard uploads"

This reverts commit 681ba107f87635d29521442b150e84dcc1e2a05f.

Reason for revert: break uploading on Mac (https://ci.chromium.org/buildbot/chromium.perf/mac-10_13_laptop_high_end-perf/167)

Original change's description:
> Parallelizing perf dashboard uploads
> 
> This change also update the oauth token to generating
> ne per run to one per benchmark
> 
> Bug:713357, 854162 
> Change-Id: If06e71c53fe8083f82307584a6e92104e33b2f65
> 
> 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:obbs_fyi
> 
> Change-Id: If06e71c53fe8083f82307584a6e92104e33b2f65
> Reviewed-on: https://chromium-review.googlesource.com/1114690
> Commit-Queue: Ned Nguyen <nednguyen@google.com>
> Reviewed-by: Ned Nguyen <nednguyen@google.com>
> Cr-Commit-Position: refs/heads/master@{#571212}

TBR=nednguyen@google.com,eyaich@chromium.org

Change-Id: Icc0093c203e0fa335c550c8af5686af1786d6625
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  713357 ,  854162 
Cq-Include-Trybots: master.tryserver.chromium.perf:obbs_fyi
Reviewed-on: https://chromium-review.googlesource.com/1120125
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#571441}
[modify] https://crrev.com/cce5450a6d372adba5fbaa2a46e2065382e4b503/tools/perf/core/oauth_api.py
[modify] https://crrev.com/cce5450a6d372adba5fbaa2a46e2065382e4b503/tools/perf/core/results_dashboard.py
[modify] https://crrev.com/cce5450a6d372adba5fbaa2a46e2065382e4b503/tools/perf/process_perf_results.py

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/933e64e8d889dbe40b69074038b09e67892551d4

commit 933e64e8d889dbe40b69074038b09e67892551d4
Author: Emily Hanley <eyaich@google.com>
Date: Mon Jul 02 14:49:31 2018

Reland parallelizing perf dashboard uploads

Failures in this revert https://chromium-review.googlesource.com/c/chromium/src/+/1120125
were for failing mac, but the everything was succeeding, we were
failing when trying to right out not valid json as perf results.

I have added logic in line 447 of process_perf_results.py to catch
these errors in the future.  Note this was also an issue in the old
recipe, it just shows one step as failed instead of the entire suite.

Bug:  713357 , 854162 ,  859073 , 858995
Change-Id: I37c8f8fe3d7973962a17bbd64b758c7c98517799
Reviewed-on: https://chromium-review.googlesource.com/1122478
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Emily Hanley <eyaich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571893}
[modify] https://crrev.com/933e64e8d889dbe40b69074038b09e67892551d4/tools/perf/core/oauth_api.py
[modify] https://crrev.com/933e64e8d889dbe40b69074038b09e67892551d4/tools/perf/core/results_dashboard.py
[modify] https://crrev.com/933e64e8d889dbe40b69074038b09e67892551d4/tools/perf/process_perf_results.py

Cc: -eakuefner@chromium.org
Owner: ----
Status: Fixed (was: Assigned)
I think this was fixed with the reland in Comment 8. Please feel free to re-open/reassign if necessary.

Sign in to add a comment