Skip to content

Commit fd3abb3

Browse files
committed
reproducing the problem
1 parent f711be9 commit fd3abb3

File tree

2 files changed

+34
-10
lines changed

2 files changed

+34
-10
lines changed

packages/service-library/src/servicelib/redis.py

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ class CouldNotConnectToRedisError(BaseRedisError):
4949
msg_template: str = "Connection to '{dsn}' failed"
5050

5151

52+
class LockLostError(BaseRedisError):
53+
msg_template: str = "Lock {lock.name} has been lost"
54+
55+
5256
async def _cancel_or_warn(task: Task) -> None:
5357
if not task.cancelled():
5458
task.cancel()
@@ -179,13 +183,16 @@ async def lock_context(
179183
raise CouldNotAcquireLockError(lock=ttl_lock)
180184

181185
async def _extend_lock(lock: Lock) -> None:
182-
with (
183-
log_context(_logger, logging.DEBUG, f"Extending lock {lock_unique_id}"),
184-
log_catch(_logger, reraise=False),
185-
):
186-
# TODO: if we cannot re-acquire that means the lock is lost, and we are not anymore safe and should raise all the way to the caller
187-
188-
await lock.reacquire()
186+
try:
187+
with (
188+
log_context(
189+
_logger, logging.DEBUG, f"Extending lock {lock_unique_id}"
190+
),
191+
):
192+
# TODO: if we cannot re-acquire that means the lock is lost, and we are not anymore safe and should raise all the way to the caller
193+
await lock.reacquire()
194+
except redis.exceptions.LockNotOwnedError as exc:
195+
raise LockLostError(lock=lock) from exc
189196

190197
try:
191198
async with periodic_task(

packages/service-library/tests/test_redis.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from servicelib import redis as servicelib_redis
1818
from servicelib.redis import (
1919
CouldNotAcquireLockError,
20+
LockLostError,
2021
RedisClientSDK,
2122
RedisClientsManager,
2223
RedisManagerDBConfig,
@@ -35,7 +36,7 @@
3536
]
3637

3738
pytest_simcore_ops_services_selection = [
38-
# "redis-commander",
39+
"redis-commander",
3940
]
4041

4142

@@ -48,7 +49,7 @@ async def _is_locked(redis_client_sdk: RedisClientSDK, lock_name: str) -> bool:
4849
async def redis_client_sdk(
4950
get_redis_client_sdk: Callable[
5051
[RedisDatabase], AbstractAsyncContextManager[RedisClientSDK]
51-
]
52+
],
5253
) -> AsyncIterator[RedisClientSDK]:
5354
async with get_redis_client_sdk(RedisDatabase.RESOURCES) as client:
5455
yield client
@@ -165,6 +166,23 @@ async def test_lock_context(
165166
assert await ttl_lock.owned() is False
166167

167168

169+
async def test_issue_cannot_acquire_lock_anymore(
170+
redis_client_sdk: RedisClientSDK, faker: Faker
171+
):
172+
lock_name = faker.pystr()
173+
with pytest.raises(LockLostError):
174+
async with redis_client_sdk.lock_context(lock_name) as ttl_lock:
175+
assert await _is_locked(redis_client_sdk, lock_name) is True
176+
assert await ttl_lock.owned() is True
177+
# let's force release the lock
178+
await redis_client_sdk._client.delete(lock_name)
179+
180+
await asyncio.sleep(20)
181+
# assert await _is_locked(redis_client_sdk, lock_name) is False
182+
# assert await ttl_lock.owned() is False
183+
# actually it should even raise here
184+
185+
168186
async def test_lock_context_with_already_locked_lock_raises(
169187
redis_client_sdk: RedisClientSDK, faker: Faker
170188
):
@@ -272,7 +290,6 @@ async def _inc_counter() -> None:
272290
async def test_redis_client_sdks_manager(
273291
mock_redis_socket_timeout: None, redis_service: RedisSettings
274292
):
275-
276293
all_redis_configs: set[RedisManagerDBConfig] = {
277294
RedisManagerDBConfig(db) for db in RedisDatabase
278295
}

0 commit comments

Comments
 (0)