Skip to content

Conversation

@seongjunnoh
Copy link
Collaborator

@seongjunnoh seongjunnoh commented Oct 29, 2025

#️⃣ 연관된 이슈

closes #325

📝 작업 내용

  • '인기 작가 모임방에 여러 유저들이 동시에 모임방 참여 요청을 보낼 경우' 에 대한 시나리오를 설정하고, 이를 테스트 코드와 부하 테스트 스크립트로 재현하여 테스트 하였습니다.
  1. 테스트 코드
  • 동시에 여러 유저들이 모임방 참여 요청을 보내는 상황을 재현하기 위해, java.util.concurrent.ExecutorService 를 활용하여 여러 스레드가 동시에 모임방 참여 api 요청을 보내도록 구현
  • 모임방 제한인원보다 많은 수의 200 OK 응답(= 모임방 참여 OK 응답) 이 발생하는 것을 확인
  • 마찬가지로 DB에 insert 된 room_participants 레코드의 숫자가 해당 rooms 레코드의 member_count 값보다 많은 것을 확인
  1. 부하 테스트 스크립트
  • 'per-vu-iterations(iterations : 1)' 시나리오를 설정하여, 수많은 사람들이 동시에 방 참여를 1번씩 요청하는 상황에 대하여 부하 테스트 진행
  • 모든 요청에 대한 응답으로 200 OK(= 모임방 참여 성공), 400 Error(= 모임방 인원 수 초과) 응답이 와야 정상이지만, 다수의 500 Error 발생을 확인하였음(-> 콘솔 로그 확인 결과 deadlock 이슈로 인한 500 Error)

📸 스크린샷

image -> 테스트 코드 결과

https://separate-snowplow-d8b.notion.site/API-1-29ab701eb0b880c79fd7ed9cfd744384?source=copy_link
-> 부하 테스트 결과와 데드락 원인에 대해 정리한 글

💬 리뷰 요구사항

  • '방 참여 api' 의 개선 방향을
  1. 서버의 응답 안정성 향상(= 500 error 발생 포인트 확인 및 해결 -> 방 참여 요청에 대해서는 '방 참여 OK' or '모임방 인원 수 초과' 의 응답만 발생하도록)
  2. 서버의 응답 처리율 향상 -> 동시에 많은 요청이 몰리더라도, 응답 시간을 안정적으로 유지

로 설정하였습니다.

추후 새로 브랜치파서 위와 관련된 작업 진행해보겠습니다.

  • 부하테스트 패키지를 프로젝트 최상위 디렉토리 하위에 위치시켰습니다. 혹시 다른 의견 있으시면 리뷰 부탁드립니다!
  • 멀티스레드로 많은 요청을 보내는 테스트 코드는 CI 과정에서 제외하고 빌드되는게 맞는거 같아서 @tag 어노테이션 활용 및 build.gradle 에서 test 설정 부분을 수정하였습니다! 이 부분 확인 부탁드립니다!

📌 PR 진행 시 이러한 점들을 참고해 주세요

* P1 : 꼭 반영해 주세요 (Request Changes) - 이슈가 발생하거나 취약점이 발견되는 케이스 등
* P2 : 반영을 적극적으로 고려해 주시면 좋을 것 같아요 (Comment)
* P3 : 이런 방법도 있을 것 같아요~ 등의 사소한 의견입니다 (Chore)

Summary by CodeRabbit

Release Notes

  • Tests

    • 룸 동시 참여 기능에 대한 동시성 테스트 추가
    • 데이터베이스 플러시 작업 검증 테스트 추가
  • Chores

    • 로드 테스트 스크립트 추가
    • CI 환경에서 동시성 관련 테스트 제외 설정

- 멀티스레드 환경으로 동시에 방 참여 api 요청이 몰리는 상황 재현
- 현재 방 참여 api 코드에 동시성 이슈가 있음을 테스트 코드로 확인
- 해당 테스트 코드는 CI 과정에서 제외될 수 있도록 태그 부여 및 build.gradle 의 test 부분 설정 수정
- k6를 활용하여 방 참여 api 부하테스트를 수행할 수 있는 스크립트 추가
- '인기 작가 모임방에 동시에 여러 유저들이 방 참여를 원할 경우' 에 대한 시나리오 테스트 수행
- 이때 서버의 요청처리 안정성, 처리율을 매트릭 지표로 수집
- 부하 테스트 결과를 통해 현재 방 참여 api 코드에는 500 error 발생 지점, 응답 지연 구간이 존재함을 확인
- 신규 엔티티와, dirty 엔티티에 대해서 flush 시점에 어떤 sql이 DB로 나가는지 확인
@seongjunnoh seongjunnoh linked an issue Oct 29, 2025 that may be closed by this pull request
3 tasks
@coderabbitai
Copy link

coderabbitai bot commented Oct 29, 2025

Walkthrough

방 참여 API의 동시성 및 부하 테스트를 위한 테스트 코드와 스크립트를 추가합니다. CI 환경에서 동시성 테스트를 조건부로 제외하도록 빌드 설정을 변경하고, k6 부하테스트 스크립트, 멀티스레드 통합 테스트, 지원 테스트 메서드 및 팩토리 메서드를 추가합니다.

Changes

Cohort / File(s) 변경 요약
빌드 설정
build.gradle
CI 환경 변수가 'true'일 때 동시성 태그 테스트를 JUnit Platform에서 제외하도록 조건부 설정 추가
부하 테스트
loadtest/room_join_load_test.js
k6 기반 버스트 조인 시나리오 부하 테스트 스크립트 추가 (토큰 배치 발급, 동기화된 시작, 응답 지표 기록, 에러 분류 포함)
테스트 헬퍼
src/test/java/konkuk/thip/common/util/TestEntityFactory.java
지정된 recruitCount를 가진 RoomJpaEntity를 생성하는 새로운 팩토리 메서드 createCustomRoom 추가
Persistence 테스트
src/test/java/konkuk/thip/common/persistence/JpaRepositoryMethodTest.java
flush 시점의 SQL 쿼리 동작을 검증하는 두 개의 새로운 테스트 메서드 (dirty 엔티티, 신규 엔티티) 추가
동시성 통합 테스트
src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java
500개의 동시 스레드로 방 참여 요청을 수행하고 경쟁 조건을 검증하는 새로운 통합 테스트 클래스 추가

Sequence Diagram(s)

sequenceDiagram
    participant LoadTest as k6 Load Test
    participant Setup as Setup Phase
    participant Token as Token Service
    participant Server as API Server
    participant Metrics as Metrics Collector

    LoadTest->>Setup: Initialize test
    Setup->>Token: Batch token requests<br/>(BATCH_SIZE chunks)
    Token-->>Setup: tokens array
    Setup->>LoadTest: Return tokens + startAt
    
    par Burst Join Requests
        LoadTest->>LoadTest: Wait until startAt<br/>(synchronized start)
        LoadTest->>Server: POST /rooms/{roomId}/join<br/>(bearer token)
        Server-->>LoadTest: Response (2xx/4xx/5xx)
        LoadTest->>Metrics: Record joinLatency
        LoadTest->>Metrics: Categorize error<br/>(if applicable)
    end
    
    Metrics-->>LoadTest: Validate thresholds<br/>(p95 latency, 5xx count)
Loading
sequenceDiagram
    participant Test as RoomJoinConcurrencyTest
    participant Setup as Setup Phase
    participant ThreadPool as Thread Pool
    participant MockMvc as MockMvc
    participant DB as Database
    participant Validation as Validation Phase

    Test->>Setup: Create book, room (capacity: 10)<br/>Create 500 users
    Setup-->>Test: Initial state ready
    
    Test->>ThreadPool: Create 500 concurrent tasks
    par Concurrent Join Requests
        ThreadPool->>MockMvc: POST /rooms/{roomId}/join<br/>(500 concurrent requests)
        MockMvc->>DB: Process join requests
        DB-->>MockMvc: Response (200 OK / error)
    end
    
    ThreadPool-->>Test: All tasks completed
    
    Test->>Validation: Query room state
    Validation->>DB: Count participants, memberCount,<br/>recruitCount
    DB-->>Validation: Query results
    Validation->>Test: Assert participant count ><br/>recruitCount, memberCount <br/>< participant count
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 분

  • 주의 필요 영역:
    • RoomJoinConcurrencyTest.java: 동시성 검증 로직의 정확성, 카운트다운 래치 사용, 데이터베이스 상태 검증 단언문 검토 필요
    • room_join_load_test.js: k6 시나리오 구성, 토큰 배치 처리, 에러 파싱 로직 및 메트릭 임계값 검증 필요
    • JpaRepositoryMethodTest.java: flush 타이밍과 예상되는 SQL 쿼리 타입 (INSERT/UPDATE) 검증 필요

Possibly related PRs

Suggested labels

🧹 chore, 👻 성준

Suggested reviewers

  • buzz0331
  • hd0rable

Poem

🐰 부하 테스트 스크립트 뛰다
k6 번들 속 500 마리 토끼,
동시에 방으로 콩콩콩!
경쟁 조건도 잡아내고,
성능 지표도 기록하고,
우리 서버는 안녕할까요? 🚀

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning 대부분의 변경사항은 모임방 참여 API 부하 테스트와 직접 관련이 있습니다: build.gradle(CI 환경에서 동시성 테스트 제외 설정)은 테스트 실행 환경 구성에 필요하고, TestEntityFactory의 추가 메서드는 RoomJoinConcurrencyTest에서 테스트 데이터 생성에 필요합니다. 다만 src/test/java/konkuk/thip/common/persistence/JpaRepositoryMethodTest.java의 변경사항(더티 엔티티와 새로운 엔티티의 SQL 플러시 동작 테스트)은 모임방 참여 API 부하 테스트 요구사항과 직접적인 연관성이 없으며, 일반적인 JPA 리포지토리 동작 검증에 관한 것으로 보입니다. JpaRepositoryMethodTest.java의 변경사항은 이슈 #325의 요구사항 범위를 벗어나는 것으로 보이므로 제거하거나 별도의 PR로 분리하는 것을 권장합니다. 현재 PR의 범위를 명확히 모임방 참여 API 부하 테스트에만 집중시키면 리뷰 및 유지보수가 더 용이할 것입니다.
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed PR 제목 "[test] 모임방 참여 api 부하 테스트 진행"은 변경사항의 주요 목표인 모임방 참여 API에 대한 부하 테스트 수행을 명확하고 간결하게 전달합니다. 제목은 변경사항의 핵심인 부하 테스트(load test)를 명시적으로 지칭하며, PR 목표에서 우선적으로 언급된 내용을 반영합니다. 다만 제목은 멀티스레드 동시성 테스트 코드 측면도 동등하게 포함되어 있음을 완전히 나타내지 못합니다.
Linked Issues Check ✅ Passed 링크된 이슈 #325의 요구사항은 다음 세 가지입니다: (1) 멀티스레드 환경에서 동시 모임방 참여 요청 테스트 코드 작성 [RoomJoinConcurrencyTest.java 추가로 충족], (2) 부하 테스트를 통한 동시 요청 시나리오 테스트 [room_join_load_test.js k6 스크립트 추가로 충족], (3) 발생하는 문제 상황 파악 [500 에러, 데드락, 동시성으로 인한 허용 초과 등을 식별할 수 있는 테스트 설계로 충족]. PR의 변경사항들이 세 가지 요구사항을 모두 구현하였습니다.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch test/#325-room-join-concurrency

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • API-1: Request failed with status code 404

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

Test Results

488 tests   488 ✅  47s ⏱️
145 suites    0 💤
145 files      0 ❌

Results for commit 974f520.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/test/java/konkuk/thip/common/persistence/JpaRepositoryMethodTest.java (1)

120-138: 테스트 로직은 정확하나, SQL 검증은 콘솔 로그에 의존합니다.

테스트는 dirty entity에 대한 UPDATE 쿼리 발생을 검증하는 것이 목적이지만, 실제로는 주석에서 "UPDATE 쿼리가 발생하는 것 확인"이라고 명시하고 콘솔 로그 관찰에 의존합니다. JPA 동작을 학습하기 위한 테스트로는 적절하지만, 자동화된 검증은 없습니다.

필요시 @Sql 또는 테스트용 SQL 로깅 검증 라이브러리를 활용하여 실제 SQL 실행을 자동으로 검증할 수 있습니다.

loadtest/room_join_load_test.js (1)

5-11: 설정 값이 하드코딩되어 있습니다.

ROOM_IDUSERS_START가 하드코딩되어 있어 테스트 실행 전 수동 설정이 필요합니다. Line 65 주석에서 사전 준비를 언급하고 있습니다.

환경 변수를 통해 설정값을 주입할 수 있도록 개선하면 재사용성이 향상됩니다:

const BASE_URL      = __ENV.BASE_URL || 'http://localhost:8080';
const ROOM_ID       = parseInt(__ENV.ROOM_ID || '12345');
const USERS_START   = parseInt(__ENV.USERS_START || '10000');
const USERS_COUNT   = parseInt(__ENV.USERS_COUNT || '500');
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 7c65945 and 974f520.

📒 Files selected for processing (5)
  • build.gradle (1 hunks)
  • loadtest/room_join_load_test.js (1 hunks)
  • src/test/java/konkuk/thip/common/persistence/JpaRepositoryMethodTest.java (1 hunks)
  • src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1 hunks)
  • src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#285
File: src/main/java/konkuk/thip/room/adapter/out/jpa/RoomStatus.java:1-7
Timestamp: 2025-08-31T05:25:14.845Z
Learning: seongjunnoh는 enum 의존성에 대해 유연한 접근을 선호하며, 도메인→어댑터 레이어 참조와 같은 아키텍처 layering 원칙보다 실용적인 구현을 우선시한다.
🧬 Code graph analysis (1)
src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java (2)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
  • TestEntityFactory (35-417)
loadtest/room_join_load_test.js (3)
  • body (118-118)
  • i (70-70)
  • userIds (67-67)
⏰ 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 (9)
build.gradle (1)

122-127: CI 환경에서 동시성 테스트 제외 구성이 적절합니다.

동시성 테스트는 비결정적이고 실행 시간이 길 수 있으므로, CI 환경에서 제외하는 것이 합리적입니다. RoomJoinConcurrencyTest@Tag("concurrency")와 일치합니다.

src/test/java/konkuk/thip/common/persistence/JpaRepositoryMethodTest.java (1)

140-152: 테스트 로직은 정확하나, SQL 검증은 콘솔 로그에 의존합니다.

신규 엔티티에 대한 INSERT 쿼리 발생을 검증하는 테스트이지만, 실제 검증은 콘솔 로그 관찰에 의존합니다. JPA 학습용 테스트로는 적절합니다.

src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)

150-162: 테스트용 방 생성 팩토리 메서드가 적절합니다.

recruitCount를 파라미터로 받는 새로운 오버로드가 추가되어 동시성 테스트에서 특정 정원의 방을 생성하는데 유용합니다. roomStatusRECRUITING으로 설정한 것도 방 참여 테스트에 적합합니다.

src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java (1)

142-149: 헬퍼 메서드가 명확하고 적절합니다.

지정된 수만큼 사용자를 생성하고 ID 목록을 반환하는 로직이 올바르게 구현되었습니다.

loadtest/room_join_load_test.js (5)

13-28: 커스텀 메트릭 정의가 잘 설계되었습니다.

지연시간 추적과 HTTP 상태 코드별 카운터, 실패 원인별 세부 카운터를 통해 동시성 문제를 효과적으로 관찰할 수 있습니다. 에러 코드도 명확히 정의되어 있습니다.


30-43: 응답 파싱 로직이 방어적으로 잘 구현되었습니다.

BaseResponse 구조를 파싱하며 JSON 파싱 실패 시에도 안전하게 폴백 값을 반환합니다.


47-62: 시나리오 설계가 PR 목표와 일치합니다.

per-vu-iterations executor로 500명의 동시 요청 버스트를 재현하며, threshold에서 5xx 에러가 0건이어야 한다는 조건은 "서버 응답 안정성 향상" 목표와 정확히 일치합니다.


99-148: 메인 실행 로직이 정확하고 포괄적입니다.

VU-to-user 매핑, 동기화된 시작, 요청 실행, 메트릭 기록, 응답 검증이 모두 올바르게 구현되었습니다. 에러 분류 로직(lines 129-138)을 통해 실패 원인을 세밀하게 추적할 수 있습니다.


66-96: 토큰 API 응답 형식 확인 완료 - 코드가 올바릅니다.

TestController.java 라인 17-20에서 /api/test/token/access 엔드포인트는 public String generateAccessToken(@RequestParam Long userId)로 구현되어 있으며, 평문 문자열로 토큰을 반환합니다. 따라서 loadtest 파일의 r.body.trim() 사용이 정확하며, JSON 파싱이 필요하지 않습니다. 배치 처리, 에러 핸들링, 동기화 시작 시간 계산 모두 적절하게 구현되었습니다.

Comment on lines +66 to +99
ExecutorService pool = Executors.newFixedThreadPool(Math.min(requestUserCount, 100));
CountDownLatch ready = new CountDownLatch(requestUserCount);
CountDownLatch start = new CountDownLatch(1);
CountDownLatch finish = new CountDownLatch(requestUserCount);

RoomJoinRequest body = new RoomJoinRequest("join");
String json = objectMapper.writeValueAsString(body);

List<Future<Integer>> results = new ArrayList<>(requestUserCount);
for (int i = 0; i < requestUserCount; i++) {
final long userId = requestUserIds.get(i);
results.add(pool.submit(() -> {
ready.countDown();
start.await();
try {
mockMvc.perform(post("/rooms/{roomId}/join", room.getRoomId())
.contentType("application/json")
.content(json)
.requestAttr("userId", userId))
.andExpect(status().isOk());
return 200;
} catch (AssertionError e) {
return 400;
} finally {
finish.countDown();
}
}));
}

// 동시에 시작
ready.await(10, TimeUnit.SECONDS);
start.countDown();
finish.await(60, TimeUnit.SECONDS);
pool.shutdown();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

동시성 동기화 로직은 적절하나, 타임아웃 처리 개선이 필요합니다.

CountDownLatch를 활용한 동기화 패턴은 정확하나, 다음 사항들을 고려해야 합니다:

  1. 타임아웃 검증 부재: ready.await(10, TimeUnit.SECONDS)finish.await(60, TimeUnit.SECONDS)가 타임아웃될 경우 반환값을 확인하지 않고 진행합니다. 일부 스레드가 준비되지 않았거나 완료되지 않은 상태에서 테스트가 진행될 수 있습니다.

  2. 예외 처리 불완전: Callable 내부에서 AssertionError만 처리하고 다른 예외는 처리하지 않습니다. 예외 발생 시 Future.get()에서 ExecutionException이 발생하여 Line 103의 스트림 처리에서 문제가 될 수 있습니다.

다음과 같이 개선할 수 있습니다:

 // 동시에 시작
-ready.await(10, TimeUnit.SECONDS);
+boolean allReady = ready.await(10, TimeUnit.SECONDS);
+assertThat(allReady).isTrue();
 start.countDown();
-finish.await(60, TimeUnit.SECONDS);
+boolean allFinished = finish.await(60, TimeUnit.SECONDS);
+assertThat(allFinished).isTrue();
 pool.shutdown();

예외 처리 개선:

             try {
                 mockMvc.perform(post("/rooms/{roomId}/join", room.getRoomId())
                                 .contentType("application/json")
                                 .content(json)
                                 .requestAttr("userId", userId))
                         .andExpect(status().isOk());
                 return 200;
             } catch (AssertionError e) {
                 return 400;
+            } catch (Exception e) {
+                return 500;
             } finally {
                 finish.countDown();
             }
📝 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
ExecutorService pool = Executors.newFixedThreadPool(Math.min(requestUserCount, 100));
CountDownLatch ready = new CountDownLatch(requestUserCount);
CountDownLatch start = new CountDownLatch(1);
CountDownLatch finish = new CountDownLatch(requestUserCount);
RoomJoinRequest body = new RoomJoinRequest("join");
String json = objectMapper.writeValueAsString(body);
List<Future<Integer>> results = new ArrayList<>(requestUserCount);
for (int i = 0; i < requestUserCount; i++) {
final long userId = requestUserIds.get(i);
results.add(pool.submit(() -> {
ready.countDown();
start.await();
try {
mockMvc.perform(post("/rooms/{roomId}/join", room.getRoomId())
.contentType("application/json")
.content(json)
.requestAttr("userId", userId))
.andExpect(status().isOk());
return 200;
} catch (AssertionError e) {
return 400;
} finally {
finish.countDown();
}
}));
}
// 동시에 시작
ready.await(10, TimeUnit.SECONDS);
start.countDown();
finish.await(60, TimeUnit.SECONDS);
pool.shutdown();
ExecutorService pool = Executors.newFixedThreadPool(Math.min(requestUserCount, 100));
CountDownLatch ready = new CountDownLatch(requestUserCount);
CountDownLatch start = new CountDownLatch(1);
CountDownLatch finish = new CountDownLatch(requestUserCount);
RoomJoinRequest body = new RoomJoinRequest("join");
String json = objectMapper.writeValueAsString(body);
List<Future<Integer>> results = new ArrayList<>(requestUserCount);
for (int i = 0; i < requestUserCount; i++) {
final long userId = requestUserIds.get(i);
results.add(pool.submit(() -> {
ready.countDown();
start.await();
try {
mockMvc.perform(post("/rooms/{roomId}/join", room.getRoomId())
.contentType("application/json")
.content(json)
.requestAttr("userId", userId))
.andExpect(status().isOk());
return 200;
} catch (AssertionError e) {
return 400;
} catch (Exception e) {
return 500;
} finally {
finish.countDown();
}
}));
}
// 동시에 시작
boolean allReady = ready.await(10, TimeUnit.SECONDS);
assertThat(allReady).isTrue();
start.countDown();
boolean allFinished = finish.await(60, TimeUnit.SECONDS);
assertThat(allFinished).isTrue();
pool.shutdown();
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java
around lines 66 to 99, the test ignores await() boolean returns and only catches
AssertionError inside the tasks; update the test to (1) verify the boolean
results of ready.await(10, TimeUnit.SECONDS) and finish.await(60,
TimeUnit.SECONDS) and fail the test immediately (e.g., throw AssertionError or
call Assertions.fail) if either returns false, and (2) broaden the task's
exception handling to catch Throwable/Exception (not just AssertionError),
ensure finish.countDown() still runs in finally, and return a distinct failure
code or rethrow a runtime exception so Future.get() does not produce unexpected
ExecutionExceptions; finally, after pool.shutdown(), iterate futures and handle
InterruptedException/ExecutionException explicitly to fail the test with clear
messages if any task failed.

Comment on lines +112 to +116
long participantRows = jdbcTemplate.query(
"SELECT COUNT(*) FROM room_participants WHERE room_id = ?",
ps -> ps.setLong(1, room.getRoomId()),
rs -> {rs.next(); return rs.getLong(1); }
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

JDBC 쿼리 패턴이 비정상적입니다.

jdbcTemplate.queryResultSetExtractor 람다가 단일 값을 반환하고 있지만, query 메서드는 ResultSetExtractor가 null을 반환할 수 있어 NPE 위험이 있습니다.

더 안전하고 명확한 방법을 사용하세요:

-long participantRows = jdbcTemplate.query(
-        "SELECT COUNT(*) FROM room_participants WHERE room_id = ?",
-        ps -> ps.setLong(1, room.getRoomId()),
-        rs -> {rs.next(); return rs.getLong(1); }
-);
+Long participantRows = jdbcTemplate.queryForObject(
+        "SELECT COUNT(*) FROM room_participants WHERE room_id = ?",
+        Long.class,
+        room.getRoomId()
+);
📝 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
long participantRows = jdbcTemplate.query(
"SELECT COUNT(*) FROM room_participants WHERE room_id = ?",
ps -> ps.setLong(1, room.getRoomId()),
rs -> {rs.next(); return rs.getLong(1); }
);
Long participantRows = jdbcTemplate.queryForObject(
"SELECT COUNT(*) FROM room_participants WHERE room_id = ?",
Long.class,
room.getRoomId()
);
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java
around lines 112-116, the jdbcTemplate.query call uses a ResultSetExtractor
lambda that returns a single long and risks an NPE; replace it with
jdbcTemplate.queryForObject("SELECT COUNT(*) FROM room_participants WHERE
room_id = ?", Long.class, room.getRoomId()) (or Integer.class if preferred) and
handle possible null by unboxing safely or using Objects.requireNonNull/Optional
to ensure a non-null primitive long for assertions.

Copy link
Member

@hd0rable hd0rable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

노션이랑 테스트코드 모두 확인했습니다!! 성준띠가 어떻게 해결하실지 궁금하네여 ㅎㅎ

tasks.named('test') {
useJUnitPlatform()
useJUnitPlatform {
if (System.getenv('CI') == 'true') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 좋습니닷 👍🏻👍🏻

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 이렇게하면 깃허브 액션에서 CI 작업시에 제외시켜 주는건가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI, CD 시에 해당 테스트 코드는 제외하기 위해서 찾아봤는데 이런 방법이 있는거 같아서 적용하였습니다.
다만 실제로 잘 동작하는지는 확실하진 않습니다. 좀 더 알아보겠슴다

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI, CD 시에 해당 테스트 코드는 제외하기 위해서 찾아봤는데 이런 방법이 있는거 같아서 적용하였습니다. 다만 실제로 잘 동작하는지는 확실하진 않습니다. 좀 더 알아보겠슴다

@coderabbitai 내가 적용한 방식으로 CI 시에 해당 테스트 메서드 클래스를 제외할 수 있는게 맞나?? 그리고 실제로 잘 적용되었는지를 확인하기 위한 방법이 뭐가 있을까?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the RoomJoinConcurrencyTest has the @Tag annotation
cat src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java | head -50

Length of output: 2289


🏁 Script executed:

#!/bin/bash
# Search for any @Tag annotations in the test file
rg -n "@Tag" src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java

Length of output: 22


🏁 Script executed:

#!/bin/bash
# Check GitHub Actions workflow files to see how CI environment variable is set
fd -e yml -e yaml . .github/workflows --exec cat {}

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Find GitHub Actions workflow files
find .github -type f -name "*.yml" -o -name "*.yaml" 2>/dev/null

Length of output: 341


🏁 Script executed:

#!/bin/bash
# List all files in .github directory
ls -la .github/ 2>/dev/null || echo "No .github directory found"

Length of output: 302


🏁 Script executed:

#!/bin/bash
# Check if CI environment variable is commonly set in GitHub Actions
# Look for any workflow files
fd -t f . .github 2>/dev/null | head -20

Length of output: 32


🏁 Script executed:

#!/bin/bash
# Check the CI workflow file
cat .github/workflows/ci-workflow.yml

Length of output: 2057


🏁 Script executed:

#!/bin/bash
# Also check CD workflow files to see if they run tests
cat .github/workflows/cd-workflow-dev.yml
cat .github/workflows/cd-workflow-prod.yml

Length of output: 5559


@seongjunnoh

네, 적용한 방식이 맞습니다! 확인 결과:

✅ 설정이 올바른 이유

  1. 테스트 클래스에 태그 적용됨: RoomJoinConcurrencyTest@Tag("concurrency")가 38번째 줄에 정확히 적용되어 있습니다
  2. Gradle 설정 정확함: JUnit 5의 excludeTags 사용이 올바릅니다
  3. CI 환경변수: GitHub Actions는 기본적으로 모든 워크플로우에서 CI=true 환경변수를 자동으로 설정합니다

🔍 동작 확인 방법

1. GitHub Actions 로그 확인 (가장 확실한 방법)

PR을 올리면 CI 워크플로우가 실행되는데, Actions 탭에서 로그를 확인하세요:

Test > Task :test
> konkuk.thip.room.concurrency.RoomJoinConcurrencyTest > SKIPPED

또는 전체 테스트 수가 1개 줄어든 것을 확인할 수 있습니다.

2. 로컬에서 테스트

# CI 환경을 시뮬레이션하여 테스트
CI=true ./gradlew test --info

# 또는 상세 로그로 확인
CI=true ./gradlew test --info | grep -i "concurrency\|excluded\|skipped"

3. 임시 확인 코드 추가

build.gradle에 임시로 로그를 추가해서 확인:

tasks.named('test') {
    useJUnitPlatform {
        if (System.getenv('CI') == 'true') {
            excludeTags 'concurrency'
            println "🚫 CI 환경 감지: concurrency 태그 테스트 제외됨"
        } else {
            println "💻 로컬 환경: 모든 테스트 실행"
        }
    }
}

💡 추가 개선 제안

명시성을 위해 .github/workflows/ci-workflow.ymlenv 섹션에 CI: true를 명시적으로 추가하는 것도 좋습니다 (GitHub Actions가 자동으로 설정하지만, 코드로 명확히 표현):

env:
  CI: true  # 명시적 선언
  RESOURCE_PATH: src/main/resources
  TEST_RESOURCE_PATH: src/test/resources
  FIREBASE_KEY_PATH: ${{ secrets.FIREBASE_KEY_PATH }}

다음 PR CI 실행 시 Actions 로그에서 테스트 수를 확인해보시면 RoomJoinConcurrencyTest가 제외된 것을 확인하실 수 있을 겁니다!


🧠 Learnings used
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.

Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.

Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 285
File: src/main/java/konkuk/thip/room/adapter/out/jpa/RoomStatus.java:1-7
Timestamp: 2025-08-31T05:25:14.845Z
Learning: seongjunnoh는 enum 의존성에 대해 유연한 접근을 선호하며, 도메인→어댑터 레이어 참조와 같은 아키텍처 layering 원칙보다 실용적인 구현을 우선시한다.

Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.

Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 305
File: src/main/java/konkuk/thip/notification/adapter/out/persistence/repository/NotificationQueryRepository.java:9-13
Timestamp: 2025-09-17T06:40:49.863Z
Learning: seongjunnoh는 메서드 네이밍 시 구현 세부사항보다 비즈니스 의도를 반영하는 것을 선호하며, 미래 확장성과 아키텍처 변화를 고려한 설계 철학을 가지고 있다.

Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.

Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/common/exception/code/ErrorCode.java:210-211
Timestamp: 2025-08-24T09:40:48.595Z
Learning: seongjunnoh는 HTTP 상태 코드 선택에 대해 기술적 근거와 코드베이스 내 일관성을 중요하게 생각하며, 구체적인 사례 분석을 통한 설명을 선호한다.

Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/TestTokenController.java:0-0
Timestamp: 2025-08-24T09:33:52.982Z
Learning: seongjunnoh는 Spring의 ConditionalOnProperty 동작 원리를 정확히 이해하고 있으며, 보안 이슈에 대해서도 실질적인 위험성을 기준으로 판단하는 실용적 접근을 선호한다.

Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.

Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 166
File: src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java:70-82
Timestamp: 2025-08-07T18:19:55.908Z
Learning: seongjunnoh는 Clean Architecture 원칙을 중시하며, 어댑터는 순수하게 저장/조회 기능만 담당하고 비즈니스 로직은 서비스 레이어에서 처리하는 것을 선호한다.

return { tokens, startAt };
}

// VU : 각자 자기 토큰으로 참여 호출 & 각자 1회만 실행
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하 이렇게 진짜 어느 특정시점에 방참여가 몰리는 부하테스트를 실행하신거군요 좋네요

@Autowired private JdbcTemplate jdbcTemplate;

@Test
@DisplayName("[동시성] 방 참여 동시 요청 테스트")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이테스트 h2환경에서 실행해도 데드락 상황이 로그에 남나요??

Copy link
Collaborator Author

@seongjunnoh seongjunnoh Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아니요 테스트 코드에서 명확한 500 error 관련 로그를 확인하지는 못했습니다!
테스트 코드로는 api 요청에 대한 200 OK 응답 개수와 테스트 이후 DB 에 기록된 room_participants, rooms 레코드의 데이터를 확인하여 기댓값과 비교하는 것을 목표로 했습니다!
저희 운영환경에서의 DB는 MySQL 이므로, 로컬 서버환경에서 부하 테스트를 통해 500 error의 원인을 파악하는 것을 생각하였습니다! (테스트 코드는 동시성 이슈 발생 여부를 확인하는 정도로 생각했습니다)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트 코드로는 데이터 정합성만 판단하고 정확한 에러율과 에러 메시지는 MySQL을 사용하여 부하 테스트로 확인하는 것으로 이해했는데 맞을까요?
저번에 회의에서 말씀해주신대로 테스트 코드 목표가 명확해서 좋은 것 같네요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트 코드로는 데이터 정합성만 판단하고 정확한 에러율과 에러 메시지는 MySQL을 사용하여 부하 테스트로 확인하는 것으로 이해했는데 맞을까요? 저번에 회의에서 말씀해주신대로 테스트 코드 목표가 명확해서 좋은 것 같네요!

넵 맞습니다!

Copy link
Contributor

@buzz0331 buzz0331 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다~~ 확실히 짬바가 느껴지시네요 💯

tasks.named('test') {
useJUnitPlatform()
useJUnitPlatform {
if (System.getenv('CI') == 'true') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 이렇게하면 깃허브 액션에서 CI 작업시에 제외시켜 주는건가요?

@Autowired private JdbcTemplate jdbcTemplate;

@Test
@DisplayName("[동시성] 방 참여 동시 요청 테스트")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트 코드로는 데이터 정합성만 판단하고 정확한 에러율과 에러 메시지는 MySQL을 사용하여 부하 테스트로 확인하는 것으로 이해했는데 맞을까요?
저번에 회의에서 말씀해주신대로 테스트 코드 목표가 명확해서 좋은 것 같네요!

Comment on lines +134 to +139
// 1) participants가 recruitCount 보다 커질 수 있음
assertThat(participantRows).isGreaterThan(recruit);

// 2) memberCount가 실제 participants 수보다 작을 수 있음
// memberCount 값은 Room 도메인 규칙에 의해 recruitCount를 초과하여 증가하지 않음
assertThat(memberCountInRoom).isLessThan((int) participantRows);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 시나리오에서는 다수의 사용자가 동시에 동일한 방에 참여하려고 할 때,
1. 실제 방 참여자 수가 방의 모집 인원을 초과하고,
2. 방 테이블의 memberCount 값이 실제 참여자 수보다 적게 나타나는 문제가 발생하고 있습니다.

성준님께서 정리해주신 노션 내용을 참고했을 때, 데드락이 발생하면 하나의 트랜잭션은 롤백되고 다른 하나의 트랜잭션만 UPDATE 쿼리가 반영되는 것으로 이해했습니다.
그렇다면 이러한 트랜잭션 처리 방식이 현재 시나리오에서도 동일하게 유지된다면, 데이터 정합성 자체는 깨지지 않아야 하지 않나 라는 의문이 생기게 되었습니다. 그냥 데드락으로 인한 연쇄적인 현상으로 이해하면 될까요?

Copy link
Collaborator Author

@seongjunnoh seongjunnoh Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

노션에 희진님께서 동일한 내용을 질문해주셔서 관련해서 댓글 단 내용 참고해주시면 될 것 같습니다!

저는 데드락 상황이 발생하면 MySQL이 트랜잭션 하나를 롤백시키는게 맞지만, 데드락 상황이 발생하지 않는 경우에는 두 트랜잭션이 서로 rooms 레코드의 memberCount 의 값을 동일하게 update 시키고 commit 하므로 데이터 정합성이 깨질 수 있다(= 방의 멤버 수와 방 참여자 의 전체 수가 다를 수 있다) 라고 생각합니다

@seongjunnoh seongjunnoh merged commit 553fd16 into develop Nov 9, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[THIP2025-369] [test] 모임방 참여 api 부하테스트

4 participants