From cb118d04cea318df23831d7445f6b33c866cd0c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Herv=C3=A9=20Beraud?= <hberaud@redhat.com>
Date: Mon, 17 Jan 2022 14:59:25 +0100
Subject: [PATCH] Expose pymemcache's HashClient public params

This patch expose a couple of pymemcache's HashClient public
params that can be useful to configure HA and failover for
clustered memcached servers.

These options can be used in addition of the previously added
retrying mechanismes.

This patch rely on recent changes [1] of dogpile.cache that
aim to expose these options too.

[1] https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/3528

Change-Id: I24fc853db4237c08b87871ddff1b3ced27cc7fce
---
 oslo_cache/_opts.py                           | 17 ++++++++
 oslo_cache/core.py                            |  6 +++
 oslo_cache/tests/unit/test_cache_basics.py    | 43 +++++++++++++++++++
 ...hashclient_configure-f6f48c5ca38bce47.yaml |  8 ++++
 requirements.txt                              |  2 +-
 5 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/pymemcache_hashclient_configure-f6f48c5ca38bce47.yaml

diff --git a/oslo_cache/_opts.py b/oslo_cache/_opts.py
index 0137c815..2f4d4097 100644
--- a/oslo_cache/_opts.py
+++ b/oslo_cache/_opts.py
@@ -188,6 +188,23 @@ FILE_OPTIONS = {
             'retry_delay',
             default=0,
             help='Number of seconds to sleep between each attempt.'),
+        cfg.IntOpt(
+            'hashclient_retry_attempts',
+            min=1,
+            default=2,
+            help='Amount of times a client should be tried '
+            'before it is marked dead and removed from the pool in '
+            'the HashClient\'s internal mechanisms.'),
+        cfg.FloatOpt(
+            'hashclient_retry_delay',
+            default=1,
+            help='Time in seconds that should pass between '
+            'retry attempts in the HashClient\'s internal mechanisms.'),
+        cfg.FloatOpt(
+            'dead_timeout',
+            default=60,
+            help='Time in seconds before attempting to add a node '
+            'back in the pool in the HashClient\'s internal mechanisms.'),
     ],
 }
 
diff --git a/oslo_cache/core.py b/oslo_cache/core.py
index f27ee1df..9d26a487 100644
--- a/oslo_cache/core.py
+++ b/oslo_cache/core.py
@@ -229,6 +229,12 @@ def _build_cache_config(conf):
             conf.cache.retry_attempts
         conf_dict['%s.arguments.retry_delay' % prefix] = \
             conf.cache.retry_delay
+        conf_dict['%s.arguments.hashclient_retry_attempts' % prefix] = \
+            conf.cache.hashclient_retry_attempts
+        conf_dict['%s.arguments.hashclient_retry_delay' % prefix] = \
+            conf.cache.hashclient_retry_delay
+        conf_dict['%s.arguments.dead_timeout' % prefix] = \
+            conf.cache.dead_timeout
 
     return conf_dict
 
diff --git a/oslo_cache/tests/unit/test_cache_basics.py b/oslo_cache/tests/unit/test_cache_basics.py
index dfd9b130..ba3a3d1a 100644
--- a/oslo_cache/tests/unit/test_cache_basics.py
+++ b/oslo_cache/tests/unit/test_cache_basics.py
@@ -514,6 +514,49 @@ class CacheRegionTest(test_cache.BaseTestCase):
             42
         )
 
+    def test_cache_pymemcache_retry_with_extra_opts(self):
+        """Validate we build a valid config for the retry client."""
+        self.config_fixture.config(group='cache',
+                                   enabled=True,
+                                   config_prefix='test_prefix',
+                                   backend='dogpile.cache.pymemcache',
+                                   enable_retry_client=True,
+                                   retry_attempts=42,
+                                   retry_delay=42,
+                                   hashclient_retry_attempts=100,
+                                   hashclient_retry_delay=100,
+                                   dead_timeout=100)
+
+        config_dict = cache._build_cache_config(self.config_fixture.conf)
+
+        self.assertTrue(
+            self.config_fixture.conf.cache.enable_retry_client)
+
+        self.assertEqual(
+            config_dict['test_prefix.arguments.retry_attempts'],
+            42
+        )
+
+        self.assertEqual(
+            config_dict['test_prefix.arguments.retry_delay'],
+            42
+        )
+
+        self.assertEqual(
+            config_dict['test_prefix.arguments.hashclient_retry_attempts'],
+            100
+        )
+
+        self.assertEqual(
+            config_dict['test_prefix.arguments.hashclient_retry_delay'],
+            100
+        )
+
+        self.assertEqual(
+            config_dict['test_prefix.arguments.dead_timeout'],
+            100
+        )
+
     def test_cache_dictionary_config_builder_flush_on_reconnect_enabled(self):
         """Validate we build a sane dogpile.cache dictionary config."""
         self.config_fixture.config(group='cache',
diff --git a/releasenotes/notes/pymemcache_hashclient_configure-f6f48c5ca38bce47.yaml b/releasenotes/notes/pymemcache_hashclient_configure-f6f48c5ca38bce47.yaml
new file mode 100644
index 00000000..a4cd21f1
--- /dev/null
+++ b/releasenotes/notes/pymemcache_hashclient_configure-f6f48c5ca38bce47.yaml
@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Add new options (``hashclient_retry_attempts``, ``hashclient_retry_delay``,
+    ``dead_timeout``) to allow to configure pymemcache's HashClient use
+    through dogpile.cache's pymemcache backend. Those options expose
+    pymemcache params that allow to configure the failover for memcached
+    cluster.
diff --git a/requirements.txt b/requirements.txt
index 4b7f67c2..3b49fadc 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -2,7 +2,7 @@
 # of appearance. Changing the order has an impact on the overall integration
 # process, which may cause wedges in the gate later.
 
-dogpile.cache>=1.1.4 # BSD
+dogpile.cache>=1.1.5 # BSD
 oslo.config>=8.1.0 # Apache-2.0
 oslo.i18n>=5.0.0 # Apache-2.0
 oslo.log>=4.2.1 # Apache-2.0