서버가 클라이언트에게 Ping/Pong 요청 #776
Conversation
1분에서 10분으로 수정
ping status -> pingStatus
| @Component | ||
| @RequiredArgsConstructor | ||
| public class ChatWebSocketHandleTextMessageProvider implements WebSocketHandleTextMessageProvider { | ||
|
|
||
| private static final String PING_STATUS_KEY = "pingStatus"; |
There was a problem hiding this comment.
필수
static 필드 밑에 한 줄 개행해주세요!
| sessions.remove(session); | ||
| } | ||
|
|
||
| @Scheduled(fixedDelay = 60000) | ||
| public void sendPingSessions() { |
| return sessions.getChatRoomSessions() | ||
| .values() | ||
| .stream() | ||
| .flatMap(webSocketSessions -> webSocketSessions.getSessions().stream()) |
|
|
||
| private void sendPingMessage(final WebSocketSession session) { | ||
| final Map<String, Object> attributes = session.getAttributes(); | ||
| final boolean pingStatus = (boolean) attributes.get(PING_STATUS_KEY); |
There was a problem hiding this comment.
선택
pingStatus 대신 connected는 어떤가요?
!pingStatus라고 하니 의미르 한 번 더 고민하게 되어서요!
There was a problem hiding this comment.
connected 좋습니다!
pingStatus가 애매하다고 생각했는데 적절한 네이밍을 찾지 못했습니다..
그런데 메리가 제안해 준 connected가 적절한 것 같네요!
| try { | ||
| session.sendMessage(new PingMessage()); | ||
| } catch (IOException e) { | ||
| log.error("ping 보내기 실패 : {} ", session); |
There was a problem hiding this comment.
질문
ping 전송에 실패하면 로그만 찍고 다시 ping을 전송하지 않아도 괜찮을까요?
저는 ping 전송에 실패한 경우 정해둔 시간 동안 다시 ping 전송 요청을 해보는 것을 생각했습니다.
There was a problem hiding this comment.
저는 ping이 실패한 경우 다시 ping 요청을 수행하더라도 실패할 것이라 생각했습니다.
네트워크, 세션 종료, 메시지 등에 대한 문제로 일어날 것으로 예상됩니다.
그래서 다시 시도해도 동일한 상황 및 메시지로는 계속 실패할 것이라는 생각이 드는 데 어떻게 생각하실까요?
There was a problem hiding this comment.
실패한 로직을 재전송하는 부분이 필요할 것 같다는 생각이 들었어요!
여기 뿐만 아니라 알림 전송도 비슷한 기능이 있어야 한다고 생각합니다.
이건 나중에 논의해보고 이슈 새로 파서 진행하면 좋을 것 같아요~!
There was a problem hiding this comment.
다음 회의에서 이에 대해 한번 논의해 보기로 해요!
그 전까지 sendMessage()가 실패하는 원인에 대해 좀 더 알아보도록 하겠습니다!
그래서 일단 이번 pr에서는 해당 부분은 적용하지 않고 넘어가도록 하겠습니다.
| @@ -29,6 +29,7 @@ public boolean beforeHandshake( | |||
| ) throws Exception { | |||
| attributes.put("userId", findUserId(request)); | |||
| attributes.put("baseUrl", ImageRelativeUrl.USER.calculateAbsoluteUrl()); | |||
| attributes.put("pingStatus", true); | |||
There was a problem hiding this comment.
선택
리마인드 차원에서 코멘트 남깁니다
만약 위의 pingStatus 네이밍을 변경하게 된다면 여기도 함께 변경해주세요!
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class WebSocketHandler extends TextWebSocketHandler { | ||
|
|
||
| private static final String TYPE_KEY = "type"; | ||
| private static final String PING_STATUS_KEY = "pingStatus"; |
There was a problem hiding this comment.
선택 & 질문
여기도 함께 변경해주세요!
그런데 동일한 PingStatusKey가 여러군데에서 반복적으로 사용되고 있네요!
WebSocketAttributeKey와 같은 enum을 생성하고 따로 관리해주는건 어떤가요??
There was a problem hiding this comment.
혹시 WebSocketAttributeKey가 어떤 클래스를 이야기하는 걸까요?
아니면 다른 attribute를 포함해 enum을 만들면 좋겠다는 이야기가 맞을까요?
맞다면 제가 생각한 방식으로 만들어보았는데 확인해 주시면 감사하겠습니다!
추가적으로 WebSocketAttributeKey 클래스의 위치가 어디가 적절한지 고민이네요...
| @@ -43,4 +46,10 @@ public void afterConnectionClosed(final WebSocketSession session, final CloseSta | |||
| final WebSocketHandleTextMessageProvider provider = providerComposite.findProvider(textMessageType); | |||
| provider.remove(session); | |||
| } | |||
|
|
|||
| @Override | |||
| public void handlePongMessage(WebSocketSession session, PongMessage message) { | |||
There was a problem hiding this comment.
필수
제 눈에는 왜 Final이 이렇게 잘 보이는 걸까요..?
| @Override | ||
| public void handlePongMessage(WebSocketSession session, PongMessage message) { | ||
| final Map<String, Object> attributes = session.getAttributes(); | ||
| attributes.put(PING_STATUS_KEY, true); |
There was a problem hiding this comment.
선택
PingStatus를 Enum으로 관리하고, value로 true/false를 관리하는 것은 어떤가요?
지난번에 작업하다보니 원시값만 사용하다보면 어떤 의미로 사용했었는지 헷갈리는 경우가 많더라구요!
There was a problem hiding this comment.
PingStatus가 위 리뷰에 따라 conected로 변경되었습니다.
그리고 connected 즉 연결이 true/false 인 것은 크게 헷갈리지 않다고 생각하는 데 어떻게 생각하시나요?
어차피 enum으로 만들더라도 connected/disconnected 혹은 true/false일 것 같아 여쭤봅니다!
pingStatus -> connected
📄 작업 내용 요약
서버에서 10분에 한 번씩 클라이언트에게 Ping 요청을 수행하는 기능입니다.
만약 pong 메시지에 대해 10분마다 확인하는데, pong 메시지가 오지 않았다면 삭제하고 pong 메시지가 오면 다시 ping을 요청합니다.
🙋🏻 리뷰 시 주의 깊게 확인해야 하는 코드
크게 어려운 부분은 없다고 생각합니다.
ping 주기는 지난번 회의 때 얘기한 대로 10분으로 설정했습니다.
📎 Issue 번호