Missing archive_data_file does not stop run_benchmark run if page_set does not use a bucket |
|||||
Issue descriptionI captured new archives for CT this weekend and noticed that several slaves failed to capture them for an infrastructure reason. But interestingly the telemetry benchmark run still succeeded on those slaves. Multiple slaves should have failed because of missing archives since all CT page sets specifies a archive_data_file in the StorySet constructor: https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/page_sets/ct_page_set.py&q=ct_page&sq=package:chromium&l=41 This is similar to the other page sets: https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/page_sets/maps.py&q=archive_data_file%20file:%5Esrc/tools/perf/page_sets/&sq=package:chromium&type=cs&l=20 Note that previously a missing file meant that run benchmark would fail. To summarize: It appears that even if the archive data file is missing locally the benchmark still runs and succeeds. I do not know if it hits live data or if something else is happening.
,
Apr 5 2016
Something is strange with how ct benchmark is set up. Normally, story_runner should throw exception based on the check in https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/internal/story_runner.py#L355
,
Apr 5 2016
Thank you for investigating! I will now take a look at what the CT benchmark is doing.
,
Apr 5 2016
Looks like that check is only done if a bucket is specified: https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/internal/story_runner.py#L185 CT page sets do not need buckets, the archives are managed by the framework: https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/page_sets/ct_page_set.py We can either revisit https://codereview.chromium.org/1856603003/ or story_runner.py should be changed. WDYT?
,
Apr 5 2016
I think we should change story_runner to handle cases like yours in general.
,
Apr 5 2016
I agree that we should change story_runner.
,
Apr 5 2016
,
Apr 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7d316cee9a6ddffdc762d023650fe1e50698e8e5 commit 7d316cee9a6ddffdc762d023650fe1e50698e8e5 Author: catapult-deps-roller <catapult-deps-roller@chromium.org> Date: Tue Apr 05 22:02:59 2016 Roll src/third_party/catapult/ c76786ca9..0c61248dd (3 commits). https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/c76786ca9cf8..0c61248dd7da $ git log c76786ca9..0c61248dd --date=short --no-merges --format='%ad %ae %s' BUG= 600154 TBR=catapult-sheriff@chromium.org Review URL: https://codereview.chromium.org/1865463003 Cr-Commit-Position: refs/heads/master@{#385292} [modify] https://crrev.com/7d316cee9a6ddffdc762d023650fe1e50698e8e5/DEPS
,
Apr 6 2016
Looks like this is working. The new CT runs report less webpages but that is because they are missing archives. Doing a grep of "Archive file is missing stories" in the logs I got 69 results for the top1k which matches the number of webpage archives in Google Storage: 931. Marking as fixed.
,
Apr 6 2016
,
Apr 6 2016
Great! |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by rmis...@google.com
, Apr 4 2016