[#12] 좌석 선택 #13

SOHEELEE408 · 2024-01-13T12:32:46Z

Summary

요청된 좌석이 예매 가능한지 체크한다.
선택된 좌석을 저장한다.
잔여석 조회 API 구현 #5 에서 추가된 예매 불가 예외 처리 수정

Issue Number

좌석 선택 기능 구현 #12

Describe changes

좌석 PK로 구매 좌석(PURCHASE_SEAT)에 저장된 데이터가 있는 지 체크하였습니다.
예매 가능한 경우, PURCHASE_SEAT과 PURCHASE_INFO를 생성하여 저장하였습니다.
PURCHASE_SEAT이 저장되었다면, commit 전이어도 선택된 좌석으로 판단되어야 해서 트랜잭션 격리 수준을 READ_UNCOMMITTED으로 설정했습니다.

티켓 수령 방식(receiving_type)과 예매 번호(purchase_serial_number)는 임시 데이터로 저장하고, 예매 완료 시 업데이트 처리 예정입니다.

티켓 수령 방식과 임시 예매 번호 변수가 추가하였습니다.(207acf6)

예매 불가 예외(UnavailablePurchaseException)는 사용되는 상황이 여러 가지로 나뉠 수 있어서 구체적으로 명시해주기 위해 필드를 추가하였습니다.(7964004)

- 예매 번호(PurchaseSerialNumber)는 임시 번호로 저장 - 티켓 수령 방법(ReceivingType) 타입 추가

f-lab-maverick

확인하였습니다. 아래 리뷰를 확인해주시고 동시성 제어 방식에 대해 어떻게 접근할지 코멘트로 남겨주세요~

테스트코드 작성 부탁드립니다.
동시성과 관련되어 종시성 이슈가 발생하지 않음을 보장하는 통합테스트를 작성해주세요. 동시성 테스트는 동시성을 제어하는 역할이 어디에 있느냐에 따라 테스트 작성법이 달라집니다.

하나의 어플리케이션 내부에서 동시성을 제어한다면 단위테스트로 작성이 가능하겠지만, DB나 외부 자원으로 동시성을 제어한다면 통합테스트를 진행해주세요.

동시성을 제어하는 로직이 없습니다.
트랜잭션의 격리 레벨을 낮춘다 해도 여러 스레드가 하나의 자원에 접근하여 여러가지 일을 한다면 동시성 이슈가 발생할 여지가 여전히 존재합니다. JPA + DB를 사용하여 제어한다면 아래 Lock을 사용할 수 있습니다.

JPA Pessimistic Locking
JPA Optimistic Locking

DB를 사용한다면 쉽게 제어가 가능하지만 그만큼 여러 단점과 리스크가 있습니다. DB 말고 다른 락이나 동시성 제어 방법도 한번 고려해보고 찾아보시겠어요? DB는 유저가 많아질 경우 데이터의 읽기/쓰기 리소스도 부족해질 여지가 있습니다.

f-lab-maverick · 2024-01-14T00:59:35Z

src/main/java/com/gwangya/global/advice/GlobalExceptionHandler.java

+		ExceptionResponse exceptionResponse = ExceptionResponse.builder()
+			.message(exception.getMessage())
+			.build();
+		return ResponseEntity.badRequest().body(exceptionResponse);


UnavailablePurchaseException, 예외에 실패했을 때 400번대 에러를 주는게 적절할까요? 400 에러는 어떤 상황을 뜻할까요?

400번 에러는 클라이언트 요청에 문제가 있을 때 사용되는 코드입니다.
나머지 400번 대 에러도 클라이언트 측에 문제(권한이 없거나 URI가 틀렸을 때 등)가 있을 때 반환 됩니다.
이 경우는 요청은 정상적으로 처리 되었으나 해당하는 데이터가 없는 경우이므로 400번 대는 적절하지 않은 것 같습니다!
이 경우에는 클라이언트나 서버에 문제가 있는 상황이 아니라 비즈니스 상에서 필요한 예외 처리니까 200번으로 변경하려고 하는데 맞을까요?

HTTP Status code에 대한 문서를 보면 좋을것 같네요.
200은 요청이 성공적으로 수행되었음을 의미하기 때문에 예외상황에 적절하지 않아보입니다.

아! 해당 예외는 예매 기간이 아니거나 예매 가능 매수가 초과했을 때 발생하는 예외이니, 403 Forbidden이 적절할 것 같습니다.

f-lab-maverick · 2024-01-14T01:06:54Z

src/main/java/com/gwangya/purchase/service/PurchaseService.java

+		if (purchaseSeatRepository.existsBySeat(seat)) {
+			throw new UnavailablePurchaseException("이미 선택된 좌석입니다.", SELECTED_SEAT, seat.getPerformanceDetail().getId());
+		}
+		purchaseSeatRepository.save(PurchaseSeat.selectSeat(user, seat));


동시성 이슈가 발생할 여지가 있네요.
동시성 이슈는 동일한 자원에 두가지 이상의 액션이 있고, 그 자원을 점유하지 못한 상황일 경우 발생할 여지가 있습니다.

이 코드에서는 아래 상황에서 동시성 이슈가 발생할 수 있습니다.

purchaseSeatRepository.existsBySeat(seat) // 이 때는 좌석이 있었다 // 아래 코드를 실행하기 전 다른 트랜잭션이 좌석을 이미 예매해 버렸다 purchaseSeatRepository.save(PurchaseSeat.selectSeat(user, seat)); // 이 코드를 실행하면 정해진 좌석보다 더 많은 좌석이 예매된다

f-lab-maverick · 2024-01-14T01:21:02Z

src/main/java/com/gwangya/performance/exception/UnavailablePurchaseType.java

+import lombok.AllArgsConstructor;
+import lombok.Getter;
+
+@AllArgsConstructor


꼭 필요한 인자만을 이용해서 생성한다는 의미로 @RequiredArgsConstructor를 사용하는건 어떠신가요?

아아 넵! 그게 더 명확한 것 같습니다. 수정했습니다:) 6c169e2

…ture/12-select-seat

- 유효성 검증(checkValidSeat) - 좌석 선택(SeatService.selectSeat(24f1127)

SOHEELEE408 · 2024-02-05T19:42:07Z

좌석 선택 기능 전체 Flow

동시성 제어를 위해 분산 락을 적용하였습니다.

Hazelcast 로컬 환경 세팅(8266827)
좌석 별 Lock 획득 및 좌석 저장(24f1127)

좌석은 좌석 PK를 Key로 하고, 유저 PK를 Value로 하는 Map에 저장했습니다.

좌석 저장 후 Lock을 바로 해제하지 않고, 저장한 좌석의 삭제 이벤트가 발생하면 해제되도록 수정했습니다.(298056a)
➡️ Lock 바로 해제 시 동시성 이슈 발생

분산 락을 적용한 메서드에 대해 단위 테스트를 구현하였습니다.(1e39796, 461e8c3)

좌석 점유 실패 케이스

Map에 이미 좌석에 대한 데이터가 존재할 때
좌석에 이미 Lock이 걸려있을 때
요청한 좌석이 2개 이상일 때, 모든 좌석에 대해 Lock을 획득하지 못했을 때

SOHEELEE408 · 2024-02-05T20:01:20Z

src/main/java/com/gwangya/performance/service/SeatService.java

-		} finally {
-			validLocks.forEach(FencedLock::unlock);
-		}
-
 		if (validLocks.size() != fencedLocks.size()) {


Lock을 일부만 획득했을 때, 해제해주는 로직 필요

…오류 수정

- destory ➡️ unlock, - Map.value(userId ➡️ userId, thread id를 갖는 객체)

SOHEELEE408

lock 해제 방식을 변경하였습니다.(7c13941)

변경 전

좌석 점유 시 좌석 PK와 유저 PK 쌍 저장
점유 좌석 데이터 만료 or 삭제 이벤트 발생 시 Lock 객체 destroy

변경 후

좌석 점유 시 좌석 PK와 점유 정보 객체(유저 PK, Lock을 획득한 Thread ID) 쌍 저장
점유 좌석 데이터 만료 or 삭제 이벤트 발생 시 Thread ID를 세팅하고 unlock

SOHEELEE408 · 2024-03-16T14:30:54Z

src/main/java/com/gwangya/purchase/facade/PurchaseFacade.java

@@ -38,7 +37,6 @@ public class PurchaseFacade {
 	 * @throws UnavailablePurchaseException
 	 * @see SeatService#occupySeats(OccupySeatInfo)
 	 */
-	@Transactional


좌석 점유 메서드(occupySeats)는 DB 처리를 하지 않아 트랜잭션 설정을 제거했습니다.

f-lab-maverick

고생하셨습니다! 리뷰확인해주시고 간단히 수정해서 머지해주세요~

f-lab-maverick · 2024-03-17T16:44:27Z

src/main/java/com/gwangya/lock/HazelcastLockService.java

+		boolean isSuccessfullyOccupied = fencedLock.tryLock(time, unit);
+		if (!isSuccessfullyOccupied) {


딱 한번만 사용하는 변수는 선언하지 않는 것이 좋습니다.
변수를 선언한다는 것은 이 변수의 생명주기 종료까지 코드를 읽을 때 변수를 염두해두어야 하기 때문에 변수 선언이 많아지면 가독성이 떨어질 수 있거든요.

예외적으로 변수가 "반드시 이름이 필요한 경우" 선언하는게 좋지만, 이 코드(fencedLock.tryLock(time, unit);와 같이 메서드로 행위를 유추할 수 있는 경우에는 변수선언 없이 인라인 메서드로 처리하는게 좋지 않을까요?

Suggested change

boolean isSuccessfullyOccupied = fencedLock.tryLock(time, unit);

if (!isSuccessfullyOccupied) {

if (!fencedLock.tryLock(time, unit)) {

f-lab-maverick · 2024-03-17T17:47:17Z

src/main/java/com/gwangya/purchase/domain/LockInfo.java

+import lombok.AllArgsConstructor;
+import lombok.Getter;
+
+@AllArgsConstructor


꼭 필요한 생성자 (final 변수)만으로 생성자를 생성한다는 의미로 @RequiredArgsConstructor를 명시적으로 사용해주세요~

f-lab-maverick · 2024-03-17T17:49:23Z

src/test/java/com/gwangya/performance/service/LockServiceIntegrationTest.java

+		@DisplayName("요청 수가 100000일 때 무결성이 보장된다.")
+		@Test


이런 대규모 테스트의 경우 실제 CI마다 테스트를 돌리면 락 서버에 부하가 많이 가기떄문에, 로컬에서만 테스트로 돌려보는 경우가 있습니다. 이 경우, disable 처리를 해주시면 전체 일괄테스트 구동시에 해당 테스트가 구동되지 않게 만들 수 있습니다,.

f-lab-maverick · 2024-03-17T17:49:49Z

src/test/java/com/gwangya/performance/service/LockServiceTest.java

꼼꼼한 테스트케이스 좋습니다~ 테스트코드도 많이 좋아졌네요

sonarcloud · 2024-03-19T13:46:50Z

Quality Gate failed

Failed conditions
1 Security Hotspot
D Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

SOHEELEE408 added 8 commits January 12, 2024 17:15

feat : 예매 가능 여부 체크 기능 추가

b10b138

feat : 선택한 좌석 생성 정적 팩토리 메서드 추가

207acf6

- 예매 번호(PurchaseSerialNumber)는 임시 번호로 저장 - 티켓 수령 방법(ReceivingType) 타입 추가

feat : 선택한 좌석 저장 기능 추가

2de7141

refactor : 로직 이동

22a80ed

refactor : 동일한 타입의 파라미터 request dto로 변경

377672a

refactor : 예매 불가 예외 필드 추가

7964004

refactor : 예외 처리 분리, 인자 추가

1e385f7

refactor : 예매 수량 조회 로직 service로 이동

8e39b52

SOHEELEE408 requested a review from f-lab-maverick January 13, 2024 12:32

f-lab-maverick requested changes Jan 14, 2024

View reviewed changes

f-lab-dev and others added 18 commits January 15, 2024 11:15

Update sonarcloud-analyze.yml

33b1397

refactor : 생성자 변경

6c169e2

Merge remote-tracking branch 'origin/feature/12-select-seat' into fea…

7df988f

…ture/12-select-seat

refactor : 반환 타입 명시

0fc3d77

refactor : HTTP Status 수정

b28b056

chore : hazelcast 설정 추가

8266827

chore : local, prod 환경 분리

eaf8db5

refactor : 설정 파일 경로 설정 수정

621d20b

feat : 좌석 점유 로직 추가

24f1127

refactor : 좌석 선택 메서드 분리

fe97e4a

- 유효성 검증(checkValidSeat) - 좌석 선택(SeatService.selectSeat(24f1127)

feat : 좌석 점유 API 추가

1b633ca

refactor : 미사용 필드 제거, 스타일 변경

f9a8571

feat : lock 해제 이벤트 추가, lock 해제 시점 변경(데이터 삽입 ➡️ 데이터 삭제)

298056a

test : 좌석 점유 단위 테스트 추가

1e39796

test : 중복 요청 테스트 추가

461e8c3

refactor : 생성자 인자 추가

db55bbc

refactor : 타입 수정, 테스트 검증 메서드 수정

64a3222

refactor : 예외 타입 추가

a62b304

SOHEELEE408 commented Feb 5, 2024

View reviewed changes

SOHEELEE408 added 17 commits February 13, 2024 19:59

refactor : 패키지 위치 변경

edcbff2

feat : lock 해제 이벤트 추가(시간 초과로 삭제 되었을 때)

6eeccdd

refactor : PostConstruct으 인자 제거

a468b56

docs : 주석 추가

8866300

docs : 주석 추가

3438ea3

refactor : 메서드 명 변경

282f7fb

refactor : DB, hazelcast 설정 변경

bd092fe

fix : 회원가입 > JWT 필터에서 회원가입 제외, 이메일 중복 확인 메서드 파라미터 타입 수정, 비밀번호 정규 표현식 …

00dd70f

…오류 수정

refactor : lock 해제 방식 변경-1

7c13941

- destory ➡️ unlock, - Map.value(userId ➡️ userId, thread id를 갖는 객체)

feat : lock 통합 테스트 환경 세팅 추가

65a5948

test : lock 통합 테스트 추가

8da23be

docs : 주석 수정

bcde03d

feat : 리턴 값 추가

f1212c8

test : 좌석 점유 - 단위 테스트 항목 추가

5bd4bb9

test : 좌석 점유 - 통합 테스트 항목 추가, 검증 로직 리팩토링, 테스트 타입별로 그룹화

35c0954

feat : hazelcast 테스트 설정 추가

5e75cf1

refactor : 트랜잭션 제거

d820be7

SOHEELEE408 commented Mar 16, 2024

View reviewed changes

f-lab-maverick approved these changes Mar 17, 2024

View reviewed changes

SOHEELEE408 added 4 commits March 19, 2024 22:41

refactor : 변수 제거 후 인라인 메서드 처리

16a2230

refactor : 생성자 변경

3a31bbd

feat : 테스트 종료 후 lock 해제 로직 추가

c0848b2

refactor : 요청 수 테스트 클래스 전체 disabled

0591e3e

SOHEELEE408 merged commit dab30fb into main Mar 19, 2024
1 of 2 checks passed

SOHEELEE408 deleted the feature/12-select-seat branch March 19, 2024 13:50

f-lab-maverick linked an issue Aug 7, 2024 that may be closed by this pull request

좌석 선택 기능 구현 #12

Closed

6 tasks

f-lab-maverick removed a link to an issue Aug 7, 2024

좌석 선택 기능 구현 #12

Closed

6 tasks

SOHEELEE408 linked an issue Aug 17, 2024 that may be closed by this pull request

좌석 선택 기능 구현 #12

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#12] 좌석 선택 #13

[#12] 좌석 선택 #13

SOHEELEE408 commented Jan 13, 2024

f-lab-maverick left a comment

f-lab-maverick Jan 14, 2024

SOHEELEE408 Jan 16, 2024

f-lab-maverick Jan 16, 2024

SOHEELEE408 Jan 25, 2024

f-lab-maverick Jan 14, 2024

f-lab-maverick Jan 14, 2024

SOHEELEE408 Jan 16, 2024

SOHEELEE408 commented Feb 5, 2024 •

edited

Loading

SOHEELEE408 Feb 5, 2024

SOHEELEE408 left a comment •

edited

Loading

SOHEELEE408 Mar 16, 2024

f-lab-maverick left a comment

f-lab-maverick Mar 17, 2024

f-lab-maverick Mar 17, 2024

f-lab-maverick Mar 17, 2024

f-lab-maverick Mar 17, 2024

sonarcloud bot commented Mar 19, 2024

		boolean isSuccessfullyOccupied = fencedLock.tryLock(time, unit);
		if (!isSuccessfullyOccupied) {

	boolean isSuccessfullyOccupied = fencedLock.tryLock(time, unit);
	if (!isSuccessfullyOccupied) {
	if (!fencedLock.tryLock(time, unit)) {

		@DisplayName("요청 수가 100000일 때 무결성이 보장된다.")
		@Test

[#12] 좌석 선택 #13

[#12] 좌석 선택 #13

Conversation

SOHEELEE408 commented Jan 13, 2024

Summary

Issue Number

Describe changes

f-lab-maverick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SOHEELEE408 commented Feb 5, 2024 • edited Loading

좌석 선택 기능 전체 Flow

동시성 제어를 위해 분산 락을 적용하였습니다.

좌석 점유 실패 케이스

Choose a reason for hiding this comment

SOHEELEE408 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

f-lab-maverick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Mar 19, 2024

Quality Gate failed

SOHEELEE408 commented Feb 5, 2024 •

edited

Loading

SOHEELEE408 left a comment •

edited

Loading