Skip to content
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

[머지 금지!] 리뷰용입니다 #455

Open
wants to merge 7 commits into
base: backend
Choose a base branch
from

Conversation

0chil
Copy link
Collaborator

@0chil 0chil commented Feb 14, 2024

관련 이슈번호


작업 사항


기타 사항


Copy link
Collaborator

@Songusika Songusika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0chil
고생하셨습니다!
이벤트 발송 테스트 코드에 주석으로 처리된 부분이 있네요
이부분 다시 작업해서 어떤 branch로 push 하면 될까요??

username: ${DEV_DATASOURCE_USERNAME}
password: ${DEV_DATASOURCE_PASSWORD}
driver-class-name: com.mysql.cj.jdbc.Driver
jpa:
database: mysql
generate-ddl: false
hibernate:
ddl-auto: validate
ddl-auto: none
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thdwoqor
validate 에서 none 으로 바뀐 이유가 궁금해요!

Member member = memberRepository.save(파워());
Room room = roomRepository.save(차이());

roomService.dislike(member.getId(), room.getId());

assertThat(member.getDislikeRooms()).isNotEmpty();
// verify(publisher).publishEvent(any(DefaultMemberGenreEvent.class));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분과 아래 주석까지해서 테스트 코드를 다시 작성해서 어떤 branch에 PR을 작성하면 될까요??

Copy link
Collaborator Author

@0chil 0chil Mar 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위 브랜치(refactor/add-aggregate-2)에 해주시면 됩니다! 기다리고 있겠습니당!

Copy link
Collaborator

@kong-hana01 kong-hana01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요. UUID 질문만 남겼습니다. 다른 질문은 다른 pr에 남겼어요!

Comment on lines +21 to +23
@Id
@GeneratedValue(strategy = GenerationType.UUID)
private UUID id;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id를 UUID로 하는 이유가 있을까요?

UUID를 기준으로 데이터를 저장하면 클러스터링 인덱스로 인한 장점을 모두 잃어버리는 것 같은데 이 부분을 변경하신 이유가 있으신지 궁금합니다. ㅎㅎ
제가 생각한 단점들은 다음과 같아요.

  • id 기반 범위 탐색을 못한다.
  • 다른 인덱스를 추가해서 조회를 하더라도 항상 랜덤 io로만 데이터를 조회한다.
    • 클러스터링 인덱스를 활용하면 순차 io가 발생해서 조회 성능이 훨씬 개선 됩니다.
  • 기존 레포지토리에서 id 범위탐색을 했던 것을 모두 변경해야한다.
    • 예를 들어 댓글 레포지토리에서는 id를 무한 스크롤에 사용하는데 이를 모두 변경해야합니다. createdAt으로 조회해도 되지만 세컨더리 인덱스를 추가해야하고, 성능 상 이점도 잃어버리게 됩니다.
  • pk의 크기가 커진다. 세컨더리 인덱스는 리프 노드에 pk를 두는 것으로 알고있습니다. uuid가 128비트라고 하면 16바이트를 사용하는데 그러면 데이터가 많아질수록 부담이 클 것 같아요.

cf) 처음 UUID를 도입한 목적이 무엇이신가요? - 땡칠 질문

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 비슷한 다른 작업(페이징)을 하다가 이 질문에 대해 생각나서 결정에 도움이 될까 답을 달아봅니다~

  • id 기반 범위 탐색을 못한다.

기술적으로 사실입니다.
UUID를 도입하게 되어 지금의 정렬 기준을 createdAt + UUID로 대체하게 된다면?
createdAt 및 UUID로 세컨더리 인덱스를 만들고 유지하는 것은 sequential id에 비해 읽기 뿐 아니라 쓰기 성능도 후퇴하게 됩니다.
게다가 id 자체도 크기도 작고 연산도 단순한 숫자 id에 비해서 사전순으로 정렬하는데 더 많은 연산이 필요하므로
sequential id에 비해 몇 배 이상 느려질 가능성이 있습니다.

대신 UUID를 사용하면, 기존에 PK에 의미를 부여(생성 순서대로 정렬된다)하고 의존하던 것을 벗어날 수 있어서 확장성을 늘릴 수 있을 것이라는 생각이 들었습니다.
PK에 의미를 부여하고 의존하게 되면, PK가 관리(락)되는 범위(단일 DB 인스턴스)에서 벗어나기 어렵습니다.
이 경우 DB를 다중화한다면 별도의 동기화가 필요할 거라고 추측이 드네요.

하지만 UUID를 사용하면 DB 복제, 다중화가 용이해져서 인프라 차원에서 학습을 위한 다양한 경험을 할 수 있는 기반이 될 것이라고 생각합니다.

~를 블랙캣과 파워가 얘기하고 싶었던게 아닐까 라는 생각이 들었습니다 ㅋㅋㅋ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants