[refactor][queue-service] Lua Script + INCR 기반 tie-breaker 도입#36
Merged
Conversation
- enqueue.lua / delete.lua: SETNX + ZADD + HSET 을 단일 Lua 스크립트로 통합 (원자성) - score 생성: epoch_milli * 1000000 + INCR 시퀀스 (동시 진입 FIFO 보장) - RedisScriptConfig: DefaultRedisScript 빈 등록 - RedisQueueTokenRepository: Lua 스크립트 기반 enqueue/delete 정정 - deleteAllByProgramId: 역인덱스 compare-and-delete (동시 발급 토큰 보호) + seq 키 정리 Closes #9, #11
7 tasks
📝 WalkthroughWalkthroughenqueue/delete/deleteAllByProgramId를 Redis Lua 스크립트로 전환하고 스크립트를 Spring 빈으로 등록해 엔큐·삭제·프로그램 전체 삭제를 Redis에서 원자적으로 처리하도록 변경했다. ChangesLua 스크립트 기반 원자 처리 통합
Sequence Diagram(s)sequenceDiagram
participant Client
participant Repository
participant RedisTemplate
participant Redis
rect rgba(100, 150, 255, 0.5)
Note over Client,Redis: Enqueue Flow
Client->>Repository: enqueue(tokenId, userId, programId)
Repository->>RedisTemplate: execute(enqueueScript, keys, argv)
RedisTemplate->>Redis: Lua script execution (SET NX + INCR + ZADD + HSET + EXPIRE + SADD)
alt duplicate token
Redis-->>RedisTemplate: 0
RedisTemplate-->>Repository: 0
Repository->>Client: DuplicateTokenException
else first entry
Redis-->>RedisTemplate: 1
RedisTemplate-->>Repository: 1
Repository-->>Client: queued
end
end
rect rgba(150, 100, 255, 0.5)
Note over Client,Redis: Delete Flow
Client->>Repository: delete(tokenId, userId, programId)
Repository->>RedisTemplate: execute(deleteScript, keys, argv)
RedisTemplate->>Redis: Lua script execution (GET compare + DEL userProgramKey if match + ZREM + DEL tokenHash + SREM programTokens)
Redis-->>RedisTemplate: 1 or 2
RedisTemplate-->>Repository: deletion result
Repository-->>Client: deletion completed
end
rect rgba(200, 200, 100, 0.5)
Note over Client,Redis: DeleteAllByProgram Flow
Client->>Repository: deleteAllByProgramId(programId)
Repository->>RedisTemplate: execute(deleteAllByProgramScript, keys, argv)
RedisTemplate->>Redis: Lua script execution (SMEMBERS programTokens -> per-token HGET/GET/DEL -> DEL programKey/seqKey/programTokensKey)
Redis-->>RedisTemplate: processedCount
RedisTemplate-->>Repository: processedCount
Repository-->>Client: processedCount
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/firstticket/queueservice/queuetoken/infrastructure/redis/RedisQueueTokenRepository.java`:
- Around line 329-334: In RedisQueueTokenRepository, the current
read-then-batch-delete flow that builds userProgramKeysToDelete by comparing
tokenKey.substring(tokenKeyPrefix.length()) to
redisTemplate.opsForValue().get(userProgramKey) is vulnerable to TOCTOU; replace
the separate comparison + later DEL with a Lua compare-and-delete executed at
deletion time (use redisTemplate.execute or script execution) that takes
userProgramKey and expected tokenIdStr and only deletes if the stored value
equals the expected tokenIdStr, ensuring atomicity for the check-and-delete
operation for tokenKey, tokenKeyPrefix, userProgramKey, and
userProgramKeysToDelete handling.
In `@src/main/resources/lua/enqueue.lua`:
- Around line 47-50: The score calculation uses issuedAtEpochMilli * 1000000 +
seq which overflows double precision and zeroes out small seq values; change the
score to use seconds instead of milliseconds by computing issuedAtEpochSeconds =
math.floor(issuedAtEpochMilli / 1000) and then score = issuedAtEpochSeconds *
1000000 + seq (update the lines that compute seq and score around seqKey,
ttlSeconds, issuedAtEpochMilli), so the base is smaller and the seq tie-breaker
is preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: first-ticket/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d33db9f-7f88-464c-ad0d-9cd206c8d309
📒 Files selected for processing (4)
src/main/java/com/firstticket/queueservice/queuetoken/infrastructure/redis/RedisQueueTokenRepository.javasrc/main/java/com/firstticket/queueservice/queuetoken/infrastructure/redis/RedisScriptConfig.javasrc/main/resources/lua/delete.luasrc/main/resources/lua/enqueue.lua
straycat405
approved these changes
May 19, 2026
- enqueue.lua: score 생성 시 epoch_milli → epoch_second 정정 double 정밀도 손실 (ULP=256) 로 같은 ms 안에 128 명 이상 enqueue 시 seq 손실 → FIFO 깨지는 본문 정정 - deleteAllByProgram.lua 신규: SCAN + compare-and-delete + 일괄 정리를 단일 Lua 로 통합 TOCTOU 안전 보장 (역인덱스 삭제 시 새 토큰 보호) Java 측 SCAN 제거 - RedisScriptConfig: deleteAllByProgramScript 빈 등록 - RedisQueueTokenRepository.deleteAllByProgramId 정정: Lua 호출로 단일화 Related to #11
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/firstticket/queueservice/queuetoken/infrastructure/redis/RedisQueueTokenRepository.java`:
- Around line 82-91: Wrap all direct calls to redisTemplate.execute (used with
enqueueScript in enqueue, the delete script in delete, and the delete-all script
in deleteAllByProgramId) in try-catch blocks that catch RedisSystemException
(and broader DataAccessException/RuntimeException as needed) and rethrow a
repository-layer/domain-friendly exception (e.g., QueueTokenRepositoryException
or InfrastructureException) with contextual message (including which operation:
enqueue/delete/deleteAllByProgramId) and the original exception as the cause;
ensure this same conversion policy is applied to the three code sites referenced
and keep the thrown exception type consistent across the repository so
domain/application layers never see raw Redis exceptions.
- Around line 56-58: The constructor injection in RedisQueueTokenRepository uses
three parameters of the same type DefaultRedisScript<Long> (enqueueScript,
deleteScript, deleteAllByProgramScript) which can cause
NoUniqueBeanDefinitionException because parameter names are not preserved; fix
by adding explicit `@Qualifier` annotations to the corresponding constructor
parameters (or on the fields) referencing the bean names for enqueueScript,
deleteScript and deleteAllByProgramScript in the RedisQueueTokenRepository
constructor so Spring can disambiguate, or alternatively enable the Java
compiler -parameters option in the build configuration to preserve parameter
names.
In `@src/main/resources/lua/deleteAllByProgram.lua`:
- Around line 36-66: The Lua script currently scans and deletes all matching
token keys in one run (using SCAN, ZREM, DEL, cursor and processedCount), which
can block Redis; change it to a batched, non-blocking approach: iterate only a
limited number of items per invocation (e.g., add a maxBatch param and stop the
repeat loop when processedCount >= maxBatch), or switch to targeting the
program-specific sorted set (programKey) and delete members in small slices
(ZRANGE/ZPOP or ZRANGE with LIMIT) instead of scanning the entire keyspace;
preserve the compare-and-delete logic using userProgramKeyPrefix + userId +
programKeyInfix + programIdStr and keep deleting tokenKey and removing
tokenIdStr from programKey, but ensure each run only processes up to the batch
limit and returns the cursor/remaining flag so the caller can re-invoke until
completion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: first-ticket/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 82db390a-c377-453c-84bb-272a24af9b70
📒 Files selected for processing (4)
src/main/java/com/firstticket/queueservice/queuetoken/infrastructure/redis/RedisQueueTokenRepository.javasrc/main/java/com/firstticket/queueservice/queuetoken/infrastructure/redis/RedisScriptConfig.javasrc/main/resources/lua/deleteAllByProgram.luasrc/main/resources/lua/enqueue.lua
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/resources/lua/enqueue.lua
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/resources/lua/deleteAllByProgram.lua (1)
36-57:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift대량 토큰에서 단일 Lua 실행 블로킹 위험이 남아 있습니다.
Line 36의
SMEMBERS전체 로드와 Line 40-57의 전량 순회 삭제를 한 번에 수행하면, 프로그램별 토큰이 큰 경우 Redis 이벤트 루프를 오래 점유할 수 있습니다.maxBatch(예: ARGV) 기반으로 건수를 제한하고, 남은 작업은 호출 측에서 반복 실행하도록 쪼개는 방식이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/lua/deleteAllByProgram.lua` around lines 36 - 57, The current logic loads all token IDs with SMEMBERS into tokenIds and iterates them (tokenIds, processedCount, tokenKeyPrefix, userProgramKeyPrefix, programKeyInfix, programIdStr), which can block Redis for large sets; change the Lua to read a max batch only (pass maxBatch and a cursor via ARGV), replace the SMEMBERS call with an SSCAN-based fetch using the supplied cursor and COUNT=maxBatch, iterate and delete only the returned members (maintain the same per-token actions: check HGET on tokenKey, compare GET on userProgramKey and DEL where needed, DEL tokenKey), increment processedCount for only those processed, and return the new cursor plus processedCount so the caller can repeat until the cursor is 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/main/resources/lua/deleteAllByProgram.lua`:
- Around line 36-57: The current logic loads all token IDs with SMEMBERS into
tokenIds and iterates them (tokenIds, processedCount, tokenKeyPrefix,
userProgramKeyPrefix, programKeyInfix, programIdStr), which can block Redis for
large sets; change the Lua to read a max batch only (pass maxBatch and a cursor
via ARGV), replace the SMEMBERS call with an SSCAN-based fetch using the
supplied cursor and COUNT=maxBatch, iterate and delete only the returned members
(maintain the same per-token actions: check HGET on tokenKey, compare GET on
userProgramKey and DEL where needed, DEL tokenKey), increment processedCount for
only those processed, and return the new cursor plus processedCount so the
caller can repeat until the cursor is 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: first-ticket/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 219d1798-6d1f-40b3-923f-e1e041e97041
📒 Files selected for processing (4)
src/main/java/com/firstticket/queueservice/queuetoken/infrastructure/redis/RedisQueueTokenRepository.javasrc/main/resources/lua/delete.luasrc/main/resources/lua/deleteAllByProgram.luasrc/main/resources/lua/enqueue.lua
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/resources/lua/enqueue.lua
- src/main/java/com/firstticket/queueservice/queuetoken/infrastructure/redis/RedisQueueTokenRepository.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🌱 설명
Redis 자료구조의 원자성 강화 및 동시 진입 FIFO 보장을 위한 리팩토링
MULTI/EXEC기반enqueue/delete로직을 Lua Script 로 전환하여 단일 호출의 원자성 보장deleteAllByProgramId에 compare-and-delete 적용 (동시 발급된 토큰의 역인덱스 보호)📌 관련 이슈
💻 커밋 유형
📝 체크리스트
📚 추가 설명
Summary by CodeRabbit
릴리스 노트