From badd8c83aed9cf6de2c993e03e48c7b2ce87d1bb Mon Sep 17 00:00:00 2001
From: joereyna <joseph.reyna@gmail.com>
Date: Mon, 23 Mar 2026 22:49:32 -0700
Subject: [PATCH] fix: add Lua fallback on execution error + test coverage gaps
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address Greptile review feedback on #24466:

1. Wrap Lua script execution in try/except — if Redis clears loaded
   scripts (restart) or scripting is disabled, fall back to GET+DEL
   rather than letting the exception propagate and leave the lock held
   until TTL. Reset cached script handle so the next call re-registers.

2. Add test_release_lock_lua_path_emits_released_event — verifies
   _emit_released_lock_event is called when Lua path returns 1.

3. Add test_release_lock_falls_back_to_get_del_when_lua_execution_fails
   — verifies the fallback path is taken and script handle is reset.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../db_transaction_queue/pod_lock_manager.py  | 24 +++++++----
 .../test_pod_lock_manager.py                  | 42 +++++++++++++++++++
 2 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/litellm/proxy/db/db_transaction_queue/pod_lock_manager.py b/litellm/proxy/db/db_transaction_queue/pod_lock_manager.py
index ef1805e4af0..5e0ddef9eaa 100644
--- a/litellm/proxy/db/db_transaction_queue/pod_lock_manager.py
+++ b/litellm/proxy/db/db_transaction_queue/pod_lock_manager.py
@@ -162,14 +162,24 @@ end
         """
         script_register = getattr(self.redis_cache, "async_register_script", None)
         if callable(script_register):
-            if self._release_lock_script is None:
-                self._release_lock_script = script_register(
-                    self._COMPARE_AND_DELETE_LOCK_SCRIPT
+            try:
+                if self._release_lock_script is None:
+                    self._release_lock_script = script_register(
+                        self._COMPARE_AND_DELETE_LOCK_SCRIPT
+                    )
+                result = await self._release_lock_script(
+                    keys=[lock_key], args=[self.pod_id]
+                )
+                return int(result or 0)
+            except Exception:
+                # Lua execution failed (e.g. Redis restart cleared loaded scripts,
+                # or scripting is disabled). Reset cached script handle and fall
+                # through to the GET + DEL fallback so the lock is still released.
+                self._release_lock_script = None
+                verbose_proxy_logger.warning(
+                    "Lua compare-and-delete failed for lock_key=%s, falling back to GET+DEL",
+                    lock_key,
                 )
-            result = await self._release_lock_script(
-                keys=[lock_key], args=[self.pod_id]
-            )
-            return int(result or 0)
 
         current_value = await self.redis_cache.async_get_cache(lock_key)  # type: ignore
         if isinstance(current_value, bytes):
diff --git a/tests/test_litellm/proxy/db/db_transaction_queue/test_pod_lock_manager.py b/tests/test_litellm/proxy/db/db_transaction_queue/test_pod_lock_manager.py
index 138bed0e991..eb795fbc012 100644
--- a/tests/test_litellm/proxy/db/db_transaction_queue/test_pod_lock_manager.py
+++ b/tests/test_litellm/proxy/db/db_transaction_queue/test_pod_lock_manager.py
@@ -345,3 +345,45 @@ async def test_release_lock_reuses_registered_script(pod_lock_manager, mock_redi
     await pod_lock_manager.release_lock(cronjob_id="test_job")
 
     assert mock_redis.async_register_script.call_count == 1
+
+
+@pytest.mark.asyncio
+async def test_release_lock_lua_path_emits_released_event(
+    pod_lock_manager, mock_redis
+):
+    """
+    Test that _emit_released_lock_event is called when the Lua path returns 1
+    (successful release).
+    """
+    script_callable = AsyncMock(return_value=1)
+    mock_redis.async_register_script = MagicMock(return_value=script_callable)
+
+    with patch.object(pod_lock_manager, "_emit_released_lock_event") as mock_emit:
+        await pod_lock_manager.release_lock(cronjob_id="test_job")
+
+    mock_emit.assert_called_once_with(
+        cronjob_id="test_job", pod_id=pod_lock_manager.pod_id
+    )
+
+
+@pytest.mark.asyncio
+async def test_release_lock_falls_back_to_get_del_when_lua_execution_fails(
+    pod_lock_manager, mock_redis
+):
+    """
+    Test that release_lock falls back to GET+DEL when Lua script execution
+    raises (e.g. Redis restart cleared loaded scripts).
+    """
+    script_callable = AsyncMock(side_effect=Exception("NOSCRIPT"))
+    mock_redis.async_register_script = MagicMock(return_value=script_callable)
+    mock_redis.async_get_cache.return_value = pod_lock_manager.pod_id
+    mock_redis.async_delete_cache.return_value = 1
+
+    await pod_lock_manager.release_lock(cronjob_id="test_job")
+
+    # Lua failed — should have fallen back to GET+DEL
+    lock_key = pod_lock_manager.get_redis_lock_key(cronjob_id="test_job")
+    mock_redis.async_get_cache.assert_called_once_with(lock_key)
+    mock_redis.async_delete_cache.assert_called_once_with(lock_key)
+    # Cached script handle should be reset so next call re-registers
+    assert pod_lock_manager._release_lock_script is None