[ARVADOS] updated: b445db93ece1069c949f0c02d0564e578e453d12

git at public.curoverse.com git at public.curoverse.com
Tue Feb 2 10:03:42 EST 2016


Summary of changes:
 .../nodemanager/arvnodeman/computenode/__init__.py | 82 +++++++++++++---------
 .../arvnodeman/computenode/dispatch/__init__.py    | 30 ++++----
 .../arvnodeman/computenode/dispatch/slurm.py       |  8 +--
 .../arvnodeman/computenode/driver/__init__.py      | 17 +++--
 4 files changed, 75 insertions(+), 62 deletions(-)

       via  b445db93ece1069c949f0c02d0564e578e453d12 (commit)
      from  85c71a36173550f14fb1d5f4092f2050ec8dc033 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit b445db93ece1069c949f0c02d0564e578e453d12
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Tue Feb 2 10:03:39 2016 -0500

    8206: Refactor _retry to RetryMixin.  Make retry timing consistent.

diff --git a/services/nodemanager/arvnodeman/computenode/__init__.py b/services/nodemanager/arvnodeman/computenode/__init__.py
index bc8ada5..3921e09 100644
--- a/services/nodemanager/arvnodeman/computenode/__init__.py
+++ b/services/nodemanager/arvnodeman/computenode/__init__.py
@@ -44,7 +44,7 @@ def arvados_node_missing(arvados_node, fresh_time):
     else:
         return not timestamp_fresh(arvados_timestamp(arvados_node["last_ping_at"]), fresh_time)
 
-def _retry(errors=()):
+class RetryMixin(object):
     """Retry decorator for an method that makes remote requests.
 
     Use this function to decorate method, and pass in a tuple of exceptions to
@@ -55,40 +55,54 @@ def _retry(errors=()):
     is a timer actor.)
 
     """
-
-    def decorator(orig_func):
-        @functools.wraps(orig_func)
-        def retry_wrapper(self, *args, **kwargs):
-            start_time = time.time()
-            while True:
-                try:
-                    ret = orig_func(self, *args, **kwargs)
-                except Exception as error:
-                    if not (isinstance(error, errors) or
-                            self._cloud.is_cloud_exception(error)):
-                        raise
-                    self._logger.warning(
-                        "Client error: %s - waiting %s seconds",
-                        error, self.retry_wait)
-                    if self._timer:
-                        # reschedule to be called again
-                        self._timer.schedule(start_time + self.retry_wait,
-                                             getattr(self._later,
-                                                     orig_func.__name__),
-                                             *args, **kwargs)
+    def __init__(self, retry_wait, max_retry_wait,
+                 logger, cloud, timer=None):
+        self.min_retry_wait = retry_wait
+        self.max_retry_wait = max_retry_wait
+        self.retry_wait = retry_wait
+        self._logger = logger
+        self._cloud = cloud
+        self._timer = timer
+
+    @staticmethod
+    def _retry(errors=()):
+        def decorator(orig_func):
+            @functools.wraps(orig_func)
+            def retry_wrapper(self, *args, **kwargs):
+                while True:
+                    try:
+                        ret = orig_func(self, *args, **kwargs)
+                    except Exception as error:
+                        if not (isinstance(error, errors) or
+                                self._cloud.is_cloud_exception(error)):
+                            self.retry_wait = self.min_retry_wait
+                            raise
+
+                        self._logger.warning(
+                            "Client error: %s - waiting %s seconds",
+                            error, self.retry_wait, exc_info=error)
+
+                        if self._timer:
+                            start_time = time.time()
+                            # reschedule to be called again
+                            self._timer.schedule(start_time + self.retry_wait,
+                                                 getattr(self._later,
+                                                         orig_func.__name__),
+                                                 *args, **kwargs)
+                        else:
+                            # sleep on it.
+                            time.sleep(self.retry_wait)
+
+                        self.retry_wait = min(self.retry_wait * 2,
+                                              self.max_retry_wait)
+                        if self._timer:
+                            # expect to be called again by timer so don't loop
+                            return
                     else:
-                        # sleep on it.
-                        time.sleep(self.retry_wait)
-                    self.retry_wait = min(self.retry_wait * 2,
-                                          self.max_retry_wait)
-                    if self._timer:
-                        # expect to be called again by timer so don't loop
-                        return
-                else:
-                    self.retry_wait = self.min_retry_wait
-                    return ret
-        return retry_wrapper
-    return decorator
+                        self.retry_wait = self.min_retry_wait
+                        return ret
+            return retry_wrapper
+        return decorator
 
 class ShutdownTimer(object):
     """Keep track of a cloud node's shutdown windows.
diff --git a/services/nodemanager/arvnodeman/computenode/dispatch/__init__.py b/services/nodemanager/arvnodeman/computenode/dispatch/__init__.py
index fcc1376..8c983c1 100644
--- a/services/nodemanager/arvnodeman/computenode/dispatch/__init__.py
+++ b/services/nodemanager/arvnodeman/computenode/dispatch/__init__.py
@@ -11,11 +11,11 @@ import pykka
 
 from .. import \
     arvados_node_fqdn, arvados_node_mtime, arvados_timestamp, timestamp_fresh, \
-    arvados_node_missing, _retry
+    arvados_node_missing, RetryMixin
 from ...clientactor import _notify_subscribers
 from ... import config
 
-class ComputeNodeStateChangeBase(config.actor_class):
+class ComputeNodeStateChangeBase(config.actor_class, RetryMixin):
     """Base class for actors that change a compute node's state.
 
     This base class takes care of retrying changes and notifying
@@ -24,14 +24,14 @@ class ComputeNodeStateChangeBase(config.actor_class):
     def __init__(self, logger_name, cloud_client, arvados_client, timer_actor,
                  retry_wait, max_retry_wait):
         super(ComputeNodeStateChangeBase, self).__init__()
+        RetryMixin.__init__(self,
+                            retry_wait,
+                            max_retry_wait,
+                            logging.getLogger(logger_name),
+                            cloud_client,
+                            timer_actor)
         self._later = self.actor_ref.proxy()
-        self._logger = logging.getLogger(logger_name)
-        self._cloud = cloud_client
         self._arvados = arvados_client
-        self._timer = timer_actor
-        self.min_retry_wait = retry_wait
-        self.max_retry_wait = max_retry_wait
-        self.retry_wait = retry_wait
         self.subscribers = set()
 
     def _finished(self):
@@ -84,18 +84,18 @@ class ComputeNodeSetupActor(ComputeNodeStateChangeBase):
         else:
             self._later.prepare_arvados_node(arvados_node)
 
-    @_retry(config.ARVADOS_ERRORS)
+    @RetryMixin._retry(config.ARVADOS_ERRORS)
     def create_arvados_node(self):
         self.arvados_node = self._arvados.nodes().create(body={}).execute()
         self._later.create_cloud_node()
 
-    @_retry(config.ARVADOS_ERRORS)
+    @RetryMixin._retry(config.ARVADOS_ERRORS)
     def prepare_arvados_node(self, node):
         self.arvados_node = self._clean_arvados_node(
             node, "Prepared by Node Manager")
         self._later.create_cloud_node()
 
-    @_retry()
+    @RetryMixin._retry()
     def create_cloud_node(self):
         self._logger.info("Creating cloud node with size %s.",
                           self.cloud_size.name)
@@ -106,7 +106,7 @@ class ComputeNodeSetupActor(ComputeNodeStateChangeBase):
         self._logger.info("Cloud node %s created.", self.cloud_node.id)
         self._later.update_arvados_node_properties()
 
-    @_retry(config.ARVADOS_ERRORS)
+    @RetryMixin._retry(config.ARVADOS_ERRORS)
     def update_arvados_node_properties(self):
         """Tell Arvados some details about the cloud node.
 
@@ -130,7 +130,7 @@ class ComputeNodeSetupActor(ComputeNodeStateChangeBase):
         self._logger.info("%s updated properties.", self.arvados_node['uuid'])
         self._later.post_create()
 
-    @_retry()
+    @RetryMixin._retry()
     def post_create(self):
         self._cloud.post_create_node(self.cloud_node)
         self._logger.info("%s post-create work done.", self.cloud_node.id)
@@ -197,7 +197,7 @@ class ComputeNodeShutdownActor(ComputeNodeStateChangeBase):
         return stop_wrapper
 
     @_stop_if_window_closed
-    @_retry()
+    @RetryMixin._retry()
     def shutdown_node(self):
         if not self._cloud.destroy_node(self.cloud_node):
             if self._cloud.broken(self.cloud_node):
@@ -212,7 +212,7 @@ class ComputeNodeShutdownActor(ComputeNodeStateChangeBase):
         else:
             self._later.clean_arvados_node(arv_node)
 
-    @_retry(config.ARVADOS_ERRORS)
+    @RetryMixin._retry(config.ARVADOS_ERRORS)
     def clean_arvados_node(self, arvados_node):
         self._clean_arvados_node(arvados_node, "Shut down by Node Manager")
         self._finished(success_flag=True)
diff --git a/services/nodemanager/arvnodeman/computenode/dispatch/slurm.py b/services/nodemanager/arvnodeman/computenode/dispatch/slurm.py
index c395a30..43f61c7 100644
--- a/services/nodemanager/arvnodeman/computenode/dispatch/slurm.py
+++ b/services/nodemanager/arvnodeman/computenode/dispatch/slurm.py
@@ -8,7 +8,7 @@ import time
 from . import \
     ComputeNodeSetupActor, ComputeNodeUpdateActor, ComputeNodeMonitorActor
 from . import ComputeNodeShutdownActor as ShutdownActorBase
-from .. import _retry
+from .. import RetryMixin
 
 class ComputeNodeShutdownActor(ShutdownActorBase):
     SLURM_END_STATES = frozenset(['down\n', 'down*\n',
@@ -43,7 +43,7 @@ class ComputeNodeShutdownActor(ShutdownActorBase):
     # of the excessive memory usage that result in the "Cannot allocate memory"
     # error are still being investigated.
 
-    @_retry((subprocess.CalledProcessError, OSError))
+    @RetryMixin._retry((subprocess.CalledProcessError, OSError))
     def cancel_shutdown(self, reason):
         if self._nodename:
             if self._get_slurm_state() in self.SLURM_DRAIN_STATES:
@@ -55,14 +55,14 @@ class ComputeNodeShutdownActor(ShutdownActorBase):
                 pass
         return super(ComputeNodeShutdownActor, self).cancel_shutdown(reason)
 
-    @_retry((subprocess.CalledProcessError, OSError))
+    @RetryMixin._retry((subprocess.CalledProcessError, OSError))
     @ShutdownActorBase._stop_if_window_closed
     def issue_slurm_drain(self):
         self._set_node_state('DRAIN', 'Reason=Node Manager shutdown')
         self._logger.info("Waiting for SLURM node %s to drain", self._nodename)
         self._later.await_slurm_drain()
 
-    @_retry((subprocess.CalledProcessError, OSError))
+    @RetryMixin._retry((subprocess.CalledProcessError, OSError))
     @ShutdownActorBase._stop_if_window_closed
     def await_slurm_drain(self):
         output = self._get_slurm_state()
diff --git a/services/nodemanager/arvnodeman/computenode/driver/__init__.py b/services/nodemanager/arvnodeman/computenode/driver/__init__.py
index 779209b..b696677 100644
--- a/services/nodemanager/arvnodeman/computenode/driver/__init__.py
+++ b/services/nodemanager/arvnodeman/computenode/driver/__init__.py
@@ -9,9 +9,9 @@ import libcloud.common.types as cloud_types
 from libcloud.compute.base import NodeDriver, NodeAuthSSHKey
 
 from ...config import NETWORK_ERRORS
-from .. import _retry
+from .. import RetryMixin
 
-class BaseComputeNodeDriver(object):
+class BaseComputeNodeDriver(RetryMixin):
     """Abstract base class for compute node drivers.
 
     libcloud drivers abstract away many of the differences between
@@ -26,7 +26,7 @@ class BaseComputeNodeDriver(object):
     """
     CLOUD_ERRORS = NETWORK_ERRORS + (cloud_types.LibcloudError,)
 
-    @_retry()
+    @RetryMixin._retry()
     def _create_driver(self, driver_class, **auth_kwargs):
         return driver_class(**auth_kwargs)
 
@@ -44,12 +44,11 @@ class BaseComputeNodeDriver(object):
           libcloud driver's create_node method to create a new compute node.
         * driver_class: The class of a libcloud driver to use.
         """
-        self.min_retry_wait = retry_wait
-        self.max_retry_wait = max_retry_wait
-        self.retry_wait = retry_wait
-        self._cloud = type(self)
-        self._logger = logging.getLogger(str(self._cloud))
-        self._timer = None
+
+        super(BaseComputeNodeDriver, self).__init__(retry_wait, max_retry_wait,
+                                         logging.getLogger(str(type(self))),
+                                         type(self),
+                                         None)
         self.real = self._create_driver(driver_class, **auth_kwargs)
         self.list_kwargs = list_kwargs
         self.create_kwargs = create_kwargs

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list