From fb1a67fe6bc919002e9a33aaf73a973fc48b7828 Mon Sep 17 00:00:00 2001
From: Zhenguo Niu <niuzhenguo@huawei.com>
Date: Fri, 16 Oct 2015 17:18:12 +0800
Subject: [PATCH] Move hash_ring refresh logic out of sync_local_state
As sync_local_state periodic task can be disabled, which will lose
out on hash ring refresh, so the refresh logic should be independent
of it. this patch adds a updated_at to HashRingManager to track of
when the ring was last refreshed, and when checks the ring is set,
also check the age and rebuild if older than the new config option
hash_ring_reset_interval which is default to 180s.
Change-Id: Ie46dbf93b920543f99e11774a29878aaf27c3400
Closes-Bug: #1506657
---
etc/ironic/ironic.conf.sample | 4 ++++
ironic/common/hash_ring.py | 12 ++++++++++--
ironic/conductor/manager.py | 1 -
ironic/tests/unit/common/test_hash_ring.py | 12 ++++++++++++
ironic/tests/unit/conductor/test_manager.py | 2 --
5 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample
index c1362dbf5a..5415fabdf4 100644
--- a/etc/ironic/ironic.conf.sample
+++ b/etc/ironic/ironic.conf.sample
@@ -67,6 +67,10 @@
# (integer value)
#hash_distribution_replicas=1
+# Interval (in seconds) between hash ring resets. (integer
+# value)
+#hash_ring_reset_interval=180
+
#
# Options defined in ironic.common.images
diff --git a/ironic/common/hash_ring.py b/ironic/common/hash_ring.py
index b345251caa..baeb957432 100644
--- a/ironic/common/hash_ring.py
+++ b/ironic/common/hash_ring.py
@@ -16,6 +16,7 @@
import bisect
import hashlib
import threading
+import time
from oslo_config import cfg
import six
@@ -47,6 +48,9 @@ hash_opts = [
'conductor services to prepare deployment environments '
'and potentially allow the Ironic cluster to recover '
'more quickly if a conductor instance is terminated.')),
+ cfg.IntOpt('hash_ring_reset_interval',
+ default=180,
+ help=_('Interval (in seconds) between hash ring resets.')),
]
CONF = cfg.CONF
@@ -166,17 +170,21 @@ class HashRingManager(object):
def __init__(self):
self.dbapi = dbapi.get_instance()
+ self.updated_at = time.time()
@property
def ring(self):
+ interval = CONF.hash_ring_reset_interval
+ limit = time.time() - interval
# Hot path, no lock
- if self._hash_rings is not None:
+ if self._hash_rings is not None and self.updated_at >= limit:
return self._hash_rings
with self._lock:
- if self._hash_rings is None:
+ if self._hash_rings is None or self.updated_at < limit:
rings = self._load_hash_rings()
self.__class__._hash_rings = rings
+ self.updated_at = time.time()
return self._hash_rings
def _load_hash_rings(self):
diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 0f01ec5baf..9312ea8e2d 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -1406,7 +1406,6 @@ class ConductorManager(periodic_task.PeriodicTasks):
The ensuing actions could include preparing a PXE environment,
updating the DHCP server, and so on.
"""
- self.ring_manager.reset()
filters = {'reserved': False,
'maintenance': False,
'provision_state': states.ACTIVE}
diff --git a/ironic/tests/unit/common/test_hash_ring.py b/ironic/tests/unit/common/test_hash_ring.py
index 832c975b28..b5a36c2d9a 100644
--- a/ironic/tests/unit/common/test_hash_ring.py
+++ b/ironic/tests/unit/common/test_hash_ring.py
@@ -14,6 +14,7 @@
# under the License.
import hashlib
+import time
import mock
from oslo_config import cfg
@@ -249,3 +250,14 @@ class HashRingManagerTestCase(db_base.DbTestCase):
self.assertRaises(exception.DriverNotFound,
self.ring_manager.__getitem__,
'driver1')
+
+ def test_hash_ring_manager_refresh(self):
+ CONF.set_override('hash_ring_reset_interval', 30)
+ # Initialize the ring manager to make _hash_rings not None, then
+ # hash ring will refresh only when time interval exceeded.
+ self.assertRaises(exception.DriverNotFound,
+ self.ring_manager.__getitem__,
+ 'driver1')
+ self.register_conductors()
+ self.ring_manager.updated_at = time.time() - 30
+ self.ring_manager.__getitem__('driver1')
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index f86026bb30..942249fd45 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -3830,7 +3830,6 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
self._assert_get_nodeinfo_args(get_nodeinfo_mock)
mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver)
self.assertFalse(acquire_mock.called)
- self.service.ring_manager.reset.assert_called_once_with()
def test_already_mapped(self, get_nodeinfo_mock, mapped_mock,
acquire_mock):
@@ -3846,7 +3845,6 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
self._assert_get_nodeinfo_args(get_nodeinfo_mock)
mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver)
self.assertFalse(acquire_mock.called)
- self.service.ring_manager.reset.assert_called_once_with()
def test_good(self, get_nodeinfo_mock, mapped_mock, acquire_mock):
get_nodeinfo_mock.return_value = self._get_nodeinfo_list_response()