Skip to content
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

[Feature/526] 탭바 SSE를 적용한다 #536

Merged
merged 5 commits into from
Nov 4, 2024
Merged

[Feature/526] 탭바 SSE를 적용한다 #536

merged 5 commits into from
Nov 4, 2024

Conversation

miseongk
Copy link
Member

💡 이슈 번호

close: #526

✨ 작업 내용

  • 탭바 new 뱃지를 위한 SSE를 적용했습니다.

🚀 전달 사항

  • 클라이언트에서 connect api를 통해 각 탭에 대해 각각 커넥션을 맺으면, 이후 변경사항이 생기면 변경되었다는 이벤트를 전송합니다.

@miseongk miseongk self-assigned this Oct 29, 2024
@injoon2019
Copy link
Collaborator

오 이거 시간을 가지고 꼼꼼히 볼게요 1시간 안에는 봅니당

Copy link
Collaborator

@injoon2019 injoon2019 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다! 🚀

@@ -8,6 +8,7 @@ spring:
hibernate:
jdbc:
time_zone: Asia/Seoul
open-in-view: false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 요거는 어쩌다 보게 된거에요?
https://w1725778411-x0b416331.slack.com/archives/C07LHK2S0MV/p1730289607847939

저희 종종 발생하는 lock 에러 잡을 수 있으려나요..?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open-in-view를 사용하면 http connection 맺고 있는동안 DB connection도 같이 열려있어서 커넥션이 고갈되는 문제가 있다고 해요! 그래서 false로 설정했어요.

음 lock에러가 저거랑 관련이 있나요..?-?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 모르겠어요 ㅋㅋ 시간될 때 lock 에러 한번 봐볼게요!

Comment on lines 18 to 23
@GetMapping("/connect", produces = [MediaType.TEXT_EVENT_STREAM_VALUE])
@AuthRequired
fun connect(@AuthUserId userId: Long, @ModelAttribute("tapType") tapType: TabTypeRequest): SseEmitter {
val sseEmitter = tabEventService.connect(userId, tapType.tabType)
return sseEmitter
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 나중에 소켓이나 Stomp 같은 것도 연결할 수 있다 가정하면 connectSse 요렇게 안나타내도 되려나요?
  • ModelAttribute로 tapType을 받는데 그럼 각 탭별로 다 커넥트를 맺는 구조인가요? 한번 커넥션을 맺고, 거기서 탭별 응답을 주는 구조는 안되는건지 궁금해요!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

나중에 소켓이나 Stomp 같은 것도 연결할 수 있다 가정하면 connectSse 요렇게 안나타내도 되려나요?

오 좋아요!

ModelAttribute로 tapType을 받는데 그럼 각 탭별로 다 커넥트를 맺는 구조인가요? 한번 커넥션을 맺고, 거기서 탭별 응답을 주는 구조는 안되는건지 궁금해요!

한번만 커넥션을 맺고 이벤트 발생할 때 전달하는 데이터에 어떤 탭인지 알려주는 구조로 할 수 있을 것 같아요!
처음에는 새로운 이벤트가 발생했다는 확인용 이벤트만 보내기로 했기 때문에, 클라이언트에서도 이벤트를 감지할 때 탭 별로 커넥션을 맺으면 해당 커넥션에서 이벤트가 오는것만 감지해서 처리하기 쉬울거라고 생각했어요.
그런데 생각해보니까 클라이언트마다 세개의 커넥션을 맺고 있는게 불필요하게 연결을 하고 있는것 같네요! 그리고 또 뱃지 유무 여부를 데이터로 전달하기로 하기도 했으니까 말씀하신 방법대로 하는게 더 좋아보여요!

@Service
class TabEventService {

private val sseEmitters: MutableMap<Long, MutableMap<String, SseEmitter>> = ConcurrentHashMap()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예전에 우테코 때 옆 팀에서 sse를 쓰면서, SSE를 제대로 쓰려면 레디스가 필수다라고 했던 말이 기억나는데 그때 이유는 생각이 안나거든요. 지금보니, 인스턴스별로 sseEmitters를 들고 있으면, 각 인스턴스별로 요청을 보낼때 연결이 일관되게 유지되지 않는 문제가 있을 수 있겠군요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 맞아요! 인스턴스가 여러대가 된다면 일관되게 처리하지 못하겠네요

@miseongk
Copy link
Member Author

miseongk commented Nov 1, 2024

탭 별로 연결하던것을 한번만 연결하도록 구조를 변경했습니다. 이에 따라 이벤트 발생 시 어떤 탭인지와 뱃지 표시 유무를 DTO로 전달하도록 수정했어요!

모래사장 탭과 호감 탭은 읽음 표시하는 api 구현해야해요! 주말에 할게요

@miseongk
Copy link
Member Author

miseongk commented Nov 3, 2024

기존에는 핑퐁 중인 보틀에 대해서만 읽음 여부를 가지고 있는 구조여서 (LetterisReadByOtherUser 필드로 관리) 랜덤 보틀과 호감 보틀에 대해서 읽음 여부를 관리하는 BottleReadHistory 객체를 만들었어요!
만들고나니까 핑퐁 중인 보틀과 다른 보틀을 구분해서 읽음 여부를 관리할 필요없이 BottleReadHistory로 모두 관리하면 좋을 것 같다는 생각이 들어요.
그래서 우선은 기존 api와 호환성을 위해서 보틀 읽음 표시 api는 v2로 만들었고, 이후에 클라이언트에서 핑퐁 보틀 읽음 표시도 v2로 변경한 후에 수정해야할 것 같아요! 수정할 부분은 TODO로 남겨두었어요.

변경 사항이 좀 있어서 다시한번 리뷰 부탁드려요 🙇‍♀️

Copy link
Collaborator

@injoon2019 injoon2019 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니다 수고하셨어요!!

@miseongk miseongk merged commit 7e462a8 into develop Nov 4, 2024
1 check passed
@miseongk miseongk deleted the feature/526 branch November 4, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

탭바 상태 변경 구현하기
2 participants