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.

Sign up for GitHub

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

Jump to bottom

Feat/#89 lock #92

Open

sunghyun0610 wants to merge 5 commits into main from feat/#89-Lock

+62 −6

Collaborator

sunghyun0610 commented Oct 25, 2024 •

edited

Loading

📌 과제 설명

👩‍💻 요구 사항과 구현 내용

[feat] : 티켓 구매 완료시 구매수량만큼 남은티켓수량 감소 구현
[feat] : 남은 티켓이 구매를 희망하는 티켓보다 적은경우 예외처리 구현
[feat] : 비관적 락 x Lock을 이용해 동시성 제어 구현

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

https://moon99610.tistory.com/66
->자세한 설명은 여기에 기록해두었습니다!

sunghyun0610 added 5 commits

October 24, 2024 15:35


          feat : 티켓구매 완료시 그만큼 티켓수량 감소 구현

8aa987c


          feat : 남은 티켓이 구매를 희망하는 티켓보다 적은경우 예외처리 구현

18a189f


          feat : 비관적 락 x lock을 이용해 구현

0efbaff


          chore : 비관적 락 성공여부확인을 위한 로그 추가

88b2824


          chore : 락 테스트를 위한 10초대기 코드 주석

b640a4f

sunghyun0610 requested review from skfk286, macmorning0116 and Icecoff22

October 25, 2024 06:55

sunghyun0610 self-assigned this

sunghyun0610 linked an issue

that may be closed by this pull request

티켓 동시성 제어 기능 구현 #89

Open

yeonsu00 approved these changes

View reviewed changes

Collaborator

yeonsu00 left a comment

수고하셨습니다!
근데 혹시 티켓 취소할 때도 lock을 걸어야 할까요??

Collaborator Author

sunghyun0610 commented Oct 25, 2024

수고하셨습니다! 근데 혹시 티켓 취소할 때도 lock을 걸어야 할까요??

오 그건 생각못했네요 취소할때도 lock적용하시죠!

Icecoff22 reviewed

View reviewed changes

Collaborator

Icecoff22 left a comment

락 구현하고 직접 테스트하는데 어려웠을 텐데 잘 적용하신 것 같네요!
질문 몇개 남겨봅니다..!

src/main/java/org/socialculture/platform/ticket/service/TicketServiceImpl.java

+                                  .orElseThrow(() -> new GeneralException(ErrorStatus.PERFORMANCE_NOT_FOUND));
+                      } catch (PessimisticLockException e) {
+                          log.error("공연 락 획득 실패, 공연 ID: " + email);
+                          throw e;

Collaborator

Icecoff22 Oct 25, 2024

throw e를 하게되면 일반 에러가 발생하는건가요?
커스텀 에러로 처리하거나, 메세지를 담아서 스프링에서 제공해주는 예외로 던지는 것이 예외 처리 및 디버깅할 때 용이할 것 같습니다!

src/main/java/org/socialculture/platform/ticket/service/TicketServiceImpl.java

                   @Override
                   @Transactional
                   public TicketResponseDto registerTicket(String email, TicketRequestDto ticketRequest) {

Collaborator

Icecoff22 Oct 25, 2024

티켓을 등록할 때 코드의 책임을 명확히 하면 좋을 것 같습니다!
객체지향 원칙중에 단일책임 원칙을 고려해보시는게 좋을 것 같아요!

Collaborator Author

sunghyun0610 Oct 25, 2024

좋은 피드백 감사합니다! 분리하도록 하겠습니다!

src/main/java/org/socialculture/platform/ticket/service/TicketServiceImpl.java

-                      PerformanceEntity performanceEntity = findPerformanceById(ticketRequest.performanceId());
+                      log.info("공연에 대한 락을 획득 시도 중, 공연 ID: " + email);
+                      PerformanceEntity performanceEntity;

Collaborator

Icecoff22 Oct 25, 2024

performance를 불러오는것을 하나의 함수로 분리시키는게 어떨까요?
한줄로 끝나는 것도 아니고, try-catch문까지 있어서 락을 가진 공연을 불러온다 <- 이 책임을 가진 함수를 하나 생성하는 것이 어떨까 싶습니다.

src/main/java/org/socialculture/platform/ticket/service/TicketServiceImpl.java


		log.info("공연 락 획득 완료, 공연 ID: " + email);

		if (performanceEntity.getRemainingTickets() < ticketRequest.quantity()) {

Collaborator

Icecoff22 Oct 25, 2024

이 부분도 공연의 티켓의 수가 유효한지 확인하는 부분이라 validate 관련 네이밍을 가진 함수로 분리하는 것이 좋아보입니다!

src/main/java/org/socialculture/platform/ticket/service/TicketServiceImpl.java

+                          log.warn("남은 티켓 수 부족, 공연 ID: " + ticketRequest.performanceId());
+                          throw new GeneralException(ErrorStatus._NOT_ENOUGH_TICKETS);
+                      }
+                      //비관적 락이 걸린 후에 대기 시간 추가 (시각적으로 확인하기 위함)

Collaborator

Icecoff22 Oct 25, 2024

주석은 코드의 가독성을 해치게 되니 제거 부탁드립니다..!

src/main/java/org/socialculture/platform/ticket/service/TicketServiceImpl.java

                       int finalPrice = calculateFinalPrice(performanceEntity.getPrice(), ticketRequest.quantity(), ticketRequest.couponId());
-                      // 티켓 생성 및 저장
                       TicketEntity ticketEntity = createAndSaveTicket(memberEntity, performanceEntity, ticketRequest.quantity(), finalPrice);

Collaborator

Icecoff22 Oct 25, 2024

Suggested change

      
            private updateRemainingTickets(PerformanceEntity performanceEntity, int ticketQuantity) {
          
                int remainTicket = performanceEntity.getRemainingTickets() - ticketEntity.getQuantity();
          
                log.info("티켓 등록 후 남은 티켓 수량: " + remainTicket);
          
                performanceEntity.updateTicket(remainTicket);
          
                log.info("공연 테이블의 남은 티켓 수 업데이트 완료: " + performanceEntity.getRemainingTickets());
          
            }

이 부분도 공연의 티켓을 수정한다는 개념으로 따로 분리가 되어있으면 좋을 것 같아요!

macmorning0116 reviewed

View reviewed changes

src/main/java/org/socialculture/platform/ticket/service/TicketServiceImpl.java

@@ @@ -33,10 +37,10 @@ @@
               @RequiredArgsConstructor
               public class TicketServiceImpl implements TicketService {
+                  private static final Logger log = LoggerFactory.getLogger(TicketServiceImpl.class);

Member

macmorning0116 Oct 25, 2024

@slf4j를 사용해보시는건 어떠신가요?

macmorning0116 approved these changes

View reviewed changes

Member

macmorning0116 left a comment

고생하셨습니다~!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet