-
Notifications
You must be signed in to change notification settings - Fork 0
[fix] 팔로잉 동시성 이슈 문제 1차 해결시도 #337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough팔로우 기능의 동시성 문제를 해결하기 위해 데이터베이스 레벨의 유니크 제약조건, 비관적 잠금, 재시도 로직을 추가합니다. 두 개의 k6 로드테스트와 동시성 통합테스트도 새로 도입됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant VU as VU (클라이언트)
participant API as API 서버
participant Lock as 잠금<br/>관리자
participant DB as 데이터베이스
participant Recovery as 복구 핸들러
VU->>API: 팔로우 요청 (token)
activate API
API->>Lock: findByIdWithLock(targetUserId)
activate Lock
Lock->>DB: SELECT with PESSIMISTIC_WRITE<br/>(5초 타임아웃)
activate DB
DB-->>Lock: User 데이터 반환 (잠금 획득)
Lock-->>API: User 객체
deactivate DB
deactivate Lock
alt 잠금 획득 성공
API->>DB: INSERT followings<br/>(user_id, following_user_id)
alt 유니크 제약 위반
DB-->>API: ConstraintViolation
API-->>VU: 409 Conflict
else 성공
DB-->>API: 성공
API-->>VU: 200 OK
end
else 타임아웃 (3회 재시도)
API->>Recovery: `@Recover` 호출
activate Recovery
Recovery->>Recovery: RESOURCE_LOCKED 예외 생성
Recovery-->>VU: 500 RESOURCE_LOCKED
deactivate Recovery
end
deactivate API
sequenceDiagram
participant Test as 동시성 테스트
participant Barrier as 배리어<br/>동기화
participant Task as 팔로우 태스크
participant REST as REST API
participant JDBC as JDBC/DB
Test->>Test: 대상 사용자 1명 생성
Test->>Test: 팔로워 500명 생성
loop 각 팔로워별 스레드
Test->>Barrier: await() - ready
activate Barrier
Barrier-->>Task: 준비 신호
deactivate Barrier
activate Task
Task->>Barrier: await() - start
activate Barrier
Barrier-->>Test: 모든 스레드 준비 완료
Barrier-->>Task: 시작 신호
deactivate Barrier
Task->>REST: POST /users/following/{targetId}
REST-->>Task: 200 OK
deactivate Task
end
Test->>Barrier: await() - finish
Test->>JDBC: SELECT COUNT(\\*) FROM followings
JDBC-->>Test: 팔로우 수
Test->>JDBC: SELECT follower_count FROM users
JDBC-->>Test: 팔로워 카운트
Test->>Test: assert followings ≤ followers
Estimated code review effort🎯 4 (복잡함) | ⏱️ ~50분 Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (10)
loadtest/follow_change_toggle_load_test.js (1)
5-27: k6 토글 시나리오/메트릭 구성이 명확하고 실험 목적에 잘 맞습니다 (소소한 정리 제안)
- 토큰 배치 발급 +
per-vu-iterations+START_DELAY_S조합으로 동시 토글 부하를 재현하는 구조가 명확하고, 토큰 발급 실패시 빈 토큰을 넣고 VU 쪽에서!token이면 조용히 스킵하는 전략도 실용적인 선택으로 보입니다.- 전역
isFollowing를 VU별 토글 상태로 사용하는 것도 k6의 VU 격리 특성 상 의도대로 동작할 것이므로 유지해도 괜찮아 보입니다.- 자잘한 부분으로는,
setup마지막의if (tokens.length > USERS_COUNT) tokens.length = USERS_COUNT;는 현재 로직 상 항상 길이가 정확히USERS_COUNT가 되므로 삭제해도 동작에는 영향이 없습니다.- 또 하나는
ERR상수와 서버 쪽 에러 코드가 반드시 함께 유지되어야 하니, 상수 선언부에 “서버 ErrorCode 변경 시 함께 수정 필요” 정도의 주석을 추가해 두면 나중에 코드 동기화가 조금 더 쉬울 것 같습니다.Also applies to: 35-47, 49-64, 66-93, 95-162
src/main/java/konkuk/thip/user/application/port/out/UserCommandPort.java (1)
8-16: CommandPort에 Lock 버전 조회 메서드 추가는 설계상 자연스럽습니다 (간단한 Javadoc 추천)
findById옆에findByIdWithLock를 두고, follow 쓰기 플로우에서만 사용하는 구조는 CommandPort에 도메인 엔티티 조회 메서드를 두는 기존 CQRS 컨벤션과 잘 맞습니다.- 다만 이 메서드가 “비관적 write 락 + 타임아웃” 전제를 가지는 만큼, 인터페이스 수준에 간단히 Javadoc으로 락 타입·타임아웃·예외 동작을 명시해 두면 다른 Adapter/UseCase에서 오용될 여지를 줄일 수 있을 것 같습니다. (기존 CQRS Port 분리 컨벤션 learnings 기준입니다.)
src/main/resources/db/migration/V251120__Add_following_unique_constraint.sql (1)
1-3: 팔로잉 유니크 제약 추가는 타당하며, 운영 DB의 중복 레코드 여부만 한 번 점검해 두면 좋겠습니다
(user_id, following_user_id)에 유니크 제약을 거는 방향은 follow 관계를 1회로 제한하고, 이후 동시성 이슈에 대한 방어선으로도 적절해 보입니다.다만 운영 DB에 이미 중복 레코드가 있다면 이 마이그레이션이 바로 실패할 수 있으니, 적용 전에 예를 들어 아래와 같은 쿼리로 중복 여부를 한 번 확인하고 필요 시 정리하는 절차를 추천드립니다.
SELECT user_id, following_user_id, COUNT(*) AS cnt FROM followings GROUP BY user_id, following_user_id HAVING COUNT(*) > 1;이 제약은 동시에 유니크 인덱스를 생성하므로,
(user_id, following_user_id)기준 조회/삽입 성능에도 도움이 될 것입니다.src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.java (1)
3-4: PESSIMISTIC_WRITE 기반findByUserIdWithLock추가가 의도에 잘 맞습니다 (타임아웃·트랜잭션 사용 컨벤션만 정리 권장)
findByUserId를 건드리지 않고, 별도 메서드에@Lock(PESSIMISTIC_WRITE)와 5초lock.timeout힌트를 부여한 구조는 “동시성 민감한 write 플로우 전용 조회”라는 의도가 잘 드러나고, 이전에 정리한findByUserId선호 컨벤션과도 일관적입니다. (UserJpaRepository 관련 learnings 기준입니다.)- 한편 이 메서드는 락 대기만 최대 5초까지 걸릴 수 있고, 상위 레이어에서
@Retryable까지 적용된 상태라면 재시도 횟수에 따라 최악 응답 시간이 꽤 길어질 수 있습니다. 현재 Retry 설정(시도 횟수, backoff)과 함께 전체 upper bound를 한 번 계산해 보고, 필요한 경우 락 타임아웃이나 retry 정책을 조금 더 보수적으로 조정하는 것도 고려해 볼 만합니다.- 또한 이 메서드는 반드시
@Transactional(readOnly = false)문맥 안에서만 호출되어야 하니, follow 서비스 레벨에서 “락 기반 조회는 write 트랜잭션에서만 사용” 정도의 팀 컨벤션을 명시해 두면 이후 유지보수 시 안전할 것 같습니다.Also applies to: 7-10, 22-27
src/test/java/konkuk/thip/user/concurrency/UserFollowConcurrencyTest.java (2)
36-120: 동시성 테스트의 검증 조건이 지나치게 느슨해 회귀를 잘 못 잡을 수 있습니다.지금은
followings행 수와follower_count가 단순히followerCount이하인지만 검사해서, 요청이 거의 다 실패하거나 0건이어도 테스트가 통과할 수 있습니다. 최소한okCount와 DB 값 간의 관계(예:followingRows≥storedFollowerCount, 또는okCount와의 기대 관계 등)를 한두 개 정도 추가로 assert 해 두면 실제 정합성 문제가 다시 생겼을 때 더 일찍 감지할 수 있을 것 같습니다.
123-129: 테스트 유저 생성 시 oauth2Id 를 고유하게 만들어 두면 더 안전합니다.
createUsersRange에서TestEntityFactory.createUser를 반복 호출할 때, 팩토리 내부가 고정된oauth2Id를 사용한다면 해당 컬럼에 유니크 제약을 추가하는 순간 이 테스트가 제약 위반으로 깨질 수 있습니다.\"kakao_12345678_\" + i와 같이 follower 별로 다른 oauth2Id 를 부여하도록 팩토리를 확장하거나, 여기서만 별도 빌더를 사용해 고유한 값을 주는 방향을 고려해 보시면 좋겠습니다.src/main/java/konkuk/thip/user/application/service/following/UserFollowService.java (2)
35-46:@Retryable/@Recover예외 분류는 의도가 잘 드러나지만, 범위/로깅을 약간 좁히는 걸 추천드립니다.
notRecoverable+noRetryFor에BusinessException,InvalidStateException을 넣어 도메인 예외는 재시도/복구를 완전히 건너뛰는 구조는 좋아 보입니다.- 다만
retryFor를 지정하지 않아, 도메인 예외를 제외한 모든 예외(예: 잠금/DB 예외뿐 아니라 NPE, 버그성 RuntimeException 등)까지 3회 재시도 후RESOURCE_LOCKED로 매핑됩니다. 이 경우 실제 버그가 “락 문제”로 보일 수 있어 원인 파악이 어려워질 수 있습니다.- 가능하다면:
- 잠금/DB 관련 예외(예: JPA 락 타임아웃 계열)만
retryFor로 한정하거나,- 최소한
@Recover안에서e와followCommand를 로그로 남기거나(또는BusinessException에 cause 를 연결) 해두면, 운영 중 디버깅이 훨씬 수월해질 것 같습니다.1차 대응으로는 충분히 간단하고 명확한 설계라 유지하셔도 되지만, 다음 튜닝 단계에서 한 번 정도는 재시도 대상/복구 대상 예외 범위를 정리해 보면 좋겠습니다. 과도한 복잡도는 피하고자 하는 선호를 고려해 권장사항 수준으로만 제안드립니다. Based on learnings, ...
Also applies to: 73-76
55-55:findByIdWithLock적용으로 타깃 유저에 대한 선점 락을 거는 방향은 타당해 보입니다만, 다른 경로와의 락 순서 일관성은 한 번 더 확인해 주세요.
userCommandPort.findByIdWithLock(targetUserId)로 X-lock(또는 P-lock)을 먼저 잡도록 바꾼 건 데드락 재현 케이스를 줄이는 데 도움이 될 것 같습니다.- 다만 이 메서드 밖의 다른 플로우(예: 언팔로우 이외의 팔로우 관련 유스케이스, 배치/정합성 보정 로직 등)에서도 같은 테이블에 접근할 때 락 획득 순서가 동일한지 확인해 두지 않으면, 새로운 데드락 패턴이 생길 여지가 있습니다.
- 성능 테스트에서 관찰하신 대로 고부하 환경에서는 여기서 대기열이 길어질 수 있으니, 운영에 반영할 때는 모니터링 지표(락 대기 시간, DB wait event 등)를 한 번 더 보면서 튜닝 포인트로 잡아두시면 좋겠습니다.
loadtest/follow_change_state_load_test.js (2)
24-28: 4xx 허용 범위와 에러 코드 매핑을 조금 더 좁히면 장애 감지가 쉬워질 것 같습니다.
ERR.*상수로 “이미 팔로우/언팔로”, “자기 자신 팔로우”를 구분하고, 4xx 일 때 각각 카운터를 나눠 쌓는 구조는 매우 좋습니다.- 다만
check에서로 모든 4xx 를 “expected” 로 허용하고 있어, 인증 실패(401), 권한 오류(403), 라우팅 오류(404) 등도 체크상으로는 통과하게 됩니다.'follow_change 200 or expected 4xx': (r) => r.status === 200 || (r.status >= 400 && r.status < 500),- 부하 테스트가 “락/중복 팔로우에 대한 4xx 는 허용하되, 그 외 4xx 는 빨리 드러내고 싶다”는 목적이라면:
fail_OTHER_4XX에 대해threshold: ['count==0']를 추가하거나,- check 식을
status === 200 || (status >= 400 && status < 500 && [ERR.USER_ALREADY_FOLLOWED, ...].includes(err.code))형태로 좁히는 방안을 고려해 볼 수 있습니다. (이 경우parseError재호출 비용과 가독성을 감안해 선택)- 지금도 메트릭을 보면 구분은 가능하지만, CI 등 자동 판단에는 위와 같이 한 번 더 조건을 좁혀 두는 편이 안정적일 것 같습니다.
Also applies to: 123-138, 146-148
63-93: 토큰 발급 실패 시 VU 수가 줄어드는 현상은 의도인지 한 번만 더 확인해 보시면 좋겠습니다.
setup()에서 토큰을 배치로 발급하고, 실패한 경우에도 인덱스를 맞추기 위해''를 push 한 뒤token_issue_failed를 올리는 구조는 이해하기 쉽습니다.- 이후
default함수에서:으로 토큰이 비어 있으면 해당 VU 는 팔로우 요청을 아예 보내지 않고 종료합니다. 토큰 발급 실패가 많아지면, 실제 동시 팔로우 요청 수가const token = data.tokens[idx]; ... if (!token) { return; }USERS_COUNT보다 적어질 수 있습니다.- 만약 “토큰 발급 실패가 1건이라도 있으면 테스트 자체를 실패로 보겠다”는 의도라면:
options.thresholds에token_issue_failed: ['count==0']를 추가하거나,setup()내에서token_issue_failed가 0이 아니면fail을 던져 테스트를 중단하는 방법도 있습니다.- 반대로 “일부 토큰 실패는 감수하고, 가능한 만큼만 부하를 걸겠다”는 의도라면 지금 구조도 충분히 실용적이니, 주석으로 그 의도를 한 줄 남겨 두면 나중에 스크립트를 보는 사람이 이해하기 더 쉬울 것 같습니다.
Also applies to: 105-107
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
build.gradle(1 hunks)loadtest/follow_change_state_load_test.js(1 hunks)loadtest/follow_change_toggle_load_test.js(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/config/RetryConfig.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/jpa/FollowingJpaEntity.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.java(2 hunks)src/main/java/konkuk/thip/user/application/port/out/UserCommandPort.java(1 hunks)src/main/java/konkuk/thip/user/application/service/following/UserFollowService.java(5 hunks)src/main/resources/db/migration/V251120__Add_following_unique_constraint.sql(1 hunks)src/test/java/konkuk/thip/user/concurrency/UserFollowConcurrencyTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 309
File: src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java:36-44
Timestamp: 2025-09-23T08:31:05.161Z
Learning: buzz0331은 기술적 이슈에 대해 실용적인 해결책을 제시하면서도 과도한 엔지니어링을 피하는 균형감을 선호한다. 복잡도 대비 실제 발생 가능성을 고려하여 "굳이" 불필요한 솔루션보다는 심플함을 유지하는 것을 중요하게 생각한다.
📚 Learning: 2025-07-29T08:11:23.599Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 109
File: src/main/java/konkuk/thip/user/adapter/in/web/UserQueryController.java:37-46
Timestamp: 2025-07-29T08:11:23.599Z
Learning: THIP 프로젝트에서 ExceptionDescription 어노테이션은 비즈니스 로직에서 발생하는 커스텀 에러 코드가 있는 API에만 사용하며, bean validation만 수행하는 API에는 사용하지 않는다.
Applied to files:
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
📚 Learning: 2025-09-01T13:18:13.652Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 287
File: src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.java:8-14
Timestamp: 2025-09-01T13:18:13.652Z
Learning: seongjunnoh는 JpaRepository의 findById 메서드 재정의보다는 도메인별 명시적 메서드(findByUserId, findByRoomId 등)를 정의하여 Hibernate Filter 적용을 보장하는 방식을 선호하며, 이를 통해 더 안전하고 의도가 명확한 코드 구조를 구축한다.
Applied to files:
src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.javasrc/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.javasrc/main/java/konkuk/thip/user/application/port/out/UserCommandPort.java
📚 Learning: 2025-06-29T09:47:31.299Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
Applied to files:
src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.java
📚 Learning: 2025-07-03T03:05:05.031Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Applied to files:
src/main/java/konkuk/thip/user/application/port/out/UserCommandPort.java
📚 Learning: 2025-10-13T08:39:43.833Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 323
File: build.gradle:102-104
Timestamp: 2025-10-13T08:39:43.833Z
Learning: Spring AI 1.0.0-M6에서 Google AI Gemini 전용 스타터가 빈 등록에 실패하는 경우, spring-ai-openai-spring-boot-starter를 사용하고 외부 설정(환경 변수 등)으로 spring.ai.openai.base-url을 Google의 OpenAI 호환 엔드포인트로, spring.ai.openai.api-key를 Google Cloud 액세스 토큰으로, spring.ai.openai.chat.options.model을 Gemini 모델명으로 지정하여 우회할 수 있습니다.
Applied to files:
build.gradle
🧬 Code graph analysis (2)
src/test/java/konkuk/thip/user/concurrency/UserFollowConcurrencyTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(35-417)
src/main/java/konkuk/thip/user/application/service/following/UserFollowService.java (1)
src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java (1)
Service(29-150)
⏰ 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
🔇 Additional comments (6)
build.gradle (1)
102-107: spring-retry 의존성 추가 방향은 적절합니다 (Retry 설정값만 SLA 기준으로 재확인 권장)
- Spring Boot BOM을 쓰고 있어서 별도 버전 명시는 없어도 되고, 이미
spring-boot-starter-aop가 포함되어 있어@Retryable기반 프록시 구성이 가능한 상태라 의존성 추가 자체는 무난해 보입니다.- 이번 팔로우 동시성 이슈가 응답 지연으로 이어지고 있는 상황이니,
RetryConfig에 설정한maxAttempts, backoff, 그리고 JPA 락 타임아웃(5초)과의 조합으로 최악 응답 시간이 얼마까지 길어질 수 있는지 한 번만 계산·검토해 보시면 좋겠습니다.src/main/java/konkuk/thip/config/RetryConfig.java (1)
6-8: 전역 Retry 설정 클래스는 현재 형태로 충분해 보입니다.팔로우 동시성 처리에 필요한 @EnableRetry만 노출하고 있어 단순하고 목적에 맞게 구성된 것 같습니다.
spring-retry의존성이build.gradle에 누락되지 않았는지만 한 번만 더 확인해 주세요.src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
35-36: RESOURCE_LOCKED 에러 코드 추가가 전체 규칙과 잘 맞습니다.50000대 시스템/인프라 계열 코드 구간에 배치되어 있고,
HttpStatus.LOCKED및 메시지도 의도(락 경합 시 응답)과 잘 맞아 보입니다. 이 코드가 내려갈 때 클라이언트에서 423 상태 코드에 대한 공통 처리(문구, 재시도 전략 등)가 준비되어 있는지만 한번 점검해 두시면 좋겠습니다.src/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.java (1)
40-46: findByIdWithLock 추가가 기존 패턴과 일관되고 캡슐화가 잘 되어 있습니다.기존
findById와 동일한 예외 처리·매핑을 유지하면서 잠금 전용 경로를 메서드로 분리해 둔 점이 좋습니다.UserJpaRepository.findByUserIdWithLock쪽에서 실제로 필요한 LockMode(예: PESSIMISTIC_WRITE)가 적용되어 있는지만 한 번만 더 확인해 주세요.src/main/java/konkuk/thip/user/adapter/out/jpa/FollowingJpaEntity.java (1)
8-16: user_id + following_user_id 유니크 제약으로 중복 팔로잉 방지가 명확해졌습니다.엔티티 레벨에서 DB 유니크 제약을 그대로 선언해 두어 의도가 잘 드러나고, Hibernate 가 생성하는 예외 메시지도 일관될 것 같습니다. 마이그레이션(
V251120__Add_following_unique_constraint.sql)의 제약 이름과 컬럼 구성과 완전히 동일한지만 한번 더 확인해 두시면 좋겠습니다.src/main/java/konkuk/thip/user/application/service/following/UserFollowService.java (1)
22-22: 자기 자신 팔로우 방지는 이 위치에서 처리하는 것이 깔끔합니다.
validateParams에서userId.equals(targetUserId)를 바로 막고,USER_CANNOT_FOLLOW_SELF도메인 에러를 던지는 구조는 명확하고 재시도/락과도 잘 분리되어 있습니다.- 현재 형태 그대로 유지해도 충분해 보이며, 추후 검증 항목이 늘어나더라도 이 메서드에 모으는 패턴을 유지하면 가독성이 좋을 것 같습니다.
Also applies to: 83-87
Test Results488 tests 488 ✅ 47s ⏱️ Results for commit 9ade7f6. ♻️ This comment has been updated with latest results. |
seongjunnoh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니다!!
| @Configuration | ||
| @EnableRetry(proxyTargetClass = true) | ||
| public class RetryConfig { | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅋㅋㅋㅋ 동일한 retryConfig 3개가 반복되네요
| notRecoverable = { | ||
| BusinessException.class, | ||
| InvalidStateException.class | ||
| }, | ||
| noRetryFor = { | ||
| BusinessException.class, | ||
| InvalidStateException.class | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 재시도 X, recover X 이렇게 설정하셨군요 좋습니다!
그런데 혹시 발생할 수 있는 재시도 상황에서 changeFollowingState() 메서드가 새로 트랜잭션을 시작하는게 아니라, 기존 트랜잭션을 이어가도록 하신 이유가 있을까요??
(락 타임아웃 상황이 비교적 긴 상황에서 재시도가 발생하는 상황이 있을까 싶긴하지만) 혹시나 발생할 수 있는 재시도 상황에서 기존 트랜잭션으로 서비스 메서드를 재시도하게 되면 해당 요청 쓰레드가 DB 커넥션을 그만큼 길게 물고있다고 생각해서 저는 매번 새로 DB 커넥션을 맺도록 트랜잭션 propagation 설정을 REQUIRES_NEW로 설정하긴 했습니다!
단순 궁금증에 여쭤봅니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사실 구현 당시에는 재시도 시 각 시도마다 독립적인 트랜잭션으로 수행되는 것으로 이해하고 별도로 설정을 추가하지 않았습니다. 다만 성준님 질문을 보고 궁금해서 Spring Retry 공식 내용을 확인해보니, 관련해서 논의된 이슈가 있었습니다.
해당 이슈(#22)에서는 AOP로 동작하는 @Retryable의 order 기본값을 Ordered.LOWEST_PRECEDENCE - 1로 두어, 일반적으로 LOWEST_PRECEDENCE인 @Transactional보다 Retry AOP가 먼저 적용되도록(바깥에서 감싸도록) 하는 방향을 다루고 있습니다.
즉 기본 구성에서는 @Retryable이 바깥, @Transactional이 안쪽 구조가 되어 호출 흐름이 @Retryable -> @Transactional 순서로 형성되고, 결과적으로 재시도 시에는 이전 시도의 트랜잭션이 종료(롤백)된 뒤 다음 시도에서 트랜잭션이 새로 시작되는 형태로 동작하게 됩니다.
따라서 현재 제 코드에서도 재시도는 매번 새로운 트랜잭션에서 수행되는 것으로 보시면 될 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hd0rable
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨어용~~👍🏻🔥👍🏻 너무늦게봤네요 ㅎ..허허
|
|
||
| @Override | ||
| @Transactional | ||
| @Retryable( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니다 ~~ 룻구투미~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 309
File: src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java:36-44
Timestamp: 2025-09-23T08:31:05.161Z
Learning: buzz0331은 기술적 이슈에 대해 실용적인 해결책을 제시하면서도 과도한 엔지니어링을 피하는 균형감을 선호한다. 복잡도 대비 실제 발생 가능성을 고려하여 "굳이" 불필요한 솔루션보다는 심플함을 유지하는 것을 중요하게 생각한다.
📚 Learning: 2025-07-29T08:11:23.599Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 109
File: src/main/java/konkuk/thip/user/adapter/in/web/UserQueryController.java:37-46
Timestamp: 2025-07-29T08:11:23.599Z
Learning: THIP 프로젝트에서 ExceptionDescription 어노테이션은 비즈니스 로직에서 발생하는 커스텀 에러 코드가 있는 API에만 사용하며, bean validation만 수행하는 API에는 사용하지 않는다.
Applied to files:
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
🪛 GitHub Actions: CI with Gradle
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
[error] 36-36: ErrorCode is not abstract and does not override abstract method getMessage() in ResponseCode
[error] 36-36: RESOURCE_LOCKED is already defined in enum ErrorCode
[error] 36-36: Duplicate or conflicting enum entry detected for RESOURCE_LOCKED

#️⃣ 연관된 이슈
📝 작업 내용
첫번째 해결책으로는 User 조회시에 비관락을 걸어서 해결해보려고 하였습니다.
현재 팔로잉 코드 흐름이
FollowingCommandPort.findByUserIdAndTargetUserId()UserCommandPort.findById()followingCommandPort.save()다음과 같을때, 2번에서 User의 X-Lock을 먼저 획득하면 뒤에서 S-Lock -> X-Lock으로의 승격을 기다리지 않아 데드락을 해결할 수 있을 거라고 추론하였습니다.
부하테스트 결과, 데드락이 발생하지 않았습니다. 다만, 점진적으로 부하를 올리면서 측정해본 결과 VU = 5000에서 최대 20초정도의 요청 지연이 발생했고, VU = 10000에서는 무수히 많은 request timeout과 최대 1분까지의 요청 지연이 발생하는 것을 볼 수 있었습니다.
우선, 이 과정을 통해 비관락을 걸었을 때 데이터 정합성이 지켜지는 것을 보장하긴 하지만, 요청 지연이 너무 발생하는 것과 VU = 10000이 넘어가는 순간 요청 실패율이 급격하게 올라가는 것을 확인했습니다.
따라서, 이후 해결과정에서는 이벤트 기반 비동기 집계를 도입하여 요청 지연을 줄이고 조금더 대용량 트래픽을 견딜 수 있는 해결법을 강구해볼 생각입니다.
정리한 노션입니다. 참고해주세요.
https://separate-snowplow-d8b.notion.site/API-2b2b701eb0b880ae9cfbdff2ba1f5d2f?source=copy_link
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
릴리스 노트
버그 수정
개선 사항
✏️ Tip: You can customize this high-level summary in your review settings.