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

🔀 :: erd에 맞춰 Entity 생성 #4

Merged
merged 20 commits into from
Sep 30, 2024

Conversation

Umjiseung
Copy link
Collaborator

@Umjiseung Umjiseung commented Sep 27, 2024

💡 배경 및 개요

erd에 설계한대로 엔티티를 추가하였습니다

Resolves: #3

📃 작업내용

엔티티 추가

  • Applicant
  • Attendance
  • Club
  • ClubMember
  • ClubNotice
  • Notice
  • User, UserMajor, UserRole

Enum 추가

  • Period
  • Status
  • Type
  • Authority
  • Major

🙋‍♂️ 리뷰노트

erd에 처음에 원래 각각의 id가 UUID 또는 Long 타입이였지만 중간에 수정을 진행했습니다.
ClubNotice, Notice는 Long 타입으로 설계하였고 나머지는 ULID를 택하여 UUID의 단점인 정렬이 불가능한 것을 보완하기 위해 변경했습니다. 그 점 유의하여 코드 리뷰 부탁드립니다

✅ PR 체크리스트

템플릿 체크리스트 말고도 추가적으로 필요한 체크리스트는 추가해주세요!

  • 이 작업으로 인해 변경이 필요한 문서가 변경되었나요? (e.g. .env, 노션, README)
  • 이 작업을 하고나서 공유해야할 팀원들에게 공유되었나요? (e.g. "API 개발 완료됐어요", "환경값 추가되었어요")
  • 작업한 코드가 정상적으로 동작하나요?
  • Merge 대상 브랜치가 올바른가요?
  • PR과 관련 없는 작업이 있지는 않나요?

🎸 기타

@Umjiseung Umjiseung added the ⚙ Setting 환경 세팅 label Sep 27, 2024
@Umjiseung Umjiseung self-assigned this Sep 27, 2024
@Umjiseung Umjiseung linked an issue Sep 27, 2024 that may be closed by this pull request
Comment on lines 30 to 40
private static String toULIDString(byte[] bytes) {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < bytes.length; i++) {
if (i == 6 || i == 8 || i == 10 || i == 12) {
sb.append('-');
}
int value = bytes[i] & 0xFF;
sb.append(Integer.toHexString(value >>> 4));
sb.append(Integer.toHexString(value & 0x0F));
}
return sb.toString();
Copy link

Choose a reason for hiding this comment

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

코드를 보니 필요할 때에만 Byte[] 형태의 ULID를 String 형태로 변환할 수 있네요
Pk의 자료형을 String으로 다루는것보다 byte로 다루는 것도 나쁘지 않아 보입니다
어떻게 생각하시나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

별 다른 문제가 없다면 좋아보이네요 조금의 성능향상도 기대해볼 수 있을 것 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

근데 갑자기 든 생각이지만 api에서 id 값을 사용한다면 불편한 점도 존재할 것 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

제 생각으로는 지금 그대로 유지해도 괜찮을 것 같아요

Copy link

Choose a reason for hiding this comment

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

지성님이 생각하신 비즈니스 로직을 작성할 때 불편할 점이 무엇인지 궁금하네요 알려주세요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저도 pk를 byte를 사용한적이 없어 추측한거지만 데이터베이스에 byte[]로 저장한다면 사용할 때마다 String으로 바꿔야하는 불편함이 있지 않을까라는 생각이 들어서 말했어요

Copy link

Choose a reason for hiding this comment

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

애플리케이션 관점에서 보신 것이군요 하지만 UUID도 클라이언트와 통신할 때 .toString() .fromString()을 많이 사용하기 때문에 별 차이는 없을 것이라 생각하고요.
지금 보시면 String 으로 선언한 ULID PK는 VARCHAR(255)로 선언이 되는데 이는 한 자리당 UTF-8 인코딩 시 3byte를 차지합니다(자릿수를 지정하는 공간을 포함하면 +n byte겠죠)
하지만 Byte[]로 선언을 하시게 된다면 BINARY자료형을 사용해 한 자리당 1byte만의 공간을 차지하게 됩니다.

원래는 ULID의 자릿수가 변하지 않기 때문에 VARCHAR 보다는 CHAR이 적절해 보이긴 합니다.

이는 MYSQL의 InnoDB 엔진에서의 데이터 조회(여러 데이터를 가져와야 하는 상황을 주력으로 보죠)에서도 성능 차이를 가져오게 되는데요 MySQL은 데이터를 조회(하게 되면 Disk I/O를 Page 단위로 수행하게 됩니다. Page의 크기는 정해져 있기 때문에 하나의 Page를 읽어온 후 데이터가 완전하지 않다면 다음 Page를 가져오는 식으로 진행됩니다. 여기서 BINARY 자료형인 PK는 16 byte VARCHAR 자료형인 PK는 48 byte + @ 가 되겠네요. 그렇기 때문에 Disk I/O가 BINARY에 비해 VARCHAR가 빈번하게 이루어지게 되겠네요

장기적인 관점에서 보게 된다면 VARCHAR 자료형이 가져다 주는 latency는 적지 않을 것이라고 판단되네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋은 정보 감사합니다 DB에 대한 지식이 부족해서 깊게 생각해보지 못한 부분인 것 같아요
알려주신 정보대로 서치해보니 성능부분이 확실히 차이가 날 것 같습니다 변경하는 게 좋을 것 같네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cacdb35
9ac98f5
toULIDString 메서드를 static을 빼고 public으로 바꾸었습니다 그리고 pk의 타입을 String에서 byte[]로 바꾸었습니다

return bytes;
}

public String toULIDString(byte[] bytes) {
Copy link

Choose a reason for hiding this comment

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

메서드 명도 UUID처럼 fromString 사용하는게 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

72154a6
수정했습니다

@Umjiseung Umjiseung merged commit 5311305 into main Sep 30, 2024
1 check passed
@Umjiseung Umjiseung deleted the setting/3-erd-according-entity-add branch September 30, 2024 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚙ Setting 환경 세팅
Projects
None yet
Development

Successfully merging this pull request may close these issues.

erd에 따른 엔티티 추가
3 participants