openstack/neutron
Code Issues Proposed changes

[OVN] Rate limit the "Disallow caching" log from hash ring

Browse Source 
This patch adds a condition prior to logging the "Disallow caching" log
message from the hash ring.

Prior to this patch, this message was logged when the number of nodes
connected to the hash ring was different from the number of API workers
at Neutron's startup. This is because the hash ring waits until all API
workers are connected to build the hash ring cache.

With this patch, we will only log the message once (per worker) until
the number of connected nodes changes. When nodes connect and the cache
is built the _wait_startup_before_caching() is no longer used until the
service is restarted again.

Change-Id: I4f62b723083215483a2277cfcb798506671e1a2d
Closes-Bug: 1989480
Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
This commit is contained in:
Lucas Alvares Gomes
2022-09-13 16:11:37 +01:00
parent 0a6b9cc395
commit 9655466763
2 changed files with 22 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
neutron/common/ovn/hash_ring_manager.py
View File

@@ -35,6 +35,8 @@ class HashRingManager(object):
self._last_time_loaded = None
self._check_hashring_startup = True
self._group = group_name
# Flag to rate limit the caching log
self._prev_num_nodes = -1
self.admin_ctx = context.get_admin_context()
@property
@@ -56,11 +58,18 @@ class HashRingManager(object):
self.admin_ctx,
constants.HASH_RING_CACHE_TIMEOUT, self._group, from_host=True)
if len(nodes) >= api_workers:
LOG.debug("Allow caching, nodes %s>=%s", len(nodes), api_workers)
num_nodes = len(nodes)
if num_nodes >= api_workers:
LOG.debug("Allow caching, nodes %s>=%s", num_nodes, api_workers)
self._check_hashring_startup = False
return False
LOG.debug("Disallow caching, nodes %s<%s", len(nodes), api_workers)
# NOTE(lucasagomes): We only log when the number of connected
# nodes are different to prevent this message from being spammed
if self._prev_num_nodes != num_nodes:
LOG.debug("Disallow caching, nodes %s<%s", num_nodes, api_workers)
self._prev_num_nodes = num_nodes
return True
def _load_hash_ring(self, refresh=False):

11
neutron/tests/unit/common/ovn/test_hash_ring_manager.py
View File

@@ -110,14 +110,23 @@ class TestHashRingManager(testlib_api.SqlTestCaseLight):
# The ring should re-balance and as it was before
self._verify_hashes(hash_dict_before)
@mock.patch.object(hash_ring_manager.LOG, 'debug')
@mock.patch.object(service, '_get_api_workers', return_value=2)
def test__wait_startup_before_caching(self, api_workers):
def test__wait_startup_before_caching(self, api_workers, mock_log):
db_hash_ring.add_node(self.admin_ctx, HASH_RING_TEST_GROUP, 'node-1')
# Assert it will return True until until we equal api_workers
self.assertTrue(self.hash_ring_manager._wait_startup_before_caching)
self.assertTrue(self.hash_ring_manager._check_hashring_startup)
# Call it again with the same number of workers to test the
# log rating
self.assertTrue(self.hash_ring_manager._wait_startup_before_caching)
self.assertTrue(self.hash_ring_manager._check_hashring_startup)
# Assert the cache message has been logged only once
self.assertEqual(1, mock_log.call_count)
db_hash_ring.add_node(self.admin_ctx, HASH_RING_TEST_GROUP, 'node-2')
# Assert it's now False. Waiting is not needed anymore