-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: 실시간 교양 여석 조회 기능 구현 #19
Conversation
Test Results33 tests 33 ✅ 5s ⏱️ Results for commit 40bc606. ♻️ This comment has been updated with latest results. |
@Getter | ||
@AllArgsConstructor | ||
public class Seat { | ||
|
||
private Subject subject; | ||
private int seatCount; | ||
private LocalDateTime queryTime; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 엔티티가 아닌가요??
당연히 엔티티가 될 것이라고 생각하고 있었어서, 엔티티로 설정하지 않은 이유가 궁금해요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
구현하던 당시에는 이게 엔티티인지 아닌지 판단하기 어려워서 이렇게 두었어요 😅
판단하기 어려웠던 지점은 Seat 정보는 인메모리에서 관리할 생각이었기 때문이에요. 사용자에게 실시간으로 여석 정보를 제공해야 하니까, 매번 DB에 조회하는 것이 아닌 인메모리에서 관리하는 것을 생각했어요. 그렇기에 엔티티여야 할까? 하는 의문이 들었어요. 이건 #12 와 관련이 있겠어요
@Component | ||
public class SeatStorage { | ||
|
||
private final List<Seat> seats; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 혹시 DB까지 가지 않고 빠르게 전달하기위해 엔티티로 등록하지 않은 걸까요?!?
그런데 저희 DB에 마지막 여석 정보 저장하기로 하지 않았었나요? 이후 좌석 여석 정보를 가져올때 터진다거나...하는 상황을 대비해서요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 PR 보고 잊고 있던 게 생각났네요!!
아 혹시 DB까지 가지 않고 빠르게 전달하기위해 엔티티로 등록하지 않은 걸까요?!?
맞아요!!
그런데 저희 DB에 마지막 여석 정보 저장하기로 하지 않았었나요? 이후 좌석 여석 정보를 가져올때 터진다거나...하는 상황을 대비해서요
이것도 맞아요!! 하지만 DB에 저장하는 역할은 크롤러 쪽에 있어요. 크롤러가 저장과 동시에 seat-finder에 전송해요. 전송 받은 seat-finder는 인메모리 저장소에 저장하구요. 그 데이터를 빠르게 사용자에게 제공해요. 만약 어떤 이유에서 크롤러와 통신되지 않는다면 가장 최근에 조회한 데이터라도 제공하기 위해 DB 조회를 시작해요. 이 흐름을 생각했어요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아~ 맞네요 여석 정보에 대한 저장은 크롤러가 하니까 크롤러가 저장한 시간 정보 저장+여석 정보 저장
을 이 아이는 받아와서 쓰는거지요
그럼 이 구조가 맞네요오
저도 놓치고 있던 흐름 다시 기억 살렸어요 감사합니당🐥🐥
} | ||
|
||
public void propagate(String eventName, Object data) { | ||
sseEmitterStorage.getEmitters().forEach(emitter -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getEmitters를 할때 remove한 emitter가 포함될 수 있으니까 (삭제+조회 동시성) 터지는거자나요?
그럼 주기적으로 remove를 해주는 방법은 어떨까요??...?? 주기적으로 안쓰는게 있으면 삭제해주고 emitters 재가동하기 이런느낌...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getEmitters를 할때 remove한 emitter가 포함될 수 있으니까 (삭제+조회 동시성) 터지는거자나요?
그럼 주기적으로 remove를 해주는 방법은 어떨까요??...?? 주기적으로 안쓰는게 있으면 삭제해주고 emitters 재가동하기 이런느낌...?
이 방식도 고려했어요!! 버퍼처럼 삭제할 과목 쌓아뒀다가 한번에 비우는 방식, 꽤 괜찮을 것 같아요.
지금까지 겪은 문제SseEmitters 동시성 문제SseEmitterStorage는 클라이언트와 연결된 SSE 커넥션들을 관리하는 인메모리 자료구조입니다. 내부적으로 어떤 자료구조를 사용하는지 결정해야 합니다. // 현재 pr 버전
@Component
public class SseEmitterStorage {
/*
ExecutorService로 변경이 필요할 수 있습니다.
*/
private final ConcurrentLinkedQueue<SseEmitter> emitters;
/*
동시성 문제로 자료구조 변경이 필요할 수 있습니다.
*/
public SseEmitterStorage() {
this.emitters = new ConcurrentLinkedQueue<>();
} 처음에는 ArrayList를 사용했어요. 그랬더니 동시성 예외가 발생했어요. 이유는 다음 작업이 동시에 일어나기 때문이에요.
// sseStorage
public void add(SseEmitter sseEmitter) {
emitters.add(sseEmitter);
log.info("[SSE] 새로운 연결이 추가되었습니다. 현재 연결 수: {}", emitters.size());
sseEmitter.onTimeout(() -> {
emitters.remove(sseEmitter);
log.debug("[SSE] 연결이 타임아웃으로 종료되었습니다. 현재 연결 수: {}", emitters.size());
});
sseEmitter.onError(e -> {
emitters.remove(sseEmitter);
});
}
// sseService.class
public void propagate(String eventName, Object data) {
sseEmitterStorage.getEmitters().forEach(emitter -> {
SseEventBuilder eventBuilder = SseEventBuilderFactory.create(eventName, data);
sendEvent(emitter, eventBuilder);
});
} 그래서 동시성을 보장해주는 ConcurrentLinkedQueue로 변경했어요. 하지만 LinkedQueue인 만큼 조회, 삭제 등에 속도가 느려요.. 추후 변경이 필요할 것으로 생각해요. HashMap은 어떨까 고민하고 있어요. 사용자마다 하나의 sse 커넥션을 맺는다는 점을 고려하여 key: token, value: sseEmitter로 두어서 관리하는 것이죠. 이렇게 하면 한 사용자는 반드시 하나의 연결만 맺도록 관리할 수 있을 것 같아요. 끊긴 SSE 커넥션에 데이터를 보내는 문제SseEmitter에 설정한 timeout 시간이 지나면 sse 연결은 끊겨요! sse 연결이 끊기면 SseEmitter를 SseEmitterStorage에 저장할 때 등록한 콜백에 따라 스토리지에서 제거돼요. public void add(SseEmitter sseEmitter) {
emitters.add(sseEmitter);
log.info("[SSE] 새로운 연결이 추가되었습니다. 현재 연결 수: {}", emitters.size());
sseEmitter.onTimeout(() -> {
emitters.remove(sseEmitter);
log.debug("[SSE] 연결이 타임아웃으로 종료되었습니다. 현재 연결 수: {}", emitters.size());
});
sseEmitter.onError(e -> {
emitters.remove(sseEmitter);
});
} 문제는 SseEmitterStorage의 getEmitters에 있는데요. public List<SseEmitter> getEmitters() {
return emitters.stream().toList();
} 이때 위 코드와 같이 작성하면 getEmitters 메서드를 호출하는 시점에 가지고 있는 SseEmitter 목록을 그대로 전달해요. 갖고있는 참조를 그대로 보내지 않고 복사를 떠서 보내기 때문이에요. 관련해서는 이 글을 참고하면 좋아요. getEmitters 메서드 호출한 후에 timeout으로 인해 일부 SseEmitter 객체의 커넥션이 끊길 수 있어요. 하지만 스케줄러에서는 그것을 알지 못하고 데이터를 전송하려하고 예외가 발생해요. ![]() 이거는,,, 일단은 무시해도 치명적인 에러가 발생하지 않아서 무시하기로 했어요. 또한 timeout 시간을 길게 잡는다면 크게 문제가 안될 수 있어요. SSE에서 에러가 발생하면 어디에서 핸들링하는 것이 좋은가SSE 통신에서 문제가 생기면 예외를 잡을 수 있는 지점은 두 곳이 있어요.
저는 SseEmitter의 콜백함수에서 잡는 것이 좋다고 생각하는데요. SSE 연결을 다루는 근원이기 때문에 이것에서 핸들링하는 것이 예외 전파를 막을 수 있겠다 생각했어요. 하지만 지금은 그러지 않았는데요. 이유는,,, SseEmitter에서 에러처리해도 얻을 수 있는 정보가 매우 제한되더라고요 예시 상황은 위에서 본, 클라이언트가 일방적으로 연결을 끊은 상황이에요. 콜백함수에서 잡게 되면 다음과 같아요. sseEmitter.onError(e -> {
emitters.remove(sseEmitter);
log.error("SSE 예외가 발생했습니다. {}", e);
}); ![]() 단지 IOException만 발생하고 끝나더군요.. 관련 정보를 더 얻을 수 없어서 에러 핸들링이 어려웠어요. 대신에 비즈니스 로직에서 잡게 되면 다음과 같이 좀더 자세한 정보가 나와요. private void sendEvent(SseEmitter sseEmitter, SseEventBuilder eventBuilder) {
try {
sseEmitter.send(eventBuilder);
} catch (IOException e) {
SseErrorHandler.handle(e);
}
} ![]() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사용자가 어떤 전공과목을 화면에서 선택하고있는지 어떻게 알고 응답을 줄 것인지 논의하면 좋을 것 같아요
사용자가 전공과목 탭을 옮기면 어떻게 SSE의 응답을 변경할 지 논의해보고싶어요
제가 생각한 것은 현재 사용자 토큰에 따라 SseEmitter를 하나 열잖아요? 그래서 그냥 교양, 컴공전공,소웨전공,전정통 전공을 차례대로 계속 내려주는건 어떤가 싶어요...^^ 저희가 아직 전공 과목 서비스를 여러 개 하지 않잖아요, 우선 빠른 구현이 목표라고 생각해서요! 리팩토링 지점으로 남겨두고 이런 방식으로 구현해보는건 어떨까요? 데이터 양도 방대한 것이 아니라 총 40개 정도 되는 거잖아요? 그래서 이런 제안을 해봅니다
작업 고생하셨어용🐥🐥
변경 사항 정리SseStorage 구현체 변경SseStorage 내부 구현체를 ConcurrentLinkedQueue에서 ConcurrentHashMap으로 변경했어요. ConcurrentLinkedQueue는 다음 단점이 있어요.
HashMap의 구현체인 ConcurrentHashMap으로 변경하면 예상되는 효과는 다음과 같아요.
LocalDateTime.class가 직렬화 안되는 문제ObjectMapper는 LocalDateTime.class를 제대로 직렬화하지 못하는데요.
실제로 직렬화한 결과는 다음과 같아요. // expected
{
"queryTime":"2025-1-28T23:55:23.434294"
}
// actual (아마 이랬던 것 같아요)
{
"queryTime": [
"2025", "1", "28", "23", "55", "23", "434294"
]
} 따라서 ObjectMapper에 모듈을 추가하고, 또 하나의 설정을 해주었어요. |
작업 내용
실시간으로 여석을 조회하는 기능을 구현했습니다. 데이터를 실시간으로 전달하기 위해 SSE를 사용했습니다.
아직 테스트를 작성하지 않았는데, 추가할게요! sse 경험이 없으면 코드 리뷰에 좀 오랜 시간이 걸릴 것 같아서 구현한 내용부터 올려요. 이후에 구현부가 달라질 수 있다는 점을 고려해 주세요 😀
고민 지점과 리뷰 포인트