-
Notifications
You must be signed in to change notification settings - Fork 0
[test] 팔로잉 동시성 이슈 문제 확인 및 원인 파악 #335
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
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough팔로우/언팔로우 기능의 동시성 안정성을 검증하기 위해 k6 기반 부하 테스트 두 가지와 JPA 엔티티 고유 제약 조건, 데이터베이스 마이그레이션, 그리고 멀티스레드 통합 테스트를 추가했습니다. 중복 팔로우 관계 생성을 차단합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트 VU
participant Setup as setup()
participant TokenAPI as 토큰 API
participant FollowAPI as 팔로우 API
participant DB as 데이터베이스
Setup->>TokenAPI: 배치 토큰 발급 요청
TokenAPI-->>Setup: 토큰 배열 반환
Setup-->>Client: {tokens, startAt} 반환
Note over Client: startAt 시간 대기
activate Client
Client->>FollowAPI: POST /users/following/{TARGET_USER_ID}<br/>(Authorization: 토큰)
FollowAPI->>DB: 팔로우 관계 조회/생성
alt 고유 제약 조건 위반
DB-->>FollowAPI: 중복 오류 또는 200
else 새로운 팔로우
DB->>DB: 행 삽입
DB-->>FollowAPI: 성공 (200)
end
FollowAPI-->>Client: 응답 (상태 코드, 오류 코드)
Client->>Client: 지연 시간 기록 및 메트릭 업데이트
deactivate Client
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 분
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
Test Results488 tests 488 ✅ 47s ⏱️ Results for commit d8a5057. |
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 (3)
loadtest/follow_change_toggle_load_test.js (1)
1-162: 토글 부하 시나리오 구성과 메트릭 정의가 목적에 잘 맞습니다
- 토큰 배치 발급,
startAt기반 동시 시작,per-vu-iterations+ VU‑로컬isFollowing토글 조합이 의도한 “팔로우/언팔 반복 스팸” 시나리오를 잘 표현하고 있고, 메트릭/threshold 네이밍도 기존 스크립트와 일관적이라 이해하기 쉽습니다. 이 파일에서 눈에 띄는 기능적 버그는 없습니다.- 다만
check에서follow_change 200 or expected 4xx조건이 모든 4xx(예: 실수로 404/401)가 포함되도록 꽤 넓게 잡혀 있어서, 나중에 엔드포인트 오타나 인증 설정 문제 같은 걸 자동으로 잡고 싶다면ERR에 정의된 코드만 허용하도록 좁히는 것도 한 가지 방법일 것 같습니다(지금 PR 목적상 필수는 아니니 참고용).follow_change_state_load_test.js와 공통 로직을 유틸로 빼는 것도 가능하지만, 테스트 스크립트 특성상 현재처럼 각 파일이 독립적으로 완결된 형태가 가독성과 유지보수 면에서 더 단순해 보여서 그대로 두셔도 충분해 보입니다.Based on learnings
src/main/resources/db/migration/V251120__Add_following_unique_constraint.sql (1)
1-3: 기존 중복 데이터 존재 시 마이그레이션 실패 가능성 점검 권장
followings(user_id, following_user_id)에 UNIQUE 제약을 바로 추가하면, 이미 중복 로우가 쌓여 있는 환경에서는ALTER TABLE자체가 실패합니다(이번 이슈의 원인이 중복/정합성 문제였다면 특히 가능성이 높습니다).- 운영 DB에 반영하기 전에, 아래처럼 사전 중복 여부를 확인하거나 별도 정리용 마이그레이션을 한 번 두는 게 안전할 것 같습니다.
SELECT user_id, following_user_id, COUNT(*) AS cnt FROM followings GROUP BY user_id, following_user_id HAVING COUNT(*) > 1;src/test/java/konkuk/thip/user/concurrency/UserFollowConcurrencyTest.java (1)
36-120: 동시성 테스트의 동작은 타당하지만, 동시 출발 제어와 검증 강도 측면에서 약간 개선 여지가 있습니다
ExecutorService pool = newFixedThreadPool(Math.min(followerCount, 100))인데ready/finish래치 카운트는followerCount(500)라서, 실제로는 최대 100개 작업만start.await()에서 동시에 대기하고 나머지 400개는start해제 이후에 순차적으로 실행됩니다. “최대 동시 100개 정도면 충분하다”는 의도라면 동작 자체는 문제 없지만, 동시 출발을 래치로 제어하고 싶다면 카운트를 풀 사이즈로 맞추는 편이 코드 의도와 더 잘 맞습니다.- ExecutorService pool = Executors.newFixedThreadPool(Math.min(followerCount, 100)); - CountDownLatch ready = new CountDownLatch(followerCount); + int poolSize = Math.min(followerCount, 100); + ExecutorService pool = Executors.newFixedThreadPool(poolSize); + CountDownLatch ready = new CountDownLatch(poolSize);
- 또한 현재는 모든 비‑200 응답을
400으로 치환해서okCount만 세고, DB 검증도<= followerCount만 체크하기 때문에, 극단적으로는 API가 전부 4xx/5xx를 반환하거나(=okCount == 0), follower_count가 실제 성공 횟수보다 작게 저장되는 경우에도 이 테스트는 통과하게 됩니다.
- 이번 PR 목적이 “이슈 재현 및 원인 파악용” 로그/관찰 테스트라면 지금처럼 느슨한 검증도 이해되지만, 나중에 동시성 버그를 fix한 뒤 회귀 테스트로 쓰고 싶다면
okCount,followingRows,storedFollowerCount사이의 관계를 더 강하게 검증하는 별도 테스트를 추가하는 걸 추천드립니다. 예를 들어, 안정화 단계에서는followingRows와storedFollowerCount를okCount와 같다고 단언하는 테스트를 하나 더 두는 식입니다.
📜 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 (5)
loadtest/follow_change_state_load_test.js(1 hunks)loadtest/follow_change_toggle_load_test.js(1 hunks)src/main/java/konkuk/thip/user/adapter/out/jpa/FollowingJpaEntity.java(1 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 (1)
📓 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은 기술적 이슈에 대해 실용적인 해결책을 제시하면서도 과도한 엔지니어링을 피하는 균형감을 선호한다. 복잡도 대비 실제 발생 가능성을 고려하여 "굳이" 불필요한 솔루션보다는 심플함을 유지하는 것을 중요하게 생각한다.
🧬 Code graph analysis (1)
src/test/java/konkuk/thip/user/concurrency/UserFollowConcurrencyTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(35-417)
⏰ 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 (2)
loadtest/follow_change_state_load_test.js (1)
1-149: 동시 팔로우 버스트(k6) 시나리오가 명확하고 기존 구조와 잘 정렬돼 있습니다
USERS_COUNTVU를 1:1로 사용자에 매핑하고,setup()에서 토큰을 1:1로 미리 발급한 뒤startAt기준으로 동기화해서 한 번씩만 팔로우를 쏘는 구조가 실제 “인기 작가 프로모션 순간 트래픽”을 직관적으로 잘 모사하고 있습니다.- 앞선 토글 스크립트와 동일한 메트릭/에러 코드 체계를 재사용하고, 5xx=0 / p95<1000ms threshold를 준 것도 일관적이라 분석할 때 헷갈릴 지점이 없어 보입니다. 현재 상태로 충분히 목적에 부합해 보여 별도 수정을 권할 부분은 없습니다.
src/main/java/konkuk/thip/user/adapter/out/jpa/FollowingJpaEntity.java (1)
8-16: 엔티티 레벨 unique 제약 설정이 SQL 마이그레이션과 일관적입니다
@Table의uniqueConstraints이름과 컬럼 목록이V251120__Add_following_unique_constraint.sql의 제약과 정확히 일치해서, JPA 엔티티와 DB 스키마 사이에 불일치가 생길 여지가 없어 보입니다.- 이 제약 추가로 인해 중복 팔로우 저장 시
DataIntegrityViolationException등이 발생할 텐데, 상위 계층에서 이를 적절히 매핑(예: 도메인 에러 코드 70001/75001)만 하고 있다면 현재 변경 자체에는 문제 없어 보입니다.
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.
수고하셨습니닷!!~~ 노션 설명진짜 가독성 넘넘좋네여 ㅎ 👍🏻
|
|
||
| @Entity | ||
| @Table(name = "followings") | ||
| @Table( |
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.
확인했습니닷 !! 👍🏻
| @@ -0,0 +1,3 @@ | |||
| ALTER TABLE followings | |||
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.
확인했습니닷 !! 별거아니지만 COMMENT나 주석처리 가능할까요??
다른 마이그레이션 파일들도 변경사항이 COMMENT나 주석으로 추가설명이 기재되어있고 추후에 마이그레이션 파일들을보며 어떤사항때문에 db가 변경되었다는걸 알기가 편할것같습니다!!
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.
네넵 다음 브랜치에서 주석 추가하도록 하겠습니다!
근데 저희 지금까지 마이그레이션 파일 대부분 주석처리 안해뒀던 것 같네요 😂
#️⃣ 연관된 이슈
📝 작업 내용
k6 부하 테스트 시나리오 2가지
테스트 결과와 원인 분석은 노션에 정리해두었으니 확인 부탁드립니다! 앞에 분들이 이미 답을 잘 찾아두셔서 한번더 정리하는 느낌이였습니다,, 😅
https://separate-snowplow-d8b.notion.site/API-2aab701eb0b880ef8dd9e542c732d4a8?source=copy_link
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
개선 사항
테스트
✏️ Tip: You can customize this high-level summary in your review settings.