[이예은] sprint12#127
Hidden character warning
Conversation
Highjune
left a comment
There was a problem hiding this comment.
예은님 아주 수준높게 구현 잘하셧습니다~!
코멘트 남긴 것만 조금 확인부탁드려요~!
마지막 구현 어려우면서도 중요해서 복기 한번 해보셔요. 그리고 가벼운 아키텍쳐 한번 그려보세요~!
질문 몇 가지만 남겨놓을게요~!
카프카가 현재 2가지 역할 하잖아요?
역할 A: 알림 저장 등 1번만 처리 (고정 groupId)
역할 B: SSE 이벤트를 모든 인스턴스에 브로드캐스트 (고유 groupId)
Q1) 위에서 redis pub/sub 으로 대체할 수 있는 영역이랑 없는 영역은? 바꿀 수 있다면 장/단점은?
현재 SseMessageRepository는 JVM 메모리(ConcurrentHashMap)에 이벤트를 저장하고 있잖아요.
Q2) 분산 환경(인스턴스 3개)에서 이 구현이 실패하는 시나리오를 설명하고, Redis로 어떻게 개선할 수 있는지 한번 고민해봐주세요~!
| @Override | ||
| public void configureClientInboundChannel(ChannelRegistration registration) { | ||
| registration.interceptors( | ||
| jwtAuthenticationChannelInterceptor, |
There was a problem hiding this comment.
오 아주 훌륭합니다.
jwt -> 시큐리티 설정 -> role 기반 인가까지 해주셨네요.
| .orElseThrow(() -> MessageNotFoundException.withId(messageId)); | ||
|
|
||
| String destination = "/sub/channels." + event.channelId() + ".messages"; | ||
| messagingTemplate.convertAndSend(destination, messageMapper.toDto(message)); |
There was a problem hiding this comment.
WebSocket은 분산 환경에서 로컬 전송(지금 클라이언트한테만) 되는 것 같은데요!?
@TransactionalEventListener는 현재 JVM에서만 동작하잖아요. WebSocketKafkaEvent record를 정의해뒀지만, 실제로 WebSocket 메시지를 Kafka를 통해 모든 인스턴스에 분배하는 consumer가 없는 것 같은데 맞을까요?
아래와 같이, 예를 들어
App-1에서 메시지 생성 → App-1의 WebSocket 클라이언트만 실시간 수신
그런데 App-2, App-3의 WebSocket 클라이언트는 못 받지 않나요. 이 부분이 문제인 것 같아요.
SSE는 Kafka로 분배했으니, WebSocket도 동일 패턴 적용 필요해 보여요. WebSocketKafkaEvent를 Kafka로 발행 → 모든 인스턴스에서 수신 → 그 후에 messagingTemplate.convertAndSend().
즉 중간에 카프카 메시지 발행한번 해주고, 다른 인스턴스들에도 알려주는 것이 필요해요
| import java.util.UUID; | ||
| import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; | ||
|
|
||
| public interface SseService { |
There was a problem hiding this comment.
오 인터페이스 설정 아주 잘하셨어요. 왜냐하면 나중에 redis pub/sub 으로 변경할 때 그대로 갈아끼우기만 하면 되고 나중에 테스트 mock 만들기도 좋으니까요
| UUID eventId = sseMessageRepository.save(eventName, data); | ||
|
|
||
| SseKafkaEvent kafkaEvent = new SseKafkaEvent(eventId, receiverIds, eventName, data); | ||
| kafkaTemplate.send(SSE_TOPIC, kafkaEvent); |
There was a problem hiding this comment.
분산 서버 고려해서 잘하셨습니다.
카프카 SSE_TOPIC 에 보내기만 하고 끝이라서 직접 emitter 에 보내지 않고, 수신하고 있는 모든 인스턴스들에서 자기 인스턴스의 emitter 에서 해당 유저 찾아서 전송하니까 메세지 이벤트 받고 나서 그 서버만 처리하는 형태가 아닌거죠. 잘하셧어요.
| public void releaseLock(String key) { | ||
| String lockKey = LOCK_KEY_PREFIX + key; | ||
| try { | ||
| redisTemplate.delete(lockKey); |
There was a problem hiding this comment.
현재 누구거든 그냥 삭제하고 있잖아요?
보통 분산 락 구현시에는 값이 내 것일때만 삭제하게 하는 로직이 있거든요.(app1, 2, 3 구분 필요)
lock:유저A 일 때 무조건 샂게하는데, 값이 app1 이든 app2 이든 상관없이 삭제해요.
케이스로 보면,
App2 가 정당하게 가지고 있는 락을 app1 이 지워버리거든요.
app2 는 락을 가지고 있다고 생각하지만 실제로는 없고
app3 이 락을 새로 획득 가능해요
app2 와 app3 이 동시에 같은 유저의 jwt 를 조작하게 됩니다.
즉 maxActiveJwtCount 제한이 깨져버려요
'내 것만 삭제' 하게 로직 넣는 것이 좋을 것 같아요.
그리고 이미 락 만들때 고유한 키 값 만들고 계시네요.
그것으로 relsaeLock 떄도 사용하시면 됩니다. lockvalue
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게