[BE] Refactor/#430 핀 복사 시 JdbcTemplate을 이용하여 bulk insert하도록 변경 #617

yoondgu · 2023-11-01T15:05:47Z

작업 대상

#430 참조부탁드립니다.
조금씩 오랜 기간 작업했다보니 공유/기록할 만한 사항이 많아 보여 다소 긴 내용 양해부탁드려요 🥲

추가로 함께 변경된 사항

Jdbc Url query string에 batchupdate 쿼리 반영을 위한 설정이 하나 추가되었습니다. (테스트 컨테이너에서도 이를 추가했습니다)
N+1 문제로 인한 pinRepository.findAllByIdIn(Long pinId)에 fetch join 적용 (이로 인해 Latency가 악화되지는 않는 것을 확인했습니다)

📄 작업 내용

MySQL의 Identity 기본키 전략을 사용함에 따라 saveAll 메서드가 bulk insert를 지원받지 못해 발생하는 쿼리 문제를 해결하는 PR입니다.

용어 정리

초반에 헷갈려서 단어를 섞어 썼는데요. 지금은 아래와 같이 이해하고 사용하고 있습니다!

bulk insert: insert into Pin values (?,?,?), (?,?,?), (?,?,?)...와 같이 하나의 insert 쿼리에서 여러 행을 추가하는 것
batch update: 한번에 여러 변경 사항을 만드는 것으로, JdbcTemplate에서 제공하는 batchUpdate() 메서드를 표현할 때 해당 단어를 사용했습니다.

(결론부터) 성능 비교

이슈에 올렸던 성능 비교에서는 핀 이미지가 없는 핀으로만 테스트했기 때문에 논외로 치고 다시 진행했습니다.

대상 API: 실제로 여러 개의 핀 복사를 요청하는 데 가장 많이 쓰이는 /topics/new
기준 데이터: 복사할 핀 약 10,000개 / 각 핀 당 이미지 1개(이미지도 총 약 10,000개)

10만개는 되어야 유의미한 차이가 있지 않을까? 했는데 제 로컬 환경에서 given절을 세팅하는 데에만 힙 메모리가 부족한 상황이 발생하더라구요. 그런데 사실 지금 서비스에 전국 데이터가 있는 지도만 해도, 핀이 800개가 최대이기 때문에 10,000개 정도를 척도로 두는 게 오히려 적절하다고 생각했어요.

Bulk Insert 적용 이전

 Latency : 37.422s, Query count : 30754, Request URI : /topics/new

Pin, PinImage를 각각 insert하는 데에만 20,000개이고 findAllByIdIn(Long pinId)에서의 N+1 문제도 함께 존재했습니다.

Bulk Insert 적용 이후

Latency : 7.938s, Query count : 7, Request URI : /topics/new

실제 쿼리는 위 기록된 것에 PinBatchRepository에서 수행되는 쿼리 3개가 추가됩니다.
저장하는 데이터가 20,000개라는 걸 감안해야 하지만 Latency는 조금 아쉬운데요. 다른 변수가 많은 부분이지만 적용 이전과 비교하면 의미있는 개선인 듯합니다.

겪었던 문제

생각하지 못한 어려움을 해결하고 조금씩 작업하느라 오래 걸렸네요.
슬랙 및 대화로 충분히 공유되었다고 생각하지만, 문서화를 위해 추가 기록해둡니다.

1. `1:N` 연관 관계가 있는 엔티티들을 Bulk Insert 할 때, `1`에 해당하는 엔티티(pin)의 Id를 모두 알아야 `N`(pinImage)을 저장할 수 있다.

해결: MySQL의 last_insert_id() 함수를 사용해 bulk insert한 pin의 첫번째 id를 받아온 뒤 해당하는 개수만큼 계산해 pinId를 가진 pinImage를 저장합니다.
마지막 id가 아닌 첫번째 id를 받아오기 때문에, bulk insert한 row의 개수만큼 id를 보장받은 상태로 작업할 수 있습니다.
참고 링크: https://helloworld.kurly.com/blog/bulk-performance-tuning/

2. JdbcTemplate을 주입받는 Repository로 인해 DataJpaTest가 터지는 문제

해결: 해당 Repository를 Spring Data Jpa에서 제공하는 커스텀 리포지토리로 정의하면 JpaRepository로 주입 가능
사용자 정의 리포지토리에 대해서는 코멘트에서 추가로 설명드리겠습니다.
참고 링크: https://docs.spring.io/spring-data/jpa/reference/repositories/custom-implementations.html

그밖에 고려한 사항

1. Pin을 복사해올 때, Topic의 pinCount, lastPinUpdatdAt도 업데이트 해야 함

처음에는 한 비즈니스 로직 안에 묶어서, 데이터 정합성을 보장하기 위해 batchUpdate가 이루어지는 saveAllToTopic 내에서 topic.update...를 해주었습니다.
그러나 두 가지가 우려되어서 이를 분리했습니다.
1. JPA 쿼리와 jdbcTemplate 쿼리가 함께 발생하는 것에 대한 사이드 이펙트가 있지 않다고 보장하기에는 아직 학습이 더 필요하다고 느낌
2. 추상화된 PinBatchRepositoryCustomImpl의 영역에 변경 감지 로직을 넣어두면 숨겨진 코드처럼 여겨져 추후 디버깅이 어려울 수도 있음
  이 부분에 대해서는 다른 분들의 의견도 들어보고 싶어요!

2. SQL 로깅

한 가지 인지해야 할 것이, 현재 구현된 bulk insert는 hibernate에서 수행되는 쿼리가 아니기 때문에 저희의 QueryCounter에 걸리지 않고, Hibernate 로그에도 찍히지 않습니다.

그래서 대신 아래와 같은 설정을 jdbc url에 추가해주면, Hibernate 이외에도 모든 SQL 관련 로그를 확인할 수 있습니다.

&profileSQL=true&logger=Slf4JLogger&maxQuerySizeToLog=9999

그러나 이 설정을 켜두면, Hibernate의 SQL 로그와 중복된 로그가 발생하겠죠? 게다가 필요 이상의 정보 (setAutoCommit에 대한 로그 등..)도 너무 많이 나와요.
가끔 발생하는 bulk insert를 확인하기 위해 중복된 로그, 불필요한 로그를 모두 찍을 필요는 없다고 생각했어요. 로그 설정을 더 잘 조정해봐도 좋겠지만요..
일단은 그래서 PinBatchRepository 클래스 에서 커스텀 로그를 작성하는 방식을 선택했습니다.

🙋🏻 주의 사항

앞서 말한 것처럼, 현재 구현된 bulk insert는 hibernate에서 수행되는 쿼리가 아니기 때문에 저희의 QueryCounter에 걸리지 않고, Hibernate 로그에도 찍히지 않습니다!
만약 개발하다가 해당 쿼리 로그를 확인할 필요가 있다면 로컬 환경에서 위 설정을 추가해주세요.

스크린샷

📎 관련 이슈

레퍼런스

- datasource url에 &rewriteBatchedStatements=true&profileSQL=true&logger=Slf4JLogger&maxQuerySizeToLog=999999 추가할 것

- 추상화 후 실패하는 테스트의 영속성 컨텍스트 문제 해결

- 실제 서브모듈에도 반영할것

github-actions · 2023-11-01T15:09:57Z

Unit Test Results

  70 files   70 suites 45s ⏱️
346 tests 346 ✔️ 0 💤 0 ❌
369 runs 369 ✔️ 0 💤 0 ❌

Results for commit 986ffb1.

♻️ This comment has been updated with latest results.

junpakPark

도이 코드 잘 봤습니당~
2가지 질문 남겼는데 답변 해주시면 감사하겠습니당 ㅎㅎ

junpakPark · 2023-11-01T15:08:50Z

...nd/src/main/java/com/mapbefine/mapbefine/common/repository/PinBatchRepositoryCustomImpl.java

@@ -0,0 +1,168 @@
+package com.mapbefine.mapbefine.common.repository;


전 pin 패키지 내에 infrastructure 패키지 만들어 준 후에 거기서 사용하는 걸로 생각했었어요!
제가 말을 명쾌하게 드리지 못한 것 같네요ㅠ

앗 아직 Draft 상태인 PR이라.. 본문 작성 및 보완하고 나면 Ready for Review로 전환 후 다시 말씀드리겠습니다 ㅠ

junpakPark · 2023-11-01T15:09:07Z

backend/src/main/java/com/mapbefine/mapbefine/common/repository/PinBatchRepositoryCustom.java

+
+public interface PinBatchRepositoryCustom {
+
+    int[] saveAllToTopic(Topic topicForCopy, List<Pin> originalPins);


Collection으로 반환은 안되는건가용?

Draft 상태지만 미리 남겨주셔서 말씀드리자면, 기존 batchUpdate의 시그니처를 그대로 가져가고자 했습니다!
batchUpdate에서 변경된 행 수를 int[] 로 주는데, 굳이 Collection으로 바꿔줄 이유를 모르겠어서용

yoondgu · 2023-11-02T01:26:21Z

backend/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustom.java

+public interface PinBatchRepositoryCustom {
+
+    int[] saveAllToTopic(Topic topicForCopy, List<Pin> originalPins);
+


커스텀 리포지토리에 구현할 메서드를 정의한 인터페이스를 만들어주어야 합니다.

yoondgu · 2023-11-02T01:28:21Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+
+@Slf4j
+@Repository
+public class PinBatchRepositoryCustomImpl implements PinBatchRepositoryCustom {


1에서 만든 인터페이스의 구현체를 만드는데, 이때 중요한 것은 클래스명입니다! 인터페이스 이름 + Impl 로 지어주어야해요.

Impl 로 만들지 않는다면 작동을 안하나요?

@kpeel5839 https://cs-ssupport.tistory.com/511 다른 이름도 설정할 수 있는 방법이 있긴한데, 널리 쓰이진 않는 것 같아요 ㅋㅋ

Impl 이 짱이긴하죠 껄껄

yoondgu · 2023-11-02T01:32:09Z

backend/src/main/java/com/mapbefine/mapbefine/pin/domain/PinRepository.java

@@ -9,11 +10,14 @@
 import org.springframework.stereotype.Repository;

 @Repository
-public interface PinRepository extends JpaRepository<Pin, Long> {
+public interface PinRepository extends JpaRepository<Pin, Long>, PinBatchRepositoryCustom {


2에서 만든 커스텀 리포지토리 구현체를 이렇게 JpaRepository와 합쳐서 사용할 수 있습니다.
그리고 사용자 정의로 구현한 내용이 기본 구현이나 Repository aspect(공식문서 워딩인데 Repository에 대해 전역적으로 적용되는 기능들을 말하는 게 아닐까 싶어요)보다 우선시 된다고 하네용

yoondgu · 2023-11-02T01:41:59Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+    private static class PinImageInsertDto {
+
+        private final String imageUrl;
+        private final Long pinId;
+        private final boolean isDeleted;


불가피하게 DB 저장 시 사용할 Dto가 필요했습니다.
딱 여기에서만 사용하는 Dto라 내부 클래스로 정의했습니다.
핀 저장 -> 아이디 조회 -> DTO 생성 -> 핀 이미지 저장 이 한 메서드에서 이루어져서 좀 더 객체 책임을 분리해보는 방법을 고민해보고 싶긴 하네요

흠... 근데 너무 묶여있는 작업이라 현재 방식도 괜찮다고 보긴하는데.. 이에 대해 도이 생각은 어떠신가요??

그러게요. 사실 saveAll 메서드가 bulk insert를 지원했다면 한번에 이루어졌을 로직이라, 억지로 분리할 필요는 없다는 생각도 드네요!

yoondgu · 2023-11-02T01:58:31Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+    private int[] bulkInsertPins(Topic topicForCopy, List<Pin> originalPins) {
+        String bulkInsertSql = "INSERT INTO pin ("
+                + "name, description, member_id, topic_id, location_id,"
+                + " created_at, updated_at)"
+                + " VALUES ("
+                + " ?, ?, ?, ?, ?,"
+                + " ?, ?)";
+        LocalDateTime createdAt = topicForCopy.getLastPinUpdatedAt();
+        Long topicId = topicForCopy.getId();
+        Long creatorId = topicForCopy.getCreator().getId();
+        log.debug("[Query] bulk insert size {} : {}", originalPins.size(), bulkInsertSql);
+
+        return jdbcTemplate.batchUpdate(bulkInsertSql, new BatchPreparedStatementSetter() {


bulk insert하는 부분입니다.

복사할 Pin들을 bulk insert한다.

bulk insert 쿼리 실행 후, 해당 쿼리의 첫번째 id값을 받아온다.

id값을 이용해 새로 저장할 PinImage의 내용을 만든다.

PinImageInsertDto를 bulk insert한다.

오우 설명들이 살벌하네요 이해에 도움이 많이 됩니다.

kpeel5839

도이 코드 잘 봤어요!

역시 천재 또또이.. 아주 잘 해내주셨군요

그냥 간단하게 코멘트 남겼습니다!

읽어보시고 답변해주시면 감사할 것 같아요!

kpeel5839 · 2023-11-02T11:43:04Z

backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java

@@ -55,12 +56,12 @@ public Long saveTopic(AuthMember member, TopicCreateRequest request) {
        Topic topic = convertToTopic(member, request);
        List<Long> pinIds = request.pins();

+        topicRepository.save(topic);
+        topicRepository.flush();


flush 하지 않으면 어떤 일이 발생하나요?

JdbcTemplate은 바로 쿼리문을 쏘니까 필요한 듯요..

다들 꼼꼼히 봐주셔서 감사합니다!!

JdbcTemplate은 바로 쿼리문을 쏘니까 필요한 듯요..

맞아요 저도 그런 이유로 flush를 넣었었는데요.. (JdbcTemplate의 쿼리 이전에 topic을 먼저 save하려고)
아래 다른 코멘트에서 확인되듯이 flush를 안해도 topic이 먼저 save 되는거였네요.
왜냐하면 saveAllToTopic 메서드 내에서 batchUpdate를 실행하기 전에 전달받은 topic을 조회하기 때문에,
조회하는 일이 발생했으니까 JPA에서 insert를 먼저 시켜주는 것 같아요. 제가 생각한 게 맞을까요?
++ flush는 지우겠습니다!

backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java

kpeel5839 · 2023-11-02T11:47:28Z

backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java

+        topicRepository.save(topic);
+        topicRepository.flush();


Suggested change

topicRepository.save(topic);

topicRepository.flush();

topicRepository.saveAndFlush(topic);

은 어떠신가요?

헐 댑악 ~

근데 flush 필요 없는 것 맞았습니다 ^.^ ㅜ 지울게요! 좋은거 알려주셔서 감사해여

kpeel5839 · 2023-11-02T11:49:21Z

backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java

@@ -105,14 +106,15 @@ private void copyPinsToTopic(
    ) {
        List<Pin> originalPins = findAllPins(pinIds);
        validateCopyablePins(member, originalPins);
+        topic.increasePinCount(pinIds.size());
+        topic.updateLastPinUpdatedAt(LocalDateTime.now());


@PrePersist protected void prePersist() { lastPinUpdatedAt = LocalDateTime.now(); }

현재 Topic 에는 위와 같은 코드가 존재하는데요!

그렇기 때문에, Save 하면서 LastPinupdateAt 은 이미 현재 시각으로 적용되어 있지 않나용?

아니라면 따끔하게 말씀해주십숑

헉 맞아요!! id 채번하는 데에만 정신이 팔려서 꼼꼼하지 못했던 부분이 많네요 ㅠㅠ 감사합니당

kpeel5839 · 2023-11-02T11:50:18Z

backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java

@@ -105,14 +106,15 @@ private void copyPinsToTopic(
    ) {
        List<Pin> originalPins = findAllPins(pinIds);
        validateCopyablePins(member, originalPins);
+        topic.increasePinCount(pinIds.size());
+        topic.updateLastPinUpdatedAt(LocalDateTime.now());
+        pinRepository.flush();


여기도 flush 가 있는 것을 보니 batch update 를 진행하기 위해서는 명시적인 flush 호출이 필요한가보군요 흠흠

update할때는 flush를 해주어야 하는 게 맞아요!
이전에 delete가 반영안되는 문제 해결할 때, member update가 되지 않았던 부분 문제랑 동일하다고 보면 될거같아용
https://map-befine-official.github.io/trouble-shooting-jpa-delete-and-persistence/

kpeel5839 · 2023-11-02T12:03:53Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+    private static class PinImageInsertDto {
+
+        private final String imageUrl;
+        private final Long pinId;
+        private final boolean isDeleted;


흠... 근데 너무 묶여있는 작업이라 현재 방식도 괜찮다고 보긴하는데.. 이에 대해 도이 생각은 어떠신가요??

kpeel5839 · 2023-11-02T12:04:34Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+                + "(image_url, pin_id) "
+                + "VALUES "
+                + "(?, ?)";
+        log.debug("[Query] bulk insert size {} : {}", pinImages.size(), bulkInsertSql);


여기서 왜 log 를 따로 찍는지도 PR 에 너무 상세하게 적어주셔서 이해가 쉬웠습니다!

역시 도이짱!

kpeel5839 · 2023-11-02T12:08:45Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+                + " VALUES ("
+                + " ?, ?, ?, ?, ?,"
+                + " ?, ?)";
+        LocalDateTime createdAt = topicForCopy.getLastPinUpdatedAt();


아 이거를 위해서 topic.updateLastPinUpdatedAt 를 만들어주신건가??!

근데 그렇다기에는 flush 해주면서 @PrePersist 가 진행되어서 위 메서드를 호출해서 현재 시각으로 설정해주지 않더라도 설정이 되어 있지 않나요??

잘 모르겠네요 허허허허

kpeel5839 · 2023-11-02T12:10:19Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+        validateId(firstIdFromBatch);
+
+        return IntStream.range(0, originalPins.size())
+                .filter(index -> rowCount[index] != -3)


오 minus 3 이면 무슨 의미를 가지나요

batchUpdate 의 반환값인 int[]에서는, 각 행에 대해 반영된 상태를 나타내는 정수를 담아 주는데요.
batch statement 실행 중 에러를 만나 실패했다면 -3을 반환합니다!

저는 int[]에 무조건 true, false로 1 아니면 0인 값을 넣어서 반환할 줄 알았는데 디버깅 중에 -2가 들어있는 걸 본 적이 있어서 의아해서 찾아보니 여러 상태에 따라 다르더라구요.

그런데 이를 누구나 이해할 수 있게, 해당 상수를 가져오거나 상수화를 시켜야겠네요!!

Class JdbcTemplate Javadoc
Interface Statement Javadoc

꼭 -3 뿐만 아니라, 음수 값일때 수행하지 못하도록 할 필요는 없을까요 ?
SUCCESS_NO__INFO가 어떤 상황에 발생하는건지는 알 수 없지만, 성공은 한거니까 상관없을까요 ?

넵 저는 성공이니까 상관없다고 생각했어요! 성공해서 insert된 레코드들에 대한 작업을 해주니까요.
쥬니 생각은 어떠신가요?

kpeel5839 · 2023-11-02T12:11:27Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+                .filter(index -> rowCount[index] != -3)
+                .mapToObj(index -> {
+                    Pin pin = originalPins.get(index);
+                    return PinImageInsertDto.of(pin.getPinImages(), firstIdFromBatch + index);


이 부분 아주 깔끔하게 해결해주셨네요 멋집니다 도이

cpot5620

도이 고생 많으셨어요 !!

뒤늦게 알고리즘에 뛰어들어서, 리뷰가 늦어졌네요 😢
이미 리뷰가 꼼꼼하게 남겨져있는 것 같아서, 중복 질문은 피해서 코멘트 남겼습니다 !

cpot5620 · 2023-11-03T04:43:48Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+        String bulkInsertSql = "INSERT INTO pin ("
+                + "name, description, member_id, topic_id, location_id,"
+                + " created_at, updated_at)"
+                + " VALUES ("
+                + " ?, ?, ?, ?, ?,"
+                + " ?, ?)";


Suggested change

String bulkInsertSql = "INSERT INTO pin ("

+ "name, description, member_id, topic_id, location_id,"

+ " created_at, updated_at)"

+ " VALUES ("

+ " ?, ?, ?, ?, ?,"

+ " ?, ?)";

String bulkInsertSql = "INSERT INTO pin "

+ "(name, description, member_id, topic_id, location_id, "

+ "created_at, updated_at)"

+ "VALUES ("?, ?, ?, ?, ?, ?, ?)";

이런 개행이 더 예쁜 것 같은데, 기존과 같이 개행을 적용하신 이유가 있나요 !?

쥬니짱 오늘 잠실갔어용?

필드를 두 줄로 나눈 부분을 ?이랑 같게 두는 게 ? 개수가 맞는지 확인하기 편하다고 생각했어요!
근데 괄호는 제가 봐도 안예쁘네여 ㅋㅋ 괄호만 수정해볼게요

cpot5620 · 2023-11-03T04:46:40Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+    private List<PinImageInsertDto> createPinImageInsertDtos(List<Pin> originalPins, int[] rowCount) {
+        Long firstIdFromBatch = jdbcTemplate.queryForObject("SELECT last_insert_id()", Long.class);
+        validateId(firstIdFromBatch);


해당 쿼리문에 대한 queryForObject 메서드의 반환값이 Null일 가능성이 존재하나요 ?

DB 자체의 문제로 발생하는 거면, 내부적으로 다른 예외를 던져주지 않을까 싶은데.. 아닌가요 ?

맞네요. null일 가능성은 없네요!
근데 last_insert_id()가 트랜잭션 별로 관리되어서 만약 이전에 insert를 성공한 적이 없다면 0을 반환하더라구요.
이 부분에 대한 예외로 수정하겠습니당

cpot5620 · 2023-11-03T04:58:57Z

backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java

@@ -55,12 +56,12 @@ public Long saveTopic(AuthMember member, TopicCreateRequest request) {
        Topic topic = convertToTopic(member, request);
        List<Long> pinIds = request.pins();

+        topicRepository.save(topic);
+        topicRepository.flush();


JdbcTemplate은 바로 쿼리문을 쏘니까 필요한 듯요..

cpot5620 · 2023-11-03T04:59:41Z

backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java


+        topicRepository.save(topic);


엥 그러네 ...

cpot5620 · 2023-11-03T05:01:15Z

backend/src/test/java/com/mapbefine/mapbefine/pin/domain/PinRepositoryTest.java

+        assertSoftly(softly -> {
+            softly.assertThat(copiedPins).extracting("pinInfo")
+                    .usingRecursiveComparison()
+                    .isEqualTo(originalPinInfos);
+            softly.assertThat(copiedPins.get(0).getCreator())
+                    .isEqualTo(copier);
+            softly.assertThat(copiedPins).hasSize(originalPins.size())
+                    .flatMap(Pin::getPinImages)
+                    .allSatisfy(pinImage -> {
+                        assertThat(pinImage.getImageUrl()).isEqualTo(PinImageFixture.IMAGE_URL);
+                    });
+        });
+    }


assertSoftly를 사용하면, assertAll을 사용하는 것보다 테스트 성능이 현저히 떨어진다는 사실을 알고 계신가요 ?

전 몰랐어요

허걱스 몰랐어요. 근데 성능이 현저히 떨어진다는 건 테스트 시간이 많이 길어진다는 뜻인가요?! assertSoftly로 하면 assertAll과 달리 각 테스트가 실패한 코드 라인 위치까지 보여줘서 편하다구 생각했는데 이 점은 어떻게 생각하시나요..?!

cpot5620 · 2023-11-03T05:03:34Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+        validateId(firstIdFromBatch);
+
+        return IntStream.range(0, originalPins.size())
+                .filter(index -> rowCount[index] != -3)


꼭 -3 뿐만 아니라, 음수 값일때 수행하지 못하도록 할 필요는 없을까요 ?
SUCCESS_NO__INFO가 어떤 상황에 발생하는건지는 알 수 없지만, 성공은 한거니까 상관없을까요 ?

kpeel5839

도이 커밋단위가 세세해서, 제가 작업하고 있는 느낌이 들 정도로 읽기 수월했어요!

고생하셨습니다!

빠르게 머지 하시죵~

kpeel5839 · 2023-11-05T05:27:16Z

backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java

@@ -57,7 +57,6 @@ public Long saveTopic(AuthMember member, TopicCreateRequest request) {
        List<Long> pinIds = request.pins();

        topicRepository.save(topic);
-        topicRepository.flush();


kpeel5839 · 2023-11-05T05:27:40Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

@@ -78,7 +80,7 @@ private List<PinImageInsertDto> createPinImageInsertDtos(List<Pin> originalPins,
        validateId(firstIdFromBatch);

        return IntStream.range(0, originalPins.size())
-                .filter(index -> rowCount[index] != -3)
+                .filter(index -> rowCount[index] != EXECUTE_FAILED)


kpeel5839 · 2023-11-05T05:28:06Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+        String bulkInsertSql = "INSERT INTO pin "
+                + "(name, description, member_id, topic_id, location_id, "
+                + "created_at, updated_at) "
+                + "VALUES "
+                + "(?, ?, ?, ?, ?, "
+                + "?, ?)";


깔꼼스 하군요!

kpeel5839 · 2023-11-05T05:28:35Z

...d/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java

+    private record PinImageInsertDto(
+            String imageUrl,
+            Long pinId,
+            boolean isDeleted
+    ) {


👍 반영해주셔서 감사합니다~!

kpeel5839 · 2023-11-05T05:28:50Z

backend/src/main/java/com/mapbefine/mapbefine/topic/domain/Topic.java

-    public void increasePinCount() {
-        pinCount++;
-    }


kpeel5839 · 2023-11-05T05:29:18Z

backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java

@@ -106,7 +105,6 @@ private void copyPinsToTopic(
        List<Pin> originalPins = findAllPins(pinIds);
        validateCopyablePins(member, originalPins);
        topic.increasePinCount(pinIds.size());
-        topic.updateLastPinUpdatedAt(LocalDateTime.now());


kpeel5839 · 2023-11-05T05:29:27Z

backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java

@@ -139,7 +137,6 @@ public Long merge(AuthMember member, TopicMergeRequest request) {
        List<Pin> originalPins = getAllPinsFromTopics(originalTopics);

        topic.increasePinCount(originalPins.size());
-        topic.updateLastPinUpdatedAt(LocalDateTime.now());


yoondgu added 17 commits November 1, 2023 23:59

feat: batch update 적용 중

ca47a30

chore: batch update 적용 중

11f64ff

feat: pin image를 고려한 batch update 적용

331db76

- datasource url에 &rewriteBatchedStatements=true&profileSQL=true&logger=Slf4JLogger&maxQuerySizeToLog=999999 추가할 것

feat: batch update 시 pinImage N+1 문제 해결을 위한 fetch join

2486358

refactor: PinBatchRepository 중복 제거 및 메서드 분리

79dcbce

refactor: jdbcTemplate 사용하는 리포지토리를 사용자 정의 리포지토리로 추상화

12ab047

- 추상화 후 실패하는 테스트의 영속성 컨텍스트 문제 해결

refactor: PinBatchRepository 인덴트 개선

974de2b

refactor: 테스트 컨테이너 jdbcURl에 batch update를 위한 설정 추가

5186cc8

- 실제 서브모듈에도 반영할것

refactor: saveAllToTopic 불필요한 인자 제거로 인한 시그니처 변경

e6a30ba

refactor: 사용자 정의 리포지토리의 saveAllToTopic 테스트 작성

60cab90

refactor: jdbcTemplate, hibernate 중복 로그 하는 대신 커스텀 로그 설정

dc9a5b7

test: 테스트 내 불필요한 출력 삭제

f907795

test: JPA 쿼리 호출과 JdbcTemplate 쿼리 호출 분리

26db14d

chore: batch update를 위한 jdbc url 설정 추가

585e135

chore: 로컬 환경 batch update 설정 추가

7512b63

refactor: 불필요한 flush 삭제

0073ed2

refactor: 개행 추가

a57a16c

junpakPark approved these changes Nov 1, 2023

View reviewed changes

chore: 커스텀 리포지토리 패키지 컨텍스트 별로 변경

de403b5

yoondgu commented Nov 2, 2023

View reviewed changes

refactor: 메서드명 수정 및 가독성 개선

58fc4ea

yoondgu commented Nov 2, 2023

View reviewed changes

yoondgu marked this pull request as ready for review November 2, 2023 01:59

kpeel5839 assigned kpeel5839 and yoondgu and unassigned kpeel5839 Nov 2, 2023

kpeel5839 added BE 백엔드 관련 이슈 feat 새로운 기능 개발 labels Nov 2, 2023

kpeel5839 requested review from cpot5620 and kpeel5839 November 2, 2023 02:18

kpeel5839 requested changes Nov 2, 2023

View reviewed changes

cpot5620 approved these changes Nov 3, 2023

View reviewed changes

yoondgu added 6 commits November 3, 2023 17:40

refactor: 불필요한 flush 삭제

ab519fb

refactor: rowCount 상태코드 상수화

eabc420

refactor: sql 문자열 개행 방식 수정

bd7f38c

refactor: DB용 Dto record로 변경

6976452

refactor: 사용하지 않는 메서드 삭제

bda1f24

refactor: 불필요한 update 호출 삭제

986ffb1

yoondgu added 우선순위 : 중 refactor 리팩토링 관련 이슈 labels Nov 3, 2023

kpeel5839 approved these changes Nov 5, 2023

View reviewed changes

yoondgu merged commit 9a26db2 into develop-BE Nov 6, 2023
3 checks passed

semnil5202 deleted the refactor/#430-re branch February 9, 2024 23:46

		@@ -0,0 +1,168 @@
		package com.mapbefine.mapbefine.common.repository;


		public interface PinBatchRepositoryCustom {

		int[] saveAllToTopic(Topic topicForCopy, List<Pin> originalPins);

	topicRepository.save(topic);
	topicRepository.flush();
	topicRepository.saveAndFlush(topic);

[BE] Refactor/#430 핀 복사 시 JdbcTemplate을 이용하여 bulk insert하도록 변경 #617

[BE] Refactor/#430 핀 복사 시 JdbcTemplate을 이용하여 bulk insert하도록 변경 #617

Conversation

yoondgu commented Nov 1, 2023 • edited Loading

작업 대상

추가로 함께 변경된 사항

📄 작업 내용

용어 정리

(결론부터) 성능 비교

Bulk Insert 적용 이전

Bulk Insert 적용 이후

겪었던 문제

1. 1:N 연관 관계가 있는 엔티티들을 Bulk Insert 할 때, 1에 해당하는 엔티티(pin)의 Id를 모두 알아야 N(pinImage)을 저장할 수 있다.

2. JdbcTemplate을 주입받는 Repository로 인해 DataJpaTest가 터지는 문제

그밖에 고려한 사항

1. Pin을 복사해올 때, Topic의 pinCount, lastPinUpdatdAt도 업데이트 해야 함

2. SQL 로깅

🙋🏻 주의 사항

스크린샷

📎 관련 이슈

레퍼런스

github-actions bot commented Nov 1, 2023 • edited Loading

Unit Test Results

junpakPark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoondgu Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoondgu Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoondgu Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpeel5839 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoondgu Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpot5620 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpeel5839 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoondgu commented Nov 1, 2023 •

edited

Loading

1. `1:N` 연관 관계가 있는 엔티티들을 Bulk Insert 할 때, `1`에 해당하는 엔티티(pin)의 Id를 모두 알아야 `N`(pinImage)을 저장할 수 있다.

github-actions bot commented Nov 1, 2023 •

edited

Loading

yoondgu Nov 2, 2023 •

edited

Loading

yoondgu Nov 2, 2023 •

edited

Loading

yoondgu Nov 2, 2023 •

edited

Loading

yoondgu Nov 2, 2023 •

edited

Loading