refactor

Author: sanderegg

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

@@ -40,6 +40,8 @@
     COUNT_SEMAPHORE_SCRIPT,
     RELEASE_SEMAPHORE_SCRIPT,
     RENEW_SEMAPHORE_SCRIPT,
+    SCRIPT_BAD_EXIT_CODE,
+    SCRIPT_OK_EXIT_CODE,
 )
 
 _logger = logging.getLogger(__name__)
@@ -210,11 +212,11 @@ async def release(self) -> None:
         )
 
         assert isinstance(result, list)  # nosec
-        success, status, current_count, expired_count = result
+        exit_code, status, current_count, expired_count = result
         result = status
 
         if result == "released":
-            assert success == 1  # nosec
+            assert exit_code == SCRIPT_BAD_EXIT_CODE  # nosec
             _logger.debug(
                 "Released semaphore '%s' (instance: %s, count: %s, expired: %s)",
                 self.key,
@@ -225,7 +227,7 @@ async def release(self) -> None:
         else:
             # Instance wasn't in the semaphore set - this shouldn't happen
             # but let's handle it gracefully
-            assert success == 0  # nosec
+            assert exit_code == SCRIPT_OK_EXIT_CODE  # nosec
             raise SemaphoreNotAcquiredError(name=self.key)
 
     async def _try_acquire(self) -> bool:
@@ -238,11 +240,11 @@ async def _try_acquire(self) -> bool:
             client=self.redis_client.redis,
         )
 
-        # Lua script returns: [success, status, current_count, expired_count]
+        # Lua script returns: [exit_code, status, current_count, expired_count]
         assert isinstance(result, list)  # nosec
-        success, status, current_count, expired_count = result
+        exit_code, status, current_count, expired_count = result
 
-        if success == 1:
+        if exit_code == SCRIPT_OK_EXIT_CODE:
             _logger.debug(
                 "Acquired semaphore '%s' (instance: %s, count: %s, expired: %s)",
                 self.key,
@@ -285,11 +287,11 @@ async def reacquire(self) -> None:
         )
 
         assert isinstance(result, list)  # nosec
-        success, status, current_count, expired_count = result
+        exit_code, status, current_count, expired_count = result
 
         # Lua script returns: 'renewed' or status message
         if status == "renewed":
-            assert success == 1  # nosec
+            assert exit_code == SCRIPT_OK_EXIT_CODE  # nosec
             _logger.debug(
                 "Renewed semaphore '%s' (instance: %s, count: %s, expired: %s)",
                 self.key,
@@ -298,7 +300,7 @@ async def reacquire(self) -> None:
                 expired_count,
             )
         else:
-            assert success == 0  # nosec
+            assert exit_code == SCRIPT_BAD_EXIT_CODE  # nosec
             if status == "expired":
                 _logger.warning(
                     "Semaphore '%s' holder key expired (instance: %s, count: %s, expired: %s)",

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

@@ -26,3 +26,6 @@ def _load_script(script_name: str) -> str:
 RELEASE_SEMAPHORE_SCRIPT: Final[str] = _load_script("release_semaphore")
 RENEW_SEMAPHORE_SCRIPT: Final[str] = _load_script("renew_semaphore")
 COUNT_SEMAPHORE_SCRIPT: Final[str] = _load_script("count_semaphore")
+
+SCRIPT_BAD_EXIT_CODE: Final[int] = 255
+SCRIPT_OK_EXIT_CODE: Final[int] = 0

packages/service-library/src/servicelib/redis/lua/acquire_semaphore.lua

@@ -5,8 +5,8 @@
 -- ARGV[2]: capacity (max concurrent holders)
 -- ARGV[3]: ttl_seconds
 --
--- Returns: {success, status, current_count, expired_count}
---   success: 1 if acquired, 0 if failed
+-- Returns: {exit_code, status, current_count, expired_count}
+--   exit_code: 0 if acquired, 255 if failed
 --   status: 'acquired' or 'capacity_full'
 --   current_count: number of holders after operation
 --   expired_count: number of expired entries cleaned up
@@ -34,7 +34,7 @@ if current_count < capacity then
     redis.call('ZADD', semaphore_key, current_time, instance_id)
     redis.call('SETEX', holder_key, ttl_seconds, '1')
 
-    return {1, 'acquired', current_count + 1, expired_count}
+    return {0, 'acquired', current_count + 1, expired_count}
 else
-    return {0, 'capacity_full', current_count, expired_count}
+    return {255, 'capacity_full', current_count, expired_count}
 end

packages/service-library/src/servicelib/redis/lua/release_semaphore.lua

@@ -5,7 +5,7 @@
 -- ARGV[2]: ttl_seconds
 --
 -- Returns: {success, status, current_count, expired_count}
---   success: 1 if released, 0 if failed
+--   exit_code: 0 if released, 255 if failed
 --   status: 'released', 'not_held', or 'already_expired'
 --   current_count: number of holders after operation
 --   expired_count: number of expired entries cleaned up
@@ -39,8 +39,8 @@ local removed_holder = redis.call('DEL', holder_key)
 local current_count = redis.call('ZCARD', semaphore_key)
 
 if removed_from_zset == 1 then
-    return {1, 'released', current_count, expired_count}
+    return {0, 'released', current_count, expired_count}
 else
     -- This shouldn't happen since we checked ZSCORE above, but handle it
-    return {0, 'already_expired', current_count, expired_count}
+    return {255, 'already_expired', current_count, expired_count}
 end

packages/service-library/src/servicelib/redis/lua/renew_semaphore.lua

@@ -5,7 +5,7 @@
 -- ARGV[2]: ttl_seconds
 --
 -- Returns: {success, status, current_count, expired_count}
---   success: 1 if renewed, 0 if failed
+--   exit_code: 0 if renewed, 255 if failed
 --   status: 'renewed', 'not_held', or 'expired'
 --   current_count: number of holders after operation
 --   expired_count: number of expired entries cleaned up
@@ -29,7 +29,7 @@ local score = redis.call('ZSCORE', semaphore_key, instance_id)
 if score == false then
     -- Instance doesn't hold the semaphore
     local current_count = redis.call('ZCARD', semaphore_key)
-    return {0, 'not_held', current_count, expired_count}
+    return {255, 'not_held', current_count, expired_count}
 end
 
 -- Step 3: Check if the holder key still exists (not expired)
@@ -38,12 +38,12 @@ if exists == 0 then
     -- Holder key expired, remove from semaphore and fail renewal
     redis.call('ZREM', semaphore_key, instance_id)
     local current_count = redis.call('ZCARD', semaphore_key)
-    return {0, 'expired', current_count, expired_count + 1}
+    return {255, 'expired', current_count, expired_count + 1}
 end
 
 -- Step 4: Renew both the semaphore entry and holder key
 redis.call('ZADD', semaphore_key, current_time, instance_id)
 redis.call('SETEX', holder_key, ttl_seconds, '1')
 
 local current_count = redis.call('ZCARD', semaphore_key)
-return {1, 'renewed', current_count, expired_count}
+return {0, 'renewed', current_count, expired_count}