여행 생성 및 여행 상세 조회 쿼리 개선 #651

jjongwa · 2023-10-03T07:50:42Z

📄 Summary

네~ 개선했습니다~

Trip 생성

최악의 경우: 쿼리 110번, 134ms

dayLog saveAll 배치로 해결: 110 → 48번
tripCity saveAll 배치로 해결: 48 → 18번
city 검증 로직 한번에 해결 18 → 4번

결론 쿼리 110번 -> 4번, 속도 134ms -> 83ms

Trip 조회

최악의 경우: 쿼리 3783번, 7446ms

Fetch Join & DayLog, Item List -> Set으로 변경: 3783 -> 9번
카테고리 각각 조회 -> 한번에 가지고 와서 필요한 것만 반환: 9 -> 4번

결론 쿼리 3783 -> 4, 속도 7446ms -> 600ms

refresh와 writer를 하나로 합칠 수 있을 것 같은데.. 쿼리 하나 줄이는 것보다 다른 일이 훨씬 급해서 여기까지..! 다음 사람이 맡아주시길...!!

🙋🏻 More

close #645
25: 싱싱미역
달리: 싱싱한 미라이돈 2만원
홍고: 진짜말도안됩니다.

Co-authored-by: mcodnjs <[email protected]>

Co-authored-by: hgo641 <[email protected]>

github-actions · 2023-10-03T07:53:29Z

📝 Jacoco Test Coverage

Total Project Coverage	82.66%	🍏

File	Coverage [96.5%]
TripExpenseResponse.java	100%	🍏
DayLog.java	100%	🍏
CustomTripCityRepositoryImpl.java	100%	🍏
CustomDayLogRepositoryImpl.java	100%	🍏
Trip.java	98.45%	🍏
TripService.java	97.33%	🍏
RequestLoggingFilter.java	55.56%	❌

waterricecake

수고하셧습니다. 홍디오

waterricecake · 2023-10-04T06:15:27Z

backend/src/main/java/hanglog/expense/dto/response/TripExpenseResponse.java

@@ -60,4 +61,37 @@ public static TripExpenseResponse of(
                dayLogExpenseResponses
        );
    }
+
+    public static TripExpenseResponse of2(


이거 사용이 안되고 있는것 같습니다.

waterricecake · 2023-10-04T06:20:51Z

backend/src/main/java/hanglog/global/detector/ConnectionProxyHandler.java

@@ -0,0 +1,45 @@
+package hanglog.global.detector;


쿼리 카운터 이 pr에서 넣기로 한건가요?

i want...
이제 각자 쿼리 개선할텐데, 그때마다 N+1 detector 코드 복사해오고, push할 때 다시 없애는 일이 너무 번거로울 것 같아서요! 희망사항이긴합니다

#651 (comment) QueryCounter

waterricecake · 2023-10-04T06:22:48Z

backend/src/main/java/hanglog/global/filter/RequestLoggingFilter.java

+        if (!message.contains("prometheus")) {
+            logger.info(message);
+        }


오 ㅋㅋㅋ 👍 👍 👍 👍 👍

waterricecake · 2023-10-04T06:24:11Z

backend/src/main/java/hanglog/global/filter/RequestLoggingFilter.java

    }

    @Override
    protected void afterRequest(@NonNull final HttpServletRequest request, @NonNull final String message) {
-        logger.info(message);
+//        logger.info(message);


주석 처리하지말고 그냥 지우는건 어떤가요?

waterricecake · 2023-10-04T06:26:37Z

backend/src/main/java/hanglog/trip/domain/DayLog.java

@@ -47,7 +49,7 @@ public class DayLog extends BaseEntity {

    @OneToMany(mappedBy = "dayLog", cascade = REMOVE, orphanRemoval = true)
    @OrderBy(value = "ordinal ASC")
-    private List<Item> items = new ArrayList<>();
+    private Set<Item> items = new HashSet<>();


Suggested change

private Set<Item> items = new HashSet<>();

private Set<Item> items = new LinkedHashSet<>();

아이템 순서도 보장하게 LinkedHashSet을 사용하는건 어떤가요?

@OrderBy가 붙여져있으면 내부적으로 LinkedHashSet으로 처리해줘서 그냥 두었는데 명시적으로 LinkedHashSet으로 할당하는 것도 좋겠군여

순서는 @OrderBy로 보장하고 있어서
굳이 HashSet보다 성능이 떨어지는(물론 둘 다 O(1)이긴 하지만) LinkedHashSet로 바꿀 필요까지는 없다는 생각입니다.!

내부적으로 LinkedHashSet으로 처리한다면, 어떤 부분에서 성능이 다른가용 ?? 갖고 있을 때 메모리를 더 잡아먹는다 ??
저는 명시적으로 + 혹시 모를 버그 예방을 위해 LinkedHashSet에 한 표 하겠습니다 ㅎㅎ

@OrderBy를 쓰면 HashSet으로 생성을 해도 순서가 보장이 되는지 몰랐네요.

저도 홍고와 라온 말처럼 명시적으로 혹은 혹시 모를 버그를 방지하기 위한 LinkedHashSet으로 할당하는 것에 한표 하겠습니다.

성능 차이는 일반 List와 LinkedList의 차이와 같습니다.!
어떤 자료구조를 사용하냐인데 이건 답글로 일일이 달 필요까지는 없다고 생각합니다.(저희 ArrayList LinkedList 미니 미션도 했으니까요..!)

그리고 명시적으로 라는 부분도 잘 모르곘는게,
Trip의 DayLogs라면 이해가 됩니다. 저희가 추가 삭제 할 때마다 순서에 맞게 기타 DayLog의 실제 위치를 변경했으니까요.!

그런데 DayLog의 Items는 다르다고 생각합니다.
저희가 아이템 위치 바꿀 때 기존에 존재하는 아이템들 사이에 끼워 넣나요?
그냥 order 필드만 바꾸고 있습니다..! 따라서 시간복잡도는 그냥 O(1)이죠
애초에 저희가 순서 변경을 할 때 컬랙션 내에서 item의 위치를 바꾸는게 아닌데
왜 LinkedHashSet으로 바꾸는게 명시적인 효과가 있는건지 잘 모르겠다는 의견입니다..!

그으으으런데 홍고의 말대로 @OrderBy가 붙여져있을 때
HashSet으로 선언해놔도 알아서 LinkedHashSet 형태로 저장되어 있다면
LinkedHashSet으로 바꿔도 좋을 것 같습니다. (근데 진짜 그렇게 바뀌나요? 그러면 코드 단에서 밑줄로 잡아줘야 하는거 아닌가)

LJW25

진짜 길었던 오류픽스,,

LJW25 · 2023-10-04T06:38:43Z

backend/src/main/java/hanglog/global/detector/PreparedStatementProxyHandler.java

+    private static final List<String> JDBC_QUERY_METHOD =
+            List.of("executeQuery", "execute", "executeUpdate");


줄바꿈..뭔가 묘하게 불편한데..List.of를 붙이고 안에 내용을 한줄에 하나씩으로 띄우는거 어떨까요

#651 (comment) QueryCounter

LJW25 · 2023-10-04T06:39:28Z

backend/src/main/java/hanglog/global/detector/QueryCounterAop.java

+    public void loggingAfterApiFinish() {
+        final LoggingForm loggingForm = getCurrentLoggingForm();
+
+        ServletRequestAttributes attributes =


얜 final 아니에용?

#651 (comment) QueryCounter

LJW25 · 2023-10-04T06:40:43Z

backend/src/main/java/hanglog/trip/domain/repository/CustomDayLogRepositoryImpl.java

+    }
+
+    private MapSqlParameterSource getDayLogToSqlParameterSource(final DayLog dayLog) {
+        LocalDateTime now = LocalDateTime.now();


LJW25 · 2023-10-04T06:40:58Z

backend/src/main/java/hanglog/trip/domain/repository/CustomTripCityRepositoryImpl.java

+    }
+
+    private MapSqlParameterSource getTripCityToSqlParameterSource(final City city, final Long tripId) {
+        LocalDateTime now = LocalDateTime.now();


LJW25 · 2023-10-04T06:41:32Z

backend/src/main/java/hanglog/trip/service/TripService.java

-        trip.getDayLogs()
-                .removeIf(dayLog -> dayLog.getOrdinal() >= requestPeriod + 1 && dayLog.getOrdinal() <= currentPeriod);
+        trip.getDayLogs().stream()
+                .filter(dayLog -> dayLog.getOrdinal() >= requestPeriod + 1 && dayLog.getOrdinal() <= currentPeriod)


filter 안에 부분 메소드로 분리하는건 과할까요? 한눈에 안들어와서

f812570 이렇게 바꿔봤는데 어떤가요..!

jjongwa · 2023-10-04T07:12:00Z

QueryCounter 부분은 이전에 올렸던 PR로 이관하겠습니다.!

mcodnjs

고생하셨습니다 ~ 커멘트 한번 확인해주십셔 ~~

mcodnjs · 2023-10-04T07:31:46Z

backend/src/main/java/hanglog/trip/domain/repository/TripRepository.java

+            LEFT JOIN FETCH place.category place_category
+            WHERE dayLogs.trip.id = :tripId
+            """)
+    Optional<Trip> findById(@Param("tripId") Long tripId);


mcodnjs · 2023-10-04T07:51:23Z

backend/src/test/java/hanglog/trip/service/TripServiceTest.java

-                        softly.assertThat(trip.getDayLogs()).containsExactly(dayLog1, dayLog2, dayLog3, extraDayLog);
+                        softly.assertThat(trip.getDayLogs())
+                                .usingRecursiveComparison()
+                                .ignoringCollectionOrder()


ignoringCollectionOrder() 이 메서드가 쓰인 이유는 HashSet을 사용해서이고,
실제로는 @OrderBy로 주입해줘서 정상적으로 동작하는데, 모킹테스트라서 이렇게 쓰인게 맞나요 ??

넵 맞습니닷

mcodnjs · 2023-10-04T07:54:28Z

backend/src/main/java/hanglog/trip/service/TripService.java

+        final List<City> cities = cityRepository.findCitiesByIds(tripCreateRequest.getCityIds());
+        if (cities.size() != tripCreateRequest.getCityIds().size()) {
+            throw new BadRequestException(NOT_FOUND_CITY_ID);
+        }


mcodnjs · 2023-10-04T08:01:37Z

backend/src/main/java/hanglog/trip/domain/DayLog.java

@@ -47,7 +49,7 @@ public class DayLog extends BaseEntity {

    @OneToMany(mappedBy = "dayLog", cascade = REMOVE, orphanRemoval = true)
    @OrderBy(value = "ordinal ASC")
-    private List<Item> items = new ArrayList<>();
+    private Set<Item> items = new HashSet<>();


내부적으로 LinkedHashSet으로 처리한다면, 어떤 부분에서 성능이 다른가용 ?? 갖고 있을 때 메모리를 더 잡아먹는다 ??
저는 명시적으로 + 혹시 모를 버그 예방을 위해 LinkedHashSet에 한 표 하겠습니다 ㅎㅎ

mcodnjs · 2023-10-04T08:12:13Z

backend/src/test/java/hanglog/integration/service/TripServiceIntegrationTest.java

+        entityManager.flush();
+        entityManager.clear();


이 부분 이 부분 설명해주실 수 있나용 ? 궁금해요 대면으로 설명해주셔도 괜찬항여

DataJpaTest 를 사용했을 때 해당 시점에 자식 객체를 가져오지 못합니다.
https://stackoverflow.com/questions/33587221/spring-data-jpa-test-returns-null-list-even-after-child-is-saved

그래서 사실 TripId에 해당되는 Trip을 조회한다. 테스트도 가져온 Trip의 DayLogs size가 0인데
ignoringFields("id", "dayLogs") 가 되어있어서 통과할 수 있었던 거구요.!

waterricecake

HashSet은 순서 보장이 된다면 저는 어느 것이든 상관 없어서 괜찮을 것 같습니다.

* refactor: queryCounter 적용 Co-authored-by: mcodnjs <[email protected]> * refactor: prometheus 관련 로그 출력되지 않도록 변경 * feat: DayLog saveAll 메서드 구현 Co-authored-by: hgo641 <[email protected]> * feat: tripCity saveAll 메서드 구현 Co-authored-by: hgo641 <[email protected]> * refactor: trip 검증 로직 변경 Co-authored-by: hgo641 <[email protected]> * refactor: fetch join으로 trip의 모든 정보 조회 최적화 Co-authored-by: hgo641 <[email protected]> * refactor: TripServiceTest 수정 Co-authored-by: hgo641 <[email protected]> * feat: addDayLog, addItem 메서드 생성 * test: ignoringCollectionOrder 추가 * rafactor: Left join 적용 및 테스트 리팩터링 * test: flush & clear 통한 JpaTest 오류 해결 * refactor: 컨벤션 및 사용하지 않는 메서드 제거 * refactor: 새롭게 수정된 일정을 벗어난 DayLog 삭제하는 로직 메서드 분리 * refactor: QueryCounter 제거 * refactor: 매개변수 final 추가 --------- Co-authored-by: mcodnjs <[email protected]> Co-authored-by: hgo641 <[email protected]>

jjongwa and others added 11 commits September 28, 2023 14:49

refactor: queryCounter 적용

82186ed

Co-authored-by: mcodnjs <[email protected]>

refactor: prometheus 관련 로그 출력되지 않도록 변경

a172e46

feat: DayLog saveAll 메서드 구현

70ff2c3

Co-authored-by: hgo641 <[email protected]>

feat: tripCity saveAll 메서드 구현

3e9a3be

Co-authored-by: hgo641 <[email protected]>

refactor: trip 검증 로직 변경

899b2af

Co-authored-by: hgo641 <[email protected]>

refactor: fetch join으로 trip의 모든 정보 조회 최적화

7b6b60c

Co-authored-by: hgo641 <[email protected]>

refactor: TripServiceTest 수정

de40180

Co-authored-by: hgo641 <[email protected]>

feat: addDayLog, addItem 메서드 생성

4d464e9

test: ignoringCollectionOrder 추가

b8f9432

rafactor: Left join 적용 및 테스트 리팩터링

033e403

test: flush & clear 통한 JpaTest 오류 해결

a23f0b9

jjongwa added 🔨 Refactor BE labels Oct 3, 2023

jjongwa requested review from mcodnjs and waterricecake October 3, 2023 07:50

jjongwa assigned LJW25, hgo641 and jjongwa Oct 3, 2023

waterricecake reviewed Oct 4, 2023

View reviewed changes

LJW25 approved these changes Oct 4, 2023

View reviewed changes

jjongwa added 3 commits October 4, 2023 16:05

refactor: 컨벤션 및 사용하지 않는 메서드 제거

d3ab7a8

refactor: 새롭게 수정된 일정을 벗어난 DayLog 삭제하는 로직 메서드 분리

f812570

refactor: QueryCounter 제거

75c3a7c

mcodnjs reviewed Oct 4, 2023

View reviewed changes

jjongwa added 2 commits October 4, 2023 17:55

refactor: 매개변수 final 추가

1b1a4c6

refactor: conflict 해결

f5ea739

waterricecake approved these changes Oct 6, 2023

View reviewed changes

jjongwa merged commit a053cc0 into develop Oct 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

여행 생성 및 여행 상세 조회 쿼리 개선 #651

여행 생성 및 여행 상세 조회 쿼리 개선 #651

jjongwa commented Oct 3, 2023 •

edited

Loading

github-actions bot commented Oct 3, 2023 •

edited

Loading

waterricecake left a comment

waterricecake Oct 4, 2023

waterricecake Oct 4, 2023

hgo641 Oct 4, 2023

jjongwa Oct 4, 2023

waterricecake Oct 4, 2023

waterricecake Oct 4, 2023

waterricecake Oct 4, 2023

hgo641 Oct 4, 2023

jjongwa Oct 4, 2023 •

edited

Loading

mcodnjs Oct 4, 2023

waterricecake Oct 4, 2023

jjongwa Oct 4, 2023

LJW25 left a comment

LJW25 Oct 4, 2023

jjongwa Oct 4, 2023

LJW25 Oct 4, 2023

jjongwa Oct 4, 2023

LJW25 Oct 4, 2023

LJW25 Oct 4, 2023

LJW25 Oct 4, 2023

jjongwa Oct 4, 2023

LJW25 Oct 4, 2023

jjongwa commented Oct 4, 2023

mcodnjs left a comment

mcodnjs Oct 4, 2023

mcodnjs Oct 4, 2023

jjongwa Oct 4, 2023

mcodnjs Oct 4, 2023

mcodnjs Oct 4, 2023

mcodnjs Oct 4, 2023

jjongwa Oct 4, 2023

waterricecake left a comment

	private Set<Item> items = new HashSet<>();
	private Set<Item> items = new LinkedHashSet<>();

		private static final List<String> JDBC_QUERY_METHOD =
		List.of("executeQuery", "execute", "executeUpdate");

여행 생성 및 여행 상세 조회 쿼리 개선 #651

여행 생성 및 여행 상세 조회 쿼리 개선 #651

Conversation

jjongwa commented Oct 3, 2023 • edited Loading

📄 Summary

Trip 생성

결론 쿼리 110번 -> 4번, 속도 134ms -> 83ms

Trip 조회

결론 쿼리 3783 -> 4, 속도 7446ms -> 600ms

🙋🏻 More

github-actions bot commented Oct 3, 2023 • edited Loading

📝 Jacoco Test Coverage

waterricecake 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

jjongwa Oct 4, 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

LJW25 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

jjongwa commented Oct 4, 2023

mcodnjs 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

waterricecake left a comment

Choose a reason for hiding this comment

jjongwa commented Oct 3, 2023 •

edited

Loading

github-actions bot commented Oct 3, 2023 •

edited

Loading

jjongwa Oct 4, 2023 •

edited

Loading