[Chat] 채팅방에 userId로 초대하기 API 구현 #163
Conversation
minisundev
changed the title
yudonggeun
left a comment
yudonggeun
left a comment
There was a problem hiding this comment.
락 획득에 실패한 경우에는 요청이 실패로 끝나게 되는데, 사용자가 성공할 때까지 계속해서 api를 호출을 해야하는 구현으로 보여서 이 부분은 개선의 여지가 필요하다고 생각이 들어요.
분산락을 mongodb를 통해서 관리하도록 구현을 하셨는데요. 확장성이 부족해서 mongo를 사용했다는 주장이 설득이 안되네요. 왜냐하면 redis 또한 확장성이 좋은 nosql 기반 데이터베이스 중에 하나이기 때문이에요.
오히려 redis는 in memory 데이터베이스이기 때문에 영속성이 필요하지 않은 분산락에 더 어울리고 캐시로 많이 활용되는 만큼 성능면에서 큰 차이가 있다고 생각이 들거든요.
그리고 redis는 분산락을 지원하는 클라이언트가 이미 구현되어있어서 손쉽게 구현이 가능한 반면에 mongo는 관련된 구현체를 아직 발견하지 못했어요.
자료의 차이로 봐서는 redis가 좀 더 분산락을 구현하는 것에서는 장점이 클 것 같은 신호로 생각이 되네요.
테스트 코드를 조금씩 작성해주시는 부분은 너무 좋은 것 같아요.
고생하셨습니다!
| var id: String? = null | ||
|
|
||
| var members: MutableList<String> = mutableListOf() | ||
| var members: MutableSet<String> = mutableSetOf() |
There was a problem hiding this comment.
id와 member를 public이면서 var이면 위험해보여요
| val lock = lockService.getLock(chatRoomId) | ||
|
|
||
| inviteToChatRoomByUserId(userId, inviterId, chatRoomId) | ||
|
|
||
| lockService.releaseLock(lock.lockId, lock.owner) |
There was a problem hiding this comment.
inviteToChatRoomByUserId에서 예외가 발생하면 락을 회수하지 못하는 상황이 발생할 것 같아요.
| val lockId = "chatRoom:$chatRoomId:lock" | ||
| val owner = UUID.randomUUID().toString() | ||
|
|
||
| val lockAcquired = acquireLock(lockId, owner, 10000) // 10초 동안 잠금 유지 |
There was a problem hiding this comment.
트랜잭션의 평균 실행시간이 10초 보다는 짧을 것 같아서 1초로 시작해서 튜닝하는 것이 좋아보여요. 설정 값은 하드 코딩하기 보다는 설정을 따로 빼는 것이 좋아보여요.
| } | ||
|
|
||
| fun getLock(chatRoomId: String): Lock { | ||
| val lockId = "chatRoom:$chatRoomId:lock" |
There was a problem hiding this comment.
lock service인데 chatRoom의 도메인이 쓰였기 때문에 결합도가 높아지는 원인으로 보이네요.
| CHATROOM_NOT_FOUND(HttpStatus.NOT_FOUND.value(), "해당 id로 chatroom을 찾을 수 없습니다"), | ||
|
|
||
| // 500 | ||
| CONCURRENCY_CONFLICTION(HttpStatus.CONFLICT.value(), "동시성 충돌이 발생했습니다."), |
There was a problem hiding this comment.
외부에 동시성 충돌을 알려주는 것은 구현을 알려주는 것이라서 지양하는 것이 좋다고 생각합니다.
2 participants
#️⃣연관된 이슈
#84
📝작업 내용
userId로 채팅방에 초대하는 기능을 구현했습니다.
💬리뷰 요구사항(선택)
분산락을 쓰기 위해서 레디스를 쓰게되면 추후에 확장성이 떨어질거 같아서 mongoDB로 분산락을 만들어보았습니다
코드리뷰 부탁드립니다 😉✨👍💡