-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor/#634: Bookmark 동시성 이슈 해결 및 이벤트 발행 #640
base: refactor/dependency
Are you sure you want to change the base?
Conversation
package com.mapbefine.mapbefine.topic.domain; | ||
|
||
public record BookmarkAdditionEvent(Long topicId) { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기술 스택으로 Java17을 선택한 만큼, 레코드를 사용했는데요.
이를 도메인 패키지에 두었는데 너무 DTO 같아 보이려나요 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아니요!
보니까 PinUpdateEvent 도 Record 네용 ㅎㅎ
오히려 일관성을 위해서 더욱이 Record 로 두어야 할 것 같습니다.
@Modifying(clearAutomatically = true, flushAutomatically = true) | ||
@Query("update Topic t set t.bookmarkCount = t.bookmarkCount + 1 where t.id = :topicId") | ||
void increaseBookmarkCountById(@Param("topicId") Long topicId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
왜 이 녀석만 flushAutomatically 옵션을 두었는지 궁금해하실 것 같아요 ㅎㅎ
이거 진짜 팀 블로그에 쓸건데... 최대한 간략하게 설명드리자면 다음과 같아요.
이전에 discussion에서, JPQL로 작성된 메서드를 호출하면 영속성 컨텍스트가 flush된다고 말씀드렸는데요.
이때 flush되는 대상들이 해당 JPQL을 수행하는 객체와 관련된 내용만 flush가 된다고 합니다.
그래서, Bookmark 저장 -> 이벤트 발행 -> Topic JPQL 수행
의 과정으로 로직이 수행된다면..
clearAutomatically
옵션에 의해서 Bookmark 저장과 관련하여 쓰기 지연 저장소에 대기중인 쿼리가 삭제되버리게 되는 것이죠..
그래서 위 옵션을 통해 영속성 컨텍스트에 있는 모든 것(?)들을 flush 해주는 옵션이 생겨나게 되었답니다 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Topic JPQL 이 수행될 때, 자기 것만 flush 하고, 또한 clearAutomatically 옵션으로 인해서, 영속성 컨텍스트가 비워져 Bookmark 저장 쿼리가 사라진다는 의미인가요?? 제가 이해한 게 맞나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Topic -> Bookmark 로 가는 의존성이 많이 끊어져서 굉장히 좋은 것 같아요!
그리고 listener 패키지로 따로 구분해 놓은 것도 굉장히 좋은 것 같습니다.
다만, 개인적인 의견으로는 BookmarkAdditionEvent
, BookmarkDeleteEvent
를 Listener
와 같은 패키지에 두어, 같이 사용되고 있음을 명확히 나타내고, 다른 패키지와의 의존성도 생기지 않는 것이 좋아보여요!
라고 할려고 했는데, 이 논리면 Service 와 Domain 객체들도 모두 같은 패키지에 놓아야 하네요.
좋은 것 같아요! 굳굳
아 마지막으로 현재 PinUpdateEvent 는 pin -> event 패키지에 있는데, 이도 Domain 으로 옮겨야 하는 것일까요?
아니면 현재 BookmarkAdditionEvent
, BookmarkDeleteEvent
를 Event 라는 패키지를 만들어 옮겨야 할까요?
@cpot5620 답변 기다리는데 언제 달아주시죠? 🙏 🙏 |
이거 진짜 언제하지.. |
👀 |
가자! |
작업 대상
#634
#639
📄 작업 내용
JPQL을 통한 Bookmark 동시성 이슈 해결
이벤트 발행을 통한 의존성 분리(?)
🙋🏻 주의 사항
코드 관련해서는 크게 어려운 부분은 없는 것 같은데요.
코드 외적(?)으로 이야기 해보아야할 사항이 있는 것 같아요.
1. 이벤트 관련 객체들의 패키지 위치
현재 Bookmark 추가, 삭제에 대한 이벤트 객체는
topic/domain
패키지에 두었습니다.조금 어색해보일 수 있는데, 이는 네이밍 때문에 발생하는 문제라고 생각해요 !
또한, 지금
topic
하위 패키지에listener
패키지를 두었는데요.사실,
TopicCommandService
에 추가하려다가 이후에 더 많은 이벤트 발행이 발생하게 되면 해당 서비스 클래스가 너무 커질 것 같아서 별도로 분리했습니다.위와 같은 패키지 구조에 대해서 어떻게 생각하십니까 !?
스크린샷
📎 관련 이슈
레퍼런스