minor ping/keepalive tweaks

1. we do no use lock for the ping spdk client since this client is used by a single thread (keap_alive). no need to support configurable lock ping mode 2. clean up usage of ping spdk client during initialization 3. mv keap alive variables initialization from __init__ to keep_alive. Signed-off-by: Alexander Indenbaum <[email protected]>
ceph · Jul 30, 2024 · 16d306a · 16d306a
1 parent c348cab
commit 16d306a
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 27 deletions.
diff --git a/ceph-nvmeof.conf b/ceph-nvmeof.conf
@@ -29,7 +29,6 @@ enable_spdk_discovery_controller = False
 #verify_nqns = True
 #allowed_consecutive_spdk_ping_failures = 1
 #spdk_ping_interval_in_seconds = 2.0
-#ping_spdk_under_lock = False
 
 [gateway-logs]
 log_level=debug

diff --git a/control/server.py b/control/server.py
@@ -90,18 +90,6 @@ def __init__(self, config: GatewayConfig):
             self.name = socket.gethostname()
         self.logger.info(f"Starting gateway {self.name}")
 
-        self.allowed_consecutive_spdk_ping_failures = self.config.getint_with_default("gateway",
-                                                                                      "allowed_consecutive_spdk_ping_failures", 1)
-        self.spdk_ping_interval_in_seconds = self.config.getfloat_with_default("gateway", "spdk_ping_interval_in_seconds", 2.0)
-        if self.spdk_ping_interval_in_seconds < 0.0:
-            self.logger.warning(f"Invalid SPDK ping interval {self.spdk_ping_interval_in_seconds}, will reset to 0")
-            self.spdk_ping_interval_in_seconds = 0.0
-        self.ping_spdk_under_lock = self.config.getboolean_with_default("gateway", "ping_spdk_under_lock", False)
-        if self.ping_spdk_under_lock:
-            self.ping_lock = self.rpc_lock
-        else:
-            self.ping_lock = contextlib.suppress()
-
     def __enter__(self):
         return self
 
@@ -268,7 +256,7 @@ def _start_discovery_service(self):
             return
 
         try:
-            rpc_nvmf.nvmf_delete_subsystem(self.spdk_rpc_ping_client, GatewayUtils.DISCOVERY_NQN)
+            rpc_nvmf.nvmf_delete_subsystem(self.spdk_rpc_client, GatewayUtils.DISCOVERY_NQN)
         except Exception:
             self.logger.exception(f"Delete Discovery subsystem returned with error")
             raise
@@ -422,17 +410,16 @@ def _start_spdk(self, omap_state):
         for trtype in spdk_transports.split():
             self._create_transport(trtype.lower())
 
-        with self.ping_lock:
+        try:
+            return_version = spdk.rpc.spdk_get_version(self.spdk_rpc_client)
             try:
-                return_version = spdk.rpc.spdk_get_version(self.spdk_rpc_ping_client)
-                try:
-                    version_string = return_version["version"]
-                    self.logger.info(f"Started SPDK with version \"{version_string}\"")
-                except KeyError:
-                    self.logger.error(f"Can't find SPDK version string in {return_version}")
-            except Exception:
-                self.logger.exception(f"Can't read SPDK version")
-                pass
+                version_string = return_version["version"]
+                self.logger.info(f"Started SPDK with version \"{version_string}\"")
+            except KeyError:
+                self.logger.error(f"Can't find SPDK version string in {return_version}")
+        except Exception:
+            self.logger.exception(f"Can't read SPDK version")
+            pass
 
     def _stop_subprocess(self, proc, timeout):
         """Stops SPDK process."""
@@ -514,6 +501,12 @@ def _create_transport(self, trtype):
 
     def keep_alive(self):
         """Continuously confirms communication with SPDK process."""
+        self.allowed_consecutive_spdk_ping_failures = self.config.getint_with_default("gateway",
+                                                                                      "allowed_consecutive_spdk_ping_failures", 1)
+        self.spdk_ping_interval_in_seconds = self.config.getfloat_with_default("gateway", "spdk_ping_interval_in_seconds", 2.0)
+        if self.spdk_ping_interval_in_seconds < 0.0:
+            self.logger.warning(f"Invalid SPDK ping interval {self.spdk_ping_interval_in_seconds}, will reset to 0")
+            self.spdk_ping_interval_in_seconds = 0.0
 
         consecutive_ping_failures = 0
         # we spend 1 second waiting for server termination so subtract it from ping interval
@@ -540,8 +533,7 @@ def keep_alive(self):
     def _ping(self):
         """Confirms communication with SPDK process."""
         try:
-            with self.ping_lock:
-                ret = spdk.rpc.spdk_get_version(self.spdk_rpc_ping_client)
+            ret = spdk.rpc.spdk_get_version(self.spdk_rpc_ping_client)
             return True
         except Exception:
             self.logger.exception(f"spdk_get_version failed")

diff --git a/tests/ceph-nvmeof.tls.conf b/tests/ceph-nvmeof.tls.conf
@@ -28,7 +28,6 @@ enable_spdk_discovery_controller = False
 #verify_nqns = True
 #allowed_consecutive_spdk_ping_failures = 1
 #spdk_ping_interval_in_seconds = 2.0
-#ping_spdk_under_lock = False
 
 [gateway-logs]
 log_level=debug