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

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 <[email protected]>
(cherry picked from commit 9655466)

Author: umago

neutron/common/ovn/hash_ring_manager.py

@@ -35,6 +35,8 @@ def __init__(self, group_name):
         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 @@ def _wait_startup_before_caching(self):
             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):

neutron/tests/unit/common/ovn/test_hash_ring_manager.py

@@ -110,14 +110,23 @@ def test_ring_rebalance(self):
         # 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