Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(329)

Issue 391383005: Fix sharded perf test host forwarder sharing (Closed)

Created:
6 years, 5 months ago by tonyg
Modified:
6 years, 5 months ago
Reviewers:
jbudorick
CC:
chromium-reviews, klundberg+watch_chromium.org, yfriedman+watch_chromium.org, ilevy-cc_chromium.org
Project:
chromium
Visibility:
Public.

Description

Fix sharded perf test host forwarder sharing This code is subtle. Since killing the host forwarder is a global operation, it keeps a temp file with the shared pid and process start time so that it knows whether the forwarder is the correct one that should be shared or a stray one that should be killed. This failed because not all of the sharded perf test processes had the same parent pid. This caused the sharded tests to stomp on the global forwarder. We didn't notice this previously because we only had narrow sharding, and 3 retries so the flake was papered over. When we added 8-way sharding on chromium.perf, the flake rose to the level where it appeared every run. This patch fixes it by using the process group instead of using the parent process ID as a proxy. BUG=163503 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283683

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M build/android/pylib/forwarder.py View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
tonyg
6 years, 5 months ago (2014-07-17 00:38:19 UTC) #1
jbudorick
lgtm Awesome!
6 years, 5 months ago (2014-07-17 01:32:25 UTC) #2
tonyg
The CQ bit was checked by tonyg@chromium.org
6 years, 5 months ago (2014-07-17 01:33:44 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tonyg@chromium.org/391383005/1
6 years, 5 months ago (2014-07-17 01:35:35 UTC) #4
commit-bot: I haz the power
6 years, 5 months ago (2014-07-17 07:16:56 UTC) #5
Message was sent while issue was closed.
Change committed as 283683

Powered by Google App Engine
This is Rietveld 408576698