[volume-8] Redis 기반 주문 대기열 구현 - #362
Merged
Merged
Conversation
📝 Walkthrough변경 목적: 주문 API 폭주 시 Redis 기반 대기열로 진입을 제어해 DB/재고/결제 하류에 대한 부담을 줄이기 위한 기능을 추가했습니다. WalkthroughRedis 기반 주문 대기열(waiting-queue)과 입장 토큰(entry-token) 도메인이 새로 추가됐다. 대기열 진입/순번 조회 API, 토큰 발급 스케줄러, 주문 생성 게이트 인터셉터가 구현됐고, 로그인 헤더 인증 로직은 HeaderAuthenticator로 통합됐다. 관련 설정, 통합/E2E 테스트, 설계 문서도 함께 추가됐다. Changes대기열 및 주문 게이트 기능
Sequence Diagram(s)sequenceDiagram
participant Client
participant QueueV1Controller
participant QueueApplicationService
participant EntryTokenScheduler
participant EntryTokenInterceptor
participant OrderController
Client->>QueueV1Controller: POST /queue/enter
QueueV1Controller->>QueueApplicationService: enter(userId)
QueueApplicationService-->>Client: waiting position
loop 폴링
Client->>QueueV1Controller: GET /queue/position
QueueV1Controller->>QueueApplicationService: getPosition(userId)
QueueApplicationService-->>Client: position / token
end
EntryTokenScheduler->>EntryTokenScheduler: 주기적 토큰 발급
Client->>OrderController: POST /orders (X-Entry-Token)
OrderController->>EntryTokenInterceptor: preHandle
EntryTokenInterceptor->>EntryTokenInterceptor: 토큰 검증 + in-flight 선점
EntryTokenInterceptor-->>OrderController: 통과 또는 403/409
OrderController-->>Client: 주문 결과
🚥 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: 2
🧹 Nitpick comments (5)
docs/week2/01-requirements.md (1)
426-427: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win100ms를 고정값처럼 쓰지 말아야 한다.
현재 문서는 발급 속도를 런타임 설정으로 역산한다고 해놓고 바로 100ms를 사실상 상수처럼 적고 있다. 실제 구현은
queue.scheduler.interval-ms로 바뀌므로, 이 표현을 남기면 운영에서 간격을 조정할 때 문서와 TPS 산정이 어긋난다.100ms는 기본값 또는 예시로 낮추고, 실제 기준은 설정값이라고 명시해야 한다. 스케줄러 바인딩 테스트에서 interval 이 설정에서 읽히는지도 함께 고정해야 한다.♻️ 제안 수정
- 발급 간격은 100ms로 분산한다. + 발급 간격은 설정값으로 분산한다(기본 예시는 100ms다).🤖 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 `@docs/week2/01-requirements.md` around lines 426 - 427, The AC-37-2 wording is treating 100ms as a fixed requirement even though the scheduler cadence is meant to come from runtime settings. Update the requirement text in the relevant AC-37 entries to make `queue.scheduler.interval-ms` the source of truth, and rephrase 100ms as only a default/example if needed; also note that the scheduler binding/test coverage should verify the interval is read from configuration, not hardcoded.apps/commerce-api/src/main/java/com/loopers/interfaces/api/config/EntryTokenInterceptor.java (1)
62-96: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win게이트 로직은 견고하나, Redis 동기 호출이 주문 경로의 단일 실패 지점이 될 수 있다.
find/acquireInFlight/delete/releaseInFlight가 모두 주문 요청 스레드에서 동기로 실행되며 타임아웃·대체 흐름이 없다. Redis 장애나 지연 시 주문 생성 전체가 함께 지연되고 요청 스레드가 묶여 가용성이 저하될 수 있다. 대기열은 DB 를 보호하는 장치이지만, 이 장치 자체가 장애 지점이 되면 보호 대상보다 먼저 무너진다.PR 문서에 graceful degradation 을 향후 과제로 명시한 점은 확인했다. 병합 전 최소한 (1) Redis 커맨드 타임아웃, (2) Redis 미가용 시 fail-open/fail-closed 정책 결정과 근거, (3) 해당 정책을 검증하는 테스트 추가를 권장한다.
🤖 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/config/EntryTokenInterceptor.java` around lines 62 - 96, The entry-token gate in EntryTokenInterceptor currently depends on synchronous Redis calls in the request thread, so add explicit timeout handling and a clear fallback policy for Redis unavailability. Update the find, acquireInFlight, delete, and releaseInFlight paths to respect a configured Redis command timeout and decide whether the interceptor should fail-open or fail-closed when Redis is down, then apply that policy consistently in preHandle and afterCompletion. Add tests around EntryTokenInterceptor and entryTokenRepository behavior to verify timeout and outage scenarios follow the chosen policy.apps/commerce-api/src/test/java/com/loopers/domain/queue/QueuePolicyTest.java (1)
14-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win경계값·예외 케이스 보강 필요
밴드 경계값은 잘 커버하고 있으나, 다음 케이스가 누락됐다:
- 빈 밴드 리스트: 모든 position →
defaultIntervalMs반환 검증position=0: 입장 완료 상태의estimatedWaitSeconds(0)=0,pollAfterMillis(0)동작 검증- 생성자 검증 실패 (위
QueuePolicy검증 추가 후):batchSize=0또는intervalMs=0설정 시IllegalArgumentException발생 검증🧪 제안 추가 테스트
+ `@Nested` + `@DisplayName`("빈 밴드 — 모든 순번이 기본 주기") + class EmptyBands { + private final QueuePolicy emptyBandsPolicy = new QueuePolicy(new QueueProperties( + true, 300, + new QueueProperties.Scheduler(100, 14), + new QueueProperties.Polling(List.of(), 5000) + )); + + `@Test` + void returnsDefaultForAnyPosition() { + assertThat(emptyBandsPolicy.pollAfterMillis(1)).isEqualTo(5000); + assertThat(emptyBandsPolicy.pollAfterMillis(99999)).isEqualTo(5000); + } + } + + `@Nested` + `@DisplayName`("잘못된 설정 — 생성자 검증") + class InvalidConfig { + `@Test` + void rejectsZeroBatchSize() { + assertThatThrownBy(() -> new QueuePolicy(new QueueProperties( + true, 300, + new QueueProperties.Scheduler(100, 0), + new QueueProperties.Polling(List.of(), 5000) + ))).isInstanceOf(IllegalArgumentException.class); + } + + `@Test` + void rejectsZeroIntervalMs() { + assertThatThrownBy(() -> new QueuePolicy(new QueueProperties( + true, 300, + new QueueProperties.Scheduler(0, 14), + new QueueProperties.Polling(List.of(), 5000) + ))).isInstanceOf(IllegalArgumentException.class); + } + }🤖 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/domain/queue/QueuePolicyTest.java` around lines 14 - 63, Add missing edge-case coverage in QueuePolicyTest for QueuePolicy and QueueProperties: verify that an empty QueueProperties.Polling band list makes pollAfterMillis(position) fall back to defaultIntervalMs for any position, add position=0 assertions for estimatedWaitSeconds(0)=0 and pollAfterMillis(0), and include constructor validation tests confirming IllegalArgumentException when QueueProperties.Scheduler or QueueProperties.Polling is built with invalid values such as batchSize=0 or intervalMs=0.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/domain/queue/QueuePolicy.java (1)
38-44: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win밴드 정렬 보장 없음 — 설정 순서 오류 시 폴링 주기 왜곡
pollAfterMillis는findFirst()로 첫 번째 매칭 밴드를 반환하므로,bands가maxPosition오름차순이 아닐 경우 잘못된 폴링 주기를 반환한다. Javadoc에 전제조건을 명시했지만 런타임 보장이 없어 YAML 순서 실수가 silent bug가 된다.생성자에서
maxPosition기준 오름차순 정렬을 적용하라.♻️ 제안 리팩터링
public QueuePolicy(QueueProperties properties) { this.issueRatePerSecond = properties.scheduler().issueRatePerSecond(); - this.bands = properties.polling().bands(); + this.bands = properties.polling().bands().stream() + .sorted(Comparator.comparingLong(QueueProperties.Polling.Band::maxPosition)) + .toList(); this.defaultIntervalMs = properties.polling().defaultIntervalMs(); }추가 테스트: 밴드를 역순으로 설정해도 올바른
pollAfterMillis가 반환됨을 검증하라.🤖 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/QueuePolicy.java` around lines 38 - 44, `QueuePolicy.pollAfterMillis` currently relies on `bands.stream().findFirst()`, so the returned interval depends on list order and can be wrong when YAML order is off. Fix this by enforcing `maxPosition` ascending order in the `QueuePolicy` constructor (or immediately after `bands` is built) before any lookup logic runs, using the `bands` field and `pollAfterMillis` as the key symbols. Add a test that creates bands in reverse order and verifies `pollAfterMillis` still returns the interval for the correct `maxPosition` band.Source: Path instructions
apps/commerce-api/src/test/java/com/loopers/infrastructure/queue/EntryTokenSchedulerIntegrationTest.java (1)
48-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win발급 실패 시 행별 격리 동작에 대한 테스트가 없다.
EntryTokenScheduler는issue예외를 행별로 catch 하여 한 유저의 실패가 배치 전체를 중단시키지 않도록 설계됐다(EntryTokenScheduler.java:49-57). 그러나 이 동작을 검증하는 테스트가 없다. 운영 중 Redis 일시 장애 등으로 특정 유저의 토큰 발급이 실패할 수 있으며, 격리 동작이 회귀하면 일부 유저의 토큰 유실이 전체 배치 중단으로 확대된다.
@SpyBean등으로EntryTokenRepository를 감싸 특정 userId에서 예외를 던지도록 설정한 뒤, 이후 userId의 토큰이 정상 발급되는지 확인하는 테스트를 추가할 것을 권장한다.🧪 제안 테스트 코드
+ `@Test` + `@DisplayName`("발급 실패한 유저는 건너뛰고 나머지는 정상 발급된다 — 행별 격리 검증.") + void skipsFailedUserAndIssuesRest(`@Autowired` EntryTokenRepository real) { + for (long userId = 1; userId <= 3; userId++) { + waitingQueueRepository.enter(userId, userId * 100); + } + EntryTokenRepository spy = Mockito.spy(real); + Mockito.doThrow(new RuntimeException("Redis transient error")) + .when(spy).issue(eq(2L), anyString(), any()); + EntryTokenScheduler scheduler = new EntryTokenScheduler( + waitingQueueRepository, spy, scheduler(10).properties + ); + + scheduler.issueTokens(); + + assertThat(entryTokenRepository.find(1L)).isPresent(); + assertThat(entryTokenRepository.find(2L)).isEmpty(); // 실패한 유저 + assertThat(entryTokenRepository.find(3L)).isPresent(); // 격리 확인 + }경계값/실패 케이스/예외 흐름을 포함하는지 점검하라는 테스트 리뷰 기준에 근거한다. As per coding guidelines,
**/*Test*.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/test/java/com/loopers/infrastructure/queue/EntryTokenSchedulerIntegrationTest.java` around lines 48 - 93, Add a regression test for the per-user failure isolation in EntryTokenScheduler.issueTokens(); mock or spy EntryTokenRepository so one specific userId throws during issue, then verify later userIds in the same batch still get tokens and the queue continues processing. Place the test alongside issuesAtMostBatchSizePerTick/issuesDistinctTokens in EntryTokenSchedulerIntegrationTest and target the issue method path exercised by EntryTokenScheduler.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/domain/queue/QueuePolicy.java`:
- Around line 26-30: `QueuePolicy` 생성자에서
`QueueProperties.scheduler().issueRatePerSecond()`가 0 또는 무한대가 되지 않도록 fail-fast
검증을 추가하고, `QueueProperties.polling().defaultIntervalMs()`가 0 이하인지도 함께 확인해 설정 오류를
즉시 `IllegalArgumentException`으로 막으세요. `QueuePolicy(QueueProperties)`와
`estimatedWaitSeconds` 경로를 기준으로 문제를 잡되, `batchSize=0` 또는 `intervalMs=0` 입력 시 예외가
발생하는 테스트를 추가해 동작을 고정하세요.
In `@docs/week2/04-erd.md`:
- Around line 426-430: The `processing-token` description in the ERD should be
rewritten as a correctness guard, not just a fairness or load-shedding
mechanism. Update the `processing-token:{userId}` and `SETNX EX` explanation to
state that it is a temporary lease enforcing “one token, one order” and
preventing the TOCTOU gap between validation and consumption, with concurrent
duplicate orders returning 409. Also add a brief example or test note in the
`processing-token` / `entry-token` section showing two simultaneous orders from
the same user where only one succeeds so the intended invariant stays
documented.
---
Nitpick comments:
In `@apps/commerce-api/src/main/java/com/loopers/domain/queue/QueuePolicy.java`:
- Around line 38-44: `QueuePolicy.pollAfterMillis` currently relies on
`bands.stream().findFirst()`, so the returned interval depends on list order and
can be wrong when YAML order is off. Fix this by enforcing `maxPosition`
ascending order in the `QueuePolicy` constructor (or immediately after `bands`
is built) before any lookup logic runs, using the `bands` field and
`pollAfterMillis` as the key symbols. Add a test that creates bands in reverse
order and verifies `pollAfterMillis` still returns the interval for the correct
`maxPosition` band.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/config/EntryTokenInterceptor.java`:
- Around line 62-96: The entry-token gate in EntryTokenInterceptor currently
depends on synchronous Redis calls in the request thread, so add explicit
timeout handling and a clear fallback policy for Redis unavailability. Update
the find, acquireInFlight, delete, and releaseInFlight paths to respect a
configured Redis command timeout and decide whether the interceptor should
fail-open or fail-closed when Redis is down, then apply that policy consistently
in preHandle and afterCompletion. Add tests around EntryTokenInterceptor and
entryTokenRepository behavior to verify timeout and outage scenarios follow the
chosen policy.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/queue/QueuePolicyTest.java`:
- Around line 14-63: Add missing edge-case coverage in QueuePolicyTest for
QueuePolicy and QueueProperties: verify that an empty QueueProperties.Polling
band list makes pollAfterMillis(position) fall back to defaultIntervalMs for any
position, add position=0 assertions for estimatedWaitSeconds(0)=0 and
pollAfterMillis(0), and include constructor validation tests confirming
IllegalArgumentException when QueueProperties.Scheduler or
QueueProperties.Polling is built with invalid values such as batchSize=0 or
intervalMs=0.
In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/queue/EntryTokenSchedulerIntegrationTest.java`:
- Around line 48-93: Add a regression test for the per-user failure isolation in
EntryTokenScheduler.issueTokens(); mock or spy EntryTokenRepository so one
specific userId throws during issue, then verify later userIds in the same batch
still get tokens and the queue continues processing. Place the test alongside
issuesAtMostBatchSizePerTick/issuesDistinctTokens in
EntryTokenSchedulerIntegrationTest and target the issue method path exercised by
EntryTokenScheduler.
In `@docs/week2/01-requirements.md`:
- Around line 426-427: The AC-37-2 wording is treating 100ms as a fixed
requirement even though the scheduler cadence is meant to come from runtime
settings. Update the requirement text in the relevant AC-37 entries to make
`queue.scheduler.interval-ms` the source of truth, and rephrase 100ms as only a
default/example if needed; also note that the scheduler binding/test coverage
should verify the interval is read from configuration, not hardcoded.
🪄 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: 851db5df-9b37-4326-b309-ec29931340ef
📒 Files selected for processing (27)
apps/commerce-api/src/main/java/com/loopers/application/queue/QueueApplicationService.javaapps/commerce-api/src/main/java/com/loopers/application/queue/QueueInfo.javaapps/commerce-api/src/main/java/com/loopers/domain/queue/EntryTokenRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/queue/QueuePolicy.javaapps/commerce-api/src/main/java/com/loopers/domain/queue/WaitingQueueRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/queue/EntryTokenRedisRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/queue/EntryTokenScheduler.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/queue/WaitingQueueRedisRepository.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/auth/HeaderAuthenticator.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/auth/LoginUserArgumentResolver.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/config/EntryTokenInterceptor.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/config/WebConfig.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/config/QueueProperties.javaapps/commerce-api/src/main/resources/application.ymlapps/commerce-api/src/test/java/com/loopers/domain/queue/QueuePolicyTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/queue/EntryTokenSchedulerIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/queue/EntryTokenSchedulingWiringTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/queue/QueueRedisRepositoryIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/OrderQueueGateE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/QueueV1ApiE2ETest.javadocs/week2/01-requirements.mddocs/week2/02-sequence-diagrams.mddocs/week2/03-class-diagram.mddocs/week2/04-erd.md
Comment on lines
+26
to
+30
| public QueuePolicy(QueueProperties properties) { | ||
| this.issueRatePerSecond = properties.scheduler().issueRatePerSecond(); | ||
| this.bands = properties.polling().bands(); | ||
| this.defaultIntervalMs = properties.polling().defaultIntervalMs(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
issueRatePerSecond 0/Infinity 검증 누락 — 설정 오류 시 silent failure
intervalMs=0이면 issueRatePerSecond가 Infinity가 되어 estimatedWaitSeconds가 항상 0을 반환하고, batchSize=0이면 issueRatePerSecond=0으로 estimatedWaitSeconds가 Long.MAX_VALUE를 반환한다. 운영 중 YAML 설정 오타가 원인 불명 장애로 이어진다.
생성자에서 fail-fast 검증을 추가하라.
🔧 제안 수정안
public QueuePolicy(QueueProperties properties) {
this.issueRatePerSecond = properties.scheduler().issueRatePerSecond();
+ if (issueRatePerSecond <= 0 || Double.isInfinite(issueRatePerSecond) || Double.isNaN(issueRatePerSecond)) {
+ throw new IllegalArgumentException(
+ "scheduler.issueRatePerSecond must be positive and finite (check intervalMs > 0 and batchSize > 0)"
+ );
+ }
this.bands = properties.polling().bands();
this.defaultIntervalMs = properties.polling().defaultIntervalMs();
}추가 테스트: batchSize=0 또는 intervalMs=0인 QueueProperties로 QueuePolicy 생성 시 IllegalArgumentException 발생을 검증하라.
📝 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
| public QueuePolicy(QueueProperties properties) { | |
| this.issueRatePerSecond = properties.scheduler().issueRatePerSecond(); | |
| this.bands = properties.polling().bands(); | |
| this.defaultIntervalMs = properties.polling().defaultIntervalMs(); | |
| } | |
| public QueuePolicy(QueueProperties properties) { | |
| this.issueRatePerSecond = properties.scheduler().issueRatePerSecond(); | |
| if (issueRatePerSecond <= 0 || Double.isInfinite(issueRatePerSecond) || Double.isNaN(issueRatePerSecond)) { | |
| throw new IllegalArgumentException( | |
| "scheduler.issueRatePerSecond must be positive and finite (check intervalMs > 0 and batchSize > 0)" | |
| ); | |
| } | |
| this.bands = properties.polling().bands(); | |
| this.defaultIntervalMs = properties.polling().defaultIntervalMs(); | |
| } |
🤖 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/QueuePolicy.java`
around lines 26 - 30, `QueuePolicy` 생성자에서
`QueueProperties.scheduler().issueRatePerSecond()`가 0 또는 무한대가 되지 않도록 fail-fast
검증을 추가하고, `QueueProperties.polling().defaultIntervalMs()`가 0 이하인지도 함께 확인해 설정 오류를
즉시 `IllegalArgumentException`으로 막으세요. `QueuePolicy(QueueProperties)`와
`estimatedWaitSeconds` 경로를 기준으로 문제를 잡되, `batchSize=0` 또는 `intervalMs=0` 입력 시 예외가
발생하는 테스트를 추가해 동작을 고정하세요.
Source: Path instructions
Comment on lines
+426
to
+430
| | `processing-token:{userId}` | String (owner token) | 유저당 in-flight 주문 마크. `SETNX` 로 선점해 동시 주문을 1건으로 제한(2건째부터 409), 완료 시 값 일치할 때만 DEL(compare-and-delete) | 30s (크래시 자동 해제 안전망) | | ||
|
|
||
| - **왜 RDB 가 아닌가** — 대기열은 폭증 트래픽의 **완충 지대**다. 진입을 DB INSERT 로 받으면 보호하려는 커넥션 풀을 보호 장치가 소모한다(모순). 순번 조회는 `ZRANK` 로 대기 인원과 무관하게 싸고, 토큰 만료는 TTL 이 자연 처리해 만료 배치·상태 컬럼이 필요 없다. 데이터가 행사 종료와 함께 버려지는 휘발성이라 내구성 요구도 없다. | ||
| - **진입 멱등 = `ZADD NX`** — 재진입 시 score(진입 시각)를 갱신하지 않아 순번이 보존된다. 단순 ZADD 면 대기 화면 새로고침(=재진입)마다 줄 맨 뒤로 리셋된다. | ||
| - **동시 주문 마크를 토큰과 분리한 이유** — 토큰(`entry-token`)은 "입장 권한"이고 성공 시에만 소진(실패 보존)이라, 여기에 "처리 중" 상태를 겸하게 하면 실패 시 보존과 충돌한다. 별도 `processing-token` 을 `SETNX EX` 로 선점(선점·TTL 원자화)해 in-flight 배타를 표현하면, 토큰은 소진 규칙만 지키고 마크는 동시성만 막아 두 관심사가 독립적이다. 오버셀 자체는 재고 핫로우 비관락이 이미 막으므로, 이 마크는 정확성이 아니라 공정성·하류 증폭 방지가 목적이다. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
processing-token는 정확성 가드로 설명해야 한다.
지금 문구는 이 마크를 공정성·증폭 방지용으로만 읽히게 한다. 그러나 실제 역할은 검증-소진 사이 TOCTOU를 막고, 같은 토큰의 동시 주문을 409로 차단하는 correctness guard다. 이 의미가 빠지면 후속 정리 때 마크를 제거하거나 TTL 만으로 충분하다고 오해할 수 있다. 설명을 토큰 1장 = 주문 1건을 보장하는 임시 lease 로 바꾸고, 동일 사용자 동시 주문 2건 중 1건만 통과하는 시나리오를 테스트와 문서에 함께 고정해야 한다.
♻️ 제안 수정
- | `processing-token:{userId}` | String (owner token) | 유저당 in-flight 주문 마크. `SETNX` 로 선점해 동시 주문을 1건으로 제한(2건째부터 409), 완료 시 값 일치할 때만 DEL(compare-and-delete) | 30s (크래시 자동 해제 안전망) |
+ | `processing-token:{userId}` | String (owner token) | 유저당 in-flight 주문 lease. `SETNX` 로 선점해 검증-소진 TOCTOU를 막고, 동시 주문을 1건으로 제한(2건째부터 409)한다. 완료 시 값 일치할 때만 DEL(compare-and-delete) 하며, TTL 은 크래시 복구용 안전망이다. | 30s (크래시 자동 해제 안전망) |📝 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
| | `processing-token:{userId}` | String (owner token) | 유저당 in-flight 주문 마크. `SETNX` 로 선점해 동시 주문을 1건으로 제한(2건째부터 409), 완료 시 값 일치할 때만 DEL(compare-and-delete) | 30s (크래시 자동 해제 안전망) | | |
| - **왜 RDB 가 아닌가** — 대기열은 폭증 트래픽의 **완충 지대**다. 진입을 DB INSERT 로 받으면 보호하려는 커넥션 풀을 보호 장치가 소모한다(모순). 순번 조회는 `ZRANK` 로 대기 인원과 무관하게 싸고, 토큰 만료는 TTL 이 자연 처리해 만료 배치·상태 컬럼이 필요 없다. 데이터가 행사 종료와 함께 버려지는 휘발성이라 내구성 요구도 없다. | |
| - **진입 멱등 = `ZADD NX`** — 재진입 시 score(진입 시각)를 갱신하지 않아 순번이 보존된다. 단순 ZADD 면 대기 화면 새로고침(=재진입)마다 줄 맨 뒤로 리셋된다. | |
| - **동시 주문 마크를 토큰과 분리한 이유** — 토큰(`entry-token`)은 "입장 권한"이고 성공 시에만 소진(실패 보존)이라, 여기에 "처리 중" 상태를 겸하게 하면 실패 시 보존과 충돌한다. 별도 `processing-token` 을 `SETNX EX` 로 선점(선점·TTL 원자화)해 in-flight 배타를 표현하면, 토큰은 소진 규칙만 지키고 마크는 동시성만 막아 두 관심사가 독립적이다. 오버셀 자체는 재고 핫로우 비관락이 이미 막으므로, 이 마크는 정확성이 아니라 공정성·하류 증폭 방지가 목적이다. | |
| | `processing-token:{userId}` | String (owner token) | 유저당 in-flight 주문 lease. `SETNX` 로 선점해 검증-소진 TOCTOU를 막고, 동시 주문을 1건으로 제한(2건째부터 409)한다. 완료 시 값 일치할 때만 DEL(compare-and-delete) 하며, TTL 은 크래시 복구용 안전망이다. | 30s (크래시 자동 해제 안전망) | | |
| - **왜 RDB 가 아닌가** — 대기열은 폭증 트래픽의 **완충 지대**다. 진입을 DB INSERT 로 받으면 보호하려는 커넥션 풀을 보호 장치가 소모한다(모순). 순번 조회는 `ZRANK` 로 대기 인원과 무관하게 싸고, 토큰 만료는 TTL 이 자연 처리해 만료 배치·상태 컬럼이 필요 없다. 데이터가 행사 종료와 함께 버려지는 휘발성이라 내구성 요구도 없다. | |
| - **진입 멱등 = `ZADD NX`** — 재진입 시 score(진입 시각)를 갱신하지 않아 순번이 보존된다. 단순 ZADD 면 대기 화면 새로고침(=재진입)마다 줄 맨 뒤로 리셋된다. | |
| - **동시 주문 마크를 토큰과 분리한 이유** — 토큰(`entry-token`)은 "입장 권한"이고 성공 시에만 소진(실패 보존)이라, 여기에 "처리 중" 상태를 겸하게 하면 실패 시 보존과 충돌한다. 별도 `processing-token` 을 `SETNX EX` 로 선점(선점·TTL 원자화)해 in-flight 배타를 표현하면, 토큰은 소진 규칙만 지키고 마크는 동시성만 막아 두 관심사가 독립적이다. 오버셀 자체는 재고 핫로우 비관락이 이미 막으므로, 이 마크는 정확성이 아니라 공정성·하류 증폭 방지가 목적이다. |
🤖 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 `@docs/week2/04-erd.md` around lines 426 - 430, The `processing-token`
description in the ERD should be rewritten as a correctness guard, not just a
fairness or load-shedding mechanism. Update the `processing-token:{userId}` and
`SETNX EX` explanation to state that it is a temporary lease enforcing “one
token, one order” and preventing the TOCTOU gap between validation and
consumption, with concurrent duplicate orders returning 409. Also add a brief
example or test note in the `processing-token` / `entry-token` section showing
two simultaneous orders from the same user where only one succeeds so the
intended invariant stays documented.
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
문제 정의
현재 동작/제약:
문제(또는 리스크):
성공 기준(완료 정의):
선택지와 결정
고려한 대안:
A: Rate Limiting으로 초과 요청 거부
B: Redis Sorted Set 대기열 + 입장 토큰
최종 결정: B
트레이드오프:
추후 개선 여지:
🤔 고민한 점 / 막혔던 부분
🙋 기타