Volume 8#381
Merged
Merged
Conversation
- 대기열(Sorted Set): 진입(ZADD NX)/순번(ZRANK)/전체인원(ZCARD)/꺼내기(ZPOPMIN) - 입장 토큰(String+TTL): 발급/조회/삭제, TTL 자동 만료 - 토큰 발급 스케줄러: 100ms마다 앞 N명 꺼내 토큰 발급(예외 삼킴으로 조용한 취소 방지) - 주문 API 관문: 토글(loopers.queue.gate.enabled) + 토큰 검증 + 성공 후 삭제 - API: POST /queue/enter, GET /queue/position - 테스트: 스케줄러 전역 비활성화(SchedulerTestConfig)로 공유 Redis 오염 방지 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- 내 차례 도달(큐에서 빠짐 + 토큰 보유) 시 position 응답에 token 동봉 - QueueInfo.of(대기중)/admitted(내차례) 상태 구분 - position 3경우: 대기중 / 내차례(토큰) / 이탈(404) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- 동시 진입 100건: 성공한 진입은 유실·중복 없이 고유 순번 (ZADD 원자성) - 처리량 상한: 배치 초과 인원도 틱당 배치 크기만큼만 (back-pressure) - 토큰 만료: TTL 초과 시 관문 검증 거부 - TokenIssueScheduler @ConditionalOnProperty(enabled, matchIfMissing=true) → 테스트에선 빈 미생성으로 공유 Redis 오염 원천 차단(SchedulerTestConfig), 운영 kill-switch 겸용 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 Walkthrough변경 목적: Redis 기반 대기열과 입장 토큰을 도입하고, 주문 API에 토큰 기반 진입 관문을 추가했습니다. Walkthrough대기열 진입·순번 조회 API와 Redis 기반 대기열을 추가했다. 배치 스케줄러가 입장 토큰을 발급하며, 주문 생성은 토큰 검증과 성공 후 소진을 수행한다. 저장소, 동시성, 만료, API 통합 테스트도 추가했다. Changes대기열 및 입장 관문
Sequence Diagram(s)sequenceDiagram
participant Client
participant QueueV1Controller
participant QueueFacade
participant RedisWaitingQueueRepository
participant RedisEntryTokenStore
participant OrderV1Controller
participant OrderFacade
Client->>QueueV1Controller: POST /api/v1/queue/enter
QueueV1Controller->>QueueFacade: enter(userId)
QueueFacade->>RedisWaitingQueueRepository: enter(userId)
QueueFacade->>RedisWaitingQueueRepository: findRank(userId)
RedisWaitingQueueRepository-->>QueueFacade: 순번
QueueFacade-->>QueueV1Controller: QueueInfo
QueueV1Controller-->>Client: 대기 순번 및 예상 시간
Client->>OrderV1Controller: POST /api/v1/orders + X-Entry-Token
OrderV1Controller->>QueueFacade: validateEntry(userId, entryToken)
QueueFacade->>RedisEntryTokenStore: find(userId)
RedisEntryTokenStore-->>QueueFacade: 유효 토큰
OrderV1Controller->>OrderFacade: placeOrder(request)
OrderV1Controller->>QueueFacade: consumeEntry(userId)
QueueFacade->>RedisEntryTokenStore: remove(userId)
OrderV1Controller-->>Client: 주문 응답
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 (4)
apps/commerce-api/src/test/java/com/loopers/application/queue/QueueFacadeGateTest.java (1)
41-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win검증 실패 시 구체적 ErrorType 검증 권장
CoreException만 검증하고 있으나, 실제로는ErrorType.FORBIDDEN이어야 한다. 다른 ErrorType으로 변경되더라도 테스트가 통과하는 구멍이 생긴다.♻️ 제안: ErrorType까지 검증
`@DisplayName`("토큰이 없으면 예외가 발생한다.") `@Test` void validateEntry_throws_withoutToken() { assertThatThrownBy(() -> queueFacade.validateEntry(1L, null)) - .isInstanceOf(CoreException.class); + .isInstanceOfSatisfying(CoreException.class, ex -> + assertThat(ex.getErrorType()).isEqualTo(ErrorType.FORBIDDEN)); } `@DisplayName`("토큰이 일치하지 않으면 예외가 발생한다.") `@Test` void validateEntry_throws_withMismatchedToken() { entryTokenStore.issue(1L); assertThatThrownBy(() -> queueFacade.validateEntry(1L, "wrong-token")) - .isInstanceOf(CoreException.class); + .isInstanceOfSatisfying(CoreException.class, ex -> + assertThat(ex.getErrorType()).isEqualTo(ErrorType.FORBIDDEN)); }🤖 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/QueueFacadeGateTest.java` around lines 41 - 54, Update validateEntry_throws_withoutToken and validateEntry_throws_withMismatchedToken to assert that the thrown CoreException contains ErrorType.FORBIDDEN, rather than only checking the exception class. Use the exception’s existing error-type accessor and preserve the current setup for missing and mismatched tokens.apps/commerce-api/src/test/java/com/loopers/application/queue/TokenIssueSchedulerTest.java (1)
33-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win예외 처리 경로 테스트 누락
issueTokens의 catch 블록은 스케줄러가 중단되지 않도록 하는 핵심 방어 로직이다. 그러나entryTokenStore.issue()또는waitingQueueRepository.popMin()이 예외를 던지는 시나리오에 대한 테스트가 없다. 이 경로가 검증되지 않으면, 예외 처리 로직이 회귀되어도 테스트가 이를 잡아내지 못한다.추가 테스트 방향:
entryTokenStore를 모킹하여 특정 userId에서 예외를 던지도록 구성하고, (1) 예외가 전파되지 않는지, (2) 이후issueTokens()호출이 정상 동작하는지 검증한다.🤖 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/TokenIssueSchedulerTest.java` around lines 33 - 73, TokenIssueSchedulerTest의 예외 처리 경로 테스트가 누락되어 있습니다. entryTokenStore.issue() 또는 waitingQueueRepository.popMin()이 특정 사용자에서 예외를 던지도록 모킹한 테스트를 추가하고, scheduler.issueTokens()가 예외를 전파하지 않는지 검증하세요. 이어서 issueTokens()를 다시 호출해 후속 실행이 정상 처리되는지도 확인하며, 기존 테스트 설정과 격리되도록 모킹을 정리하세요.apps/commerce-api/src/main/java/com/loopers/application/queue/TokenIssueScheduler.java (1)
23-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
batchSize필드 주입 방식 개선 제안
@RequiredArgsConstructor를 사용하면서batchSize만 필드 주입(@Valueon non-final field)을 혼용하고 있다. 생성자 주입으로 통일하면 불변성을 보장하고 테스트에서 명시적 주입이 가능해진다.♻️ 제안: 생성자 주입으로 통일
`@Slf4j` -@RequiredArgsConstructor `@Component` `@ConditionalOnProperty`(prefix = "loopers.queue.scheduler", name = "enabled", havingValue = "true", matchIfMissing = true) public class TokenIssueScheduler { private final WaitingQueueRepository waitingQueueRepository; private final EntryTokenStore entryTokenStore; - `@Value`("${loopers.queue.scheduler.batch-size:18}") - private int batchSize; + private final int batchSize; + public TokenIssueScheduler( + WaitingQueueRepository waitingQueueRepository, + EntryTokenStore entryTokenStore, + `@Value`("${loopers.queue.scheduler.batch-size:18}") int batchSize + ) { + this.waitingQueueRepository = waitingQueueRepository; + this.entryTokenStore = entryTokenStore; + this.batchSize = batchSize; + }🤖 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/TokenIssueScheduler.java` around lines 23 - 24, TokenIssueScheduler의 batchSize가 필드 주입과 생성자 주입을 혼용하고 있다. batchSize를 생성자 매개변수로 주입하도록 변경하고 final 필드로 선언하며, `@Value`("${loopers.queue.scheduler.batch-size:18}")를 생성자 매개변수에 적용해 `@RequiredArgsConstructor` 기반 주입과 일관되게 수정하라.apps/commerce-api/src/test/java/com/loopers/interfaces/api/OrderQueueGateE2ETest.java (1)
66-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win잘못된 토큰 값에 대한 테스트 케이스 누락
validateEntry()는 세 가지 403 경로를 가진다: (1) 토큰 없음, (2) 저장된 토큰 없음, (3) 토큰 불일치. 현재 테스트는 (1)만 검증한다. (3) 토큰 불일치는 사용자가 이전 토큰으로 재시도하는 실제 운영 시나리오에서 발생한다.운영 관점: 토큰 검증 로직의 예외 경로가 테스트되지 않아, 회귀 시 403 대신 500이 반환되어도 발견되지 않는다.
추가 테스트: 잘못된 토큰 값으로 403을 반환하는 테스트를 추가한다.
🧪 proposed test
+ `@DisplayName`("관문 ON — 잘못된 토큰으로 주문하면 403 이다.") + `@Test` + void order_withInvalidToken_isForbidden() { + Long userId = 1L; + entryTokenStore.issue(userId); + OrderV1Dto.OrderRequest body = new OrderV1Dto.OrderRequest( + List.of(new OrderV1Dto.OrderItemRequest(productId, 1))); + + ParameterizedTypeReference<ApiResponse<OrderV1Dto.OrderResponse>> type = new ParameterizedTypeReference<>() {}; + ResponseEntity<ApiResponse<OrderV1Dto.OrderResponse>> response = + testRestTemplate.exchange(ENDPOINT, HttpMethod.POST, request(body, userId, "wrong-token"), type); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); + }🤖 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/interfaces/api/OrderQueueGateE2ETest.java` around lines 66 - 93, validateEntry()의 토큰 불일치 경로를 검증하는 테스트가 누락되어 있습니다. 기존 order_withoutToken_isForbidden()과 유사한 E2E 테스트를 추가하되, entryTokenStore.issue()로 저장된 토큰과 다른 잘못된 토큰을 요청 헤더에 전달하고 응답 상태가 HttpStatus.FORBIDDEN인지 단언하세요.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/QueueFacade.java`:
- Around line 24-28: QueueFacade.enter()의 findRank() 결과가 비어 있을 때 0순위 응답을 반환하지
않도록 수정하세요. enter()에서 findRank()가 empty이면 entryTokenStore.find(userId)를 조회하고, 발급된
토큰이 있으면 QueueInfo.admitted(token)을 반환하며 토큰도 없을 때만 기존 대기 응답을 반환하세요.
waitingQueueRepository.enter(), findRank(), entryTokenStore.find() 흐름을 검증하는 동시성
테스트도 추가해 스케줄러의 popMin() 이후 admitted 응답과 토큰이 반환되는지 확인하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/application/queue/TokenIssueScheduler.java`:
- Around line 30-42: TokenIssueScheduler.issueTokens must prevent one failed
token issuance from losing users removed by popMin. Handle exceptions per user
inside the admitted-user loop, re-enqueue the failed user through the
waiting-queue repository while preserving queue semantics, and log the failure
without aborting processing for remaining users. Add a test covering
entryTokenStore.issue failing for a specific user and verifying that user is
re-enqueued.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Controller.java`:
- Around line 36-39: 주문 생성 후 OrderV1Controller의 consumeEntry() 실패가 주문 성공 응답을
500으로 바꾸지 않도록 수정하세요. consumeEntry(userId)를 try-catch로 감싸고 예외 발생 시 사용자 응답은 정상 주문
결과로 반환하되, 예외 정보와 사용자 식별자를 포함해 로깅하세요. consumeEntry() 예외 상황에서도 주문 응답이 200으로 반환되는
테스트를 추가하세요.
In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/queue/QueueConcurrencyTest.java`:
- Around line 42-66: Update QueueConcurrencyTest to await the latch with a
finite timeout and ensure the executor is shut down in a finally block, failing
the test if workers do not finish. Add an explicit acceptable error threshold
before calculating expected results, so excessive or complete worker failures
fail the test instead of being masked by errors.size().
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/queue/TokenIssueScheduler.java`:
- Around line 23-24: TokenIssueScheduler의 batchSize가 필드 주입과 생성자 주입을 혼용하고 있다.
batchSize를 생성자 매개변수로 주입하도록 변경하고 final 필드로 선언하며,
`@Value`("${loopers.queue.scheduler.batch-size:18}")를 생성자 매개변수에 적용해
`@RequiredArgsConstructor` 기반 주입과 일관되게 수정하라.
In
`@apps/commerce-api/src/test/java/com/loopers/application/queue/QueueFacadeGateTest.java`:
- Around line 41-54: Update validateEntry_throws_withoutToken and
validateEntry_throws_withMismatchedToken to assert that the thrown CoreException
contains ErrorType.FORBIDDEN, rather than only checking the exception class. Use
the exception’s existing error-type accessor and preserve the current setup for
missing and mismatched tokens.
In
`@apps/commerce-api/src/test/java/com/loopers/application/queue/TokenIssueSchedulerTest.java`:
- Around line 33-73: TokenIssueSchedulerTest의 예외 처리 경로 테스트가 누락되어 있습니다.
entryTokenStore.issue() 또는 waitingQueueRepository.popMin()이 특정 사용자에서 예외를 던지도록
모킹한 테스트를 추가하고, scheduler.issueTokens()가 예외를 전파하지 않는지 검증하세요. 이어서 issueTokens()를
다시 호출해 후속 실행이 정상 처리되는지도 확인하며, 기존 테스트 설정과 격리되도록 모킹을 정리하세요.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/OrderQueueGateE2ETest.java`:
- Around line 66-93: validateEntry()의 토큰 불일치 경로를 검증하는 테스트가 누락되어 있습니다. 기존
order_withoutToken_isForbidden()과 유사한 E2E 테스트를 추가하되, entryTokenStore.issue()로
저장된 토큰과 다른 잘못된 토큰을 요청 헤더에 전달하고 응답 상태가 HttpStatus.FORBIDDEN인지 단언하세요.
🪄 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: 4b24b842-a52d-49af-a8f8-fa19386e4f93
📒 Files selected for processing (21)
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/TokenIssueScheduler.javaapps/commerce-api/src/main/java/com/loopers/domain/queue/EntryTokenStore.javaapps/commerce-api/src/main/java/com/loopers/domain/queue/WaitingQueueRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/queue/RedisEntryTokenStore.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/queue/RedisWaitingQueueRepository.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/queue/QueueV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/queue/QueueV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/queue/QueueV1Dto.javaapps/commerce-api/src/main/java/com/loopers/support/error/ErrorType.javaapps/commerce-api/src/test/java/com/loopers/application/queue/QueueFacadeGateTest.javaapps/commerce-api/src/test/java/com/loopers/application/queue/TokenIssueSchedulerTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/queue/QueueConcurrencyTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/queue/RedisEntryTokenStoreTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/queue/RedisWaitingQueueRepositoryTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/OrderQueueGateE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/QueueV1ApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/testconfig/SchedulerTestConfig.java
Comment on lines
+24
to
+28
| public QueueInfo enter(Long userId) { | ||
| waitingQueueRepository.enter(userId); | ||
| long rank = waitingQueueRepository.findRank(userId).orElse(0L); | ||
| return QueueInfo.of(rank); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
enter() 메서드의 TOCTOU 경쟁 조건
waitingQueueRepository.enter(userId) 이후 findRank(userId)를 별도 호출한다. 두 호출 사이에 스케줄러의 popMin()이 사용자를 대기열에서 제거할 수 있다. 이 경우 findRank()는 empty를 반환하고 orElse(0L)은 0이 되어, 사용자에게 position=0, estimatedWaitSeconds=0을 응답하지만 토큰은 포함되지 않는다.
운영 관점: 사용자가 "내 차례"로 착각하고 즉시 주문을 시도하면 403을 받게 되어 CS 문의가 발생한다. 고부하 상황에서 발생 빈도가 증가한다.
수정안: findRank()가 empty일 때 entryTokenStore.find(userId)를 확인하여 토큰이 발급되었으면 admitted(token)을 반환한다.
🔧 proposed fix
public QueueInfo enter(Long userId) {
waitingQueueRepository.enter(userId);
- long rank = waitingQueueRepository.findRank(userId).orElse(0L);
- return QueueInfo.of(rank);
+ Optional<Long> rank = waitingQueueRepository.findRank(userId);
+ if (rank.isPresent()) {
+ return QueueInfo.of(rank.get());
+ }
+ // 스케줄러가 enter와 findRank 사이에 popMin으로 제거했을 수 있음 → 토큰 확인
+ return entryTokenStore.find(userId)
+ .map(QueueInfo::admitted)
+ .orElse(QueueInfo.of(0L));
}추가 테스트: enter() 호출과 동시에 스케줄러가 popMin()을 실행하는 시나리오에서 admitted 응답(토큰 포함)이 반환되는지 검증한다.
🤖 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/QueueFacade.java`
around lines 24 - 28, QueueFacade.enter()의 findRank() 결과가 비어 있을 때 0순위 응답을 반환하지
않도록 수정하세요. enter()에서 findRank()가 empty이면 entryTokenStore.find(userId)를 조회하고, 발급된
토큰이 있으면 QueueInfo.admitted(token)을 반환하며 토큰도 없을 때만 기존 대기 응답을 반환하세요.
waitingQueueRepository.enter(), findRank(), entryTokenStore.find() 흐름을 검증하는 동시성
테스트도 추가해 스케줄러의 popMin() 이후 admitted 응답과 토큰이 반환되는지 확인하세요.
Comment on lines
+30
to
+42
| @Scheduled(fixedDelayString = "${loopers.queue.scheduler.interval-ms:100}") | ||
| public void issueTokens() { | ||
| try { | ||
| List<Long> admitted = waitingQueueRepository.popMin(batchSize); | ||
| for (Long userId : admitted) { | ||
| entryTokenStore.issue(userId); | ||
| } | ||
| } catch (Exception e) { | ||
| // ★ @Scheduled 메서드에서 예외가 밖으로 나가면 이후 실행이 조용히 취소된다. | ||
| // 반드시 여기서 삼켜서 스케줄러가 계속 돌게 한다. | ||
| log.error("토큰 발급 스케줄러 실행 중 오류", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
배치 내 토큰 발급 부분 실패 시 사용자 유실 위험
popMin은 원자적으로 대기열에서 사용자를 제거하지만, 이후 issue 루프 중 한 사용자에게서 예외가 발생하면 나머지 사용자는 토큰을 받지 못한 채 대기열에서도 사라진다. 이 사용자들은 position 조회 시 404를 받게 되며, enter로 재진입해야 하지만 순번이 후순으로 밀려 불공정해진다.
운영 관점에서 Redis 일시 장애 시 이 시나리오가 현실적으로 발생할 수 있다.
🔧 제안: 사용자별 예외 처리 및 재큐잉
`@Scheduled`(fixedDelayString = "${loopers.queue.scheduler.interval-ms:100}")
public void issueTokens() {
try {
List<Long> admitted = waitingQueueRepository.popMin(batchSize);
for (Long userId : admitted) {
- entryTokenStore.issue(userId);
+ try {
+ entryTokenStore.issue(userId);
+ } catch (Exception e) {
+ log.error("토큰 발급 실패, 대기열에 재진입: userId={}", userId, e);
+ waitingQueueRepository.enter(userId);
+ }
}
} catch (Exception e) {
log.error("토큰 발급 스케줄러 실행 중 오류", e);
}
}추가 테스트: entryTokenStore.issue가 특정 userId에서 예외를 던지는 시나리오에서, 해당 사용자가 대기열에 재진입하는지 검증하는 케이스가 필요하다.
📝 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
| @Scheduled(fixedDelayString = "${loopers.queue.scheduler.interval-ms:100}") | |
| public void issueTokens() { | |
| try { | |
| List<Long> admitted = waitingQueueRepository.popMin(batchSize); | |
| for (Long userId : admitted) { | |
| entryTokenStore.issue(userId); | |
| } | |
| } catch (Exception e) { | |
| // ★ @Scheduled 메서드에서 예외가 밖으로 나가면 이후 실행이 조용히 취소된다. | |
| // 반드시 여기서 삼켜서 스케줄러가 계속 돌게 한다. | |
| log.error("토큰 발급 스케줄러 실행 중 오류", e); | |
| } | |
| } | |
| `@Scheduled`(fixedDelayString = "${loopers.queue.scheduler.interval-ms:100}") | |
| public void issueTokens() { | |
| try { | |
| List<Long> admitted = waitingQueueRepository.popMin(batchSize); | |
| for (Long userId : admitted) { | |
| try { | |
| entryTokenStore.issue(userId); | |
| } catch (Exception e) { | |
| log.error("토큰 발급 실패, 대기열에 재진입: userId={}", userId, e); | |
| waitingQueueRepository.enter(userId); | |
| } | |
| } | |
| } catch (Exception e) { | |
| // ★ `@Scheduled` 메서드에서 예외가 밖으로 나가면 이후 실행이 조용히 취소된다. | |
| // 반드시 여기서 삼켜서 스케줄러가 계속 돌게 한다. | |
| log.error("토큰 발급 스케줄러 실행 중 오류", e); | |
| } | |
| } |
🤖 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/TokenIssueScheduler.java`
around lines 30 - 42, TokenIssueScheduler.issueTokens must prevent one failed
token issuance from losing users removed by popMin. Handle exceptions per user
inside the admitted-user loop, re-enqueue the failed user through the
waiting-queue repository while preserving queue semantics, and log the failure
without aborting processing for remaining users. Add a test covering
entryTokenStore.issue failing for a specific user and verifying that user is
re-enqueued.
Comment on lines
36
to
+39
| OrderInfo info = orderFacade.placeOrder(request.toCriteria(userId)); | ||
|
|
||
| // ③ 응용 출력(OrderInfo) → API 응답(OrderResponse) 으로 변환 | ||
| // ④ 주문 성공 후 입장 토큰 소진(삭제) | ||
| queueFacade.consumeEntry(userId); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
주문 성공 후 consumeEntry() 실패 시 중복 주문 위험
placeOrder() 트랜잭션이 커밋된 후 consumeEntry()에서 Redis 오류가 발생하면, 사용자는 500 에러를 받지만 주문은 완료된 상태가 된다. 토큰이 소진되지 않았으므로 사용자가 재시도하면 동일한 토큰으로 중복 주문이 발생한다.
운영 관점: 주문 데이터 정합성이 깨지고, 사용자는 에러를 보고 재시도하여 중복 결제가 발생한다. validateEntry() 시점에는 Redis가 정상이었으므로 consumeEntry() 시점의 일시적 장애가 원인이다.
수정안: consumeEntry()를 try-catch로 감싸고, 소진 실패 시 로깅만 수행한 후 주문 성공 응답을 반환한다. 토큰이 남아있더라도 중복 주문보다 낫다.
🔧 proposed fix
import lombok.extern.slf4j.Slf4j;
+@Slf4j
`@RequiredArgsConstructor`
`@RestController`
`@RequestMapping`("/api/v1/orders")
public class OrderV1Controller implements OrderV1ApiSpec {
private final OrderFacade orderFacade;
private final QueueFacade queueFacade;
`@PostMapping`
`@Override`
public ApiResponse<OrderV1Dto.OrderResponse> createOrder(
`@RequestHeader`(value = "X-Loopers-UserId", required = false) Long userId,
`@RequestHeader`(value = "X-Entry-Token", required = false) String entryToken,
`@RequestBody` OrderV1Dto.OrderRequest request
) {
if (userId == null) {
throw new CoreException(ErrorType.BAD_REQUEST, "X-Loopers-UserId 헤더가 필요합니다.");
}
queueFacade.validateEntry(userId, entryToken);
OrderInfo info = orderFacade.placeOrder(request.toCriteria(userId));
- queueFacade.consumeEntry(userId);
+ try {
+ queueFacade.consumeEntry(userId);
+ } catch (Exception e) {
+ log.warn("입장 토큰 소진 실패 — 주문은 완료됨: userId={}", userId, e);
+ }
return ApiResponse.success(OrderV1Dto.OrderResponse.from(info));
}
}추가 테스트: consumeEntry()가 예외를 던지는 상황에서 주문 응답이 200으로 반환되는지 검증한다.
📝 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
| OrderInfo info = orderFacade.placeOrder(request.toCriteria(userId)); | |
| // ③ 응용 출력(OrderInfo) → API 응답(OrderResponse) 으로 변환 | |
| // ④ 주문 성공 후 입장 토큰 소진(삭제) | |
| queueFacade.consumeEntry(userId); | |
| OrderInfo info = orderFacade.placeOrder(request.toCriteria(userId)); | |
| // ④ 주문 성공 후 입장 토큰 소진(삭제) | |
| try { | |
| queueFacade.consumeEntry(userId); | |
| } catch (Exception e) { | |
| log.warn("입장 토큰 소진 실패 — 주문은 완료됨: userId={}", userId, e); | |
| } |
🤖 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/api/order/OrderV1Controller.java`
around lines 36 - 39, 주문 생성 후 OrderV1Controller의 consumeEntry() 실패가 주문 성공 응답을
500으로 바꾸지 않도록 수정하세요. consumeEntry(userId)를 try-catch로 감싸고 예외 발생 시 사용자 응답은 정상 주문
결과로 반환하되, 예외 정보와 사용자 식별자를 포함해 로깅하세요. consumeEntry() 예외 상황에서도 주문 응답이 200으로 반환되는
테스트를 추가하세요.
Comment on lines
+42
to
+66
| // act — 100명이 동시에 진입 (드문 커넥션 타임아웃은 인프라성이라 집계만) | ||
| for (int i = 1; i <= n; i++) { | ||
| long userId = i; | ||
| pool.submit(() -> { | ||
| try { | ||
| waitingQueueRepository.enter(userId); | ||
| } catch (Throwable t) { | ||
| errors.add(t); | ||
| } finally { | ||
| latch.countDown(); | ||
| } | ||
| }); | ||
| } | ||
| latch.await(); | ||
| pool.shutdown(); | ||
|
|
||
| // assert — 성공한 진입만큼 정확히, 각자 유일한 순번으로 반영된다 (ZADD 원자성) | ||
| int expected = n - errors.size(); | ||
| Set<Long> ranks = new HashSet<>(); | ||
| for (long u = 1; u <= n; u++) { | ||
| waitingQueueRepository.findRank(u).ifPresent(ranks::add); | ||
| } | ||
| assertThat(waitingQueueRepository.size()).isEqualTo(expected); // 유실 없음 | ||
| assertThat(ranks).hasSize(expected); // 중복 없음 (0..expected-1) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
latch.await() 타임아웃 부재와 에러 임계값 미검사로 인한 테스트 신뢰성 저하.
두 가지 문제가 있다:
-
라인 55
latch.await()타임아웃 없음: Redis 통신이 hang되면 테스트가 무한 대기한다. CI 파이프라인이 멈추고 후속 테스트가 실행되지 않으며,pool.shutdown()도 도달하지 못해 스레드 풀이 누수된다. -
라인 59 에러 임계값 미검사:
expected = n - errors.size()로 보정만 하고, 에러가 100건이어도expected=0이 되어size==0,ranks.isEmpty()로 테스트가 통과한다. 동시성 버그나 Redis 장애가 마스킹된다.
운영 관점 문제: CI가 hang되면 배포 파이프라인 전체가 차단된다. 또한 테스트가 통과한 채로 버그가 방치되면 프로덕션에서 동시성 결함이 발견되지 않는다.
수정안:
🔧 타임아웃 추가 및 에러 임계값 검사
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
// act — 100명이 동시에 진입 (드문 커넥션 타임아웃은 인프라성이라 집계만)
for (int i = 1; i <= n; i++) {
long userId = i;
pool.submit(() -> {
try {
waitingQueueRepository.enter(userId);
} catch (Throwable t) {
errors.add(t);
} finally {
latch.countDown();
}
});
}
- latch.await();
+ boolean allFinished = latch.await(30, TimeUnit.SECONDS);
+ assertThat(allFinished)
+ .as("30초 내에 모든 태스크가 완료되어야 한다")
+ .isTrue();
pool.shutdown();
+ pool.awaitTermination(5, TimeUnit.SECONDS);
// assert — 성공한 진입만큼 정확히, 각자 유일한 순번으로 반영된다 (ZADD 원자성)
- int expected = n - errors.size();
+ assertThat(errors)
+ .as("인프라성 예외 외에는 에러가 없어야 한다")
+ .isEmpty();
+ int expected = n; // errors가 비어 있으므로 expected == n추가 테스트: Redis 연결을 의도적으로 차단한 상태에서 테스트를 실행하여, 타임아웃으로 명확히 실패하는지 확인한다.
📝 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
| // act — 100명이 동시에 진입 (드문 커넥션 타임아웃은 인프라성이라 집계만) | |
| for (int i = 1; i <= n; i++) { | |
| long userId = i; | |
| pool.submit(() -> { | |
| try { | |
| waitingQueueRepository.enter(userId); | |
| } catch (Throwable t) { | |
| errors.add(t); | |
| } finally { | |
| latch.countDown(); | |
| } | |
| }); | |
| } | |
| latch.await(); | |
| pool.shutdown(); | |
| // assert — 성공한 진입만큼 정확히, 각자 유일한 순번으로 반영된다 (ZADD 원자성) | |
| int expected = n - errors.size(); | |
| Set<Long> ranks = new HashSet<>(); | |
| for (long u = 1; u <= n; u++) { | |
| waitingQueueRepository.findRank(u).ifPresent(ranks::add); | |
| } | |
| assertThat(waitingQueueRepository.size()).isEqualTo(expected); // 유실 없음 | |
| assertThat(ranks).hasSize(expected); // 중복 없음 (0..expected-1) | |
| } | |
| // act — 100명이 동시에 진입 (드문 커넥션 타임아웃은 인프라성이라 집계만) | |
| for (int i = 1; i <= n; i++) { | |
| long userId = i; | |
| pool.submit(() -> { | |
| try { | |
| waitingQueueRepository.enter(userId); | |
| } catch (Throwable t) { | |
| errors.add(t); | |
| } finally { | |
| latch.countDown(); | |
| } | |
| }); | |
| } | |
| boolean allFinished = latch.await(30, TimeUnit.SECONDS); | |
| assertThat(allFinished) | |
| .as("30초 내에 모든 태스크가 완료되어야 한다") | |
| .isTrue(); | |
| pool.shutdown(); | |
| pool.awaitTermination(5, TimeUnit.SECONDS); | |
| // assert — 성공한 진입만큼 정확히, 각자 유일한 순번으로 반영된다 (ZADD 원자성) | |
| assertThat(errors) | |
| .as("인프라성 예외 외에는 에러가 없어야 한다") | |
| .isEmpty(); | |
| int expected = n; // errors가 비어 있으므로 expected == n | |
| Set<Long> ranks = new HashSet<>(); | |
| for (long u = 1; u <= n; u++) { | |
| waitingQueueRepository.findRank(u).ifPresent(ranks::add); | |
| } | |
| assertThat(waitingQueueRepository.size()).isEqualTo(expected); // 유실 없음 | |
| assertThat(ranks).hasSize(expected); // 중복 없음 (0..expected-1) | |
| } |
🤖 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/infrastructure/queue/QueueConcurrencyTest.java`
around lines 42 - 66, Update QueueConcurrencyTest to await the latch with a
finite timeout and ensure the executor is shut down in a finally block, failing
the test if workers do not finish. Add an explicit acceptable error threshold
before calculating expected results, so excessive or complete worker failures
fail the test instead of being masked by errors.size().
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
문제 정의
POST /orders)가 대기열 없이 바로 호출된다. 블랙프라이데이 같은 피크에는 이 앞단에 입장 토큰 관문(토큰 있는 유저만 통과)을 둬야 하는데, 그 검증 로직을 어디에 둘지가 문제였다.선택지와 결정
preHandle에서 토큰 검증,afterCompletion에서 성공 시 삭제. 주문 코드는 손대지 않음. (횡단 관심사의 정석)OrderV1Controller안에서검증 → 주문 → 삭제를 직접 호출.QueueFacade에 결합되고, 잘 돌던 주문 코드를 수정했으며, 다른 API에 관문을 재사용하기 어려워졌다.loopers.queue.gate.enabled토글로 만들어, 평소·Redis 장애 시엔 꺼서 우회(bypass)할 수 있게 했다.🤔 고민한 점 / 막혔던 부분
preHandle/afterCompletion)으로 갈라지는 게 어색하게 느껴진 것도 인라인을 택한 이유 중 하나였다.🙋 기타