mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix: add Lua fallback on execution error + test coverage gaps
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>
This commit is contained in:
parent
cd004398a7
commit
badd8c83ae
2 changed files with 59 additions and 7 deletions
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue