Various changes.
[python_utils.git] / executors.py
index b9c0748391f733e0719a744d9b30280c34bb30ee..63efd812e54b5b796daff892f9ff1e75fd4e6569 100644 (file)
@@ -21,10 +21,9 @@ import cloudpickle  # type: ignore
 from ansi import bg, fg, underline, reset
 import argparse_utils
 import config
-import exec_utils
+from exec_utils import run_silently, cmd_in_background
 from decorator_utils import singleton
-import histogram
-import string_utils
+import histogram as hist
 
 logger = logging.getLogger(__name__)
 
@@ -52,38 +51,16 @@ parser.add_argument(
     action=argparse_utils.ActionNoYes,
     help='Should we schedule duplicative backup work if a remote bundle is slow',
 )
-
-rsync = 'rsync -q --no-motd -W --ignore-existing --timeout=60 --size-only -z'
-ssh = 'ssh -oForwardX11=no'
-
-
-hist = histogram.SimpleHistogram(
-    histogram.SimpleHistogram.n_evenly_spaced_buckets(
-        int(0), int(500), 25
-    )
+parser.add_argument(
+    '--executors_max_bundle_failures',
+    type=int,
+    default=3,
+    metavar='#FAILURES',
+    help='Maximum number of failures before giving up on a bundle',
 )
 
-
-def run_local_bundle(fun, *args, **kwargs):
-    logger.debug(f"Running local bundle at {fun.__name__}")
-    start = time.time()
-    result = fun(*args, **kwargs)
-    end = time.time()
-    duration = end - start
-    logger.debug(f"{fun.__name__} finished; used {duration:.1f}s")
-    hist.add_item(duration)
-    return result
-
-
-def run_cloud_pickle(pickle):
-    fun, args, kwargs = cloudpickle.loads(pickle)
-    logger.debug(f"Running pickled bundle at {fun.__name__}")
-    start = time.time()
-    result = fun(*args, **kwargs)
-    end = time.time()
-    duration = end - start
-    logger.debug(f"{fun.__name__} finished; used {duration:.1f}s")
-    return result
+RSYNC = 'rsync -q --no-motd -W --ignore-existing --timeout=60 --size-only -z'
+SSH = 'ssh -oForwardX11=no'
 
 
 def make_cloud_pickle(fun, *args, **kwargs):
@@ -92,8 +69,14 @@ def make_cloud_pickle(fun, *args, **kwargs):
 
 
 class BaseExecutor(ABC):
-    def __init__(self):
-        pass
+    def __init__(self, *, title=''):
+        self.title = title
+        self.task_count = 0
+        self.histogram = hist.SimpleHistogram(
+            hist.SimpleHistogram.n_evenly_spaced_buckets(
+                int(0), int(500), 50
+            )
+        )
 
     @abstractmethod
     def submit(self,
@@ -107,6 +90,10 @@ class BaseExecutor(ABC):
                  wait: bool = True) -> None:
         pass
 
+    def adjust_task_count(self, delta: int) -> None:
+        self.task_count += delta
+        logger.debug(f'Executor current task count is {self.task_count}')
+
 
 class ThreadExecutor(BaseExecutor):
     def __init__(self,
@@ -122,29 +109,36 @@ class ThreadExecutor(BaseExecutor):
             max_workers=workers,
             thread_name_prefix="thread_executor_helper"
         )
-        self.job_count = 0
+
+    def run_local_bundle(self, fun, *args, **kwargs):
+        logger.debug(f"Running local bundle at {fun.__name__}")
+        start = time.time()
+        result = fun(*args, **kwargs)
+        end = time.time()
+        self.adjust_task_count(-1)
+        duration = end - start
+        logger.debug(f"{fun.__name__} finished; used {duration:.1f}s")
+        self.histogram.add_item(duration)
+        return result
 
     def submit(self,
                function: Callable,
                *args,
                **kwargs) -> fut.Future:
-        self.job_count += 1
-        logger.debug(
-            f'Submitted work to threadpool; there are now {self.job_count} items.'
-        )
+        self.adjust_task_count(+1)
         newargs = []
         newargs.append(function)
         for arg in args:
             newargs.append(arg)
         return self._thread_pool_executor.submit(
-            run_local_bundle,
+            self.run_local_bundle,
             *newargs,
             **kwargs)
 
     def shutdown(self,
                  wait = True) -> None:
-        logger.debug("Shutting down threadpool executor.")
-        print(hist)
+        logger.debug(f'Shutting down threadpool executor {self.title}')
+        print(self.histogram)
         self._thread_pool_executor.shutdown(wait)
 
 
@@ -161,24 +155,41 @@ class ProcessExecutor(BaseExecutor):
         self._process_executor = fut.ProcessPoolExecutor(
             max_workers=workers,
         )
-        self.job_count = 0
+
+    def run_cloud_pickle(self, pickle):
+        fun, args, kwargs = cloudpickle.loads(pickle)
+        logger.debug(f"Running pickled bundle at {fun.__name__}")
+        result = fun(*args, **kwargs)
+        self.adjust_task_count(-1)
+        return result
 
     def submit(self,
                function: Callable,
                *args,
                **kwargs) -> fut.Future:
-        # Bundle it up before submitting because pickle sucks.
+        start = time.time()
+        self.adjust_task_count(+1)
         pickle = make_cloud_pickle(function, *args, **kwargs)
-        self.job_count += 1
-        logger.debug(
-            f'Submitting work to processpool executor; there are now {self.job_count} items.'
+        result = self._process_executor.submit(
+            self.run_cloud_pickle,
+            pickle
+        )
+        result.add_done_callback(
+            lambda _: self.histogram.add_item(
+                time.time() - start
+            )
         )
-        return self._process_executor.submit(run_cloud_pickle, pickle)
+        return result
 
     def shutdown(self, wait=True) -> None:
-        logger.debug('Shutting down processpool executor')
-        print(hist)
+        logger.debug(f'Shutting down processpool executor {self.title}')
         self._process_executor.shutdown(wait)
+        print(self.histogram)
+
+    def __getstate__(self):
+        state = self.__dict__.copy()
+        state['_process_executor'] = None
+        return state
 
 
 @dataclass
@@ -214,6 +225,7 @@ class BundleDetails:
     is_cancelled: threading.Event
     was_cancelled: bool
     backup_bundles: Optional[List[BundleDetails]]
+    failure_count: int
 
 
 class RemoteExecutorStatus:
@@ -559,10 +571,13 @@ class RemoteExecutor(BaseExecutor):
         return False
 
     def launch(self, bundle: BundleDetails) -> Any:
-        # Find a worker for bundle or block until one is available.
+        """Find a worker for bundle or block until one is available."""
+        self.adjust_task_count(+1)
         uuid = bundle.uuid
         hostname = bundle.hostname
         avoid_machine = None
+
+        # Try not to schedule a backup on the same host as the original.
         if bundle.src_bundle is not None:
             avoid_machine = bundle.src_bundle.machine
         worker = None
@@ -574,29 +589,33 @@ class RemoteExecutor(BaseExecutor):
         self.status.record_acquire_worker(worker, uuid)
         logger.debug(f'Running bundle {uuid} on {worker}...')
 
-        # Before we do work, make sure it's still viable.
+        # Before we do any work, make sure the bundle is still viable.
         if self.check_if_cancelled(bundle):
-            return self.post_launch_work(bundle)
+            try:
+                return self.post_launch_work(bundle)
+            except Exception as e:
+                logger.exception(e)
+                logger.info(f"Bundle {uuid} seems to have failed?!")
+                if bundle.failure_count < config.config['executors_max_bundle_failures']:
+                    return self.launch(bundle)
+                else:
+                    logger.info(f"Bundle {uuid} is poison, giving up on it.")
+                    return None
 
         # Send input to machine if it's not local.
         if hostname not in machine:
-            cmd = f'{rsync} {bundle.code_file} {username}@{machine}:{bundle.code_file}'
-            logger.debug(f"Copying work to {worker} via {cmd}")
-            exec_utils.run_silently(cmd)
-
-        # Before we do more work, make sure it's still viable.
-        if self.check_if_cancelled(bundle):
-            return self.post_launch_work(bundle)
-
-        # Fucking Apple has a python3 binary in /usr/sbin that is not
-        # the one we want and is protected by the OS so make sure that
-        # /usr/local/bin is early in the path.
-        cmd = (f'{ssh} {bundle.username}@{bundle.machine} '
-               f'"export PATH=/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/home/scott/bin:/home/scott/.local/bin; /home/scott/lib/python_modules/remote_worker.py'
+            cmd = f'{RSYNC} {bundle.code_file} {username}@{machine}:{bundle.code_file}'
+            logger.info(f"Copying work to {worker} via {cmd}")
+            run_silently(cmd)
+
+        # Do it.
+        cmd = (f'{SSH} {bundle.username}@{bundle.machine} '
+               f'"source remote-execution/bin/activate &&'
+               f' /home/scott/lib/python_modules/remote_worker.py'
                f' --code_file {bundle.code_file} --result_file {bundle.result_file}"')
-        p = exec_utils.cmd_in_background(cmd, silent=True)
+        p = cmd_in_background(cmd, silent=True)
         bundle.pid = pid = p.pid
-        logger.debug(f"Running {cmd} in the background as process {pid}")
+        logger.info(f"Running {cmd} in the background as process {pid}")
 
         while True:
             try:
@@ -614,7 +633,16 @@ class RemoteExecutor(BaseExecutor):
                     f"{pid}/{bundle.uuid} has finished its work normally."
                 )
                 break
-        return self.post_launch_work(bundle)
+
+        try:
+            return self.post_launch_work(bundle)
+        except Exception as e:
+            logger.exception(e)
+            logger.info(f"Bundle {uuid} seems to have failed?!")
+            if bundle.failure_count < config.config['executors_max_bundle_failures']:
+                return self.launch(bundle)
+            logger.info(f"Bundle {uuid} is poison, giving up on it.")
+            return None
 
     def post_launch_work(self, bundle: BundleDetails) -> Any:
         with self.status.lock:
@@ -628,37 +656,47 @@ class RemoteExecutor(BaseExecutor):
             # Whether original or backup, if we finished first we must
             # fetch the results if the computation happened on a
             # remote machine.
+            bundle.end_ts = time.time()
             if not was_cancelled:
                 assert bundle.machine is not None
                 if bundle.hostname not in bundle.machine:
-                    cmd = f'{rsync} {username}@{machine}:{result_file} {result_file} 2>/dev/null'
-                    logger.debug(
+                    cmd = f'{RSYNC} {username}@{machine}:{result_file} {result_file} 2>/dev/null'
+                    logger.info(
                         f"Fetching results from {username}@{machine} via {cmd}"
                     )
                     try:
-                        exec_utils.run_silently(cmd)
+                        run_silently(cmd)
                     except subprocess.CalledProcessError:
-                        pass
-                    exec_utils.run_silently(f'{ssh} {username}@{machine}'
-                                            f' "/bin/rm -f {code_file} {result_file}"')
-            bundle.end_ts = time.time()
+                        logger.critical(f'Failed to copy {username}@{machine}:{result_file}!')
+                    run_silently(f'{SSH} {username}@{machine}'
+                                 f' "/bin/rm -f {code_file} {result_file}"')
+                dur = bundle.end_ts - bundle.start_ts
+                self.histogram.add_item(dur)
             assert bundle.worker is not None
             self.status.record_release_worker_already_locked(
                 bundle.worker,
                 bundle.uuid,
                 was_cancelled
             )
-            if not was_cancelled:
-                dur = bundle.end_ts - bundle.start_ts
-                hist.add_item(dur)
 
         # Only the original worker should unpickle the file contents
-        # though since it's the only one whose result matters.
+        # though since it's the only one whose result matters.  The
+        # original is also the only job that may delete result_file
+        # from disk.  Note that the original may have been cancelled
+        # if one of the backups finished first; it still must read the
+        # result from disk.
         if is_original:
             logger.debug(f"Unpickling {result_file}.")
-            with open(f'{result_file}', 'rb') as rb:
-                serialized = rb.read()
-            result = cloudpickle.loads(serialized)
+            try:
+                with open(f'{result_file}', 'rb') as rb:
+                    serialized = rb.read()
+                result = cloudpickle.loads(serialized)
+            except Exception as e:
+                msg = f'Failed to load {result_file}'
+                logger.critical(msg)
+                bundle.failure_count += 1
+                self.release_worker(bundle.worker)
+                raise Exception(e)
             os.remove(f'{result_file}')
             os.remove(f'{code_file}')
 
@@ -673,11 +711,11 @@ class RemoteExecutor(BaseExecutor):
                     )
                     backup.is_cancelled.set()
 
-        # This is a backup.
+        # This is a backup job.
         else:
             # Backup results don't matter, they just need to leave the
-            # result file in the right place for their original to
-            # read later.
+            # result file in the right place for their originals to
+            # read/unpickle later.
             result = None
 
             # Tell the original to stop if we finished first.
@@ -689,10 +727,12 @@ class RemoteExecutor(BaseExecutor):
 
         assert bundle.worker is not None
         self.release_worker(bundle.worker)
+        self.adjust_task_count(-1)
         return result
 
     def create_original_bundle(self, pickle):
-        uuid = string_utils.generate_uuid(as_hex=True)
+        from string_utils import generate_uuid
+        uuid = generate_uuid(as_hex=True)
         code_file = f'/tmp/{uuid}.code.bin'
         result_file = f'/tmp/{uuid}.result.bin'
 
@@ -718,6 +758,7 @@ class RemoteExecutor(BaseExecutor):
             is_cancelled = threading.Event(),
             was_cancelled = False,
             backup_bundles = [],
+            failure_count = 0,
         )
         self.status.record_bundle_details(bundle)
         logger.debug(f'Created original bundle {uuid}')
@@ -746,6 +787,7 @@ class RemoteExecutor(BaseExecutor):
             is_cancelled = threading.Event(),
             was_cancelled = False,
             backup_bundles = None,    # backup backups not allowed
+            failure_count = 0,
         )
         src_bundle.backup_bundles.append(backup_bundle)
         self.status.record_bundle_details_already_locked(backup_bundle)
@@ -772,14 +814,12 @@ class RemoteExecutor(BaseExecutor):
         pickle = make_cloud_pickle(function, *args, **kwargs)
         bundle = self.create_original_bundle(pickle)
         self.total_bundles_submitted += 1
-        logger.debug(
-            f'Submitted work to remote executor; {self.total_bundles_submitted} items now submitted'
-        )
         return self._helper_executor.submit(self.launch, bundle)
 
     def shutdown(self, wait=True) -> None:
         self._helper_executor.shutdown(wait)
-        print(hist)
+        logging.debug(f'Shutting down RemoteExecutor {self.title}')
+        print(self.histogram)
 
 
 @singleton
@@ -815,8 +855,8 @@ class DefaultExecutors(object):
                     RemoteWorkerRecord(
                         username = 'scott',
                         machine = 'cheetah.house',
-                        weight = 10,
-                        count = 6,
+                        weight = 12,
+                        count = 4,
                     ),
                 )
             if self.ping('video.house'):
@@ -824,7 +864,7 @@ class DefaultExecutors(object):
                     RemoteWorkerRecord(
                         username = 'scott',
                         machine = 'video.house',
-                        weight = 2,
+                        weight = 1,
                         count = 4,
                     ),
                 )
@@ -851,8 +891,8 @@ class DefaultExecutors(object):
                     RemoteWorkerRecord(
                         username = 'scott',
                         machine = 'backup.house',
-                        weight = 3,
-                        count = 2,
+                        weight = 1,
+                        count = 4,
                     ),
                 )
             if self.ping('puma.cabin'):
@@ -860,8 +900,8 @@ class DefaultExecutors(object):
                     RemoteWorkerRecord(
                         username = 'scott',
                         machine = 'puma.cabin',
-                        weight = 10,
-                        count = 6,
+                        weight = 12,
+                        count = 4,
                     ),
                 )
             policy = WeightedRandomRemoteWorkerSelectionPolicy()