Fix non-idempotent task that /poll reaped but returned HTTP 500.

appengine/swarming/server/task_result.py

 # Copyright 2014 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 """Task execution result models.
 
 This module doesn't do the scheduling itself. It only describes the entities to
 store tasks results.
 
 - TaskResultSummary represents the overall result for the TaskRequest taking in
@@ skipping 65 matching lines @@
 
 from components import datastore_utils
 from components import utils
 from server import large
 from server import task_pack
 from server import task_request
 
 import cipd
 
 # Amount of time after which a bot is considered dead. In short, if a bot has
-# not ping in the last 5 minutes while running a task, it is considered dead.
-BOT_PING_TOLERANCE = datetime.timedelta(seconds=5*60)
+# not ping in the last 2 minutes while running a task, it is considered dead.
+#
+# task_runner.MAX_PACKET_INTERVAL is 30 seconds.
+BOT_PING_TOLERANCE = datetime.timedelta(seconds=2*60)
 
 
 class State(object):
   """States in which a task can be.
 
   It's in fact an enum. Values should be in decreasing order of importance.
   """
   RUNNING = 0x10    # 16
   PENDING = 0x20    # 32
   EXPIRED = 0x30    # 48
@@ skipping 636 matching lines @@
         output,
         output_chunk_start)
     assert self.stdout_chunks <= TaskOutput.PUT_MAX_CHUNKS
     return entities
 
   def to_dict(self):
     out = super(TaskRunResult, self).to_dict()
     out['try_number'] = self.try_number
     return out
 
+  def _pre_put_hook(self):
+    super(TaskRunResult, self)._pre_put_hook()
+    if not self.started_ts:
+      raise datastore_errors.BadValueError('Must update .started_ts')
+
 
 class TaskResultSummary(_TaskResultCommon):
   """Represents the overall result of a task.
 
   Parent is a TaskRequest. Key id is always 1.
 
   This includes the relevant result taking in account all tries. This entity is
   basically a cache plus a bunch of indexes to speed up common queries.
 
   It's primary purpose is for status pages listing all the active tasks or
@@ skipping 407 matching lines @@
 
   The caller must save it in the DB.
   """
   assert isinstance(request, task_request.TaskRequest)
   summary_key = task_pack.request_key_to_result_summary_key(request.key)
   return TaskRunResult(
       key=task_pack.result_summary_key_to_run_result_key(
           summary_key, try_number),
       bot_dimensions=bot_dimensions,
       bot_id=bot_id,
-      started_ts=utils.utcnow(),
       bot_version=bot_version,
       server_versions=[utils.get_app_version()])
 
 
 def yield_run_result_keys_with_dead_bot():
   """Yields all the TaskRunResult ndb.Key where the bot died recently.
 
   In practice it is returning a ndb.QueryIterator but this is equivalent.
   """
   # If a bot didn't ping recently, it is considered dead.
@@ skipping 35 matching lines @@
   if tags:
     # Add TaskResultSummary indexes if desired.
     if sort != 'created_ts':
       raise ValueError(
           'Add needed indexes for sort:%s and tags if desired' % sort)
     tags_filter = TaskResultSummary.tags == tags[0]
     for tag in tags[1:]:
       tags_filter = ndb.AND(tags_filter, TaskResultSummary.tags == tag)
     query = query.filter(tags_filter)
   return _filter_query(TaskResultSummary, query, start, end, sort, state)