Findit] Flake Checker: Pipeline to trigger try jobs to identify flake culprits

appengine/findit/waterfall/flake/recursive_flake_pipeline.py

Index: appengine/findit/waterfall/flake/recursive_flake_pipeline.py
diff --git a/appengine/findit/waterfall/flake/recursive_flake_pipeline.py b/appengine/findit/waterfall/flake/recursive_flake_pipeline.py
index 9c8b4918828188e0802ddb1160e5ecffb341e034..59cb6f4801a4762dcb090289779ea0d4fc1b9e57 100644
--- a/appengine/findit/waterfall/flake/recursive_flake_pipeline.py
+++ b/appengine/findit/waterfall/flake/recursive_flake_pipeline.py
@@ -55,17 +55,15 @@ def _UpdateBugWithResult(analysis, queue_name):
 
 def _UpdateAnalysisStatusUponCompletion(
     master_flake_analysis, suspected_build, status, error):
-  master_flake_analysis.end_time = time_util.GetUTCNow()
-  master_flake_analysis.status = status
-
   if error:
     master_flake_analysis.error = error
 
-  if suspected_build != -1:
-    master_flake_analysis.suspected_flake_build_number = suspected_build
-    master_flake_analysis.result_status = result_status.FOUND_UNTRIAGED
-  else:
+  if suspected_build == _NO_BUILD_NUMBER:
+    master_flake_analysis.end_time = time_util.GetUTCNow()
+    master_flake_analysis.status = status
     master_flake_analysis.result_status = result_status.NOT_FOUND_UNTRIAGED
+  else:

[chanli, 2017/01/13 00:47:33]

master_flake_analysis.status needs to be updated at all conditions. I prefer to
update it at very beginning of this function.

[lijeffrey, 2017/01/13 01:39:23]

Done.

+    master_flake_analysis.suspected_flake_build_number = suspected_build

[chanli, 2017/01/13 00:47:33]

Question: should we continue triaging at build level? Especially if try jobs
don't have results?

[lijeffrey, 2017/01/13 01:39:23]

I think try jobs will be much more reliable than at the build level, because
it's easy at the build level to come up with false positives. A wrong suspected
build number is guaranteed to yield a wrong try job result if it even finds
anything. If the build level does not find anything, then that's definitely not
found (as is the case here), but if it does, it's still not necessarily found
until try jobs determine it has, especially for false positives (i.e. we suspect
a dive from 80% to 20%, but the 80% is already very flaky so I would not have
confidence in the try job finding anything meaningful.

I do plan to redesign the triage model though to differentiate between found at
the build level vs found at the CL level, but since that's not a trivial change
it should be for a separate CL.

 
   master_flake_analysis.put()
 
@@ -317,9 +315,10 @@ def _GetNextBuildNumber(data_points, flake_settings):
   Returns:
     (next_build_number, suspected_build): The next build number to check
         and suspected build number that the flakiness was introduced in.
-        If needs to check next_build_number, suspected_build will be -1;
-        If finds the suspected_build, next_build_number will be -1;
-        If no findings evertually, both will be -1.
+        If needs to check next_build_number, suspected_build will be
+        _NO_BUILD_NUMBER; If suspected_build is found, next_build_number will be
+        _NO_BUILD_NUMBER; If no findings eventually, both will be
+        _NO_BUILD_NUMBER.
   """
   # A description of this algorithm can be found at:
   # https://docs.google.com/document/d/1wPYFZ5OT998Yn7O8wGDOhgfcQ98mknoX13AesJaS6ig/edit