[volume-8] Redis 기반 주문 대기열 구현 - #364
Merged
Merged
Conversation
- QueueDomainService.estimateWaitSeconds(rank): 순번 / SAFE_TPS로 예상 대기시간(초) 계산
- QueueInfo: 응용 계층 결과 DTO - QueueService: enter/position/validateToken/deleteToken 유스케이스, Fake Repository로 TDD 8개 테스트 - QueueFacade: Controller/Interceptor가 호출할 진입점 (단순 위임)
- QueueRepositoryImpl/EntryTokenRepositoryImpl: redisTemplateMaster 기반 Sorted Set/String+TTL 구현 - QueueServiceIntegrationTest: 실제 Redis 통합 테스트, 동시 진입 시 순번 유일성 검증 - QueueDomainService: @component 누락 수정 (Spring 빈 등록 안 되어 컨텍스트 로딩 실패하던 버그) - QueueController: POST /api/v1/queue/enter, GET /api/v1/queue/position API + E2E 테스트 4개
- QueueEntryScheduler: 100ms마다 ZPOPMIN으로 배치 크기(14명)만큼 토큰 발급 - 스케줄러 주기를 프로퍼티화(queue.scheduler.interval-ms)하고 test 프로파일에서만 비활성화 → 실제 백그라운드 스케줄러가 테스트 도중 대기열을 미리 비워버리던 문제 해결 - QueueTokenInterceptor: POST /api/v1/orders에 X-Entry-Token 검증, 성공 시(2xx) 토큰 삭제 - WebMvcConfig: LoginInterceptor 뒤에 QueueTokenInterceptor 등록 - OrderApiE2ETest: 기존 주문 생성 테스트에 유효 토큰 발급 추가, 토큰 없음/무효/성공 후 삭제 테스트 3개 신규 - StockConcurrencyE2ETest: 동일하게 주문 API를 직접 호출하므로 토큰 발급 로직 추가 (회귀 방지)
- 최종 리뷰에서 발견: fixedRateString 프로퍼티화로 인해 BATCH_SIZE(컴파일타임 상수 기반)와 실제 스케줄러 주기가 서로 다른 소스에서 나오게 되어, 운영에서 interval-ms를 바꾸면 안전마진 계산이 조용히 어긋날 수 있는 상태였음 - @scheduled를 다시 fixedRate=SCHEDULER_INTERVAL_MS(상수)로 원복하여 BATCH_SIZE와의 결합 복구 - 테스트 중 자동 실행만 끄기 위한 별도 queue.scheduler.enabled 플래그 추가 (기본 true, test 프로파일에서 false) — 테스트의 issueTokens() 직접 호출은 영향 없음
📝 Walkthrough
WalkthroughRedis 기반 대기열과 입장 토큰 발급·검증·삭제 기능을 추가하고, 큐 조회 API와 주문 인터셉터, 배치 스케줄러 및 관련 테스트를 구성했다. Changes대기열 도메인과 Redis 저장소
큐 API와 주문 토큰 보호
토큰 발급 스케줄러
대기열과 주문 통합 검증
Sequence Diagram(s)sequenceDiagram
participant Client
participant QueueController
participant QueueFacade
participant QueueService
participant Redis
Client->>QueueController: POST /api/v1/queue/enter
QueueController->>QueueFacade: enter(userId)
QueueFacade->>QueueService: enter(userId)
QueueService->>Redis: 대기열 사용자 등록 및 순위 조회
Redis-->>QueueService: 순위 반환
QueueService-->>QueueController: QueueInfo
QueueController-->>Client: PositionResponse
sequenceDiagram
participant Client
participant QueueTokenInterceptor
participant QueueFacade
participant Redis
participant OrderController
Client->>QueueTokenInterceptor: 주문 POST + X-Entry-Token
QueueTokenInterceptor->>QueueFacade: validateToken(userId, token)
QueueFacade->>Redis: 입장 토큰 조회
Redis-->>QueueFacade: 토큰 반환
QueueTokenInterceptor->>OrderController: 검증 통과
OrderController-->>QueueTokenInterceptor: 2xx 응답
QueueTokenInterceptor->>QueueFacade: completeOrder(userId)
QueueFacade->>Redis: 입장 토큰 삭제
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 4
🧹 Nitpick comments (1)
apps/commerce-api/src/main/java/com/loopers/domain/queue/QueueDomainService.java (1)
5-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value도메인 계층의
@Component어노테이션을 분리 검토하라.
QueueDomainService는 도메인 패키지(domain/queue)에 위치하지만@Component로 인해 Spring 프레임워크 의존성이 도메인 계층에 침투한다. 도메인 규칙과 인프라 관심사가 혼재되므로, 인터페이스를 도메인에 두고 구현체(또는 어댑터)를 인프라 계층에 배치하는 방식을 권장한다.As per coding guidelines,
**/domain/**/*.java경로 지침: "도메인 규칙과 인프라 관심사가 섞이면 분리하도록 제안한다."🤖 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 `@apps/commerce-api/src/main/java/com/loopers/domain/queue/QueueDomainService.java` around lines 5 - 10, QueueDomainService의 도메인 계층에 Spring 의존성이 침투해 있으므로 `@Component를` 제거하고, 도메인에는 순수한 인터페이스 또는 규칙 객체만 유지하세요. Spring 빈 등록과 의존성 주입이 필요한 구현체 또는 어댑터는 인프라 계층으로 이동한 뒤 해당 구현체에 `@Component를` 적용하고, 사용하는 코드가 이를 주입받도록 수정하세요.Source: Path instructions
🤖 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
`@apps/commerce-api/src/main/java/com/loopers/application/queue/QueueService.java`:
- Around line 26-35: QueueService.position에서 토큰과 대기열 순위가 모두 없는 경우 NOT_FOUND 예외를
던지지 말고 재진입이 필요한 상태를 반환하도록 대체 흐름을 구현하세요. QueueInfo에 만료/재진입 필요 상태를 추가하고 해당 상태를
반환하며, 토큰 만료 후 position 조회 시나리오를 검증하는 테스트도 추가하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/queue/QueueRepositoryImpl.java`:
- Around line 27-32: enter()의 addIfAbsent와 rank() 사이 TOCTOU 경쟁 조건을 제거하세요.
QueueRepositoryImpl의 enter 메서드에서 ZADD NX와 ZRANK를 하나의 Redis Lua script로 원자적으로
실행하고, 스크립트 결과로 순위를 반환하도록 변경해 popMin이 개입해 null이 발생하지 않게 하세요. 반환값 변환과 실패 시
CoreException 처리도 명확히 유지하고, 동시 popMin 상황에서 500 오류가 발생하지 않는 테스트를 추가하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/scheduler/queue/QueueEntryScheduler.java`:
- Around line 32-40: issueTokens()에서 popMin() 후 entryTokenRepository.issue()가
실패하면 사용자가 유실되지 않도록 보상 처리를 추가하세요. 각 userId의 발급을 개별 try-catch로 감싸고 실패한 사용자는
queueRepository를 통해 재큐잉하거나, 큐 제거와 토큰 발급을 원자화하는 방식으로 변경하세요. 또한 issue() 중간 실패 시
나머지 사용자와 실패 사용자가 모두 보존되는 테스트를 추가하세요.
In
`@apps/commerce-api/src/test/java/com/loopers/application/queue/QueueServiceIntegrationTest.java`:
- Around line 45-70: Queue concurrency test can hang indefinitely when a worker
fails and does not release the latch. In
enter_guaranteesUniquePositions_underConcurrency, move latch.countDown() into a
finally block, use a bounded timeout with latch.await() and assert it completes,
then call executor.shutdown() followed by awaitTermination() with a timeout and
handle interruption appropriately; preserve worker failures so the test does not
silently pass.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/domain/queue/QueueDomainService.java`:
- Around line 5-10: QueueDomainService의 도메인 계층에 Spring 의존성이 침투해 있으므로 `@Component를`
제거하고, 도메인에는 순수한 인터페이스 또는 규칙 객체만 유지하세요. Spring 빈 등록과 의존성 주입이 필요한 구현체 또는 어댑터는 인프라
계층으로 이동한 뒤 해당 구현체에 `@Component를` 적용하고, 사용하는 코드가 이를 주입받도록 수정하세요.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 63ea99dc-d2e4-4c2f-b507-cee0983e5cd5
📒 Files selected for processing (22)
apps/commerce-api/src/main/java/com/loopers/application/queue/QueueFacade.javaapps/commerce-api/src/main/java/com/loopers/application/queue/QueueInfo.javaapps/commerce-api/src/main/java/com/loopers/application/queue/QueueService.javaapps/commerce-api/src/main/java/com/loopers/config/WebMvcConfig.javaapps/commerce-api/src/main/java/com/loopers/domain/queue/EntryTokenRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/queue/QueueDomainService.javaapps/commerce-api/src/main/java/com/loopers/domain/queue/QueueRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/queue/QueueThroughputPolicy.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/queue/EntryTokenRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/queue/QueueRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/queue/QueueController.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/queue/QueueDto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/queue/QueueTokenInterceptor.javaapps/commerce-api/src/main/java/com/loopers/interfaces/scheduler/queue/QueueEntryScheduler.javaapps/commerce-api/src/main/resources/application.ymlapps/commerce-api/src/test/java/com/loopers/application/queue/QueueServiceIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/application/queue/QueueServiceTest.javaapps/commerce-api/src/test/java/com/loopers/domain/queue/QueueDomainServiceTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/OrderApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/QueueApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/StockConcurrencyE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/scheduler/queue/QueueEntrySchedulerTest.java
Comment on lines
+26
to
+35
| public QueueInfo position(Long userId) { | ||
| Optional<String> token = entryTokenRepository.find(userId); | ||
| if (token.isPresent()) { | ||
| return QueueInfo.ready(token.get()); | ||
| } | ||
|
|
||
| long rank = queueRepository.rank(userId) | ||
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "대기열에 진입한 기록이 없습니다.")); | ||
| return QueueInfo.waiting(rank, queueDomainService.estimateWaitSeconds(rank)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
토큰 만료 후 대기열에서 제거된 사용자의 position 조회 시 NOT_FOUND 예외가 발생한다.
문제: 스케줄러가 popMin으로 사용자를 대기열에서 제거하고 토큰을 발급하지만, 토큰 TTL(300초) 만료 후 사용자가 position을 조회하면 entryTokenRepository.find(userId)가 empty를 반환하고, 이어 queueRepository.rank(userId)도 empty를 반환하여 CoreException(NOT_FOUND)가 발생한다. 사용자는 대기열에서도 제거되었고 토큰도 만료된 상태로, position 조회와 주문 모두 불가능한 상태가 된다.
운영 관점 문제: 인기 상품 오픈 시나리오에서 사용자가 5분 내에 주문을 완료하지 못하는 경우(결제 수단 선택 지연, 앱 일시 정지 등) 발생 가능하다. 현재 에러 메시지("대기열에 진입한 기록이 없습니다")는 사용자에게 재진입을 유도하지 않아 혼란을 야기한다.
수정안: rank가 empty인 경우 NOT_FOUND를 던지는 대신, 사용자에게 재진입이 필요함을 알리는 응답을 반환하거나 자동으로 대기열에 재진입시키는 처리를 추가한다.
추가 테스트: "토큰 만료 후 position 조회 시, 재진입 필요 상태를 반환한다" 케이스 추가.
As per path instructions, **/*Service*.java 리뷰 기준에서 "실패 시 대체 흐름을 제안한다"를 적용했다.
🔧 수정안: 에러 메시지 개선
long rank = queueRepository.rank(userId)
- .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "대기열에 진입한 기록이 없습니다."));
+ .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "대기열 정보가 만료되었거나 존재하지 않습니다. 대기열에 진입해주세요."));🔧 대안: 재진입 유도 응답 반환 (QueueInfo에 expired 상태 추가 필요)
public QueueInfo position(Long userId) {
Optional<String> token = entryTokenRepository.find(userId);
if (token.isPresent()) {
return QueueInfo.ready(token.get());
}
- long rank = queueRepository.rank(userId)
- .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "대기열에 진입한 기록이 없습니다."));
- return QueueInfo.waiting(rank, queueDomainService.estimateWaitSeconds(rank));
+ Optional<Long> rank = queueRepository.rank(userId);
+ if (rank.isEmpty()) {
+ return QueueInfo.expired();
+ }
+ return QueueInfo.waiting(rank.get(), queueDomainService.estimateWaitSeconds(rank.get()));
}🤖 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
`@apps/commerce-api/src/main/java/com/loopers/application/queue/QueueService.java`
around lines 26 - 35, QueueService.position에서 토큰과 대기열 순위가 모두 없는 경우 NOT_FOUND 예외를
던지지 말고 재진입이 필요한 상태를 반환하도록 대체 흐름을 구현하세요. QueueInfo에 만료/재진입 필요 상태를 추가하고 해당 상태를
반환하며, 토큰 만료 후 position 조회 시나리오를 검증하는 테스트도 추가하세요.
Source: Path instructions
Comment on lines
+27
to
+32
| @Override | ||
| public long enter(Long userId, long timestampMillis) { | ||
| redisTemplate.opsForZSet().addIfAbsent(QUEUE_KEY, String.valueOf(userId), timestampMillis); | ||
| return rank(userId) | ||
| .orElseThrow(() -> new CoreException(ErrorType.INTERNAL_ERROR, "대기열 진입에 실패했습니다.")); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
enter()의 TOCTOU 경쟁 조건 — addIfAbsent와 rank() 사이에 popMin이 끼어들면 500 에러 발생
enter()는 addIfAbsent(ZADD NX)와 rank()(ZRANK)를 두 개의 별도 Redis 명령으로 실행한다. 스케줄러가 100ms마다 popMin()(ZPOPMIN)으로 대기열에서 사용자를 제거하므로, 두 명령 사이에 사용자가 pop되면 rank()가 null을 반환하고 INTERNAL_ERROR(500)가 발생한다.
운영 관점 문제: 사용자는 500 에러를 수신하지만 실제로는 토큰이 발급되어 있다. 사용자가 재시도하면 대기열에 재진입하며 중복 항목이 발생한다. PR 목적인 요청 급증 상황에서 경쟁 확률이 증가한다.
수정안: Lua script로 ZADD+ZRANK를 원자화한다.
🔒 TOCTOU 경쟁 조건 수정안 (Lua script 원자화)
import com.loopers.domain.queue.QueueRepository;
import com.loopers.support.error.CoreException;
import com.loopers.support.error.ErrorType;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.data.redis.core.ZSetOperations;
+import org.springframework.data.redis.core.script.DefaultRedisScript;
import org.springframework.stereotype.Component;
-import java.util.List;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.Set;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
`@Component`
public class QueueRepositoryImpl implements QueueRepository {
private static final String QUEUE_KEY = "waiting-queue";
+ private static final DefaultRedisScript<Long> ENTER_SCRIPT;
+
+ static {
+ ENTER_SCRIPT = new DefaultRedisScript<>();
+ ENTER_SCRIPT.setScriptText(
+ "redis.call('ZADD', KEYS[1], 'NX', ARGV[2], ARGV[1]) " +
+ "local rank = redis.call('ZRANK', KEYS[1], ARGV[1]) " +
+ "if rank == false then return -1 end " +
+ "return rank"
+ );
+ ENTER_SCRIPT.setResultType(Long.class);
+ }
+
private final RedisTemplate<String, String> redisTemplate;
public QueueRepositoryImpl(`@Qualifier`("redisTemplateMaster") RedisTemplate<String, String> redisTemplate) {
this.redisTemplate = redisTemplate;
}
`@Override`
public long enter(Long userId, long timestampMillis) {
- redisTemplate.opsForZSet().addIfAbsent(QUEUE_KEY, String.valueOf(userId), timestampMillis);
- return rank(userId)
- .orElseThrow(() -> new CoreException(ErrorType.INTERNAL_ERROR, "대기열 진입에 실패했습니다."));
+ Long rank = redisTemplate.execute(
+ ENTER_SCRIPT,
+ Collections.singletonList(QUEUE_KEY),
+ String.valueOf(userId),
+ String.valueOf(timestampMillis)
+ );
+ if (rank == null || rank < 0) {
+ throw new CoreException(ErrorType.INTERNAL_ERROR, "대기열 진입에 실패했습니다.");
+ }
+ return rank;
}추가 테스트 제안:
- 스케줄러 popMin과 enter()를 동시 실행하여 경쟁 조건 재현 → 500 에러가 발생하지 않음을 검증
- Lua script 적용 후 enter()의 원자성 보장 테스트
As per coding guidelines, null 처리 및 예외 흐름의 명확성을 점검한다. enter()에서 rank()가 null을 반환하는 경쟁 상황이 예외 흐름의 불명확성을 초래한다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Override | |
| public long enter(Long userId, long timestampMillis) { | |
| redisTemplate.opsForZSet().addIfAbsent(QUEUE_KEY, String.valueOf(userId), timestampMillis); | |
| return rank(userId) | |
| .orElseThrow(() -> new CoreException(ErrorType.INTERNAL_ERROR, "대기열 진입에 실패했습니다.")); | |
| } | |
| private static final DefaultRedisScript<Long> ENTER_SCRIPT; | |
| static { | |
| ENTER_SCRIPT = new DefaultRedisScript<>(); | |
| ENTER_SCRIPT.setScriptText( | |
| "redis.call('ZADD', KEYS[1], 'NX', ARGV[2], ARGV[1]) " + | |
| "local rank = redis.call('ZRANK', KEYS[1], ARGV[1]) " + | |
| "if rank == false then return -1 end " + | |
| "return rank" | |
| ); | |
| ENTER_SCRIPT.setResultType(Long.class); | |
| } | |
| `@Override` | |
| public long enter(Long userId, long timestampMillis) { | |
| Long rank = redisTemplate.execute( | |
| ENTER_SCRIPT, | |
| Collections.singletonList(QUEUE_KEY), | |
| String.valueOf(userId), | |
| String.valueOf(timestampMillis) | |
| ); | |
| if (rank == null || rank < 0) { | |
| throw new CoreException(ErrorType.INTERNAL_ERROR, "대기열 진입에 실패했습니다."); | |
| } | |
| return rank; | |
| } |
🤖 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
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/queue/QueueRepositoryImpl.java`
around lines 27 - 32, enter()의 addIfAbsent와 rank() 사이 TOCTOU 경쟁 조건을 제거하세요.
QueueRepositoryImpl의 enter 메서드에서 ZADD NX와 ZRANK를 하나의 Redis Lua script로 원자적으로
실행하고, 스크립트 결과로 순위를 반환하도록 변경해 popMin이 개입해 null이 발생하지 않게 하세요. 반환값 변환과 실패 시
CoreException 처리도 명확히 유지하고, 동시 popMin 상황에서 500 오류가 발생하지 않는 테스트를 추가하세요.
Comment on lines
+32
to
+40
| public void issueTokens() { | ||
| List<Long> userIds = queueRepository.popMin(QueueThroughputPolicy.BATCH_SIZE); | ||
| for (Long userId : userIds) { | ||
| entryTokenRepository.issue(userId); | ||
| } | ||
| if (!userIds.isEmpty()) { | ||
| log.info("[QueueEntryScheduler] 토큰 발급 완료: count={}", userIds.size()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files
git ls-files | rg 'QueueEntryScheduler\.java|QueueRepository\.java|EntryTokenRepository\.java|Queue.*Repository\.java|Queue.*Scheduler\.java'
# Show outlines if available
for f in \
apps/commerce-api/src/main/java/com/loopers/interfaces/scheduler/queue/QueueEntryScheduler.java \
apps/commerce-api/src/main/java/com/loopers/domain/queue/QueueRepository.java \
apps/commerce-api/src/main/java/com/loopers/domain/entrytoken/EntryTokenRepository.java
do
if [ -f "$f" ]; then
echo "### OUTLINE $f"
ast-grep outline "$f" --view expanded || true
fi
done
# Read target file if present
if [ -f apps/commerce-api/src/main/java/com/loopers/interfaces/scheduler/queue/QueueEntryScheduler.java ]; then
echo "### QueueEntryScheduler"
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/scheduler/queue/QueueEntryScheduler.java
fi
# Search for methods/signatures
rg -n "popMin\(|issue\(|enter\(" apps/commerce-api/src/main/java -g'*.java'Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 5174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Broaden search if package paths differ
git ls-files | rg 'QueueEntryScheduler\.java|QueueRepository\.java|EntryTokenRepository\.java|Queue.*Repository\.java|Queue.*Scheduler\.java|entry.*Token.*Repository\.java'
# Search the whole repo for queue-related methods
rg -n "interface .*QueueRepository|class .*QueueRepository|interface .*EntryTokenRepository|class .*EntryTokenRepository|popMin\(|issue\(|enter\(" .
# Read the most relevant files if found
for f in \
apps/commerce-api/src/main/java/com/loopers/interfaces/scheduler/queue/QueueEntryScheduler.java \
apps/commerce-api/src/main/java/com/loopers/domain/queue/QueueRepository.java \
apps/commerce-api/src/main/java/com/loopers/domain/entrytoken/EntryTokenRepository.java
do
if [ -f "$f" ]; then
echo "### $f"
wc -l "$f"
cat -n "$f"
fi
doneRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 9866
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If the earlier paths are different, locate scheduler and repository implementations.
fd -i 'QueueEntryScheduler.java' apps .
fd -i 'QueueRepository.java' apps .
fd -i 'EntryTokenRepository.java' apps .
# Find tests or implementations mentioning 재큐잉 / ZPOPMIN / ZADD NX
rg -n "ZPOPMIN|ZADD NX|재큐|requeue|issueTokens|QueueEntryScheduler" apps .Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 3064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
apps/commerce-api/src/main/java/com/loopers/infrastructure/queue/QueueRepositoryImpl.java \
apps/commerce-api/src/main/java/com/loopers/infrastructure/queue/EntryTokenRepositoryImpl.java \
apps/commerce-api/src/test/java/com/loopers/interfaces/scheduler/queue/QueueEntrySchedulerTest.java
do
echo "### FILE: $f"
wc -l "$f"
cat -n "$f"
echo
doneRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 6893
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus on scheduler tests to see if failure/exception paths are covered.
cat -n apps/commerce-api/src/test/java/com/loopers/interfaces/scheduler/queue/QueueEntrySchedulerTest.java
# Also inspect implementation details for popMin and issue.
cat -n apps/commerce-api/src/main/java/com/loopers/infrastructure/queue/QueueRepositoryImpl.java
cat -n apps/commerce-api/src/main/java/com/loopers/infrastructure/queue/EntryTokenRepositoryImpl.javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 6281
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n apps/commerce-api/src/main/java/com/loopers/infrastructure/queue/QueueRepositoryImpl.java
cat -n apps/commerce-api/src/main/java/com/loopers/infrastructure/queue/EntryTokenRepositoryImpl.java
cat -n apps/commerce-api/src/test/java/com/loopers/interfaces/scheduler/queue/QueueEntrySchedulerTest.javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 6281
issueTokens()의 부분 실패를 보상해야 한다다.
popMin()으로 큐에서 먼저 제거한 뒤issue()가 예외를 던지면, 해당 배치의 남은 사용자는 토큰 없이 처리에서 빠진다다.issue()를 사용자 단위로 try-catch 처리하고 실패 시 재큐잉하거나, Redis Lua로 큐 제거와 토큰 발급을 원자화해야 한다다.issue()가 중간에 실패해도 나머지 사용자가 유실되지 않는 테스트를 추가해야 한다다.
🤖 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
`@apps/commerce-api/src/main/java/com/loopers/interfaces/scheduler/queue/QueueEntryScheduler.java`
around lines 32 - 40, issueTokens()에서 popMin() 후 entryTokenRepository.issue()가
실패하면 사용자가 유실되지 않도록 보상 처리를 추가하세요. 각 userId의 발급을 개별 try-catch로 감싸고 실패한 사용자는
queueRepository를 통해 재큐잉하거나, 큐 제거와 토큰 발급을 원자화하는 방식으로 변경하세요. 또한 issue() 중간 실패 시
나머지 사용자와 실패 사용자가 모두 보존되는 테스트를 추가하세요.
Comment on lines
+45
to
+70
| @DisplayName("동시에 여러 유저가 진입해도,") | ||
| @Test | ||
| void enter_guaranteesUniquePositions_underConcurrency() throws InterruptedException { | ||
| // arrange | ||
| int userCount = 50; | ||
| ExecutorService executor = Executors.newFixedThreadPool(10); | ||
| CountDownLatch latch = new CountDownLatch(userCount); | ||
|
|
||
| // act | ||
| for (long userId = 1; userId <= userCount; userId++) { | ||
| long id = userId; | ||
| executor.submit(() -> { | ||
| queueService.enter(id); | ||
| latch.countDown(); | ||
| }); | ||
| } | ||
| latch.await(); | ||
| executor.shutdown(); | ||
|
|
||
| // assert | ||
| Set<Long> positions = new HashSet<>(); | ||
| for (long userId = 1; userId <= userCount; userId++) { | ||
| positions.add(queueService.position(userId).position()); | ||
| } | ||
| assertThat(positions).hasSize(userCount); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
latch.await() 타임아웃 누락 — 태스크 예외 시 테스트 무한 대기
queueService.enter(id)가 예외를 throw하면 latch.countDown()이 호출되지 않아 latch.await()가 영구 대기한다. executor.shutdown()도 awaitTermination() 없이 즉시 반환하므로 후속 테스트에 영향을 줄 수 있다.
운영 관점 문제: CI 파이프라인이 무한 대기하며 빌드가 hang된다.
수정안: countDown()을 finally 블록으로 이동하고, latch.await()에 타임아웃을 추가하며, awaitTermination()을 호출한다.
🛡️ 테스트 hang 방지 수정안
for (long userId = 1; userId <= userCount; userId++) {
long id = userId;
executor.submit(() -> {
- queueService.enter(id);
- latch.countDown();
+ try {
+ queueService.enter(id);
+ } finally {
+ latch.countDown();
+ }
});
}
- latch.await();
+ assertThat(latch.await(10, java.util.concurrent.TimeUnit.SECONDS))
+ .as("모든 태스크가 타임아웃 내에 완료되어야 한다")
+ .isTrue();
executor.shutdown();
+ executor.awaitTermination(5, java.util.concurrent.TimeUnit.SECONDS);추가 테스트 제안:
enter()실패 시 타임아웃으로 테스트가 종료되는지 검증
As per coding guidelines, 통합 테스트의 플래키 가능성을 점검한다. latch.await() 무타임아웃은 플래키 위험 요소이다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @DisplayName("동시에 여러 유저가 진입해도,") | |
| @Test | |
| void enter_guaranteesUniquePositions_underConcurrency() throws InterruptedException { | |
| // arrange | |
| int userCount = 50; | |
| ExecutorService executor = Executors.newFixedThreadPool(10); | |
| CountDownLatch latch = new CountDownLatch(userCount); | |
| // act | |
| for (long userId = 1; userId <= userCount; userId++) { | |
| long id = userId; | |
| executor.submit(() -> { | |
| queueService.enter(id); | |
| latch.countDown(); | |
| }); | |
| } | |
| latch.await(); | |
| executor.shutdown(); | |
| // assert | |
| Set<Long> positions = new HashSet<>(); | |
| for (long userId = 1; userId <= userCount; userId++) { | |
| positions.add(queueService.position(userId).position()); | |
| } | |
| assertThat(positions).hasSize(userCount); | |
| } | |
| `@DisplayName`("동시에 여러 유저가 진입해도,") | |
| `@Test` | |
| void enter_guaranteesUniquePositions_underConcurrency() throws InterruptedException { | |
| // arrange | |
| int userCount = 50; | |
| ExecutorService executor = Executors.newFixedThreadPool(10); | |
| CountDownLatch latch = new CountDownLatch(userCount); | |
| // act | |
| for (long userId = 1; userId <= userCount; userId++) { | |
| long id = userId; | |
| executor.submit(() -> { | |
| try { | |
| queueService.enter(id); | |
| } finally { | |
| latch.countDown(); | |
| } | |
| }); | |
| } | |
| assertThat(latch.await(10, java.util.concurrent.TimeUnit.SECONDS)) | |
| .as("모든 태스크가 타임아웃 내에 완료되어야 한다") | |
| .isTrue(); | |
| executor.shutdown(); | |
| executor.awaitTermination(5, java.util.concurrent.TimeUnit.SECONDS); | |
| // assert | |
| Set<Long> positions = new HashSet<>(); | |
| for (long userId = 1; userId <= userCount; userId++) { | |
| positions.add(queueService.position(userId).position()); | |
| } | |
| assertThat(positions).hasSize(userCount); | |
| } |
🤖 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
`@apps/commerce-api/src/test/java/com/loopers/application/queue/QueueServiceIntegrationTest.java`
around lines 45 - 70, Queue concurrency test can hang indefinitely when a worker
fails and does not release the latch. In
enter_guaranteesUniquePositions_underConcurrency, move latch.countDown() into a
finally block, use a bounded timeout with latch.await() and assert it completes,
then call executor.shutdown() followed by awaitTermination() with a timeout and
handle interruption appropriately; preserve worker failures so the test does not
silently pass.
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
1 participant
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.
🧭 Context & Decision
문제 정의
풀 고갈 → 응답 지연 → 타임아웃 → 재시도 폭풍으로 이어져 전체 서비스가 멈춘다.
흘려보내야 한다(Back-pressure).
주문 API 진입 가능(성공 시 토큰 삭제), (4) Polling으로 순번/예상 대기시간 조회 — 이 4가지 Must-Have가 모두 동작하고 테스트로 증명되는 것.
선택지와 결정
1) 입장 토큰 검증 위치
LoginInterceptor/AdminInterceptor와 동일한 패턴으로WebMvcConfig에 등록HttpServletRequest에서 메서드/경로를 직접 파싱해야 해서 코드가 덜 명확해짐. 기존 컨벤션과의 일관성을 우선했다.
2) 스케줄러 배치 크기 산정
maximum-pool-size: 40) 기준으로 재계산BATCH_SIZE = SAFE_TPS / (1000 / SCHEDULER_INTERVAL_MS))SAFE_TPS를 주기적으로 재산정3) Redis 읽기 경로
ProductCacheService처럼 Master/Replica 분리해 읽기를 Replica로redisTemplateMaster) 사용r 전용을 선택했다. Master의 읽기 부하가 늘어나는 건 감수.
4) 입장 토큰 TTL
🤔 고민한 점 / 막혔던 부분
@Scheduled(fixedRate=100ms)로 실제 앱 컨텍스트가 뜨는 통합 테스트에서, 19명을 순차로 입력하는 setup 자체가 100ms보다 오래 걸려서 실제 스케줄러가 테스트 중간에 여러 번 자동으로 돌며 대기열을 예상보다 더 비워버렸다. 처음엔 스케줄러 주기를 프로퍼티화해서 test 프로파일에서 값을 크게 늘리는 방식으로 고쳤는데, 이후 전체
리뷰에서 그러면
BATCH_SIZE(컴파일타임 상수로 산정된 값)와 실제 주기가 서로 다른 소스에서 나오게 되어 나중에 누가 주기 값만 바꾸면 배치 크기 산정 근거가 조용히 어긋날 수 있다는 걸 발견했다.최종적으로는 주기는 다시 상수로 고정하고, "자동 실행 on/off"만 별도 플래그로 분리해 두 문제를 동시에 해결했다.
OrderApiE2ETest만 수정 대상으로 적어뒀는데, 실제로는StockConcurrencyE2ETest도 주문 API를 직접 호출하고 있어서 토큰 검증 게이트를 전역으로 걸자마자 조용히 깨질 뻔했다. 전체 테스트 스위트를 한 번씩 돌려보는 습관 덕분에 계획 단계에서 놓친 부분을 구현 단계에서 잡아낼 수 있었다.
SAFE_TPS라는 식별자 중간에 "을"이 끼어 들어가 컴파일이 안 되는 상태로 커밋된 적이 있었다. 다음 작업에서 컴파일을 시도하다가 우연히 발견해서 고쳤는데, 이후로는 커밋 전에 한 번 더 컴파일을 확인하는 습관이 필요하다는 걸 느꼈다.
🙋 기타
이번 라운드는 발제문의 예시 수치(DB 풀 50, TTL 5분 등)를 그대로 쓰지 않고, 실제 프로젝트의 설정값(HikariCP 풀 40)과 실제 주문 흐름(결제 폼 없이 단일 클릭)을 근거로 다시 계산/판단한 부분이 많
았다. "발제문이 이렇게 예시를 들었으니까"가 아니라 "우리 프로젝트는 이런데, 그래도 맞는 값인가"를 한 번 더 물어보는 게 이번 설계 과정에서 가장 크게 배웠다.