Docserver: Make the hand-written Cron methods run first rather than last, since

chrome/common/extensions/docs/server2/cron_servlet.py

 # Copyright 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import logging
 import time
 import traceback
 
 from app_yaml_helper import AppYamlHelper
 from appengine_wrappers import (
     GetAppVersion, IsDeadlineExceededError, logservice)
 from branch_utility import BranchUtility
 from compiled_file_system import CompiledFileSystem
 from data_source_registry import CreateDataSources
 from empty_dir_file_system import EmptyDirFileSystem
 from environment import IsDevServer
 from file_system_util import CreateURLsFromPaths
+from future import Gettable, Future
 from github_file_system_provider import GithubFileSystemProvider
 from host_file_system_provider import HostFileSystemProvider
 from object_store_creator import ObjectStoreCreator
 from render_servlet import RenderServlet
 from server_instance import ServerInstance
 from servlet import Servlet, Request, Response
 import svn_constants
 
 class _SingletonRenderServletDelegate(RenderServlet.Delegate):
   def __init__(self, server_instance):
@@ skipping 116 matching lines @@
     def request_files_in_dir(path, prefix=''):
       '''Requests every file found under |path| in this host file system, with
       a request prefix of |prefix|.
       '''
       files = [name for name, _ in CreateURLsFromPaths(trunk_fs, path, prefix)]
       return _RequestEachItem(path, files, render)
 
     results = []
 
     try:
+      # Run the hand-written Cron methods first; they can be run in parallel.
+      def run_cron_for_future(target):
+        title = target.__class__.__name__
+        start_time = time.time()
+        future = target.Cron()
+        assert isinstance(future, Future), (
+            '%s.Cron() did not return a Future' % title)
+        def resolve():
+          try:
+            future.Get()
+          except Exception as e:
+            _cronlog.error('%s: error %s' % (title, traceback.format_exc()))
+            results.append(False)
+            if IsDeadlineExceededError(e): raise
+          finally:
+            _cronlog.info(
+                '%s: took %s seconds' % (title, time.time() - start_time))

[Jeffrey Yasskin, 2013/11/06 22:37:09]

This may not be so useful anymore since even if the work is completely finished
by the .Get() call, you'll still get the time of the .Get call rather than the
time the action took. Maybe take a time for creating the future and a separate
time for running .Get() and add them?

[not at google - send to devlin, 2013/11/11 04:08:33]

Makes sense.

This logging is most for me to check to make sure nothing crazy is going on, but
separating out the times would be useful.

+        return Future(delegate=Gettable(resolve))
+
+      targets = (CreateDataSources(server_instance).values() +
+                 [server_instance.content_providers])
+      title = 'running %s parallel Cron targets' % len(targets)
+      start_time = time.time()
+      _cronlog.info(title)
+      try:
+        futures = [run_cron_for_future(target) for target in targets]
+        for future in futures:

[Jeffrey Yasskin, 2013/11/06 22:37:09]

You could delay this loop until the end of the function, to give it more time to
overlap with rendering.

[not at google - send to devlin, 2013/11/11 04:08:33]

Good idea.

+          future.Get()
+      finally:
+        _cronlog.info('%s took %s seconds' % (title, time.time() - start_time))
+
       # Rendering the public templates will also pull in all of the private
       # templates.
       results.append(request_files_in_dir(svn_constants.PUBLIC_TEMPLATE_PATH))
 
       # Rendering the public templates will have pulled in the .js and
       # manifest.json files (for listing examples on the API reference pages),
       # but there are still images, CSS, etc.
       results.append(request_files_in_dir(svn_constants.STATIC_PATH,
                                           prefix='static/'))
 
       # Samples are too expensive to run on the dev server, where there is no
       # parallel fetch.
       if not IsDevServer():
         # Fetch each individual sample file.
         results.append(request_files_in_dir(svn_constants.EXAMPLES_PATH,
                                             prefix='extensions/examples/'))
 
         # Fetch the zip file of each example (contains all the individual
         # files).
         example_zips = []
         for root, _, files in trunk_fs.Walk(svn_constants.EXAMPLES_PATH):
           example_zips.extend(
               root + '.zip' for name in files if name == 'manifest.json')
         results.append(_RequestEachItem(
             'example zips',
             example_zips,
             lambda path: render('extensions/examples/' + path)))
-
-      def run_cron(data_source):
-        title = data_source.__class__.__name__
-        _cronlog.info('%s: starting' % title)
-        start_time = time.time()
-        try:
-          data_source.Cron()
-        except Exception as e:
-          _cronlog.error('%s: error %s' % (title, traceback.format_exc()))
-          results.append(False)
-          if IsDeadlineExceededError(e): raise
-        finally:
-          _cronlog.info(
-              '%s: took %s seconds' % (title, time.time() - start_time))
-
-      for data_source in CreateDataSources(server_instance).values():
-        run_cron(data_source)
-
-      run_cron(server_instance.content_providers)
-
     except:
       results.append(False)
       # This should never actually happen (each cron step does its own
       # conservative error checking), so re-raise no matter what it is.
       _cronlog.error('uncaught error: %s' % traceback.format_exc())
       raise
     finally:
       success = all(results)
       _cronlog.info('finished (%s)', 'success' if success else 'FAILED')
       return (Response.Ok('Success') if success else
@@ skipping 50 matching lines @@
     branch_utility = self._delegate.CreateBranchUtility(object_store_creator)
     host_file_system_provider = self._delegate.CreateHostFileSystemProvider(
         object_store_creator, max_trunk_revision=revision)
     github_file_system_provider = self._delegate.CreateGithubFileSystemProvider(
         object_store_creator)
     return ServerInstance(object_store_creator,
                           CompiledFileSystem.Factory(object_store_creator),
                           branch_utility,
                           host_file_system_provider,
                           github_file_system_provider)