Skip to content
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

[BE] feat: 새로운 티켓팅 로직 추가 (#1007-3) #1010

Open
wants to merge 7 commits into
base: feat/#1007-2
Choose a base branch
from

Conversation

seokjin8678
Copy link
Collaborator

📌 관련 이슈

✨ PR 세부 내용

#1009 PR에 의존하는 티케팅 로직에 대한 PR 입니다.

이슈에 해당하는 핵심 PR이라고 보시면 될 것 같습니다.

코드의 핵심은 동시성을 처리하는 곳과 예매한 티켓을 영속하는 곳을 분리하여 비즈니스 로직이 특정 구현체에 의존적이지 않도록 변경되었고, 다른 타입 별로 무관하게 유연한 티켓팅이 가능하도록 하였습니다.

그에 따라 여러 정책들이 생기기 때문에 별도의 클래스로 분리되고 인터페이스를 구현하는 방식으로 약간 복잡하게 변경되었습니다.

자세한 내용과 의도했던 곳에 대해선 코드에 리뷰로 남기겠습니다!

@seokjin8678 seokjin8678 added BE 백엔드에 관련된 작업 🏗️ 기능 기능 추가에 관한 작업 labels Jun 11, 2024
@seokjin8678 seokjin8678 self-assigned this Jun 11, 2024
Copy link

github-actions bot commented Jun 11, 2024

Test Results

263 files  + 7  263 suites  +7   31s ⏱️ -1s
859 tests +16  859 ✅ +16  0 💤 ±0  0 ❌ ±0 
884 runs  +16  884 ✅ +16  0 💤 ±0  0 ❌ ±0 

Results for commit 7b43023. ± Comparison against base commit 29f42c5.

♻️ This comment has been updated with latest results.

Comment on lines +12 to +24
@Repository
@RequiredArgsConstructor
public class NewTicketDao {

private final StageTicketRepository stageTicketRepository;

public NewTicket findByIdWithTicketTypeAndFetch(Long id, NewTicketType ticketType) {
Optional<? extends NewTicket> ticket = switch (ticketType) {
case STAGE -> stageTicketRepository.findByIdWithFetch(id);
};
return ticket.orElseThrow(() -> new NotFoundException(ErrorCode.TICKET_NOT_FOUND));
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

NewTicketRepository를 사용하면 상속 계층의 객체를 알아서 찾아주기 때문에 해당 클래스가 왜 필요하지 싶겠지만, JPA로 상속 관계의 객체를 가져오는 것에는 한계가 존재합니다.

첫 번째 문제로는 상속된 모든 객체에 대해 join 쿼리가 발생하기 때문이고, 두 번째 문제는 fetch join을 할 수 없는 문제입니다.

더 정확히는 상속된 객체가 @OneToMany와 같은 다른 객체와 연관 관계를 맺어 있을 때, 해당 연관 관계를 가진 객체를 fetch join으로 한 번에 가져올 수 없습니다. 😂

그렇기 때문에 N+1 문제가 발생하고, 조회가 아닌 티켓팅에서 이러한 문제는 동시 처리 능력을 매우 떨어트리는 원인입니다.

따라서 별도의 객체를 만들어 N+1 문제를 방지하고 다형성을 지원하도록 했습니다.

Comment on lines 16 to 51
@Service
@RequiredArgsConstructor
//@Transactional 명시적으로 Transactional 사용하지 않음
public class QuantityTicketingService {

private final TicketQuantityRepository ticketQuantityRepository;
private final TicketingCommandService ticketingCommandService;
private final TicketingRateLimiter ticketingRateLimiter;

public TicketingResult ticketing(TicketingCommand command) {
TicketQuantity ticketQuantity = getTicketQuantity(command.ticketId());
validateFrequentTicketing(command.booker());
try {
ticketQuantity.decreaseQuantity();
return ticketingCommandService.reserveTicket(command);
} catch (Exception e) {
ticketQuantity.increaseQuantity();
throw e;
}
}

private TicketQuantity getTicketQuantity(Long ticketId) {
TicketQuantity ticketQuantity = ticketQuantityRepository.findByTicketId(ticketId)
.orElseThrow(() -> new NotFoundException(ErrorCode.TICKET_NOT_FOUND));
if (ticketQuantity.isSoldOut()) {
throw new BadRequestException(ErrorCode.TICKET_SOLD_OUT);
}
return ticketQuantity;
}

private void validateFrequentTicketing(Booker booker) {
if (ticketingRateLimiter.isFrequentTicketing(booker, 5, TimeUnit.SECONDS)) {
throw new BadRequestException(ErrorCode.TOO_FREQUENT_REQUESTS);
}
}
}
Copy link
Collaborator Author

@seokjin8678 seokjin8678 Jun 11, 2024

Choose a reason for hiding this comment

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

동시성 문제를 방지하고 높은 처리량을 보장하는 티켓팅 로직의 핵심입니다.

기존 티켓팅 로직은 MySQL의 select for update를 통한 락을 사용했기에 처음 요청이 처리되기 전 까지는 다른 요청이 처리될 수 없는 문제가 있었습니다.

따라서 해당 문제를 해결하기 위해 세마포어의 개념을 가지는 TicketQuantity를 사용하여 동시 요청을 처리합니다.

재고와 시퀀스를 동시에 관리하기 위해 큐를 활용하여 동시 요청을 처리하도록 변경했습니다.

해당 로직은 락을 걸지 않기 때문에 요청이 들어온 만큼 자원을 활용하여 티켓팅 로직을 처리합니다.

Comment on lines 46 to 50
private void validateFrequentTicketing(Booker booker) {
if (ticketingRateLimiter.isFrequentTicketing(booker, 5, TimeUnit.SECONDS)) {
throw new BadRequestException(ErrorCode.TOO_FREQUENT_REQUESTS);
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

해당 로직이 필요한 이유는 한 명의 사용자가 여러 번 요청을 보내 따닥 문제를 방지하기 위함입니다.
실제 티켓팅 로직에서 예매 개수를 판별하여 요청을 거부하는 검증 로직이 작성되어 있으나, 해당 로직이 없으면 동시성 문제로 인해 검증 로직이 동작하지 않습니다.

Comment on lines 28 to 34
try {
ticketQuantity.decreaseQuantity();
return ticketingCommandService.reserveTicket(command);
} catch (Exception e) {
ticketQuantity.increaseQuantity();
throw e;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

티켓 재고의 정합을 보장하기 위해 try-catch를 사용하여 예외가 발생하면 다시 재고를 증가시키도록 하였습니다.
또한 재고가 음수가 되는 것을 방지하기 위해 decreaseQuantity() 메서드를 try 블럭 안으로 위치시켰습니다.

Comment on lines 22 to 53
@Service
@Transactional
@RequiredArgsConstructor
public class TicketingCommandService {

private final NewTicketDao ticketDao;
private final ReserveTicketRepository reserveTicketRepository;
private final TicketingSequenceGenerator sequenceGenerator;
private final List<TicketingValidator> validators;
private final Clock clock;

public TicketingResult reserveTicket(TicketingCommand command) {
Long ticketId = command.ticketId();
NewTicketType ticketType = command.ticketType();
NewTicket ticket = ticketDao.findByIdWithTicketTypeAndFetch(ticketId, ticketType);
Booker booker = command.booker();
ticket.validateReserve(booker, LocalDateTime.now(clock));
validators.forEach(validator -> validator.validate(ticket, booker));
validate(ticket, booker);
int sequence = sequenceGenerator.generate(ticketId);
ReserveTicket reserveTicket = ticket.reserve(booker, sequence);
reserveTicketRepository.save(ticket.reserve(booker, sequence));
return new TicketingResult(reserveTicket.getTicketId());
}

private void validate(NewTicket ticket, Booker booker) {
long reserveCount = reserveTicketRepository.countByMemberIdAndTicketId(booker.getMemberId(), ticket.getId());
if (reserveCount >= ticket.getMaxReserveAmount()) {
throw new BadRequestException(ErrorCode.RESERVE_TICKET_OVER_AMOUNT);
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

실제 티켓팅을 담당하는 클래스 입니다.

검증 로직의 경우 티켓 당 허용하는 최대 예매 개수 이상이면 예외를 던지게 하였고, 그 외 다른 도메인의 의존이 필요한 검증은 TicketingValidator 인터페이스를 통해 검증하도록 했습니다.

#1008 PR의 본문에서도 언급했지만, Ticket.reserve() 메서드와 Ticket.validateReserve() 메서드가 별도로 분리되어 있습니다.

하나로 합치는 것이 응집도를 높일 수 있겠지만, sequence를 다시 롤백시키면 문제가 발생할 수 있으므로 별도로 검증 로직을 분리하였습니다.
(sequence를 롤백하면 같은 sequence를 가지는 경우가 발생할 수 있습니다.)
ex) 동시에 요청이 와서 1, 2 sequence가 발급되었는데 1에서 예외가 발생하여 롤백하면 다음 sequence는 3이 아닌 2가 반환

따라서 sequence 생성 후에는 절대로 예외가 발생해서는 안 되기에 검증 로직을 별도로 분리했습니다.

만약 sequence 롤백이 가능하도록 해야한다면 레디스 queue를 사용하여 미리 sequence를 만들어 놓고 예외가 발생하면 다시 삽입하는 구조로 가야할 것 같네요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그리고 공연 티켓이 아니더라도 다른 유형의 티켓팅을 지원하기 위해 특정 구현체가 아닌 NewTicket 추상 클래스를 의존합니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

추가로 티켓팅 시 퍼포먼스를 더 올리고 싶다면 ticketDao.findByIdWithTicketTypeAndFetch() 메서드에서 반환하는 객체를 캐싱하면 될 것 같네요.

해당 객체는 읽기에만 사용하고, 티켓팅 시간 이후에는 변경이 막혀있으므로 불변 객체라고 생각해도 무방합니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

재고와 시퀀스 둘 다 관리해야하고, 시퀀스를 롤백할 수 없는 문제를 해결하기 위해 기존 CAS 연산으로 수행하던 재고 관리를 큐를 활용하여 재고와 시퀀스 둘 다 관리하도록 변경했습니다.

따라서 실제 티켓팅 로직에서 예외가 발생하더라도 시퀀스가 롤백됩니다!

Comment on lines +12 to +34
/**
* 공연의 티켓에 대해 하나의 유형에 대해서만 예매가 가능하도록 검증하는 클래스 <br/> ex) 사용자가 하나의 공연에 대해 학생 전용, 외부인 전용을 모두 예매하는 상황을 방지 <br/>
*/
@Component
@RequiredArgsConstructor
public class StageSingleTicketTypeTicketingValidator implements TicketingValidator {

private final StageReservedTicketIdResolver stageReservedTicketIdResolver;

@Override
public void validate(NewTicket ticket, Booker booker) {
if (!(ticket instanceof StageTicket stageTicket)) {
return;
}
Long memberId = booker.getMemberId();
Long stageId = stageTicket.getStage().getId();
Long ticketId = stageReservedTicketIdResolver.resolve(memberId, stageId);
if (ticketId == null || Objects.equals(ticketId, ticket.getId())) {
return;
}
throw new BadRequestException(ErrorCode.ONLY_STAGE_TICKETING_SINGLE_TYPE);
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

주석에도 설명했지만, 공연에는 두 유형의 티켓이 발급될 수 있습니다.(재학생용, 외부인용)

이때 해당 검증을 하지 않으면 재학생이 두 유형의 티켓을 모두 예매하는 문제가 생길 수 있습니다.
(정책 상 달라질 수 있겠지만, 아마 거의 이런 경우를 원하지는 않을 것 같네요)

Comment on lines 8 to 38
public class MemoryTicketQuantity implements TicketQuantity {

private final AtomicInteger quantity;

public MemoryTicketQuantity(int quantity) {
this.quantity = new AtomicInteger(quantity);
}

@Override
public boolean isSoldOut() {
return quantity.get() <= 0;
}

@Override
public void decreaseQuantity() {
int remainAmount = quantity.decrementAndGet();
if (remainAmount < 0) { // 티켓 재고 음수 방지
throw new BadRequestException(ErrorCode.TICKET_SOLD_OUT);
}
}

@Override
public void increaseQuantity() {
quantity.incrementAndGet();
}

@Override
public int getQuantity() {
return quantity.get();
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

RedisTicketQuantity 구현체와 동작이 100% 같은 메모리 구현체 입니다.
따라서 동시성 테스트를 할 때 Redis를 사용하지 않아도 같은 결과를 보장합니다.

Comment on lines 62 to 84
@Test
void 티켓팅은_동시성_문제가_발생하지_않아야_한다() {
// given
int ticketAmount = 50;
long tryCount = 100;
ticketQuantity = ticketQuantityRepository.put(new FakeTicket(ticketId, ticketAmount));
given(ticketingCommandService.reserveTicket(any())).willAnswer(invoke -> {
reserveCount.incrementAndGet();
return null;
});

// when
List<CompletableFuture<Void>> futures = LongStream.rangeClosed(1, tryCount)
.mapToObj(i -> CompletableFuture.runAsync(() -> {
quantityTicketingService.ticketing(command);
}, executorService).exceptionally(throwable -> null))
.toList();
futures.forEach(CompletableFuture::join);

// then
assertThat(ticketQuantity.getQuantity()).isZero();
assertThat(reserveCount).hasValue(ticketAmount);
}
Copy link
Collaborator Author

@seokjin8678 seokjin8678 Jun 11, 2024

Choose a reason for hiding this comment

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

동시성 문제를 막는 곳과 실제 티켓팅 비즈니스 로직을 분리하여 얻은 장점 중 하나입니다.

티켓팅 로직은 동시성 처리에 대한 곳을 신경쓰지 않기 때문에 다음과 같이 실제 티켓팅 로직을 수행하지 않고도 동시성 문제 테스트가 가능합니다.

또한 동시성 처리 로직은 간단한 CAS 연산을 기반으로 구현되었기에 Atomic 자료형을 사용한 Memory 구현체로도 충분히 검증이 가능합니다.
재고와 시퀀스 둘 다 관리하기 위해 큐를 사용했고, 메모리 구현체는 ArrayBlockingQueue를 기반으로 구현했습니다.

(미덥지 못하면 @SpringBootTest를 사용해서 실제 레디스 기반으로 돌려봐도 됩니다)

Comment on lines 10 to 47
@RequiredArgsConstructor
public class RedisTicketSequence implements TicketSequence {

private final String key;
private final RedisTemplate<String, String> redisTemplate;

@Override
public boolean isSoldOut() {
Long size = redisTemplate.opsForList().size(key);
if (size == null) {
throw new InternalServerException(ErrorCode.REDIS_ERROR);
}
return size <= 0;
}

@Override
public int reserve() {
String sequence = redisTemplate.opsForList().rightPop(key);
if (sequence == null) {
throw new BadRequestException(ErrorCode.TICKET_SOLD_OUT);
}
return Integer.parseInt(sequence);
}

@Override
public void cancel(int sequence) {
redisTemplate.opsForList().leftPush(key, String.valueOf(sequence));
}

@Override
public int getQuantity() {
Long size = redisTemplate.opsForList().size(key);
if (size == null) {
throw new InternalServerException(ErrorCode.REDIS_ERROR);
}
return size.intValue();
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

기존 원자적인 단일 값 연산을 통해 재고를 관리하던 것을 큐를 사용한 방법으로 변경했습니다.

그에 따라 시퀀스 또한 롤백이 가능하니 로직이 더 간결해졌습니다.

기존 방법에 비해 성능이 나쁠까봐 5000개 재고에 대해 티켓팅 수행 중 개수를 확인해봤는데, 걱정할 필요가 없을 것 같네요. 😂

image

연타해서 조회한건데 0.1~0.2초 단위로 재고가 우르르 빠져나가는 것을 볼 수 있습니다.

역시 레디스 성능 짱이네요.

Comment on lines +25 to +37
@Override
public int reserve() {
String sequence = redisTemplate.opsForList().rightPop(key);
if (sequence == null) {
throw new BadRequestException(ErrorCode.TICKET_SOLD_OUT);
}
return Integer.parseInt(sequence);
}

@Override
public void cancel(int sequence) {
redisTemplate.opsForList().rightPush(key, String.valueOf(sequence));
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TicketSequence의 동작은 Deque 처럼 동작합니다.
기존에는 Queue 동작을 따랐으나, 취소된 시퀀스가 맨 뒤로 가버려 맨 마지막 예매한 사용자가 이른 시간을 예매할 수 있는 경우가 생기더군요. 😂

Comment on lines +39 to +43
private void validateFrequentTicketing(Booker booker) {
if (ticketingRateLimiter.isFrequentTicketing(booker, 5, TimeUnit.SECONDS)) {
throw new TooManyRequestException();
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

RateLimit 시간을 어떻게 조절해야 할 지 모르겠네요.
5초면 조금 긴 것 같기도 하고...
1초 정도만 하더라도 동시성 문제를 막기 충분한 것 같기도 하고.. 아니면 스핀락 방식으로 대기를 하도록 해야할지 모르겠네요. 😂

Comment on lines +12 to +31
@Component
@RequiredArgsConstructor
public class RedisTicketingRateLimiter implements TicketingRateLimiter {

private static final String KEY_PREFIX = "ticketing_limiter_";
private final RedisTemplate<String, String> redisTemplate;

@Override
public boolean isFrequentTicketing(Booker booker, long timeout, TimeUnit unit) {
if (timeout <= 0) {
return false;
}
String key = KEY_PREFIX + booker.getMemberId();
Boolean result = redisTemplate.opsForValue().setIfAbsent(key, "1", timeout, unit);
if (result == null) {
throw new InternalServerException(ErrorCode.REDIS_ERROR);
}
return !result;
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

RateLimiter 때문에 부하 테스트 시 테스트가 불가능한데, 해당 문제는 다음과 같이 해결할 수 있을 것 같네요.

@Primary
@Component
@Profile("local")
public class DisabledTicketingRateLimiter implements TicketingRateLimiter {

    @Override
    public boolean isFrequentTicketing(Booker booker, long timeout, TimeUnit unit) {
        return false;
    }
}

Comment on lines +21 to +50
@Service
@Transactional
@RequiredArgsConstructor
public class TicketingCommandService {

private final NewTicketDao ticketDao;
private final ReserveTicketRepository reserveTicketRepository;
private final List<TicketingValidator> validators;
private final Clock clock;

public TicketingResult ticketing(TicketingCommand command, int sequence) {
Long ticketId = command.ticketId();
NewTicketType ticketType = command.ticketType();
NewTicket ticket = ticketDao.findByIdWithTicketTypeAndFetch(ticketId, ticketType);
Booker booker = command.booker();
ticket.validateReserve(booker, LocalDateTime.now(clock));
validators.forEach(validator -> validator.validate(ticket, booker));
validate(ticket, booker);
ReserveTicket reserveTicket = ticket.reserve(booker, sequence);
reserveTicketRepository.save(reserveTicket);
return new TicketingResult(reserveTicket.getTicketId());
}

private void validate(NewTicket ticket, Booker booker) {
long reserveCount = reserveTicketRepository.countByMemberIdAndTicketId(booker.getMemberId(), ticket.getId());
if (reserveCount >= ticket.getMaxReserveAmount()) {
throw new BadRequestException(ErrorCode.RESERVE_TICKET_OVER_AMOUNT);
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

서비스 레이어는 비즈니스 로직을 갖지 않고, 도메인 레이어가 모든 비즈니스 로직을 갖게 했습니다.

다만 검증부가 조금 거슬리는데, 해당 검증을 Validator로 이동시켜야 할지 모르겠네요.

정답이 없는 문제이긴 한데, Validator로 옮긴다면 서비스 레이어 코드에는 단순히 트랜잭션을 적용하고 흐름을 제어하는 로직만 남게되니 깔끔해지긴 하는데, 검증부가 흩어져있어 가독성이 조금 낮습니다.

최대 예매 수량의 경우는 기본적인 검증에 해당하는 로직이라 여기 남겨두긴 했는데.. 이걸 옮겨야 할 지는 모르겠습니다. 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드에 관련된 작업 🏗️ 기능 기능 추가에 관한 작업
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant