From 0602d1a10ac20c48fa35ad711355c79ee5b0ec77 Mon Sep 17 00:00:00 2001
From: Andrew Bogott <abogott@wikimedia.org>
Date: Mon, 5 Dec 2022 09:25:00 -0600
Subject: [PATCH] Increase ACK_REQUEUE_EVERY_SECONDS_MAX to exceed default
 kombu_reconnect_delay

Previously the two values were the same; this caused us
to always exceed the timeout limit ACK_REQUEUE_EVERY_SECONDS_MAX
which results in various code paths never being traversed
due to premature timeout exceptions.

Also apply min/max values to kombu_reconnect_delay so it doesn't
exceed ACK_REQUEUE_EVERY_SECONDS_MAX and break things again.

Closes-Bug: #1993149
Change-Id: I103d2aa79b4bd2c331810583aeca53e22ee27a49
---
 oslo_messaging/_drivers/amqpdriver.py                | 2 +-
 oslo_messaging/_drivers/impl_rabbit.py               | 6 ++++--
 oslo_messaging/tests/test_config_opts_proxy.py       | 2 +-
 releasenotes/notes/bug-1993149-e8b231791b65e938.yaml | 9 +++++++++
 4 files changed, 15 insertions(+), 4 deletions(-)
 create mode 100644 releasenotes/notes/bug-1993149-e8b231791b65e938.yaml

diff --git a/oslo_messaging/_drivers/amqpdriver.py b/oslo_messaging/_drivers/amqpdriver.py
index 991bf46c3..5418c5897 100644
--- a/oslo_messaging/_drivers/amqpdriver.py
+++ b/oslo_messaging/_drivers/amqpdriver.py
@@ -37,7 +37,7 @@ LOG = logging.getLogger(__name__)
 # Maximum should be small enough to not get rejected ack,
 # minimum should be big enough to not burn the CPU.
 ACK_REQUEUE_EVERY_SECONDS_MIN = 0.001
-ACK_REQUEUE_EVERY_SECONDS_MAX = 1.0
+ACK_REQUEUE_EVERY_SECONDS_MAX = 5.0
 
 
 class MessageOperationsHandler(object):
diff --git a/oslo_messaging/_drivers/impl_rabbit.py b/oslo_messaging/_drivers/impl_rabbit.py
index 50e0a66a8..2c351d813 100644
--- a/oslo_messaging/_drivers/impl_rabbit.py
+++ b/oslo_messaging/_drivers/impl_rabbit.py
@@ -106,9 +106,11 @@ rabbit_opts = [
                 ),
     cfg.FloatOpt('kombu_reconnect_delay',
                  default=1.0,
+                 min=0.0,
+                 max=amqpdriver.ACK_REQUEUE_EVERY_SECONDS_MAX * 0.9,
                  deprecated_group='DEFAULT',
-                 help='How long to wait before reconnecting in response to an '
-                      'AMQP consumer cancel notification.'),
+                 help='How long to wait (in seconds) before reconnecting in '
+                      'response to an AMQP consumer cancel notification.'),
     cfg.StrOpt('kombu_compression',
                help="EXPERIMENTAL: Possible values are: gzip, bz2. If not "
                     "set compression will not be used. This option may not "
diff --git a/oslo_messaging/tests/test_config_opts_proxy.py b/oslo_messaging/tests/test_config_opts_proxy.py
index 8b332395e..e9f9421b7 100644
--- a/oslo_messaging/tests/test_config_opts_proxy.py
+++ b/oslo_messaging/tests/test_config_opts_proxy.py
@@ -70,7 +70,7 @@ class TestConfigOptsProxy(test_utils.BaseTestCase):
 
     def test_invalid_value(self):
         group = 'oslo_messaging_rabbit'
-        self.config(kombu_reconnect_delay=5.0,
+        self.config(kombu_reconnect_delay=1.0,
                     group=group)
         url = transport.TransportURL.parse(
             self.conf, "rabbit:///?kombu_reconnect_delay=invalid_value"
diff --git a/releasenotes/notes/bug-1993149-e8b231791b65e938.yaml b/releasenotes/notes/bug-1993149-e8b231791b65e938.yaml
new file mode 100644
index 000000000..f7b9c8fa6
--- /dev/null
+++ b/releasenotes/notes/bug-1993149-e8b231791b65e938.yaml
@@ -0,0 +1,9 @@
+---
+upgrade:
+  - |
+    If kombu_reconnect_delay is specified in the [oslo_messaging_rabbit] section,
+    ensure that it is less than 5.0, the value of ACK_REQUEUE_EVERY_SECONDS_MAX
+fixes:
+  - |
+    Increased ACK_REQUEUE_EVERY_SECONDS_MAX to resolve issues with rabbitmq HA
+    failover.