feat: 이벤트 신청, 수정 시 락 구현 #1239
feat: 이벤트 신청, 수정 시 락 구현 #1239
Conversation
📝 WalkthroughWalkthrough메서드 수준의 분산 락 인프라를 추가하고 주요 이벤트 참여·수정 메서드들에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Service as Service (annotated)
participant AOP as LockAspect
participant Lock as LockUtil (MySQL/InMemory)
participant Biz as Business Method
Client->>Service: invoke(...)
Service-->>AOP: intercepted (@DistributedLock)
AOP->>AOP: evaluate key (SpEL or default)
AOP->>Lock: acquireLock(key, timeoutSec)
alt lock acquired
Lock-->>AOP: true
AOP->>Biz: proceed()
Biz-->>AOP: result
AOP->>Lock: releaseLock(key)
Lock-->>AOP: true/false
AOP-->>Client: return result
else lock failed
Lock-->>AOP: false
AOP-->>Client: throw LOCK_ACQUIRE_FAILED
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (19)
src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1)
227-228: 메시지 일관성 및 다국어 대응 고려다른 코드들과 톤은 맞지만, 클라이언트 재시도 가이드(“잠시 후 다시 시도”)를 명확히 하려면 “잠시 후 다시 시도해주세요.”로 통일하거나, 영문 리소스 추가를 고려하세요.
src/main/java/com/gdschongik/gdsc/global/lock/DistributedLock.java (1)
17-21: 타임아웃 타입을 LockUtil과 일치시키고(현재 int vs long), 의미를 Javadoc에 명확히 표기하세요.주석상 “획득 대기 시간”으로 읽히므로 TTL이 아님을 문서에 명시하고, LockUtil.acquireLock의 파라미터 타입과 동일하게 정리하세요. 일관성이 없으면 AOP 파이프라인에서 캐스팅/오버로드 혼동을 야기할 수 있습니다.
- int timeoutSec() default 5; + int timeoutSec() default 5; // LockUtil도 int로 통일하거나 둘 다 long으로 통일src/main/java/com/gdschongik/gdsc/global/lock/LockUtil.java (2)
11-12: 타임아웃 파라미터 타입을 주석/어노테이션과 일치시켜 일관성을 확보하세요.DistributedLock.timeoutSec는 int, 여기서는 long입니다. 하나로 통일하세요. GET_LOCK의 timeout은 정수 초이므로 int로 가는 편이 자연스럽습니다.
- boolean acquireLock(String key, long timeoutSec); + boolean acquireLock(String key, int timeoutSec);추가로 “대기 시간(초), TTL 아님”을 Javadoc에 명시하면 오해를 줄일 수 있습니다.
5-10: 성공/실패 이원화만으로는 원인(타임아웃 vs DB오류)을 구분하기 어렵습니다.추후 운영 가시성을 위해 반환값을 enum(ACQUIRED, TIMEOUT, ERROR)으로 확장하거나, 실패 사유를 로깅/메트릭으로 노출하는 인터페이스 계약을 고려하세요.
src/main/java/com/gdschongik/gdsc/global/lock/MySqlLockUtil.java (4)
47-47: RELEASE_LOCK 반환값 주석이 부정확합니다.RELEASE_LOCK 결과는 1(성공), 0(해당 세션 소유 아님/이미 해제), NULL(락 존재하지 않음)입니다. ‘타임아웃’은 없습니다.
- boolean released = result != null && result == 1; // RELEASE_LOCK 결과: 1(성공), 0(타임아웃), null(에러) + boolean released = result != null && result == 1; // RELEASE_LOCK: 1(성공), 0(타 세션 소유/이미 해제), null(락 없음)
31-35: 락 로깅은 debug 수준으로 낮추고, 실패 케이스는 warn으로 구분하세요.락은 고빈도이므로 info는 로그 노이즈가 큽니다. 성공은 debug, 실패/예외는 warn이 적절합니다.
- log.info("[DistributedLock] 락 획득 성공: {}", lockName); + log.debug("[DistributedLock] 락 획득 성공: {}", lockName); - log.info("[DistributedLock] 락 획득 실패: {}", lockName); + log.warn("[DistributedLock] 락 획득 실패: {}", lockName);Also applies to: 49-53
18-20: PreparedStatement 사용 시 동일 SQL 상수 재사용은 좋습니다. 다만 GET_LOCK/RELEASE_LOCK를 함수 호출로 감싸 재사용성을 더 높일 수 있습니다.중복을 줄이기 위해 private 메서드로 분리하면 테스트/모킹이 수월합니다.
21-26: 문서에 “대기 시간”이 ‘락 보유 시간(TTL)’로 오해되지 않도록 명확히 적어두세요.MySQL named lock은 TTL이 없고 커넥션 생존 기간 동안 유지됩니다. 운영 핸드북/JavaDoc에 주의사항을 남기는 것을 권장합니다.
src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java (1)
314-316: 온라인 신청도 동일하게 정원 검증을 락 임계구역 내에서 처리하세요.수동/온라인 경로가 동일 락 키를 공유하므로 정원 체크를 중복 구현하지 않도록 도메인 서비스에 공통 메서드로 추출하는 것도 고려하세요.
src/main/java/com/gdschongik/gdsc/domain/event/application/EventService.java (1)
66-68: 업데이트 경합 보호 LGTM. 다만 timeoutSec을 명시해 운영 의도를 드러내세요.디폴트(5초)가 과하거나 부족할 수 있습니다. SLA/RT 기준에 맞춰
@DistributedLock(key = "'event:' + #eventId", timeoutSec = 2)처럼 명시를 권장합니다.src/test/java/com/gdschongik/gdsc/config/TestLockConfig.java (1)
12-16: 테스트 대체 구현 주입 LGTM.Primary로 인메모리 구현을 주입한 구성이 적절합니다. 단, 인메모리 구현이
timeoutSec을 무시한다면 시간 기반 경합 시 재현성이 떨어집니다. 필요하면 테스트 더블에 간단한 대기/타임아웃 처리를 추가하세요.src/test/java/com/gdschongik/gdsc/helper/InmemoryLockUtil.java (5)
14-14: ConcurrentHashMap 대신 Set로 단순화 + 입력값 검증 추가 제안키 존재 여부만 필요하므로 Set가 더 직관적이고 코드가 단순해집니다. 동시에 null/blank 키 방지를 위해 가드 추가를 권장합니다.
- private final Map<String, Boolean> locks = new ConcurrentHashMap<>(); + private final Set<String> locks = ConcurrentHashMap.newKeySet();- public boolean acquireLock(String key, long ignored) { - Boolean previous = locks.putIfAbsent(key, true); - boolean acquired = previous == null; + public boolean acquireLock(String key, long ignored) { + if (key == null || key.isBlank()) { + throw new IllegalArgumentException("lock key must not be null/blank"); + } + boolean acquired = locks.add(key);- public boolean releaseLock(String lockName) { - Boolean removed = locks.remove(lockName); - boolean released = removed != null; + public boolean releaseLock(String lockName) { + if (lockName == null || lockName.isBlank()) { + return false; + } + boolean released = locks.remove(lockName);추가로 import가 필요합니다:
- java.util.Set
Also applies to: 20-23, 37-38
24-28: 테스트 로그 노이즈 축소: INFO → DEBUG, 로그 태그 네이밍 일치테스트에서 모든 락 획득/해제를 INFO로 찍으면 로그가 과도해집니다. DEBUG로 낮추고 클래스명과 로그 태그 철자를 일치시키세요.
- log.info("[InMemoryLockUtil] 락 획득: {}", key); + log.debug("[InmemoryLockUtil] 락 획득: {}", key);- log.info("[InMemoryLockUtil] 락 획득 실패: {}", key); + log.debug("[InmemoryLockUtil] 락 획득 실패: {}", key);- log.info("[InMemoryLockUtil] 락 해제: {}", lockName); + log.debug("[InmemoryLockUtil] 락 해제: {}", lockName);- log.info("[InMemoryLockUtil] 락 해제 실패: {}", lockName); + log.debug("[InmemoryLockUtil] 락 해제 실패: {}", lockName);Also applies to: 40-44
20-23: timeout 파라미터 무시 — 의도 명시 혹은 간단 대기 루프 옵션현재 long 파라미터를 무시합니다. 테스트 의도라면 Javadoc에 “즉시 시도 후 실패 반환”을 명확히 하거나, 선택적으로 짧은 대기(예: 10~20ms backoff, 최대 timeoutSec) 루프를 두는 옵션도 고려해 보세요. 테스트 일관성 향상에 도움 됩니다.
36-46: 락 소유자 검증/재진입 고려현재 소유자(쓰레드/토큰) 개념이 없어 제3자가 같은 키를 해제할 수 있고, 재진입도 불가능합니다. 테스트 전용이면 괜찮지만, 오용을 조기에 탐지하려면 토큰 기반 인터페이스(획득 시 토큰 발급, 해제 시 토큰 확인)로의 확장을 추후 고려해 주세요.
1-48: 테스트 간 간섭 방지용 리셋 훅 제공 제안동일 스프링 컨텍스트를 캐시하는 통합 테스트 환경에서 키가 누적될 수 있습니다. 필요 시 아래 유틸을 추가하고 @AfterEach에서 호출해 테스트 간 간섭을 줄이세요.
// 클래스 하단에 추가 void clearAllForTest() { locks.clear(); }src/main/java/com/gdschongik/gdsc/global/lock/LockAspect.java (3)
6-6: 불필요한 @NotNull 제거파라미터는 프레임워크가 항상 주입합니다. jakarta @NotNull은 불필요하니 제거해 간결하게 유지하세요.
-import jakarta.validation.constraints.NotNull;-public Object around(@NotNull ProceedingJoinPoint joinPoint) throws Throwable { +public Object around(ProceedingJoinPoint joinPoint, DistributedLock distributedLock) throws Throwable {Also applies to: 34-34
60-73: 기본 키 정책 개선 + SpEL 실패 시 안전한 폴백 + 키 정규화(normalize)현재 기본키가 “클래스:메서드”로만 생성돼 불필요한 전역 직렬화가 발생할 수 있고, SpEL 실패/NULL에 대한 폴백이 없습니다. 또한(MySQL 기반 구현 시) GET_LOCK 키 길이(64자) 제한도 고려해야 합니다. 기본키에 첫 번째 인자를 포함하고, 실패 시 폴백하며, 최종 키를 normalize하는 경로를 제안합니다.
- private String parseLockKey(DistributedLock distributedLock, Method method, Object[] args, Object target) { - String key = distributedLock.key(); - - // 어노테이션에 락 이름이 비어있으면 클래스명:메서드명으로 기본 값 생성 - if (key.isEmpty()) { - return String.format("%s:%s", target.getClass().getSimpleName(), method.getName()); - } - - // SpEL 표현식 평가 - MethodBasedEvaluationContext context = - new MethodBasedEvaluationContext(target, method, args, parameterNameDiscoverer); - - return parser.parseExpression(key).getValue(context, String.class); - } + private String parseLockKey(DistributedLock distributedLock, Method method, Object[] args, Object target) { + String annotationKey = distributedLock.key(); + String defaultKey = (args != null && args.length > 0) + ? String.format("%s:%s:%s", target.getClass().getSimpleName(), method.getName(), String.valueOf(args[0])) + : String.format("%s:%s", target.getClass().getSimpleName(), method.getName()); + + if (annotationKey.isEmpty()) { + return normalizeKey(defaultKey); + } + + MethodBasedEvaluationContext context = + new MethodBasedEvaluationContext(target, method, args, parameterNameDiscoverer); + try { + String resolved = parser.parseExpression(annotationKey).getValue(context, String.class); + return normalizeKey(resolved != null ? resolved : defaultKey); + } catch (Exception e) { + log.warn("SpEL 키 파싱 실패. 기본 키로 대체합니다. key={}, method={}.{}", + annotationKey, target.getClass().getSimpleName(), method.getName(), e); + return normalizeKey(defaultKey); + } + }추가 메서드 예시(클래스 하단에 배치):
// MySQL GET_LOCK 등 이름 길이 제한(보통 64자)을 대비해 해시 서픽스 부여 private String normalizeKey(String key) { if (key == null) return "null"; final int MAX = 64; if (key.length() <= MAX) return key; String hash = java.util.UUID.nameUUIDFromBytes(key.getBytes(java.nio.charset.StandardCharsets.UTF_8)) .toString().replace("-", ""); int headLen = Math.max(0, MAX - 1 - 16); return key.substring(0, headLen) + "-" + hash.substring(0, 16); }Also applies to: 63-66
51-54: 락 해제 실패 시 경고 로그 남기기운영 락 구현(예: MySQL/Redis)에서 TTL 만료/오너 불일치로 해제 실패가 발생할 수 있습니다. 추적을 위해 WARN 로그를 남기세요.
- if (lockAcquired) { - lockUtil.releaseLock(key); - } + if (lockAcquired) { + boolean released = lockUtil.releaseLock(key); + if (!released) { + log.warn("락 해제 실패: key={}", key); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java(3 hunks)src/main/java/com/gdschongik/gdsc/domain/event/application/EventService.java(2 hunks)src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java(1 hunks)src/main/java/com/gdschongik/gdsc/global/lock/DistributedLock.java(1 hunks)src/main/java/com/gdschongik/gdsc/global/lock/LockAspect.java(1 hunks)src/main/java/com/gdschongik/gdsc/global/lock/LockUtil.java(1 hunks)src/main/java/com/gdschongik/gdsc/global/lock/MySqlLockUtil.java(1 hunks)src/test/java/com/gdschongik/gdsc/config/TestLockConfig.java(1 hunks)src/test/java/com/gdschongik/gdsc/helper/InmemoryLockUtil.java(1 hunks)src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/gdschongik/gdsc/global/lock/LockAspect.java (1)
src/main/java/com/gdschongik/gdsc/domain/discord/exception/DiscordEventHandlerAspect.java (1)
Aspect(10-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
🔇 Additional comments (2)
src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (1)
12-12: 테스트 락 구성 주입 LGTM.
TestLockConfig를 Import하여 테스트에서 인메모리 락으로 교체하는 접근이 적절합니다.인메모리 락이
timeoutSec을 무시한다면 경합 시 행태가 프로덕션과 다를 수 있습니다. 재현 테스트가 필요하면 알려주세요. 동시성 시나리오를 추가해 드리겠습니다.Also applies to: 70-70
src/main/java/com/gdschongik/gdsc/global/lock/LockAspect.java (1)
23-25: 트랜잭션보다 먼저 실행 순서 유지 확인@order(1) 설정은 적절합니다. 다만 다른 선행 Aspect(예: 보안/레이트리밋)가 있으면 순서를 재검토하세요.
| // TODO: 현재 본행사, 뒤풀이 신청 제한 인원 이하인지 검증 추가 | ||
| @DistributedLock(key = "'event:' + #request.eventId()") | ||
| @Transactional |
There was a problem hiding this comment.
용량 검증은 락 임계구역 안에서 반드시 수행하세요.
현재 TODO로 남아있는 본행사/뒤풀이 정원 검증을 락 획득 직후에 수행하지 않으면 초과 신청 레이스 조건이 남습니다. @DistributedLock 안에서 현재 카운트 조회 → 검증 → 생성/저장 순서를 보장하세요.
원하시면 정원 검증 로직과 리포지토리 카운트 쿼리 배치까지 반영한 패치를 제안드릴게요.
🤖 Prompt for AI Agents
In
src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java
around lines 239-241, the capacity check for main event/afterparty is left as a
TODO outside the distributed lock which allows race conditions; move the
validation so it runs immediately after acquiring the @DistributedLock and
inside the @Transactional method: after obtaining the lock, query the repository
for the current participant counts (separate counts for main event and
afterparty as needed), compare against the configured limits, throw a domain
exception if full, and only then proceed to create/persist the participation;
ensure the count query and validation are executed within the same
transactional/locked critical section to guarantee atomicity.
| // Lock | ||
| LOCK_ACQUIRE_FAILED(INTERNAL_SERVER_ERROR, "락 획득에 실패했습니다. 다시 시도해주세요."), | ||
| ; |
There was a problem hiding this comment.
락 획득 실패를 500이 아닌 409(Conflict)/503(Service Unavailable)로 표준화하는 것이 적절합니다.
락 경합으로 인한 획득 실패는 서버 오류(500)라기보다 상태 충돌(409) 또는 일시적 불가(503)에 가깝습니다. 503을 선택한다면 컨트롤러 어드바이스에서 Retry-After 헤더를 함께 내려 재시도 힌트를 주는 것이 좋습니다.
아래와 같이 409로 바꾸는 것을 제안합니다:
- LOCK_ACQUIRE_FAILED(INTERNAL_SERVER_ERROR, "락 획득에 실패했습니다. 다시 시도해주세요."),
+ LOCK_ACQUIRE_FAILED(CONFLICT, "락 획득에 실패했습니다. 다시 시도해주세요."),📝 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.
| // Lock | |
| LOCK_ACQUIRE_FAILED(INTERNAL_SERVER_ERROR, "락 획득에 실패했습니다. 다시 시도해주세요."), | |
| ; | |
| // Lock | |
| LOCK_ACQUIRE_FAILED(CONFLICT, "락 획득에 실패했습니다. 다시 시도해주세요."), | |
| ; |
🤖 Prompt for AI Agents
In src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java around
lines 227 to 229, the LOCK_ACQUIRE_FAILED enum currently maps to
INTERNAL_SERVER_ERROR (500); change it to use the appropriate HTTP status
constant (CONFLICT / 409) as suggested (or SERVICE_UNAVAILABLE / 503 if you
prefer retry semantics). Update the enum entry to reference the chosen status
constant and message, and if you select 503 also modify the controller
advice/exception handler to add a Retry-After header with a sensible retry
interval when returning this error.
| package com.gdschongik.gdsc.global.lock; | ||
|
|
||
| import static com.gdschongik.gdsc.global.exception.ErrorCode.*; | ||
|
|
||
| import com.gdschongik.gdsc.global.exception.CustomException; | ||
| import jakarta.validation.constraints.NotNull; | ||
| import java.lang.reflect.Method; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.aspectj.lang.ProceedingJoinPoint; | ||
| import org.aspectj.lang.annotation.Around; | ||
| import org.aspectj.lang.annotation.Aspect; | ||
| import org.aspectj.lang.reflect.MethodSignature; | ||
| import org.springframework.context.expression.MethodBasedEvaluationContext; | ||
| import org.springframework.core.DefaultParameterNameDiscoverer; | ||
| import org.springframework.core.ParameterNameDiscoverer; | ||
| import org.springframework.core.annotation.Order; | ||
| import org.springframework.expression.ExpressionParser; | ||
| import org.springframework.expression.spel.standard.SpelExpressionParser; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Slf4j | ||
| @Order(1) // 트랜잭션 AOP보다 먼저 실행되어야 합니다. | ||
| @Aspect | ||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class LockAspect { | ||
|
|
||
| private final LockUtil lockUtil; | ||
| private final ExpressionParser parser = new SpelExpressionParser(); | ||
| private final ParameterNameDiscoverer parameterNameDiscoverer = new DefaultParameterNameDiscoverer(); | ||
|
|
||
| @Around("@annotation(com.gdschongik.gdsc.global.lock.DistributedLock)") | ||
| public Object around(@NotNull ProceedingJoinPoint joinPoint) throws Throwable { | ||
| MethodSignature signature = (MethodSignature) joinPoint.getSignature(); | ||
| Method method = signature.getMethod(); | ||
| DistributedLock distributedLock = method.getAnnotation(DistributedLock.class); | ||
|
|
||
| String key = parseLockKey(distributedLock, method, joinPoint.getArgs(), joinPoint.getTarget()); | ||
| boolean lockAcquired = false; | ||
|
|
||
| try { | ||
| lockAcquired = lockUtil.acquireLock(key, distributedLock.timeoutSec()); | ||
|
|
||
| if (!lockAcquired) { | ||
| throw new CustomException(LOCK_ACQUIRE_FAILED); | ||
| } | ||
|
|
||
| return joinPoint.proceed(); | ||
| } finally { | ||
| if (lockAcquired) { | ||
| lockUtil.releaseLock(key); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * SpEL을 사용하여 키를 파싱합니다. | ||
| */ | ||
| private String parseLockKey(DistributedLock distributedLock, Method method, Object[] args, Object target) { | ||
| String key = distributedLock.key(); | ||
|
|
||
| // 어노테이션에 락 이름이 비어있으면 클래스명:메서드명으로 기본 값 생성 | ||
| if (key.isEmpty()) { | ||
| return String.format("%s:%s", target.getClass().getSimpleName(), method.getName()); | ||
| } | ||
|
|
||
| // SpEL 표현식 평가 | ||
| MethodBasedEvaluationContext context = | ||
| new MethodBasedEvaluationContext(target, method, args, parameterNameDiscoverer); | ||
|
|
||
| return parser.parseExpression(key).getValue(context, String.class); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
운영 환경 확인 스크립트: 키 길이 제한 및 프록시 모드 검증
- MySQL 기반 락 구현에서 키 길이 제한(보통 64자) 대응 코드 존재 여부 확인
- AOP 프록시 모드가 클래스 기반(CGLIB)인지 확인(자기 호출 시 미적용 이슈 방지)
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[1/2] MySQL 락 키 길이 제한 대응 여부 탐색"
rg -nPI 'GET_LOCK\s*\(|RELEASE_LOCK\s*\(|is_free_lock\s*\(' -g 'src/**' -S || true
rg -nP 'normalizeKey|key\.length|64' -g 'src/**' || true
rg -nP 'MySqlLockUtil|MysqlLockUtil|Jdbc.*Lock' -g 'src/**' || true
echo "[2/2] 프록시 모드/자기호출 관련 설정 탐색"
rg -n 'spring\.aop\.proxy-target-class|@EnableAspectJAutoProxy|proxyTargetClass' -g '**/*.yml' -g '**/*.yaml' -g '**/*.properties' -g 'src/**' -S || trueLength of output: 603
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== repo files matching names ==="
rg -n --hidden --glob 'src/**' 'MySqlLockUtil|AopConfig|DistributedLock' -S || true
echo
echo "=== src/main/java/com/gdschongik/gdsc/global/lock/MySqlLockUtil.java ==="
sed -n '1,240p' src/main/java/com/gdschongik/gdsc/global/lock/MySqlLockUtil.java || true
echo
echo "=== src/main/java/com/gdschongik/gdsc/global/config/AopConfig.java ==="
sed -n '1,240p' src/main/java/com/gdschongik/gdsc/global/config/AopConfig.java || true
echo
echo "=== Searching for DistributedLock annotation source ==="
rg -n --hidden --glob 'src/**' '@interface DistributedLock' -S || true
rg -n --hidden --glob 'src/**' 'interface DistributedLock' -S || true
DL_FILE=$(rg -n --hidden --glob 'src/**' '@interface DistributedLock' -S | cut -d: -f1 || true)
if [ -n "$DL_FILE" ]; then
echo
echo "=== $DL_FILE ==="
sed -n '1,240p' "$DL_FILE" || true
fi
DL_FILE2=$(rg -n --hidden --glob 'src/**' 'interface DistributedLock' -S | cut -d: -f1 || true)
if [ -n "$DL_FILE2" ] && [ "$DL_FILE2" != "$DL_FILE" ]; then
echo
echo "=== $DL_FILE2 ==="
sed -n '1,240p' "$DL_FILE2" || true
fiLength of output: 4994
조치 필요: MySQL 네임드락 키 길이(64바이트) 확인 및 AOP 프록시 모드 설정
-
MySqlLockUtil이 GET_LOCK에 원본 key를 그대로 전달합니다. MySQL 네임드락은 일반적으로 64바이트 제한이 있어 긴 SpEL 키는 실패/오작동을 유발할 수 있습니다. 조치: LockAspect.parseLockKey 또는 MySqlLockUtil에서 키를 정규화/축약하거나(예: 네임스페이스+SHA‑256 해시) 고정 길이 해시를 사용하도록 변경하세요.
위치: src/main/java/com/gdschongik/gdsc/global/lock/MySqlLockUtil.java, src/main/java/com/gdschongik/gdsc/global/lock/LockAspect.java -
AopConfig에 @EnableAspectJAutoProxy(proxyTargetClass = true) 설정이 없습니다(@EnableAspectJAutoProxy만 선언). 기본 JDK 프록시는 자기 호출(self-invocation)에서 AOP를 적용하지 못합니다. 조치: @EnableAspectJAutoProxy(proxyTargetClass = true)로 변경하거나 내부 호출이 아닌 프록시 경유 호출로 설계 변경하세요.
위치: src/main/java/com/gdschongik/gdsc/global/config/AopConfig.java -
DistributedLock을 사용하는 SpEL 키(예: EventService, EventParticipationService)가 길어질 수 있으니 위 정규화/해시 적용 여부를 확인하세요.
위치 예: src/main/java/com/gdschongik/gdsc/domain/event/application/EventService.java, src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java
| @Around("@annotation(com.gdschongik.gdsc.global.lock.DistributedLock)") | ||
| public Object around(@NotNull ProceedingJoinPoint joinPoint) throws Throwable { | ||
| MethodSignature signature = (MethodSignature) joinPoint.getSignature(); | ||
| Method method = signature.getMethod(); | ||
| DistributedLock distributedLock = method.getAnnotation(DistributedLock.class); | ||
|
|
There was a problem hiding this comment.
어노테이션 바인딩으로 프록시/리플렉션 이슈 제거
JDK 프록시 환경에서 구현체에만 붙은 애노테이션을 가져오지 못하는 경우가 생길 수 있습니다. 포인트컷에서 애노테이션을 바인딩해 직접 주입받으세요.
- @Around("@annotation(com.gdschongik.gdsc.global.lock.DistributedLock)")
- public Object around(@NotNull ProceedingJoinPoint joinPoint) throws Throwable {
+ @Around("@annotation(distributedLock)")
+ public Object around(ProceedingJoinPoint joinPoint, DistributedLock distributedLock) throws Throwable {
- MethodSignature signature = (MethodSignature) joinPoint.getSignature();
- Method method = signature.getMethod();
- DistributedLock distributedLock = method.getAnnotation(DistributedLock.class);
+ MethodSignature signature = (MethodSignature) joinPoint.getSignature();
+ Method method = signature.getMethod();Also applies to: 34-34
| public boolean acquireLock(String lockName, long timeoutSec) { | ||
| Integer result = jdbcTemplate.queryForObject(GET_LOCK_QUERY, Integer.class, lockName, timeoutSec); | ||
| boolean acquired = result != null && result == 1; // GET_LOCK 결과: 1(성공), 0(타임아웃), null(에러) | ||
|
|
||
| if (acquired) { | ||
| log.info("[DistributedLock] 락 획득 성공: {}", lockName); | ||
| } else { | ||
| log.info("[DistributedLock] 락 획득 실패: {}", lockName); | ||
| } | ||
|
|
||
| return acquired; | ||
| } |
There was a problem hiding this comment.
중요: GET_LOCK/RELEASE_LOCK는 동일 커넥션에서 호출되어야 합니다. 현재 구현은 커넥션 풀로 인해 락이 해제되지 않는 치명적 버그를 유발합니다.
JdbcTemplate가 매 호출마다 다른 커넥션을 획득할 수 있어, GET_LOCK을 수행한 커넥션과 RELEASE_LOCK을 수행하는 커넥션이 달라질 수 있습니다. 이 경우 락이 풀리지 않으며, 락 ‘소유’ 커넥션이 풀에 남아 교착/가용성 저하를 일으킬 수 있습니다.
다음과 같이 DataSourceUtils로 커넥션을 획득/보관(ThreadLocal)하여 동일 커넥션으로 acquire/release를 보장하세요.
package com.gdschongik.gdsc.global.lock;
-import lombok.RequiredArgsConstructor;
-import lombok.extern.slf4j.Slf4j;
-import org.springframework.jdbc.core.JdbcTemplate;
-import org.springframework.stereotype.Component;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.datasource.DataSourceUtils;
+import org.springframework.stereotype.Component;
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
@Slf4j
@Component
@RequiredArgsConstructor
public class MySqlLockUtil implements LockUtil {
- private final JdbcTemplate jdbcTemplate;
+ private final JdbcTemplate jdbcTemplate;
+ private final DataSource dataSource;
+ private final ThreadLocal<Connection> lockConnectionHolder = new ThreadLocal<>();
private static final String GET_LOCK_QUERY = "SELECT GET_LOCK(?, ?)";
private static final String RELEASE_LOCK_QUERY = "SELECT RELEASE_LOCK(?)";
- public boolean acquireLock(String lockName, long timeoutSec) {
- Integer result = jdbcTemplate.queryForObject(GET_LOCK_QUERY, Integer.class, lockName, timeoutSec);
- boolean acquired = result != null && result == 1; // GET_LOCK 결과: 1(성공), 0(타임아웃), null(에러)
+ @Override
+ public boolean acquireLock(String lockName, int timeoutSec) {
+ Connection conn = DataSourceUtils.getConnection(dataSource);
+ try (PreparedStatement ps = conn.prepareStatement(GET_LOCK_QUERY)) {
+ ps.setString(1, lockName);
+ ps.setInt(2, timeoutSec);
+ try (ResultSet rs = ps.executeQuery()) {
+ boolean acquired = false;
+ if (rs.next()) {
+ Integer result = (Integer) rs.getObject(1);
+ acquired = result != null && result == 1; // 1: 성공, 0: 타임아웃, null: 에러
+ }
+ if (acquired) {
+ lockConnectionHolder.set(conn); // 동일 커넥션 유지
+ log.debug("[DistributedLock] 락 획득 성공: {}", lockName);
+ return true;
+ } else {
+ log.debug("[DistributedLock] 락 획득 실패: {}", lockName);
+ // 획득 실패 시 커넥션 반납
+ DataSourceUtils.releaseConnection(conn, dataSource);
+ return false;
+ }
+ }
+ } catch (Exception e) {
+ log.warn("[DistributedLock] 락 획득 중 예외: {}", lockName, e);
+ DataSourceUtils.releaseConnection(conn, dataSource);
+ return false;
+ }
- if (acquired) {
- log.info("[DistributedLock] 락 획득 성공: {}", lockName);
- } else {
- log.info("[DistributedLock] 락 획득 실패: {}", lockName);
- }
- return acquired;
}
- public boolean releaseLock(String lockName) {
- Integer result = jdbcTemplate.queryForObject(RELEASE_LOCK_QUERY, Integer.class, lockName);
- boolean released = result != null && result == 1; // RELEASE_LOCK 결과: 1(성공), 0(타임아웃), null(에러)
-
- if (released) {
- log.info("[DistributedLock] 락 해제 성공: {}", lockName);
- } else {
- log.info("[DistributedLock] 락 해제 실패: {}", lockName);
- }
-
- return released;
+ @Override
+ public boolean releaseLock(String lockName) {
+ Connection conn = lockConnectionHolder.get();
+ if (conn == null) {
+ // 다른 커넥션에서 RELEASE_LOCK을 시도해도 해제되지 않으므로 경고만 남깁니다.
+ log.warn("[DistributedLock] 락 해제 실패(소유 커넥션 없음): {}", lockName);
+ return false;
+ }
+ try (PreparedStatement ps = conn.prepareStatement(RELEASE_LOCK_QUERY);
+ ) {
+ ps.setString(1, lockName);
+ try (ResultSet rs = ps.executeQuery()) {
+ boolean released = false;
+ if (rs.next()) {
+ Integer result = (Integer) rs.getObject(1);
+ released = result != null && result == 1; // 1: 성공, 0: 다른 세션 소유, null: 없음
+ }
+ if (released) {
+ log.debug("[DistributedLock] 락 해제 성공: {}", lockName);
+ } else {
+ log.warn("[DistributedLock] 락 해제 실패: {}", lockName);
+ }
+ return released;
+ }
+ } catch (Exception e) {
+ log.warn("[DistributedLock] 락 해제 중 예외: {}", lockName, e);
+ return false;
+ } finally {
+ lockConnectionHolder.remove();
+ DataSourceUtils.releaseConnection(conn, dataSource);
+ }
}추가 고려사항:
- 다중 락(중첩) 지원이 필요하면 ThreadLocal에 스택을 두거나 토큰 기반 API로 변경을 검토하세요.
- 락 키 길이는 MySQL 제한(일반적으로 64바이트)을 넘지 않도록 보장되어야 합니다. 현재 'event:{id}'는 안전합니다.
Also applies to: 45-56
🤖 Prompt for AI Agents
In src/main/java/com/gdschongik/gdsc/global/lock/MySqlLockUtil.java around lines
27-38 (and also apply same fix to 45-56), the current use of JdbcTemplate for
GET_LOCK/RELEASE_LOCK can pick different pooled connections and so fails to
release locks; modify the implementation to obtain and bind a single Connection
for the thread using DataSourceUtils.getConnection(dataSource) (store it in a
ThreadLocal/connection holder) before calling GET_LOCK, use that same Connection
for any subsequent RELEASE_LOCK, and finally unbind/close the Connection with
DataSourceUtils.releaseConnection when done; ensure error paths always
release/unbind the connection, and consider stacking or tokenizing the
ThreadLocal if nested locks must be supported.
| * 인메모리 락을 획득합니다. | ||
| * @param ignored 락 획득 대기 시간(초), 인메모리 구현에서는 사용하지 않음. | ||
| */ | ||
| public boolean acquireLock(String key, long ignored) { |
There was a problem hiding this comment.
테스트에서도 타임아웃 고려해야 하는 케이스가 있지 않을까요?
이거 초기 코드인걸로 아는데, 제가 최근에 구현했던대로 ReentrantLock 써야 타임아웃 기능이 동작합니다
There was a problem hiding this comment.
굳 수정했습니다.
근데 테스트에서도 타임아웃을 고려해야 하는 케이스가 있었나요? 락 점유 시간이 워낙 짧아서 없어도 될거라 생각했습니다.
There was a problem hiding this comment.
네. 시나리오 테스트에서 타임아웃을 넣어야 정상 로직대로 구현이 되더라고요
참고로 mysql lock도 분산락입니다. |
| import org.springframework.expression.spel.standard.SpelExpressionParser; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Slf4j |
There was a problem hiding this comment.
처음 구현할때에는 AOP에서 로그를 남겼었는데 지우는걸 까먹었네요
굿굿
| log.info("[DistributedLock] 락 획득 성공: {}", lockName); | ||
| } else { | ||
| log.info("[DistributedLock] 락 획득 실패: {}", lockName); |
There was a problem hiding this comment.
| log.info("[DistributedLock] 락 획득 성공: {}", lockName); | |
| } else { | |
| log.info("[DistributedLock] 락 획득 실패: {}", lockName); | |
| log.info("[MySqlLockUtil] 락 획득 성공: {}", lockName); | |
| } else { | |
| log.info("[MySqlLockUtil] 락 획득 실패: {}", lockName); |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/test/java/com/gdschongik/gdsc/helper/InmemoryLockUtil.java (1)
21-26: 타임아웃 고려한 tryLock 적용 굿
ReentrantLock#tryLock(timeout)으로 테스트 시 타임아웃을 다루도록 한 점 좋아요. 이전 코멘트에서 제기된 타임아웃 요구사항도 충족합니다.
🧹 Nitpick comments (6)
src/test/java/com/gdschongik/gdsc/helper/InmemoryLockUtil.java (6)
21-21: 인터페이스 구현 메서드에 @OverRide 누락컴파일러 체크와 가독성을 위해 명시해 주세요.
- public boolean acquireLock(String key, long timeoutSec) { + @Override + public boolean acquireLock(String key, long timeoutSec) { - public boolean releaseLock(String lockName) { + @Override + public boolean releaseLock(String lockName) {Also applies to: 44-44
21-23: null 키 방어 로직 추가
ConcurrentHashMap은 null 키를 허용하지 않아 NPE 위험이 있습니다. 빠르게 가드해 주세요.public boolean acquireLock(String key, long timeoutSec) { + java.util.Objects.requireNonNull(key, "lock key must not be null"); ReentrantLock lock = locks.computeIfAbsent(key, k -> new ReentrantLock());public boolean releaseLock(String lockName) { + java.util.Objects.requireNonNull(lockName, "lock key must not be null"); ReentrantLock lock = locks.get(lockName);Also applies to: 45-46
44-60: unlock 전 소유권 확인 + 사용 종료 시 엔트리 정리예외 기반 흐름을 줄이고, 더 이상 사용되지 않는 락 엔트리를 정리하면 테스트 장기 실행 시 메모리 누적을 줄일 수 있습니다.
- public boolean releaseLock(String lockName) { - ReentrantLock lock = locks.get(lockName); - - if (lock == null) { - log.info("[InMemoryLockUtil] 락 해제 실패, 존재하지 않는 락: {}", lockName); - return false; - } - - try { - lock.unlock(); - log.info("[InMemoryLockUtil] 락 해제: {}", lockName); - return true; - } catch (IllegalMonitorStateException e) { - log.error("[InMemoryLockUtil] 락 해제 실패, 현재 스레드가 락을 소유하고 있지 않음: {}", lockName, e); - return false; - } - } + public boolean releaseLock(String lockName) { + ReentrantLock lock = locks.get(lockName); + if (lock == null) { + log.warn("락 해제 실패(존재하지 않음): {}", lockName); + return false; + } + if (!lock.isHeldByCurrentThread()) { + log.warn("락 해제 실패(소유 스레드 아님): {}", lockName); + return false; + } + // 실제 해제 + lock.unlock(); + // 대기자 없고 잠금도 해제된 상태라면 맵 엔트리 제거(동일 인스턴스일 때만) + if (!lock.isLocked() && !lock.hasQueuedThreads()) { + locks.remove(lockName, lock); + } + log.debug("락 해제: {}", lockName); + return true; + }
28-31: 로그 레벨 세분화 제안정상 플로우는
debug, 이상 상황은warn/error로 구분하면 테스트 로그 소음을 줄일 수 있습니다.- log.info("[InMemoryLockUtil] 락 획득: {}", key); + log.debug("락 획득: {}", key); - log.info("[InMemoryLockUtil] 락 획득 실패: {}", key); + log.debug("락 획득 실패(타임아웃): {}", key);- log.info("[InMemoryLockUtil] 락 해제: {}", lockName); + log.debug("락 해제: {}", lockName);Also applies to: 48-49, 54-54
14-14: 네이밍 일관성: Inmemory ↔ InMemory클래스명은
InmemoryLockUtil인데 로그 프리픽스는[InMemoryLockUtil]로 표기되어 있습니다.
- 선택 1: 클래스명을
InMemoryLockUtil로 변경(테스트 설정 참조 변경 필요).- 선택 2: 수동 프리픽스 제거(로거가 클래스명을 포함하므로 중복).
22-22: 공정(fair) 락 옵션 고려테스트에서 경쟁이 심한 경우 기아 방지를 위해 공정 모드를 고려할 수 있습니다. 성능 저하는 미미한 테스트 환경에선 허용 가능할 때가 많습니다.
- ReentrantLock lock = locks.computeIfAbsent(key, k -> new ReentrantLock()); + ReentrantLock lock = locks.computeIfAbsent(key, k -> new ReentrantLock(true));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/gdschongik/gdsc/global/lock/LockAspect.java(1 hunks)src/main/java/com/gdschongik/gdsc/global/lock/MySqlLockUtil.java(1 hunks)src/test/java/com/gdschongik/gdsc/helper/InmemoryLockUtil.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/gdschongik/gdsc/global/lock/MySqlLockUtil.java
- src/main/java/com/gdschongik/gdsc/global/lock/LockAspect.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
🔇 Additional comments (1)
src/test/java/com/gdschongik/gdsc/helper/InmemoryLockUtil.java (1)
1-62: 테스트 전용 — 프로덕션에 포함되지 않음InmemoryLockUtil은 src/test/java에만 존재하며 테스트 전용 구성(TestLockConfig)에서만 @bean으로 생성됩니다 (src/test/java/com/gdschongik/gdsc/config/TestLockConfig.java:15).
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit