Skip to content

3주차 과제#95

Merged
soonhankwon merged 7 commits intohanghae-skillup:hongs429from
hongs429:3주차-과제
Feb 4, 2025

Hidden character warning

The head ref may contain hidden characters: "3\uc8fc\ucc28-\uacfc\uc81c"
Merged

3주차 과제#95
soonhankwon merged 7 commits intohanghae-skillup:hongs429from
hongs429:3주차-과제

Conversation

@hongs429
Copy link

@hongs429 hongs429 commented Feb 3, 2025

[예약 시스템 구현 및 동시성 제어]


작업 내용

  • 예약과 관련된 application, infra 모듈 작업 (presentation 미완성)

  • 동시성 해결을 위한 이중 방어 적용

    • 1차: ReservationCommandAdapter.reserve (MySQL) → Unique 제약 조건으로 동시성 제어
          @Entity
          @Builder
          @Table(name = "reservation_seat", uniqueConstraints = {
                @UniqueConstraint(
                        name = "UK_screening_seat",
                        columnNames = {"screening_id", "seat_id"}
                )
          })
          @Getter
          @AllArgsConstructor
          @NoArgsConstructor(access = AccessLevel.PROTECTED)
          public class ReservationSeatJpaEntity extends BaseJpaEntity {
    • 2차: ReservationLockPort (Redisson) → Application 레벨에서 분산 락 적용
          @Slf4j
        @Component
        @RequiredArgsConstructor
        public class ReservationLockAdapter implements ReservationLockPort {
        
            private final RedissonClient redissonClient;
        
            @Override
            public boolean tryLock(String lockKey, long waitTimeMils, long releaseTimeMils) {
                RLock lock = redissonClient.getLock(lockKey);
                try {
                    return lock.tryLock(waitTimeMils, releaseTimeMils, TimeUnit.MILLISECONDS);
                } catch (InterruptedException e) {
                    Thread.currentThread().interrupt();
                    return false;
                }
            }
        
            @Override
            public boolean tryScreeningSeatLock(List<String> lockKeys, long waitTimeMils, long releaseTimeMils) {
                RLock[] locks = lockKeys.stream()
                        .map(redissonClient::getLock)
                        .toArray(RLock[]::new);
        
                RLock multiLock = redissonClient.getMultiLock(locks);
        
                try {
                    log.info("Trying to acquire multi-lock for keys: {}", lockKeys);
                    boolean acquired = multiLock.tryLock(waitTimeMils, releaseTimeMils, TimeUnit.MILLISECONDS);
                    log.info("Multi-lock acquired: {}", acquired);
                    return acquired;
                } catch (InterruptedException e) {
                    Thread.currentThread().interrupt();
                    return false;
                }
            }
        
            @Override
            public void releaseLock(String lockKey) {
                RLock lock = redissonClient.getLock(lockKey);
                if (lock.isHeldByCurrentThread()) {
                    log.info("Release lock ...{}", lockKey);
                    lock.unlock();
                }
            }
        
            @Override
            public void releaseMultiLock(List<String> lockKeys) {
                lockKeys.forEach(lockKey -> {
                    RLock lock = redissonClient.getLock(lockKey);
                    if (lock.isHeldByCurrentThread()) {
                        log.info("Release lock ...{}", lockKey);
                        lock.unlock();
                    }
                });
            }
        }
          
  • 테스트 전략

    • application: 단위 테스트 (입력 모델 검증 + 비즈니스 로직 검증)
          // 입력모델 자체 검증 로직
      
        @Getter
        @Value
        @EqualsAndHashCode(callSuper = false)
        public class ReserveCommandParam extends SelfValidating<ReserveCommandParam> {
        
            @NotNull
            @Size(min = 1, max = 5)
            List<UUID> seatIds;
        
            @NotNull
            UUID screeningId;
        
            @NotNull
            String userName;
        
            public ReserveCommandParam(List<UUID> seatIds, UUID screeningId, String userName) {
                this.seatIds = seatIds;
                this.screeningId = screeningId;
                this.userName = userName;
                validate();
            }
        }
    • infra: ReservationCommandAdapter.reserve 단위 테스트
    • 통합 테스트: application -> infra 경로를 거치는 통합 테스트 진행
    • TestContainer 활용: 동일한 환경에서 테스트 수행
        @SpringBootApplication(scanBasePackages = {"project.redis.infrastructure"})
        public class TestConfiguration {
        }
    
    
    
    
          @Slf4j
          @SpringBootTest
          @ContextConfiguration(classes = TestConfiguration.class)
          @TestConstructor(autowireMode = TestConstructor.AutowireMode.ALL)
          @RequiredArgsConstructor
          class ReservationCommandAdapterTest extends TestContainerSupport {
          
            private final ReservationCommandAdapter reservationCommandAdapter;
            private final ReservationJpaRepository reservationJpaRepository;
            private final ReservationSeatJpaRepository reservationSeatJpaRepository;
            private final SeatJpaRepository seatJpaRepository;
            private final ScreeningJpaRepository screeningJpaRepository;
          
            @AfterEach
            void tearDown() {
                reservationSeatJpaRepository.deleteAll();
                reservationJpaRepository.deleteAll();
            }
    
    
    
    
        public abstract class TestContainerSupport {
      
          @Container
          public static final MySQLContainer<?> mysqlContainer
                  = new MySQLContainer<>("mysql:9.1.0")
                  .withDatabaseName("db")
                  .withUsername("user")
                  .withPassword("1234")
                  .withCommand(
                          "--character-set-server=utf8mb4",
                          "--collation-server=utf8mb4_unicode_ci"
                  );
      
          @Container
          public static final GenericContainer<?> redisContainer
                  = new GenericContainer<>("redis")
                  .withExposedPorts(6379);
      
          static {
              mysqlContainer.start();
              redisContainer.start();
          }
      
          @DynamicPropertySource
          static void setDatasourceProperties(DynamicPropertyRegistry registry) {
              System.out.println("mysqlContainer = " + mysqlContainer.getJdbcUrl());
              registry.add("spring.datasource.url", ()-> mysqlContainer.getJdbcUrl() + "?useSSL=false&allowPublicKeyRetrieval=true");
              registry.add("spring.datasource.username", mysqlContainer::getUsername);
              registry.add("spring.datasource.password", mysqlContainer::getPassword);
              registry.add("spring.datasource.driver-class-name", mysqlContainer::getDriverClassName);
      
              registry.add("redis.host", redisContainer::getHost);
              registry.add("redis.port", () -> redisContainer.getMappedPort(6379).toString());
          }
      }
            

발생했던 문제와 해결 과정

🚨 문제 1: 통합 테스트를 어떤 모듈에서 작성해야 하는지 고민

  • ⚠️ 고민: applicationinfra를 모르므로, infra에서 작성해야 하는지 모호함
  • ✅ 해결:
    • infraapplication을 알고 있고 DB 관련 내용도 포함하므로, 통합 테스트는 infra에서 작성하는 것으로 결정
    • infra에서는 application -> infra가 자연스럽게 연결될 수 있음

🚨 문제 2: 락을 applicationinfra 중 어디서 적용할지 고민

  • ⚠️ 고민: DB 트랜잭션(persistence)에서 락을 걸어야 할까? application 레벨에서 비즈니스 로직을 보호해야 할까?
  • ✅ 해결:
    • ReservationLockPortapplicationinfra에서 분리
    • application에서 비즈니스 로직 전에 락을 먼저 시도, 이후 infra에서 DB 트랜잭션 처리

🚨 문제 3: Redisson의 waitTime이 길 경우, 이미 예약된 좌석이 있어도 계속 예약 시도

  • ⚠️ 문제:
    • 락 대기 시간(waitTime)이 길면, 이미 예약된 좌석에 대해서도 중복 예약 시도가 발생
    • 결국 RDB 제약조건이 중복을 막지만, 불필요한 DB I/O가 발생
  • ✅ 해결:
    • 락을 획득한 요청만 실행 → waitTime을 짧게 설정
    • DB에 접근하는 동시 요청을 줄여, 불필요한 IO 최소화

이번 주차에서 고민되었던 지점이나, 어려웠던 점

  • 멀티 모듈 환경에서 통합 테스트를 어디서 작성해야 하는지 고민
    • 결론: infra에서 application -> infra 흐름을 테스트하는 것이 적절
  • 1회 예약 시 여러 좌석을 예약하는 시스템에서, 비즈니스 로직의 위치 고민
    • 도메인 로직을 application에 둘지 infra에 둘지 판단하는 것이 어려웠음
  • 트랜잭션의 범위 고민
    • 현재: 예약 및 좌석 등록을 하나의 트랜잭션으로 처리 → 큰 문제 없음
    • 하지만 경우에 따라 application까지 트랜잭션 전파가 필요할 가능성 존재

리뷰 포인트

  • infra에서 통합 테스트를 작성한 것이 적절한 판단인지?
  • application에서는 트랜잭션을 전파하지 않도록 구현했는데, 이는 올바른 설계인가?
    • 현재: 예약 & 예약-좌석 트랜잭션을 묶어도 문제 없음
    • 미래: 특정 상황에서는 application까지 트랜잭션을 잡아야 할 가능성 존재

기타 질문

  • Redisson의 waitTime이 필요한 이유가 명확하지 않음
    • 현재 가정: "락을 해제할 때까지 동일한 요청은 반드시 차단되어야 한다"
    • **따라서 waitTime을 짧게 설정하여 동시 요청을 제한

@soonhankwon soonhankwon merged commit 7e63cbe into hanghae-skillup:hongs429 Feb 4, 2025
2 checks passed
@soonhankwon
Copy link

다음주 기간까지 4주차까지 보완하셔서 올려주시면 코드 레벨 + 전체적인 피드백 추가해서 드리겠습니다 :) 화이팅!

리뷰 포인트

  • infra 에서 통합 테스트를 작성한 것이 적절한 판단인지?

    • 통합 테스트에는 보통 애플리케이션의 모든 의존성이 필요하게 됩니다. infra에서 통합 테스트를 작성한 것은 조금 의문이 드네요.
    • "개인적인 의견"으로는 통합테스트가 어느위치에 있는지 이런 고민보단, 내가 작성한 테스트가 의미가 있는지? 실제 버그상황에 대한 테스트가 작성되었는지? 등과 같은 "테스트의 목적"에 대한 고민을 더 해보시면 도움이 될 것 같습니다.
  • application에서는 트랜잭션을 전파하지 않도록 구현했는데, 이는 올바른 설계인가?

    • 적어주신대로 트랜잭션은 비즈니스 요구사항에 따라 좀더 세심하게 적용할 수 있습니다. 생각해볼 점은
      • 트랜잭션이 필요한 모든 도메인 로직에서 데이터 정합성이 보장되는가?
      • REQUIRES_NEW 같은 별도 트랜잭션이 필요한 구간이 있는가?
      • 예외 발생 시 롤백 정책이 적절하게 적용되는가?
    • 해당 부분은 좀 더 디테일한 구현 및 요구사항을 파악해야지 피드백 드릴수 있겠네요 :)
  • Redisson의 waitTime이 필요한 이유가 명확하지 않음

    • waitTime은 락을 획득하기 위해 최대한 대기할 시간을 의미합니다. 즉, 특정 스레드가 락을 점유하고 있다면, 다른 요청들은 waitTime 동안 락이 해제되기를 기다립니다. 이 시간이 지나도 락을 획득하지 못하면 작업을 포기합니다.
    • 예를 들어 waitTime을 너무 짧게 설정하면, 락을 획득할 기회가 충분함에도 불구하고 불필요하게 실패하는 경우가 발생할 수 있습니다. 정상적인 요청임에도 락을 얻지 못해 작업이 불가능해지는 비정상적인 상황을 맞이할 수 있습니다.

@hongs429
Copy link
Author

hongs429 commented Feb 4, 2025

감사합니다!
혹시 추가 질문드리자면, 현재 저의 application 코드를 보면 락을 별도의 포트로 구현했습니다.
현재 application에 트랜잭션이 전파되지않지만 레디스를 이용함 분산락을 적용한 것이라 트랜잭션과는 무관하여 별도의 포트로 구현했는데요, 이부분이 적절한지도 궁금합니다.

@soonhankwon
Copy link

락을 트랜잭션과 무관하게 별도의 포트로 구현한 것은 적절한 접근이라고 생각합니다 :)
다만 락은 트랜잭션과 독립적이지만, 락을 획득한 후 진행하는 "작업"은 "트랜잭션"이 적용되는 것이 일반적입니다(락의 오버헤드를 가만하면서까지 사용하는 중요한 작업이 일반적이기 때문).

  • 락을 획득한 후 실행하는 비즈니스 로직이 예외가 발생했을 때, 정합성이 깨지지는 않는지 확인이 필요합니다.
  • 락을 획득한 상태에서 트랜잭션이 적용되지 않으면, 중간에 예외 발생 시 락은 해제되지만 데이터는 중간 저장될 가능성이 있습니다.
  • 락을 거는 목적이 단순한 동시성 제어인지, 아니면 데이터 정합성을 보장하기 위한 것인지 확인하시면 좋을듯합니다.

@hongs429
Copy link
Author

hongs429 commented Feb 4, 2025

감사합니다!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants