-
Notifications
You must be signed in to change notification settings - Fork 22
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
Feature/kariskan step4 #15
Open
kariskan
wants to merge
33
commits into
base/kariskan
Choose a base branch
from
feature/kariskan_step4
base: base/kariskan
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- event publish 관련 코드는 admin만 접근 가능 - name duplicate validation
kariskan
requested review from
VSFe,
kochungcheon,
MinYeongPark,
semi-cloud,
seungh1024,
swonny,
Hugh-KR and
youngsoosoo
June 23, 2024 04:19
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
일단 이번 스텝 수정 사항이 매우 많을 것으로 예상되기 때문에.. 테스트는 프로덕션 코드 리뷰 후에 작성하겠습니다.
제가 이번 스텝을 진행하면서 정해야 할 몇 가지 정책들이 있었습니다.
그리고 issued_coupon_tbl을 통해서 발급받은 쿠폰의 이력을 저장하고 회수를 스케줄러로 진행했습니다.
동시성 이슈 및 부하 제어
요구사항에서 DDos 수준의 트래픽이 몰린다고 가정했으므로, 일반적인 로직에서 쿠폰 발급, 사용을 진행한다면 당연히 동시성 이슈가 생길 수 밖에 없고, 분산 서버 환경이라 가정했을 때 각 서버 내에서 동기화 로직을 사용하기 어려웠습니다.
그래서 생각한 방법은 서버의 요청이 하나로 �병목되는 db에 lock을 생각했는데, 이게 DDos 수준의 트래픽을 감당하기엔 어렵다고 생가했습니다.
그래서 다음으로 고민한 방법은 redis distributed lock을 사용하는 것이었는데, mysql이든 redis든 DDos 수준의 트래픽을 감당하기는 불안정해서 각 서버 메모리에 "이미 최대치로 발급, 또는 사용된 쿠폰의 정보"를 캐싱했습니다(CouponRestrictionManager).
이 캐싱 클래스 자료구조에는 딱히 thread-safe structure를 사용하지는 않았는데, 그 이유는 어짜피 최대치로 발급, 사용된 쿠폰 id만 저장이 되고 실시간으로 저장이 안되더라도 그 짧은 찰나 사이에 redis에 요청이 많이 가지는 않을 것이라 생각했기 때문입니다.
그래서 이 캐싱 정보를 이용해서 레디스의 접근을 최소화했습니다.
분리된 트랜잭션 간 통신
상품 주문 과정(
ConsumerService.purchaseProduct()
)에서 쿠폰의 사용을 관리하는 로직과 Order, OrderProduct 등을 저장하는 로직의 트랜잭션을 분리했습니다. 그런데 쿠폰을 사용하고usedCount++
이후 주문 관련 로직에서 어떤 예외가 발생하면 다른 트랜잭션이기 때문에 쿠폰 사용이 롤백되지 않는 문제가 있었습니다. 그래서 주문 로직에서 예외가 발생하면 또 redis 락을 걸고 보상 로직을 실행하게 했습니다.그런데 여기서 이 보상 로직 또한 계속 실패할 수 있기 때문에, 재시도 횟수를 어느정도 두고, 최대 재시도 횟수를 넘어가면 따로 로그로 저장해 스케줄러로 처리하도록 했습니다.
근데 이런 쿠폰 이벤트에서 엄청나게 빠르게 쿠폰이 소진될 것인데 이렇게 재시도를 하고 그것마저 안되면 스케줄러로 처리하는게 의미가 있는지는 잘 모르겠습니다.
두번째로,
현재 쿠폰 사용량 = 최대 쿠폰 사용량 - 1
인 상태에서 쿠폰 사용 로직 <-> 보상 로직 이 사이의 찰나에 다른 요청이 들어와 쿠폰 사용을 시도한다면 논리적으로 쿠폰을 사용할 수 있는 상태(이전 요청이 롤백이 났으므로)에도 불구하고현재 쿠폰 사용량 = 최대 쿠폰 사용량
상태가 되어서 요청이 반려됩니다. 이런 경우까지는 처리를 하지 않았는데 락의 범위를 넓혀서라도 이 상황을 처리해야 하는 건지 궁금합니다.그리고 개인적으로 서비스 코드(특히 ConsumerService)가 너무 비대해지다 보니,, 따로 리팩토링 브랜치 파서 리팩토링도 진행해야 할 것 같네요.