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

Unified Diff: dashboard/dashboard/pinpoint/models/quest/find_isolate.py

Issue 3011223003: [pinpoint] Fix race condition in building + isolate finding. (Closed)
Patch Set: _CheckCompleted Created 3 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: dashboard/dashboard/pinpoint/models/quest/find_isolate.py
diff --git a/dashboard/dashboard/pinpoint/models/quest/find_isolate.py b/dashboard/dashboard/pinpoint/models/quest/find_isolate.py
index ef1a0e4254500bbf0c42454353a43fc5d77c42d2..295a0d7ef8dc725e54f7f58a79a4f44354ddb31c 100644
--- a/dashboard/dashboard/pinpoint/models/quest/find_isolate.py
+++ b/dashboard/dashboard/pinpoint/models/quest/find_isolate.py
@@ -49,37 +49,56 @@ class _FindIsolateExecution(execution.Execution):
}
def _Poll(self):
- # Look for the .isolate in our cache.
+ if self._CheckCompleted():
+ return
+
+ if self._build:
+ self._CheckBuildStatus()
+ return
+
+ self._RequestBuild()
+
+ def _CheckCompleted(self):
+ """Checks the isolate cache to see if a build is already available.
+
+ Returns:
+ True iff the isolate was found, meaning the execution is completed.
+ """
try:
isolate_hash = isolate.Get(self._builder_name, self._change, self._target)
except KeyError:
- isolate_hash = None
+ return False
+ self._Complete(result_arguments={'isolate_hash': isolate_hash})
+ return True
- if isolate_hash:
- self._Complete(
- result_arguments={'isolate_hash': isolate_hash})
- return
+ def _CheckBuildStatus(self):
+ """Checks on the status of a previously requested build.
- # Check the status of a previously requested build.
- if self._build:
- status = buildbucket_service.GetJobStatus(self._build)
+ Raises:
+ BuildError: The build failed, was canceled, or didn't produce an isolate.
+ """
+ status = buildbucket_service.GetJobStatus(self._build)
- if status['build']['status'] != 'COMPLETED':
+ if status['build']['status'] != 'COMPLETED':
+ return
+
+ if status['build']['result'] == 'FAILURE':
+ raise BuildError('Build failed: ' + status['build']['failure_reason'])
+ elif status['build']['result'] == 'CANCELED':
+ raise BuildError('Build was canceled: ' +
+ status['build']['cancelation_reason'])
+ else:
+ if self._CheckCompleted():
return
+ raise BuildError('Buildbucket says the build completed successfully, '
+ "but Pinpoint can't find the isolate hash.")
- if status['build']['result'] == 'FAILURE':
- raise BuildError('Build failed: ' + status['build']['failure_reason'])
- elif status['build']['result'] == 'CANCELED':
- raise BuildError('Build was canceled: ' +
- status['build']['cancelation_reason'])
- else:
- # It's possible for there to be a race condition if the builder uploads
- # the isolate and completes the build between the above isolate lookup
- # and buildbucket lookup, but right now, it takes builds a few minutes
- # to package the build, so that doesn't happen.
- raise BuildError('Buildbucket says the build completed successfully, '
- "but Pinpoint can't find the isolate hash.")
+ def _RequestBuild(self):
+ """Requests a build.
+ If a previous Execution already requested a build for this Change, returns
+ that build instead of requesting a new one.
+ """
if self._change in self._previous_builds:
# If another Execution already requested a build, reuse that one.
self._build = self._previous_builds[self._change]
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698