Drop claim timeouts from resource tracker

Removed the claim timeouts.  The timeouts are basically irrelevant
since the re-work done here: https://review.openstack.org/#/c/13182/

Each claim is a marker of an operation consuming resources, valid
until the next invocation of the audit process
update_available_resource(), which does a full reconciliation of all
resource usage values from the DB.

Change-Id: I5cb0dd8903d358272ec223cf73f6eb4c06371a96

2 files changed, 18 insertions, 84 deletions

diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py
index e5e1194a5..d780da341 100644
--- a/nova/compute/resource_tracker.py
+++ b/nova/compute/resource_tracker.py
@@ -27,7 +27,6 @@ from nova.openstack.common import cfg
 from nova.openstack.common import importutils
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
-from nova.openstack.common import timeutils
 from nova import utils
 
 resource_tracker_opts = [
@@ -35,8 +34,6 @@ resource_tracker_opts = [
                help='Amount of disk in MB to reserve for the host'),
     cfg.IntOpt('reserved_host_memory_mb', default=512,
                help='Amount of memory in MB to reserve for the host'),
-    cfg.IntOpt('claim_timeout_seconds', default=600,
-               help='How long, in seconds, before a resource claim times out'),
     cfg.StrOpt('compute_stats_class',
                default='nova.compute.stats.Stats',
                help='Class that will manage stats for the local compute host')
@@ -51,22 +48,17 @@ COMPUTE_RESOURCE_SEMAPHORE = "compute_resources"
 
 class Claim(object):
     """A declaration that a compute host operation will require free resources.
+    Claims serve as marker objects that resources are being held until the
+    update_available_resource audit process runs to do a full reconciliation
+    of resource usage.
 
     This information will be used to help keep the local compute hosts's
     ComputeNode model in sync to aid the scheduler in making efficient / more
     correct decisions with respect to host selection.
     """
 
-    def __init__(self, instance, timeout):
+    def __init__(self, instance):
         self.instance = jsonutils.to_primitive(instance)
-        self.timeout = timeout
-        self.expire_ts = timeutils.utcnow_ts() + timeout
-
-    def is_expired(self):
-        """Determine if this adjustment is old enough that we can assume it's
-        no longer needed.
-        """
-        return timeutils.utcnow_ts() > self.expire_ts
 
     @property
     def claim_id(self):
@@ -129,8 +121,7 @@ class ResourceTracker(object):
         return ResourceContextManager(context, claim, self)
 
     @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)
-    def begin_resource_claim(self, context, instance_ref, limits=None,
-                             timeout=None):
+    def begin_resource_claim(self, context, instance_ref, limits=None):
         """Indicate that some resources are needed for an upcoming compute
         instance build operation.
 
@@ -141,11 +132,6 @@ class ResourceTracker(object):
         :param instance_ref: instance to reserve resources for
         :param limits: Dict of oversubscription limits for memory, disk,
                        and CPUs.
-        :param timeout: How long, in seconds, the operation that requires
-                        these resources should take to actually allocate what
-                        it needs from the hypervisor.  If the timeout is
-                        exceeded, the new resource claim will assume caller
-                        before releasing the resources.
         :returns: An integer 'claim ticket'.  This should be turned into
                   finalize  a resource claim or free resources after the
                   compute operation is finished. Returns None if the claim
@@ -157,9 +143,6 @@ class ResourceTracker(object):
         if not limits:
             limits = {}
 
-        if not timeout:
-            timeout = FLAGS.claim_timeout_seconds
-
         # If an individual limit is None, the resource will be considered
         # unlimited:
         memory_mb_limit = limits.get('memory_mb')
@@ -186,7 +169,7 @@ class ResourceTracker(object):
 
         # keep track of this claim until we know whether the compute operation
         # was successful/completed:
-        claim = Claim(instance_ref, timeout)
+        claim = Claim(instance_ref)
         self.claims[claim.claim_id] = claim
 
         # Mark resources in-use and update stats
@@ -292,10 +275,6 @@ class ResourceTracker(object):
         resources identified by 'claim' has now completed and the resources
         have been allocated at the virt layer.
 
-        Calling this keeps the available resource data more accurate and
-        timely than letting the claim timeout elapse and waiting for
-        update_available_resource to reflect the changed usage data.
-
         :param claim: A claim indicating a set of resources that were
                       previously claimed.
         """
@@ -303,10 +282,7 @@ class ResourceTracker(object):
             return
 
         if self.claims.pop(claim.claim_id, None):
-            LOG.info(_("Finishing claim: %s") % claim)
-        else:
-            LOG.info(_("Can't find claim %s.  It may have been 'finished' "
-                       "twice, or it has already timed out."), claim.claim_id)
+            LOG.debug(_("Finishing claim: %s") % claim)
 
     @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)
     def abort_resource_claim(self, context, claim):
@@ -320,21 +296,14 @@ class ResourceTracker(object):
         if self.disabled:
             return
 
-        # un-claim the resources:
         if self.claims.pop(claim.claim_id, None):
-            LOG.info(_("Aborting claim: %s") % claim)
+            LOG.debug(_("Aborting claim: %s") % claim)
             # flag the instance as deleted to revert the resource usage
             # and associated stats:
             claim.instance['vm_state'] = vm_states.DELETED
             self._update_usage_from_instance(self.compute_node, claim.instance)
             self._update(context, self.compute_node)
 
-        else:
-            # can't find the claim.  this may mean the claim already timed
-            # out or it was already explicitly finished/aborted.
-            LOG.audit(_("Claim %s not found.  It either timed out or was "
-                        "already explicitly finished/aborted"), claim.claim_id)
-
     @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)
     def update_usage(self, context, instance):
         """Update the resource usage and stats after a change in an
@@ -376,7 +345,7 @@ class ResourceTracker(object):
 
         self._report_hypervisor_resource_view(resources)
 
-        self._purge_expired_claims()
+        self._purge_claims()
 
         # Grab all instances assigned to this host:
         instances = db.instance_get_all_by_host(context, self.host)
@@ -411,16 +380,11 @@ class ResourceTracker(object):
             self._update(context, resources, prune_stats=True)
             LOG.info(_('Compute_service record updated for %s ') % self.host)
 
-    def _purge_expired_claims(self):
-        """Purge expired resource claims"""
-        for claim_id in self.claims.keys():
-            c = self.claims[claim_id]
-            if c.is_expired():
-                # if this claim is expired, just expunge it.
-                # it is assumed that the instance will eventually get built
-                # successfully.
-                LOG.audit(_("Expiring resource claim %s"), claim_id)
-                self.claims.pop(claim_id)
+    def _purge_claims(self):
+        """Purge claims.  They are no longer needed once the audit process
+        reconciles usage values from the database.
+        """
+        self.claims.clear()
 
     def _create(self, context, values):
         """Create the compute node in the DB"""
diff --git a/nova/tests/compute/test_resource_tracker.py b/nova/tests/compute/test_resource_tracker.py
index 04b2dfdba..84dd30e82 100644
--- a/nova/tests/compute/test_resource_tracker.py
+++ b/nova/tests/compute/test_resource_tracker.py
@@ -485,44 +485,14 @@ class ResourceTestCase(BaseTestCase):
         self.assertEqual(0, self.compute["local_gb_used"])
         self.assertEqual(6, self.compute["free_disk_gb"])
 
-    def testExpiredClaims(self):
-        """Test that old claims get cleaned up automatically if not finished
-        or aborted explicitly.
-        """
+    def testClaimsPurge(self):
+        """Test that claims get get purged when the audit process runs"""
+
         instance = self._fake_instance(memory_mb=2, root_gb=2, ephemeral_gb=0)
         claim = self.tracker.begin_resource_claim(self.context, instance)
-        claim.expire_ts = timeutils.utcnow_ts() - 1
-        self.assertTrue(claim.is_expired())
-
-        # and an unexpired claim
-        instance2 = self._fake_instance(memory_mb=1, root_gb=1, ephemeral_gb=0)
-        claim2 = self.tracker.begin_resource_claim(self.context, instance2)
 
-        self.assertEqual(2, len(self.tracker.claims))
-        self.assertEqual(2 + 1, self.tracker.compute_node['memory_mb_used'])
-        self.assertEqual(2 + 1, self.tracker.compute_node['local_gb_used'])
-
-        # expired claims get expunged when audit runs:
         self.tracker.update_available_resource(self.context)
-
-        self.assertEqual(1, len(self.tracker.claims))
-        self.assertEqual(2, len(self.tracker.tracked_instances))
-
-        # the expired claim's instance is assumed to still exist, so the
-        # resources should be counted:
-        self.assertEqual(2 + 1, self.tracker.compute_node['memory_mb_used'])
-        self.assertEqual(2 + 1, self.tracker.compute_node['local_gb_used'])
-
-        # this abort should do nothing because the claim was purged due to
-        # expiration:
-        self.tracker.abort_resource_claim(self.context, claim)
-
-        # call finish on claim2:
-        self.tracker.finish_resource_claim(claim2)
-
-        # should have usage from both instances:
-        self.assertEqual(1 + 2, self.tracker.compute_node['memory_mb_used'])
-        self.assertEqual(1 + 2, self.tracker.compute_node['local_gb_used'])
+        self.assertEqual({}, self.tracker.claims)
 
     def testInstanceClaim(self):
         instance = self._fake_instance(memory_mb=1, root_gb=0, ephemeral_gb=2)