[#16] 상품 아이템 정보 조회 기능 구현 #18
Conversation
nilgil
force-pushed
the
feat/16
branch
2 times, most recently
from
b762b88 to
3aec73a
Compare
nilgil
force-pushed
the
feat/16
branch
3 times, most recently
from
4f866d8 to
e29b11e
Compare
| @@ -34,6 +34,7 @@ dependencies { | |||
| runtimeOnly("com.h2database:h2") | |||
| runtimeOnly("com.mysql:mysql-connector-j") | |||
| annotationProcessor("org.springframework.boot:spring-boot-configuration-processor") | |||
| developmentOnly("org.springframework.boot:spring-boot-docker-compose") | |||
There was a problem hiding this comment.
구두로도 질문드렸지만 developmentOnly 는 어떻게 개발환경임을 알까요?_?
There was a problem hiding this comment.
developmentOnly, testAndDevelopmentOnly는 스프링 플러그인이 제공하는 의존성 스코프입니다.
스프링 플러그인에서 제공하는 bootRun, bootJar는 runtimeClassPath를 각각 다르게 구성합니다.
기본적으로 gradle 기본 스코프인 implementation, runtimeOnly와 스프링 플러그인으로 추가된 developmentOnly, testAndDevelopmentOnly 총 4개 스코프가 runtimeClassPath 하위에 포함됩니다.
bootJar는 구성시 developmentOnly, testAndDevelopmentOnly를 runtimeClassPath에서 제외합니다.
이로인해 소스 코드 컴파일본과 implementation, runtimeOnly 스코프의 의존성 라이브러리들만 포함한 jar파일이 만들어집니다.
bootRun은 소스 코드 컴파일본과 4개 스코프 의존성 라이브러리 모두 포함하여 jar로 묶지 않고 바로 실행합니다.
이렇게 bootJar로 만든 jar를 java -jar 로 실행하는 환경을 개발 환경이 아니라고 보고,
bootRun을 사용하여 실행하는 환경을 개발 환경으로 본다고 할 수 있을 것 같습니다.
| @@ -1,3 +1,5 @@ | |||
| .env | |||
| }, | ||
| ) | ||
|
|
||
| return ids.mapNotNull { responseMap[it] } |
There was a problem hiding this comment.
요건 request 한 id 의 순서보장을 위해 이렇게 한건가용?
그게 아니라면
return productItems.map { item -> ProductItemResponse.from( ) }
이렇게 해도 됐을 것 같아 궁금해서요!
There was a problem hiding this comment.
네 맞습니다. 서버에서 임의로 정렬을 수행하지 않고 요청한 순서에 맞게 응답해 줘야겠다고 생각했습니다.
중복 ID 또한 서버에서는 관여하지 않고, 요청한 쪽에서의 요청을 그대로 응답해 주는 것을 목표했습니다.
그런데 없는 데이터를 요청했을 때 오류 없이 빼고 주도록 했는데,
없는 데이터를 응답에 없다고 포함해 주지 않는 이상 위 목표가 무색해지는 것 같기도 하네요.
그냥 서버에서 순서 보장 없고, 중복 제거한 채로 응답하고, 요청한 쪽에서 알아서 잘 사용하도록 하는 게 더 나아보이기도 합니다.
페이로드 관점에서도 효율적일 것 같고, 애매하게 배려하는 게 오히려 혼란을 줄 수 있을 것 같습니다.
| private val productImageRepository: ProductImageRepository, | ||
| private val productItemOptionElementRepository: ProductItemOptionElementRepository, | ||
| ) { | ||
| fun getProductItem(id: Long): ProductItemResponse { |
There was a problem hiding this comment.
요런 메소드들에 @transactional(readOnly = true) 를 추가하는 것에 대해 어떻게 생각하시는지 궁금합니다~
There was a problem hiding this comment.
@transactional이 불필요하다 생각했고, 커넥션도 길게 잡을 것이기에 빼는게 좋다고 생각했었는데
자세히 생각해보니 놓친 부분이 많네요..
@transactional 사용에 대하여
오류 발생 가능성
프로젝트 전체적으로 연관관계를 lazy fetch로 설정해 놨습니다. 그러나 필요한 연관관계들을 미리 로딩해 왔기에 lazy loading이 사용되지 않아 당장 문제는 없습니다.
그런데 추후 미리 로딩되지 않은 연관관계 필드를 사용하려 하면 트랜잭션 범위에 있지 않아 영속성 컨텍스트를 사용하지 못해 오류가 발생할 것 같습니다.
눈에 잘 보이지 않는 오류 발생 가능 포인트를 만든 것 같습니다.
조회 데이터의 일관성
조회만 하는 경우에 트랜잭션이 필요하다는 생각을 못 했습니다.
현재 상황같이 ProductItem이라는 일관성이 보장되어야 하는 단위를 조회할 때 한 번의 쿼리로 가져오지 않고 여러 번의 쿼리로 각 테이블을 조회하여 조합하는 상황에서 일관성이 깨질 수 있다는 생각을 못 했습니다.
REPEATABLE READ 격리 수준으로 모두 트랜잭션 시작 시점의 데이터를 가져와 조합하여야 할 것 같습니다. 그러기 위해서는 @transactional로 묶을 필요가 있습니다.
커넥션 대여 시간과 대여&반납 오버헤드 관점
위와 같이 일관성이 보장되어야 하는 경우가 아닌 별도의 데이터들을 각 쿼리로 조회하는 경우에는 트랜잭션이 필수는 아닙니다.
현재는 각 쿼리마다 매번 커넥션을 풀에서 가져와 사용하고 바로 반납합니다. 그러나 트랜잭션으로 묶었다면 한 번 가져온 커넥션을 계속 사용했을 것입니다.
전자의 경우 커넥션을 오래 잡지 않고 빠르게 반납하는 것은 맞지만 만약 지금과 같이 커넥션 대여가 길어지지 않을 경우라면 오히려 커넥션 대여&반납 오버헤드가 더 클 것 같다는 생각이 듭니다.
결론
기본적으로 @transactional을 걸고 트랜잭션이 길어질 수 있는 경우 최적화를 시도하는 것이 좋을 것 같습니다.
@transactional readOnly 옵션
조회만 하는 경우에 사용하면 내부적으로 최적화해 준다고만 알고 있었습니다. 자세히 알아보니 다음과 같은 이점이 있는 것을 알게 되었습니다.
- 변경 감지를 생략하여 스냅샷 비교 등 수행하지 않아 성능 면에서 이점
- DB 레벨에서 최적화할 수 있게 DB에 읽기 전용이라는 힌트를 줌
- 실수로 데이터를 수정하는 것 방지
| productItemOptionElementRepository | ||
| .findAllWithOptionDetailsByItemIn(productItems) | ||
| .groupBy { it.item.id } | ||
| .mapValues { it.value.associate { it -> it.optionElement.option.name to it.optionElement.name } } |
There was a problem hiding this comment.
요고 컬렉션 처리 자체는 효율적인 것으로 보이는데, 중첩 it 사용으로 가독성이 떨어지고 로직 의도 파악이 어려워 보여요.
명시적인 변수명을 쓰거나 중간 단계를 쪼끔 더 분리해서 의도를 드러내면 어떨까요?
| } | ||
|
|
||
| fun getProductItems(ids: List<Long>): List<ProductItemResponse> { | ||
| val productItems = productItemRepository.findWithProductByIdIn(ids) |
There was a problem hiding this comment.
ids 의 크기를 제한하거나 chunking 할 필요는 없을까요?_?
nilgil
force-pushed
the
feat/16
branch
3 times, most recently
from
cb1ae90 to
a67397b
Compare
|
Closes #16