New issue
Advanced search Search tips

Issue 877809 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Multiple SparseDiagnostics with same test, name, end_revision

Project Member Reported by benjhayden@chromium.org, Aug 26

Issue description

This assertion is failing:
https://github.com/catapult-project/catapult/blob/master/dashboard/dashboard/models/histogram.py#L104
This assertion error indicates that there are multiple SparseDiagnostic entities with the same test and name and end_revision = sys.maxint. For the entities that I inspected, they have different start_revisions, so I manually fixed their end_revisions. One test was a system_health benchmark on ChromiumPerf, which does not upload out of order, suggesting a bug in or around SparseDiagnostic._FindOrInsertDiagnosticsLast, called from FindOrInsertDiagnostics, called from add_histograms.py:ProcessHistogramSet.
Is it possible for FindOrInsertDiagnostics to create the new entity but then hit the deadline before updating existing entities' end_revision? It yields futures for those operations together, but they aren't in a transaction as far as I can tell.
Should ProcessHistogramSet or FindOrInsertDiagnostics use a transaction?
Should SparseDiagnostic.GetMostRecentDataByNamesAsync tolerate this situation and just return the data with the largest start_revision?
 
What test path did this happen on?
ChromiumPerf/mac-10_12_laptop_low_end-perf/rendering.desktop
ChromiumPerf/android-go-perf/system_health.common_mobile
ChromiumPerf/Win 7 Nvidia GPU Perf/system_health.common_desktop
ChromiumPerf/Win 7 Perf/system_health.memory_desktop
ChromiumPerf/Android Nexus5 Perf/system_health.memory_mobile

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 30

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/cd6bcbe53ba75242740c2244d1d3b4c556e03039

commit cd6bcbe53ba75242740c2244d1d3b4c556e03039
Author: benshayden <benjhayden@chromium.org>
Date: Thu Aug 30 20:37:30 2018

Make SparseDiagnostic.GetMostRecentDataByNamesAsync more tolerant.

Currently, this function asserts that there should never be multiple entities
with the same test and name and end_revision = sys.maxint.
This assertion is failing in production suggesting a bug in add_histograms.
This CL works around the bug for now to enable tagmaps.

https://dev-benjhayden-1188393-dot-chromeperf.appspot.com/_ah/dev_console/interactive

Bug: chromium:877809
Change-Id: I462510a4e8c4bcd05375f82d8399f2cdf9705948
Reviewed-on: https://chromium-review.googlesource.com/1188393
Commit-Queue: Ben Hayden <benjhayden@chromium.org>
Reviewed-by: Simon Hatch <simonhatch@chromium.org>

[modify] https://crrev.com/cd6bcbe53ba75242740c2244d1d3b4c556e03039/dashboard/dashboard/models/histogram_test.py
[modify] https://crrev.com/cd6bcbe53ba75242740c2244d1d3b4c556e03039/dashboard/dashboard/models/histogram.py

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 30

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

commit 9369e026137f46a3bf15af0612aeb4582a066d95
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu Aug 30 23:55:40 2018

Roll src/third_party/catapult 7a770735549a..cd6bcbe53ba7 (1 commits)

https://chromium.googlesource.com/catapult.git/+log/7a770735549a..cd6bcbe53ba7


git log 7a770735549a..cd6bcbe53ba7 --date=short --no-merges --format='%ad %ae %s'
2018-08-30 benjhayden@chromium.org Make SparseDiagnostic.GetMostRecentDataByNamesAsync more tolerant.


Created with:
  gclient setdep -r src/third_party/catapult@cd6bcbe53ba7

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG=chromium:877809
TBR=sullivan@chromium.org

Change-Id: Iaeb976d7508515e648353c3b77fdc6c078098bee
Reviewed-on: https://chromium-review.googlesource.com/1198122
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#587860}
[modify] https://crrev.com/9369e026137f46a3bf15af0612aeb4582a066d95/DEPS

Cc: -eakuefner@chromium.org

Sign in to add a comment