[BE] feat: 하이라이트 벌크 삽입 추가 #961

donghoony · 2024-11-13T12:15:03Z

resolves [BE] 하이라이트 전체 저장을 Bulk insert로 진행한다. #878

🚀 어떤 기능을 구현했나요 ?

하이라이트를 하나의 쿼리로 삽입하도록 수정합니다.

🔥 어떻게 해결했나요 ?

JdbcTemplate의 batchUpdate를 사용했습니다. 자세한 사항은 이슈를 확인해주세요.

📝 어떤 부분에 집중해서 리뷰해야 할까요?

Impl은 Spring에서 이렇게 적어야 한다고 해서.. 안 적으면 제대로 잡아오지를 못하더라고요. JpaRepository를 Repository로 변경하고, saveAll 메서드를 JdbcTemplate를 활용한 것을 사용하도록 했어요.

📚 참고 자료, 할 말

[BE] 하이라이트 전체 저장을 Bulk insert로 진행한다. #878
테스트를 위한 프로덕션 코드 수정을 안 좋아할 텐데, JpaRepository의 findAll 메서드는 테스트에서만 사용되고 있었습니다. JpaRepository를 사용하는 것보다 Repository를 받아서 구현할 것만 메서드를 만드는 게 좋지 않을까요?

github-actions · 2024-11-13T12:16:18Z

Test Results

155 tests 152 ✅ 4s ⏱️
59 suites 3 💤
59 files 0 ❌

Results for commit 36d558c.

♻️ This comment has been updated with latest results.

Kimprodp

궁금증만 해소해주십쇼~

Kimprodp · 2024-11-14T14:04:00Z

backend/src/main/java/reviewme/highlight/repository/HighlightRepository.java

+
+    Highlight save(Highlight highlight);
+
+    boolean existsById(long id);


save, existsById 구현 없는데 동작 하나요? 어떻게 하는거지..

Repository 인터페이스에 대한 프록시를 생성할 때
Spring Data JPA가 메서드 이름을 분석하여 자동으로 쿼리를 생성하고 실행한다고 합니다!

산초가 이야기한것처럼 Repository의 메서드 이름으로 쿼리를 만들어줍니다요~!

아하 이해했슴다 단순 Repository 상속이라 안되는줄 알았는데 다 해주는구나..

Kimprodp · 2024-11-14T14:14:52Z

backend/src/main/java/reviewme/highlight/repository/HighlightRepository.java

 import reviewme.highlight.domain.Highlight;

-public interface HighlightRepository extends JpaRepository<Highlight, Long> {
+public interface HighlightRepository extends Repository<Highlight, Long>, HighlightJdbcRepository {
+


saveAll 같이 HighlightJdbcRepository 에서 구현된 메서드는 직접 들어가야지만 존재하는지 알 수 있으니 불편하네요. HighlightRepository 에서 사용 가능한 메서드를 다 바로 확인하면 좋을 텐데..

이건 저도 고민.. 계층을 두는 게 좋을까요?

saveAll 같이 HighlightJdbcRepository 에서 구현된 메서드는 직접 들어가야지만 존재하는지 알 수 있으니 불편하네요.

저는 이게 크게 불편하게 느껴지지 않았어요. 이대로도 괜찮다 생각해요!

보는건 그냥 찾아보면 될텐데 아마 서비스단에서 직접 레포를 사용해야 할 때 불편할 것 같아요. 이 부분은 다음에 고민하도록 해볼게요

질문)

계층을 두는 게 좋을까요?

계층을 둔다는 게 어떤 의미일까요?

제안)
HighlightRepository가 Repository, HighlightJdbcRepository를 상속받고 있는 상황에서

Repository를 통해 정의한 메서드도 명시가 되어있고,
커스텀 쿼리 메서드도 명시가 되어있는데,
HighlightJdbcRepository를 통해 정의한 메서드는 명시가 되어있지 않는 것이 헷갈리게 느껴져요.

모두 이 곳에 명시를 해서 볼 수 있게하는 것이 목적이라면 default 메서드를 활용하는 방법은 어떨까요?

public interface HighlightRepository extends Repository<Highlight, Long>, HighlightJdbcRepository { default void saveAll(Collection<Highlight> highlights) { batchSaveAll(highlights); } }

해당 Repo가 배치를 하는지 여부를 Repository가 알아야 하나요 ? 🤔 인터페이스에서는 배치와 관련된 걸 최대한 숨기고 싶었는데요..

흠 왜 불편함을 느꼈나 다시 생각해봤어요..!

제가 느꼈던 건
HighlightJdbcRepository의 saveAll을 파악하는데 어려움이 있다
→ repository가 의존하는 계층이 너무 깊어서 파악하기 어렵지 않나 때문이었다는 걸 다시 깨달았습니다😂

repository 계층이 하나로 모두 깊지않게 관리하려면,
서비스가 Highlight(Jpa)Repository와 HighlightJdbcRepository를 모두 의존하고, batch Insert 할때는 HighlightJdbcRepository.saveAll을 호출하면 되긴 하겠죠. 하지만 그렇게 하면 인지 부하의 문제가 또 있겠네요(batch insert할때는 HighlightJdbcRepository 쓰는 거 기억해야함!!)

그래서 타협안으로 HighlightJdbcRepository의 batchSaveAll 메서드를 호출하는 걸 갖고있으면 조금 덜 깊은 곳에서(?) 호출하는 느낌이 들어서 제안해보았습니다😝😝😝

아루: 지금 충분히 변경에 열려 있다고 생각해서 그대로 두는 건 어떤가요

변경에 열려있다는 점에는 동의해요! 다만, 호출하는 repository의 계층이 깊어서 항상 저 부분을 파악할 때 직관적이지 않다는 느낌을 받을 것 같다는 생각에 계속 걸렸어요. 하지만 이 역시 예상이니 현재 방식대로 적용해보고 리팩토링 하는 것 좋습니다!

nayonsoso

JdbcTemplate의 batchUpdate를 사용했습니다. 자세한 사항은 이슈를 확인해주세요.

확인했습니다😊
덕분에 saveAll 의 성능 문제에 대해서 알게 되었네요.
사실 saveAll 을 하더라도 “영속성 컨텍스트의 쓰기 지연” 때문에
한번에 DB 로 전달될 것이므로 오버헤드가 크지 않을것이라 생각했어요.
그런데 한번에 DB 에 전달하더라도 DB 에서는 순차적으로 실행될테니 bulk insert 보다는 성능이 안좋겠네요😓

Impl은 Spring에서 이렇게 적어야 한다고 해서.. 안 적으면 제대로 잡아오지를 못하더라고요.

와우 바꿔서 돌려봤는데 진짜 못잡아오네요🙄

추가로, 지금의 코드에서는 벌크 insert하는 sql이 로그에 안남는데, 이건 괜찮으려나요?
application.yml 아 아래 두라인 추가하니 로그에 남긴 합니다!

logging:
  level:
    springframework: DEBUG
    // 아래 두 라인
    org.springframework.jdbc.core.JdbcTemplate: DEBUG
    org.springframework.jdbc.core.StatementCreatorUtils: TRACE

backend/src/main/java/reviewme/highlight/repository/HighlightJdbcRepositoryImpl.java

nayonsoso · 2024-11-14T16:34:25Z

backend/src/main/java/reviewme/highlight/repository/HighlightRepository.java

 import reviewme.highlight.domain.Highlight;

-public interface HighlightRepository extends JpaRepository<Highlight, Long> {
+public interface HighlightRepository extends Repository<Highlight, Long>, HighlightJdbcRepository {


JpaRepository 상속에서 Repository 상속으로 바꾼 것은, JpaRepository에서 기본으로 제공하는 saveAll 와 HighlightJdbcRepository 에서 선언한 saveAll 것 사이에 충돌을 피하기 위해서 맞나요?

사실 다른 XXXRepository 들이 다 JpaRepository를 상속하고 있어서, 통일감이 없다는 느낌을 받았어요🥲 그래서 HighlightRepository 가 JpaRepository를 상속하면서 HighlightJdbcRepository의 saveAll 을 사용할 수 있는 방법이 있을까 알아보니 메서드 이름을 다르게 하는 방법 밖에 없네요 (e.g. saveInBatch)

그런데 이렇게 한다면 ‘데이터베이스에 저장되는 방식에 따라서 서비스에서 다른 함수를 호출하게’ 하는 것이므로 좋지 않은 것 같아요😱 서비스는 그런걸 알 필요가 없으니깐요..

결론 : 아루의 지금 방법이 좋겠다고 납득하게 되었습니다👍 이거 고민하느라 코드리뷰 오래걸림 ㅎㅎ

JpaRepository 상속에서 Repository 상속으로 바꾼 것은, JpaRepository에서 기본으로 제공하는 saveAll 와 HighlightJdbcRepository 에서 선언한 saveAll 것 사이에 충돌을 피하기 위해서 맞나요?

네, 그리고 테스트에서 findAll와 같은 메서드가 사용되는 걸 보고 (테스트에서만 사용되는 메서드를 정의한 것과 같음) 경계하게 되어 아예 내렸습니다. 충돌을 피하기 위해서 진행하긴 했지만, 후자의 이유가 조금 더 강합니다.

nayonsoso

NamedParameterJdbcTemplate 확인 완!
수고했습니다~~ 🙌

skylar1220

HighlightJdbcRepository 에서 구현된 메서드는 직접 들어가야지만 존재하는지 알 수 있으니 불편하네요

테드의 이 부분에 대해 질문 + 제안 딱 하나 있습니다!!
여담으로 아루의 이슈를 보고 벌크 삽입의 여러 경우를 알게되어서 너무 좋았어요🙌

skylar1220

벌크 인서트의 여러 방식을 비교해준 덕분에 저도 공부할 수 있었어요👍👍
하이라이트의 병목 원인 중 하나가 해결됐네요!!
호출하는 repository의 계층이 깊은 문제는 지켜보도록 해요😀

donghoony · 2024-11-17T07:00:24Z

Secret의 dev, prod-write db에 rewriteBatchedStatements=true를 추가했습니다.

feat: 하이라이트 벌크 삽입 추가

9bda0e4

donghoony requested review from Kimprodp, nayonsoso and skylar1220 as code owners November 13, 2024 12:15

donghoony self-assigned this Nov 13, 2024

donghoony added ✨ BE 🚀 기능 labels Nov 13, 2024

donghoony added this to the 6차 스프린트: 최종장 milestone Nov 13, 2024

donghoony added 3 commits November 13, 2024 21:17

refactor: 불필요한 @Repository 제거

1fc499e

fix: findAll 사용하지 않도록 수정

7279351

fix: 테스트 수정

717112e

Kimprodp requested changes Nov 14, 2024

View reviewed changes

nayonsoso approved these changes Nov 14, 2024

View reviewed changes

refactor: NamedParameterJdbcTemplate 사용하도록 수정

36d558c

nayonsoso approved these changes Nov 14, 2024

View reviewed changes

Kimprodp approved these changes Nov 16, 2024

View reviewed changes

skylar1220 requested changes Nov 16, 2024

View reviewed changes

skylar1220 approved these changes Nov 17, 2024

View reviewed changes

donghoony merged commit 8858e02 into develop Nov 17, 2024
15 checks passed

donghoony deleted the be/refactor/bulk-insert branch November 17, 2024 07:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] feat: 하이라이트 벌크 삽입 추가 #961

[BE] feat: 하이라이트 벌크 삽입 추가 #961

donghoony commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

Kimprodp left a comment

Kimprodp Nov 14, 2024

nayonsoso Nov 14, 2024

donghoony Nov 14, 2024

Kimprodp Nov 16, 2024

Kimprodp Nov 14, 2024

donghoony Nov 14, 2024

nayonsoso Nov 15, 2024

Kimprodp Nov 16, 2024

skylar1220 Nov 16, 2024 •

edited

Loading

donghoony Nov 16, 2024 •

edited

Loading

skylar1220 Nov 16, 2024 •

edited

Loading

skylar1220 Nov 17, 2024

nayonsoso left a comment

nayonsoso Nov 14, 2024 •

edited

Loading

donghoony Nov 14, 2024

nayonsoso left a comment

skylar1220 left a comment •

edited

Loading

skylar1220 left a comment

donghoony commented Nov 17, 2024


		Highlight save(Highlight highlight);

		boolean existsById(long id);

[BE] feat: 하이라이트 벌크 삽입 추가 #961

[BE] feat: 하이라이트 벌크 삽입 추가 #961

Conversation

donghoony commented Nov 13, 2024 • edited Loading

🚀 어떤 기능을 구현했나요 ?

🔥 어떻게 해결했나요 ?

📝 어떤 부분에 집중해서 리뷰해야 할까요?

📚 참고 자료, 할 말

github-actions bot commented Nov 13, 2024 • edited Loading

Test Results

Kimprodp 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

skylar1220 Nov 16, 2024 • edited Loading

Choose a reason for hiding this comment

donghoony Nov 16, 2024 • edited Loading

Choose a reason for hiding this comment

skylar1220 Nov 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nayonsoso left a comment

Choose a reason for hiding this comment

nayonsoso Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nayonsoso left a comment

Choose a reason for hiding this comment

skylar1220 left a comment • edited Loading

Choose a reason for hiding this comment

skylar1220 left a comment

Choose a reason for hiding this comment

donghoony commented Nov 17, 2024

donghoony commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

skylar1220 Nov 16, 2024 •

edited

Loading

donghoony Nov 16, 2024 •

edited

Loading

skylar1220 Nov 16, 2024 •

edited

Loading

nayonsoso Nov 14, 2024 •

edited

Loading

skylar1220 left a comment •

edited

Loading