Conversation
…g and Redisson lock
- Replace pessimistic DB lock (findByIdForUpdate) with Redisson distributed lock
- Add ROOM_SUBMIT_EVENTS topic to KafkaTopics
- Create RoomSubmitEventMessage DTO record with eventId idempotency key
- Implement RoomSubmitKafkaProducer in momogo-core for async event publishing
- Implement RoomSubmitKafkaConsumer in momogo-api with Redis deduplication
- Refactor RoomServiceImpl.submitRoomAnswer to delegate DB writes to Kafka
…rocessing and Redisson lock
- Replace pessimistic DB lock with Redisson distributed lock outside DB transaction
- Prevent TOCTOU duplicate submissions using Redis claim markers (setIfAbsent)
- Enforce synchronous producer ACK wait with 3-second timeout for message durability
- Implement RoomSubmitKafkaConsumer with atomic Redis SETNX deduplication and rollback cleanup
- Validate problem room ownership and count synchronously in Service and Consumer
- Throw NOT_ROOM_PARTICIPANT exception on missing RoomUser during attendance update
- Add LOCK_ACQUISITION_FAILED error code to RoomErrorCode for domain boundary isolation
- Add non-null invariant checks to RoomSubmitEventMessage compact constructor
…kafka-redis refactor(room): 답안 제출 API 성능 개편 (Kafka 비동기 처리 & Redisson 분산 락 적용)
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthrough방 답안 제출을 동기 저장 방식에서 Kafka 기반 비동기 처리 방식으로 변경했습니다. Redisson 분산 락과 Redis 클레임으로 중복 제출을 제어하고, 소비자에서 답안 저장과 응시 완료 상태 갱신을 수행합니다. Changes방 답안 제출 비동기 처리
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RoomServiceImpl
participant Redis
participant RoomSubmitKafkaProducer
participant Kafka
participant RoomSubmitKafkaConsumer
participant Database
Client->>RoomServiceImpl: 답안 제출 요청
RoomServiceImpl->>Redis: 분산 락 획득 및 중복 클레임
RoomServiceImpl->>RoomSubmitKafkaProducer: RoomSubmitEventMessage 발행
RoomSubmitKafkaProducer->>Kafka: room-submit-events 전송
Kafka->>RoomSubmitKafkaConsumer: 제출 이벤트 전달
RoomSubmitKafkaConsumer->>Redis: eventId 멱등성 키 선점
RoomSubmitKafkaConsumer->>Database: 문제와 참여자 검증
RoomSubmitKafkaConsumer->>Database: 답안 저장 및 응시 완료 갱신
RoomSubmitKafkaConsumer->>Redis: 커밋 완료 상태 저장
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 3
🧹 Nitpick comments (5)
momogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java (2)
268-333: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff제안: 락 처리와 도메인 검증을 분리하세요.
submitRoomAnswer는 약 78줄이고 중첩이 4단계입니다. 현재 한 메서드가 세 가지 관심사를 모두 담당합니다.
- 요청 유효성 검증
- 분산 락 획득과 해제, 인터럽트 처리
- 도메인 검증과 이벤트 발행
락 획득 코드는 다른 기능에서도 반복될 가능성이 큽니다. 락 실행을 재사용 가능한 헬퍼로 추출하면 중첩이 줄고 각 메서드의 책임이 명확해집니다. 이것은 단일 책임 원칙(SRP)의 적용 사례입니다.
- 장점: 가독성이 오르고 도메인 검증만 단위 테스트하기 쉬워집니다. 락 정책을 한 곳에서 바꿀 수 있습니다.
- 단점: 클래스가 하나 늘어납니다. 변경 범위가 이 PR보다 커집니다. 다음 작업으로 미뤄도 됩니다.
♻️ 제안 구조
`@Component` `@RequiredArgsConstructor` public class DistributedLockExecutor { private final RedissonClient redissonClient; public void runWithLock(String lockKey, long waitSeconds, Runnable action) { RLock lock = redissonClient.getLock(lockKey); try { if (!lock.tryLock(waitSeconds, TimeUnit.SECONDS)) { throw new BusinessException(RoomErrorCode.LOCK_ACQUISITION_FAILED); } try { action.run(); } finally { if (lock.isHeldByCurrentThread()) { lock.unlock(); } } } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new BusinessException(GlobalErrorCode.INTERNAL_SERVER_ERROR, "락 대기 중 인터럽트가 발생했습니다."); } } }호출부:
lockExecutor.runWithLock(SUBMIT_LOCK_PREFIX + roomId + ":" + userId, 3, () -> validateAndPublishSubmit(userId, roomId, request));참고로
leaseTime을 생략한tryLock사용과isHeldByCurrentThread()확인 후unlock처리는 정확합니다. Redisson watchdog이 락을 자동 갱신합니다.🤖 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 `@momogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java` around lines 268 - 333, Extract the Redisson lock acquisition, timeout handling, safe unlock, and InterruptedException handling from submitRoomAnswer into a reusable DistributedLockExecutor.runWithLock helper. Update submitRoomAnswer to pass its lock key, wait duration, and domain operation to that helper, leaving validation, claim handling, and event publishing in a separate method or lambda while preserving the existing lock and error behavior.
101-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedis claim 키 접두사가 두 모듈에 중복 정의되어 있습니다. 공용 상수가 없어서 같은 문자열
"room:submit:claimed:"가 서비스와 컨슈머에 각각 존재합니다. 한쪽만 변경하면 컨슈머의 롤백 보상이 다른 키를 삭제하고, claim이 7일 동안 남아 사용자가 재제출하지 못합니다. 컴파일 오류가 발생하지 않으므로 운영에서만 드러납니다.
momogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java#L101-L103:SUBMIT_LOCK_PREFIX와SUBMIT_CLAIM_PREFIX를momogo-core의 공용 상수 클래스(예:RoomRedisKeys)로 옮기고 그 상수를 참조하세요.momogo-api/src/main/java/com/momogo/api/room/consumer/RoomSubmitKafkaConsumer.java#L71-L77: Line 75의"room:submit:claimed:"리터럴과 Line 41의DEDUP_KEY_PREFIX를 같은 공용 상수 클래스 참조로 교체하세요.
KafkaTopics가 이미 토픽명을 두 모듈에서 공유하는 방식과 동일한 패턴입니다.🤖 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 `@momogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java` around lines 101 - 103, 중복된 Redis 키 접두사를 공용 상수로 통합하세요. momogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java:101-103에서는 SUBMIT_LOCK_PREFIX와 SUBMIT_CLAIM_PREFIX를 공용 RoomRedisKeys 같은 상수 클래스로 이동하고 참조하세요. momogo-api/src/main/java/com/momogo/api/room/consumer/RoomSubmitKafkaConsumer.java:71-77에서는 `"room:submit:claimed:"` 리터럴과 DEDUP_KEY_PREFIX를 동일한 공용 상수 클래스 참조로 교체하세요.momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/RoomSubmitKafkaProducer.java (2)
28-32: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win제안: 파티션 키를
roomId에서roomId:userId로 바꾸는 것을 검토하세요.현재 파티션 키는
roomId입니다. 한 시험방의 모든 제출이 하나의 파티션으로 몰립니다. 응시자 수가 많은 방에서 제출 시각이 겹치면 해당 파티션에 부하가 집중되고 컨슈머 병렬 처리 효과가 사라집니다.이 흐름에서 순서 보장이 필요한 단위는 "방 전체"가 아니라 "방+응시자" 조합입니다. 한 응시자의 제출은 한 번만 발생하며, 컨슈머는
eventId기반 멱등성으로 중복을 차단합니다. 따라서 키를 세분화해도 정합성은 유지됩니다.
- 장점: 파티션 분산이 균등해집니다. 컨슈머 스케일아웃 효과가 커집니다.
- 단점: 방 단위 전역 순서가 필요해지는 요구사항이 생기면 다시 설계해야 합니다.
♻️ 제안 코드
SendResult<String, Object> result = kafkaTemplate.send( KafkaTopics.ROOM_SUBMIT_EVENTS, - message.roomId().toString(), + message.roomId() + ":" + message.userId(), message ).get(3, TimeUnit.SECONDS);🤖 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 `@momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/RoomSubmitKafkaProducer.java` around lines 28 - 32, Update the Kafka partition key in RoomSubmitKafkaProducer’s send flow from message.roomId() to a composite key containing both roomId and userId, using the project’s existing identifier accessors and a consistent delimiter. Preserve the existing topic, message payload, timeout, and result handling.
36-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value제안: 예외 처리를 구체 타입으로 분리하세요.
현재
catch (Exception ex)는InterruptedException,ExecutionException,TimeoutException을 한 번에 잡고, 이후instanceof로 인터럽트를 다시 판별합니다. 동작은 정확합니다. 다만 다음 두 가지 개선점이 있습니다.
- 광범위 catch는 이후 이 메서드에 로직이 추가될 때 의도하지 않은 런타임 예외까지 삼킵니다.
- Java 7 이후의 multi-catch를 쓰면
instanceof분기가 필요 없습니다.또한 호출 순서상 중요한 점이 있습니다.
RoomServiceImpl은 Redis claim을 선점한 뒤 이 메서드를 호출합니다. 이 메서드가 예외를 던지면 claim이 그대로 남습니다. 보상 처리는RoomServiceImpl쪽 코멘트에서 함께 설명합니다.♻️ 제안 코드
- } catch (Exception ex) { - log.error("[RoomSubmitKafkaProducer] 카프카 메시지 전송 실패 - eventId: {}, roomId: {}, topic: {}", - message.eventId(), message.roomId(), KafkaTopics.ROOM_SUBMIT_EVENTS, ex); - if (ex instanceof InterruptedException) { - Thread.currentThread().interrupt(); - } - throw new BusinessException(GlobalErrorCode.INTERNAL_SERVER_ERROR, "답안 제출 전송 실패"); - } + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + log.error("[RoomSubmitKafkaProducer] 카프카 메시지 전송 대기 중 인터럽트 - eventId: {}, roomId: {}", + message.eventId(), message.roomId(), ex); + throw new BusinessException(GlobalErrorCode.INTERNAL_SERVER_ERROR, "답안 제출 전송 실패"); + } catch (ExecutionException | TimeoutException ex) { + log.error("[RoomSubmitKafkaProducer] 카프카 메시지 전송 실패 - eventId: {}, roomId: {}, topic: {}", + message.eventId(), message.roomId(), KafkaTopics.ROOM_SUBMIT_EVENTS, ex); + throw new BusinessException(GlobalErrorCode.INTERNAL_SERVER_ERROR, "답안 제출 전송 실패"); + }import 추가가 필요합니다.
import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException;🤖 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 `@momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/RoomSubmitKafkaProducer.java` around lines 36 - 43, Update the exception handling in the producer method containing the shown catch block to use specific catches for InterruptedException, ExecutionException, and TimeoutException, using multi-catch where appropriate. Preserve thread interruption for InterruptedException, retain the existing error logging and BusinessException conversion, and add the required concurrent exception imports.momogo-core/src/main/java/com/momogo/core/domain/room/event/RoomSubmitEventMessage.java (1)
26-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value제안:
OffsetDateTime.now()의 시간대 의존을 제거하세요.
OffsetDateTime.now()는 JVM 기본 시간대를 사용합니다. 서버 인스턴스마다 TZ 설정이 다르면 같은 순간의 이벤트에 서로 다른 오프셋이 기록됩니다. 컨테이너 환경에서는 이 값이 배포마다 달라질 수 있습니다.개선 방법은 두 가지입니다.
- 간단한 방법:
OffsetDateTime.now(ZoneOffset.UTC)로 고정합니다. 장점은 변경 범위가 작습니다. 단점은 테스트에서 시간 고정이 어렵습니다.- 권장 방법:
Clock을 파라미터로 받습니다. 장점은 테스트에서Clock.fixed로 시간을 고정할 수 있습니다. 단점은 호출부 시그니처가 바뀝니다.♻️ 간단한 방법 예시
public static RoomSubmitEventMessage of(UUID userId, UUID roomId, List<ProblemAnswerRequest> answers) { - return new RoomSubmitEventMessage(UUID.randomUUID(), userId, roomId, answers, OffsetDateTime.now()); + return new RoomSubmitEventMessage(UUID.randomUUID(), userId, roomId, answers, OffsetDateTime.now(ZoneOffset.UTC)); }🤖 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 `@momogo-core/src/main/java/com/momogo/core/domain/room/event/RoomSubmitEventMessage.java` around lines 26 - 28, Update the RoomSubmitEventMessage.of factory to avoid the JVM default timezone by obtaining the event timestamp with an explicit UTC offset, using OffsetDateTime.now(ZoneOffset.UTC). Keep the existing factory signature and other generated event values unchanged.
🤖 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
`@momogo-api/src/main/java/com/momogo/api/room/consumer/RoomSubmitKafkaConsumer.java`:
- Around line 102-119: Update the submission flow around
userRoomAnswerRepository.saveAll and RoomUserRepository.findById so
findByIdForUpdate() locks the participant row before checking existing answers,
then detect and return for duplicate submissions before attempting persistence.
Remove the DataIntegrityViolationException catch and its import, avoiding
commit-time constraint failures and unnecessary Kafka retries/DLT handling while
preserving normal attendance updates for new submissions.
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java`:
- Around line 299-302: RoomServiceImpl의 claim 선점 실패 분기에서 ALREADY_ENDED 대신
RoomErrorCode에 전용 ALREADY_SUBMITTED 오류 코드를 추가하고 사용하세요. 해당 코드는 중복 답안 제출을 나타내는
4013, CONFLICT 응답과 “이미 답안을 제출한 시험입니다.” 메시지를 사용하며, 기존 DUPLICATE_ANSWER_SUBMITTED의
의미는 변경하지 마세요.
- Around line 296-318: Move the SUBMIT_CLAIM_PREFIX claim acquisition in the
room submission flow to after the problem ownership validation completes, so
invalid roomProblemId requests never create a claim. Then wrap
roomSubmitKafkaProducer.send in a try/catch and delete the corresponding
claimKey when sending fails before rethrowing the original exception, preserving
the existing successful-send and duplicate-claim behavior.
---
Nitpick comments:
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/event/RoomSubmitEventMessage.java`:
- Around line 26-28: Update the RoomSubmitEventMessage.of factory to avoid the
JVM default timezone by obtaining the event timestamp with an explicit UTC
offset, using OffsetDateTime.now(ZoneOffset.UTC). Keep the existing factory
signature and other generated event values unchanged.
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/RoomSubmitKafkaProducer.java`:
- Around line 28-32: Update the Kafka partition key in RoomSubmitKafkaProducer’s
send flow from message.roomId() to a composite key containing both roomId and
userId, using the project’s existing identifier accessors and a consistent
delimiter. Preserve the existing topic, message payload, timeout, and result
handling.
- Around line 36-43: Update the exception handling in the producer method
containing the shown catch block to use specific catches for
InterruptedException, ExecutionException, and TimeoutException, using
multi-catch where appropriate. Preserve thread interruption for
InterruptedException, retain the existing error logging and BusinessException
conversion, and add the required concurrent exception imports.
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java`:
- Around line 268-333: Extract the Redisson lock acquisition, timeout handling,
safe unlock, and InterruptedException handling from submitRoomAnswer into a
reusable DistributedLockExecutor.runWithLock helper. Update submitRoomAnswer to
pass its lock key, wait duration, and domain operation to that helper, leaving
validation, claim handling, and event publishing in a separate method or lambda
while preserving the existing lock and error behavior.
- Around line 101-103: 중복된 Redis 키 접두사를 공용 상수로 통합하세요.
momogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java:101-103에서는
SUBMIT_LOCK_PREFIX와 SUBMIT_CLAIM_PREFIX를 공용 RoomRedisKeys 같은 상수 클래스로 이동하고 참조하세요.
momogo-api/src/main/java/com/momogo/api/room/consumer/RoomSubmitKafkaConsumer.java:71-77에서는
`"room:submit:claimed:"` 리터럴과 DEDUP_KEY_PREFIX를 동일한 공용 상수 클래스 참조로 교체하세요.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c1cf1866-de3a-481e-a947-5c865bebf56a
📒 Files selected for processing (6)
momogo-api/src/main/java/com/momogo/api/room/consumer/RoomSubmitKafkaConsumer.javamomogo-core/src/main/java/com/momogo/core/common/config/KafkaTopics.javamomogo-core/src/main/java/com/momogo/core/domain/room/event/RoomSubmitEventMessage.javamomogo-core/src/main/java/com/momogo/core/domain/room/exception/RoomErrorCode.javamomogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/RoomSubmitKafkaProducer.javamomogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java
…g, Redisson lock, and Redis claim marker
- Extract DistributedLockExecutor to decouple locking concerns from domain service
- Add RoomRedisKeys constant class to centralize Redis key prefixes across modules
- Move Redis claim marker (setIfAbsent) after validation to prevent claim leak
- Add compensation deletion of claim key on Kafka producer send failures
- Change Kafka partition key to roomId:userId for even consumer load distribution
- Handle Kafka producer exceptions explicitly (InterruptedException vs Execution/TimeoutException)
- Fix consumer to pre-check isAttended before saveAll to prevent transaction rollback-only errors
- Add ALREADY_SUBMITTED error code (HTTP 409) to RoomErrorCode to replace misleading ALREADY_ENDED
- Fix RoomSubmitEventMessage timestamp to use ZoneOffset.UTC
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.
작업 내용
변경 사항
체크리스트
참고 사항
관련 이슈
Summary by CodeRabbit
새로운 기능
버그 수정