[현승원] sprint12#125
Hidden character warning
Conversation
Highjune
left a comment
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로 어떻게 개선할 수 있는지 한번 고민해봐주세요~!
| @@ -0,0 +1,7 @@ | |||
| # IP Hash — 클라이언트 IP 기준으로 동일 백엔드에 고정 (세션/WS에 유리) | |||
| 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
| if (lastEventId != null) { | ||
| log.debug("SSE 연결, receiverId={}, lastEventId={}", receiverId, lastEventId); | ||
| } | ||
| SseEmitter emitter = new SseEmitter(EMITTER_TIMEOUT_MS); |
There was a problem hiding this comment.
타임아웃이 0으로, 즉 없는 것이 맞을까요? 클라이언트가 브라우저를 닫지 않고 방치하면 Emitter가 계~속 메모리에 남을 수 있어요
| @Repository | ||
| public class SseMessageRepository { | ||
|
|
||
| private static final int MAX_MESSAGES = 2000; |
There was a problem hiding this comment.
max message 설정 잘하셨습니다. oom 방지를 할 수 있거든요!
| private static final String USER_JWT_KEY_PREFIX = "jwt:user:"; | ||
| private static final String ACCESS_TOKEN_INDEX_KEY = "jwt:access_tokens"; | ||
| private static final String REFRESH_TOKEN_INDEX_KEY = "jwt:refresh_tokens"; | ||
| private static final Duration DEFAULT_TTL = Duration.ofMinutes(30); |
There was a problem hiding this comment.
모든 유저의 토큰이 하나의 set 인데 30분 ttl -> 30분간 로그인 없으면 전체 인증이 깨지는 것 아닌가요?
| return; | ||
| } | ||
|
|
||
| for (String userKey : Optional.ofNullable(redisTemplate.keys(USER_JWT_KEY_PREFIX + "*")).orElse(Set.of())) { |
There was a problem hiding this comment.
KEYS * 사용하게 되면 redis 를 블로킹하거든요. 만약 유저수가 많아서 조회가 많아지게 되면, 즉 유저 수가 많으면 성능 문제가 있을 수도 있어요. SCAN 명령을 사용하거나, refreshToken → userId 역인덱스 추가를 추천드려요!
| @Bean | ||
| public ConsumerFactory<String, String> realtimePushConsumerFactory( | ||
| KafkaProperties kafkaProperties, | ||
| @Value("${discodeit.kafka.realtime-push-group-id}") String groupId |
There was a problem hiding this comment.
카프카에서 groupId 가 동일하거나 같으면 어떤 특징들이 각각 있는건가요?
설정 파일에서 groupId를 주입받는데, 모든 인스턴스가 같은 설정 파일을 쓰면 같은 groupId → 1개 인스턴스만 수신 → 분산 환경에서 실패하게 됩니다.
위에 주석에 추가하신 것처럼, '푸시는 인스턴스마다 수신해야 하므로' -> docker-compose 에서 인스턴스별로 다른 값을 주입받거나, uuid 기반 자동 생성이 필요해요.
uuid 는 예시입니다.
| event.messageId()); | ||
| } catch (JsonProcessingException e) { | ||
| log.error("웹소켓 Kafka 페이로드 직렬화 실패, messageId={}", event.messageId(), e); | ||
| messagingTemplate.convertAndSend(destination, dto); |
There was a problem hiding this comment.
kafka 발행이 실패했을 때, 로컬 전송(호출) 하잖아요?
그러면 현재 인스턴스에 연결된 클라이언트만 받고 다른 인스턴스들의 클라이어늩들은 못 받게 되는데요. 에러 처리가 부분 성공 되어서 디버깅도 어려워질 수 잇어요.
부분 성공보다는 아예 깨끗한 실패가 나을 수도 있어요. 로컬 전송 말고 그냥 에러 로그만 남기는 것이 더 좋아보입니다.
메시지는 이미 디비에 저장되어 있어서 유저에게 다시 보여줄 수 있잖아요.
There was a problem hiding this comment.
추가로, JsonProcessingException은 사실 개발 버그이잖아요. 네트워크 문제도 아니고, 코드에 버그가 있는 건데요. 재시도해도 계속 실패하는 것이라서, 진짜 kafka 장애(네트워크 끊긴다거나) 는 다른 예외가 나요.
KafkaException, TimeoutException 등등 이건 retry가 의미가 잇어요
2 participants
요구사항
기본
심화
멘토에게