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

[FIX, REFACTOR] Notification 테이블에 title 컬럼 추가 및 Push 이벤트 비동기 처리 #23

Merged
merged 35 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
8f7c9e4
refacgtor: controller 모듈에서 responseDto 변환하도록 리팩토링
twoosky Nov 27, 2023
885615e
fix: notification 도메인 생성 로직에 title 추가
twoosky Nov 27, 2023
d13c4ce
feat: notificaiton 도메인에 id 필드 추가
twoosky Nov 27, 2023
1172052
feat: NotificationType에 title 추가
twoosky Nov 27, 2023
2511cad
feat: notification entity에 title 컬럼 추가
twoosky Nov 27, 2023
1bc803b
feat: async 스레드 풀 bean 등록
twoosky Nov 27, 2023
9c0a75a
fix: fcm title 상수 제거
twoosky Nov 27, 2023
aa984aa
feat: push 이벤트 비동기 처리
twoosky Nov 27, 2023
a7afbee
fix: async 스레드 풀 bean 이름 옵션 추가
twoosky Nov 27, 2023
fa8c56f
fix: thread pool size 조정
twoosky Nov 27, 2023
1aca701
fix: thread pool size 조정
twoosky Nov 27, 2023
4e5ffb1
fix: queueCapacity 옵션 제거
twoosky Nov 29, 2023
b804196
fix: 애플리케이션 레벨 스레드 풀 설정 제거
twoosky Nov 30, 2023
16bb553
feat: payload response dto 생성
twoosky Dec 5, 2023
b17bae3
feat: applyPayload 조회 outputPort 정의
twoosky Dec 5, 2023
27d48d0
refactor: zero payload 방식으로 이벤트 dto 리팩토링
twoosky Dec 5, 2023
bfe2cab
fix: in port 파라미터명 수정
twoosky Dec 5, 2023
f9303db
feat: group payload 조회 outputPort 정의
twoosky Dec 5, 2023
2fb7781
feat: apply 이벤트에 대한 알림 생성 서비스에 payload 요청 로직 구현
twoosky Dec 5, 2023
fef6098
feat: group 이벤트에 대한 알림 서비스에 payload 요청 로직 구현
twoosky Dec 5, 2023
e3ab74e
feat: 그룹 시스템으로 payload 요청을 위한 OpenFeign client 구현
twoosky Dec 5, 2023
49bc26f
feat: 그룹 시스템 base url 환경변수 설정
twoosky Dec 5, 2023
8611121
chore: internal-api 모듈에 domain, application 모듈 의존성 추가
twoosky Dec 5, 2023
cd50c18
feat: redirectId에 대한 value 객체 정의
twoosky Dec 5, 2023
1d66566
fix: 알림 title 람다식으로 선언
twoosky Dec 5, 2023
c1fc621
fix: redirectId value 객체로 생성
twoosky Dec 5, 2023
27ba90c
chore: internal-api 모듈 application.yml 파일 추가
twoosky Dec 5, 2023
2840e65
refactor: SQS 메시지 zero-payload 방식으로 재정의
twoosky Dec 5, 2023
c15d443
fix: application event mapper 수정
twoosky Dec 5, 2023
dc8fa74
fix: push 모듈 notification title값 수정
twoosky Dec 5, 2023
61d7eb5
feat: notification domain 객체에 image 필드 추가
twoosky Dec 5, 2023
418a2aa
feat: notification 객체 생성 필드에 groupImage값 추가
twoosky Dec 5, 2023
3c0493c
feat: notification entity에 image 컬럼 추가
twoosky Dec 5, 2023
e6268b2
feat: 알림 목록 조회 응답에 image, createAt 추가
twoosky Dec 5, 2023
a1db43d
fix: ObjectMapper LocalDateTime 역질렬화 설정 추가
twoosky Dec 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package gloddy.fcmToken.port.`in`

import gloddy.fcmToken.FCMToken
import gloddy.fcmToken.dto.FCMTokenCreateDto
import gloddy.fcmToken.dto.FCMTokenCreateResponse
import gloddy.notification.UserId

interface FCMTokenCreateUseCase {
fun create(userId: UserId, dto: FCMTokenCreateDto): FCMTokenCreateResponse
fun create(userId: UserId, dto: FCMTokenCreateDto): FCMToken
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import gloddy.fcmToken.port.`in`.FCMTokenCreateUseCase
import gloddy.fcmToken.port.out.FCMTokenCreatePort
import gloddy.fcmToken.FCMToken
import gloddy.fcmToken.FirebaseToken
import gloddy.fcmToken.dto.FCMTokenCreateResponse
import gloddy.fcmToken.dto.toDto
import gloddy.notification.UserId
import org.springframework.stereotype.Service

Expand All @@ -15,11 +13,11 @@ class FCMTokenCreateService(
private val fcmTokenCreatePort: FCMTokenCreatePort
): FCMTokenCreateUseCase {

override fun create(userId: UserId, dto: FCMTokenCreateDto): FCMTokenCreateResponse {
override fun create(userId: UserId, dto: FCMTokenCreateDto): FCMToken {
val fcmToken = FCMToken(
userId = userId,
token = FirebaseToken(dto.token)
)
return fcmTokenCreatePort.create(fcmToken).toDto()
return fcmTokenCreatePort.create(fcmToken)
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package gloddy.notification.port.`in`

import gloddy.notification.Notification
import gloddy.notification.dto.NotificationGetDto
import gloddy.notification.dto.NotificationResponse

interface NotificationGetUseCase {
fun getAllByUser(dto: NotificationGetDto): NotificationResponse
fun getAllByUser(dto: NotificationGetDto): List<Notification>
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class ApplyNotificationCreateService(
Notification(
redirectId = applyEvent.applyGroupId,
userId = getTargetUserId(type, applyEvent),
title = type.title,
content = type.content,
type = type
).run {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class GroupNotificationCreateService(
Notification(
redirectId = groupEvent.groupId,
userId = groupEvent.userId,
title = notificationType.title,
content = notificationType.getContent(),
type = notificationType
).run {
Expand All @@ -40,6 +41,7 @@ class GroupNotificationCreateService(
Notification(
userId = it,
redirectId = event.groupId,
title = notificationType.title,
content = notificationType.content,
type = notificationType
).run {
Expand All @@ -56,6 +58,7 @@ class GroupNotificationCreateService(
Notification(
userId = it,
redirectId = event.groupId,
title = notificationType.title,
content = notificationType.content,
type = notificationType
).run {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
package gloddy.notification.service

import gloddy.notification.Notification
import gloddy.notification.dto.NotificationGetDto
import gloddy.notification.dto.NotificationResponse
import gloddy.notification.dto.toResponse
import gloddy.notification.port.`in`.NotificationGetUseCase
import gloddy.notification.port.out.NotificationGetPort
import org.springframework.stereotype.Service
Expand All @@ -12,7 +11,7 @@ class NotificationGetService(
private val notificationGetPort: NotificationGetPort
): NotificationGetUseCase {

override fun getAllByUser(dto: NotificationGetDto): NotificationResponse =
notificationGetPort.findByUserId(dto.userId).toResponse()
override fun getAllByUser(dto: NotificationGetDto): List<Notification> =
notificationGetPort.findByUserId(dto.userId)

}
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
package gloddy.notification

import java.util.*


@JvmInline
value class UserId(val value: Long)

@JvmInline
value class NotificationId(val value: String)

data class Notification(
val id: NotificationId = NotificationId(UUID.randomUUID().toString()),
Copy link
Member Author

Choose a reason for hiding this comment

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

추후 알림 삭제와 같은 기능이 추가될 수 있기에 Notification domain에 Id 필드를 추가했습니다.

val userId: UserId,
val redirectId: Long,
val title: String,
val content: String,
val type: NotificationType
)
Original file line number Diff line number Diff line change
@@ -1,17 +1,40 @@
package gloddy.notification

enum class NotificationType(
val title: String,
val content: String
) {
APPLY_APPROVE("Your applied gathering has been approved! \uD83D\uDC4C"),
APPLY_REFUSE("The gathering you applied for has been declined. \uD83E\uDD72"),
APPLY_CREATE("A new gathering application has arrived! \uD83D\uDC8C"),
GROUP_LEAVE("has just left the group. \uD83E\uDD72"),
GROUP_ARTICLE_CREATE("Please Check the New Notice! \uD83D\uDDE3"),
GROUP_APPROACHING_START("The gathering starts in 1 hour! ⏰"),
GROUP_END("Did you enjoy the gathering? \uD83D\uDE06")
APPLY_APPROVE(
"👌Your applied gathering has been approved!",
"Wishing you enjoy a fun and safe gathering."
),
APPLY_REFUSE(
"😢The gathering you applied for has been declined.",
"There are various other gatherings available for you! Would you like to look for another gathering?"
),
APPLY_CREATE(
"💌A new gathering application has arrived!",
"We’re awaiting the host’s approval for the gathering! please warmly welcome the new members."
),
GROUP_LEAVE(
"😢has just left the group.",
"Shall we go check the group participants?"
),
GROUP_ARTICLE_CREATE(
"🗣️Please Check the New Notice!",
"A new post has been added."
),
GROUP_APPROACHING_START(
"⏰The gathering starts in 1 hour!",
"Are you ready to enjoy the gathering? \n" + "Check the announcement for a better gathering!"
),
GROUP_END(
"😆Did you enjoy the gathering?",
"Please rate the attendees of the gathering! \n" + "Select the best partner with reward stickers."
)
;


companion object {
fun of(type: String): NotificationType {
return values().firstOrNull { it.name == type }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package gloddy.fcmToken

import gloddy.fcmToken.dto.FCMTokenCreateDto
import gloddy.fcmToken.dto.FCMTokenCreateResponse
import gloddy.fcmToken.dto.toResponse
import gloddy.fcmToken.port.`in`.FCMTokenCreateUseCase
import gloddy.notification.UserId
import org.springframework.web.bind.annotation.*
Expand All @@ -16,6 +17,6 @@ class FCMTokenController(
@RequestBody request: FCMTokenCreateDto,
@RequestHeader("USER_ID") userId: UserId
): FCMTokenCreateResponse {
return fcmTokenCreateUseCase.create(userId, request)
return fcmTokenCreateUseCase.create(userId, request).toResponse()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ package gloddy.fcmToken.dto
import gloddy.fcmToken.FCMToken
import gloddy.fcmToken.FirebaseToken
import gloddy.notification.UserId
import gloddy.notification.dto.NotificationDto

data class FCMTokenCreateResponse(
val userId: UserId,
val token: FirebaseToken
)

fun FCMToken.toDto(): FCMTokenCreateResponse =
fun FCMToken.toResponse(): FCMTokenCreateResponse =
FCMTokenCreateResponse(
userId = this.userId,
token = this.token
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gloddy.notification

import gloddy.notification.dto.NotificationGetDto
import gloddy.notification.dto.toResponse
import gloddy.notification.port.`in`.NotificationGetUseCase
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.RequestHeader
Expand All @@ -10,10 +11,10 @@ import org.springframework.web.bind.annotation.RestController
@RestController
@RequestMapping("/api/v1/notifications")
class NotificationController(
private val notificationGetUseCase: NotificationGetUseCase
private val notificationGetUseCase: NotificationGetUseCase,
) {

@GetMapping
fun getAllByUser(@RequestHeader("USER_ID") userId: UserId) =
notificationGetUseCase.getAllByUser(NotificationGetDto(userId))
notificationGetUseCase.getAllByUser(NotificationGetDto(userId)).toResponse()
Copy link
Member Author

Choose a reason for hiding this comment

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

response dto는 변경 가능성이 높은 객체입니다. 이전에는 application 모듈에서 도메인 객체를 response dto로 변환했습니다. 이로 인해, dto에 변경이 발생하는 경우 비즈니스 코드를 수정해야 했습니다. 이는 외부 변화에 비즈니스 로직이 노출되어 있는 상황이라고 생각합니다. 따라서, application 모듈에서는 도메인을 반환하고, in-adapter-api 모듈에서 response dto로 변환하도록 리팩토링 했습니다.

Copy link
Member

Choose a reason for hiding this comment

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

저는 이 의견에 반대되는 입장입니다!
이유는 다음과 같습니다!!

  1. 도메인이 DTO의 역할까지 하게된다.
    비즈니스 요구사항의 정책을 담고 있는 도메인이 DTO의 역할까지 하는건 너무 많은 역할을 하게된다고 생각합니다! 해당 도메인에 DTO의 필요한 로직이 섞일 가능성이 있습니다. 또한 Presentation 계층이 직접 도메인을 참조하고 있기 때문에 변경에 유연하지 않을 가능성이 더 높다고 생각합니다.
  2. 이런식의 구조는 성능에 치명적일 수 있다.
    현재는 도메인 관점으로 생각하였을 때, 다른 도메인의 정보없이 Notification도메인 만을 위한 정보를 조회해서 보여주고 있습니다. 이럴 경우는 문제가 없을 수 있지만, 대부분의 조회는 여러 도메인의 정보를 같이 보여줘야되는 경우가 많습니다.(예를들어 유저를 조회하는데 신뢰도 및 칭찬 갯수를 같이 조회, 그룹멤버를 조회하는데 유저 이름 및 신뢰도를 같이 조회) 위와 같은 구조로 짤 경우 Presentation 계층에서 하나하나 도메인을 다 조회해서 그 도메인들을 조합해 Response DTO를 생성시켜야합니다. 그러면 쿼리의 수도 많이 발생할 가능성이 높으며, Controller단에 많은 도메인들이 참조되고, 그 ResponseDTO를 생성시키는 로직이 매우 복잡해질 수 있습니다.

ResponseDto는 대부분 조회 연산에서 정의되어집니다. DB 조회 연산의 경우 성능을 높이기 위해 ResponseDTO로 바로 가져오는 경우도 많습니다. 하늘님이 response dto는 변경 가능성이 높은 객체라고 말씀하셨는데, 저는 조회 연산이 C,U,D 연산과 같이 특별한 도메인의 정책이 있는 것은 아니지만 dto에 어떤 데이터가 들어가느냐가 비즈니스 요구사항이 된다고 생각하기 때문에 이를 Application 모듈에서 정의하는게 맞다고 생각합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

오오 !

  1. 저는 Presentation에서 Domain을 Dto로 변환하기 때문에 Presentation 계층에서 변경에 유연하다고 생각했어요. 도메인을 반환하게 되면 하나의 service 메소드를 여러 controller에서 사용할 수 있고, presentation에서 필요한 값만 response dto로 변환할 수 있기 때문에 변경에 유연하다고 생각했습니다
  2. 여러 도메인의 정보를 같이 보여주는 경우에는 application 계층에서 여러 도메인을 조회한 뒤 presentation 계층으로 전달 후 presentation에서 필요한 데이터만 response dto로 생성하는 방식을 생각했어요. 도메인은 핵심 비즈니스이기 때문에 response dto를 생성하기 위한 모든 값들을 갖고 있어요. 대부분 response dto의 변경은 핵심 비즈니스가 변경되는 것이 아닌, 도메인으로부터 필요한 값을 더 가져오거나, 제거하는 방식의 변경이 이뤄진다고 생각해요. 따라서, response dto는 비즈니스 관심사가 아니기 때문에 presentation에서 변환이 이뤄지는 것이 적합하다고 생각했어요.

response dto 객체는 외부의 변화와 가장 맞닿아있다고 생각해요. response dto를 application 모듈에서 직접 의존하면 외부의 변화가 비즈니스 로직에 영향을 미치는 구조라고 생각합니다. 핵사고날 아키텍처를 사용한 이유는 외부의 변화로부터 비즈니스 로직을 보호하는 것이 목적이였는데! 말이죵

Copy link
Member

@jihwan2da jihwan2da Nov 27, 2023

Choose a reason for hiding this comment

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

일단 조회를 위한 API 경우는 application 및 domain단에 특정한 비즈니스 정책이나 로직이 존재할 가능성이 없습니다.(특히 domain 계층은 사용하지도 않음) 또한 조회의 경우 DB 성능과 관련이 높기 때문에 하늘님이 말씀해주신

하나의 service 메소드를 여러 cotroller에서 사용할 수 있도록 재사용하는

방향이 아닌, 한 API 당 Read Model을 따로 정의해 놓는 경우가 많습니다. CQRS를 한 예라고 하겠습니다. 이 경우 Read Model이 도메인에서 정의한 조회 모델이 됩니다. 위의 ResponseDto라고 명칭을 하였기 때문에 Presentation 계층과 맞닿아 있다고 생각할 수 있지만 제가 말한 위의 ResponseDto는 ReadModel이라고 생각해주시면 되겠습니다. 위처럼 그냥 ReadModel을 정의하지 않고 그냥 Domain 그 자체를 application,domain 단의 service 인터페이스로 정의해버리면 조회 성능을 높이는데 너무 많은 제약이 있습니다. 조회 성능을 높이기 위해 out-persistence 계층에서 QueryDsl을 사용하여 바로 ReadModel을 QueryInjection해서 받아오는 경우가 많으며, row의 특정 컬럼만 받아오는 경우, 그리고 Nosql을 사용하여 ReadModel을 그대로 저장하여 그대로 반환하는 경우 또한 빈번합니다. 근데 domain 및 application에서 ReadModel을 따로 정의하지 않고 위처럼 Domain 그 자체를 반환하게 된다면 이를 적용하는데 한계가 있습니다.

따라서 정리하면 ReadModel은 조회의 애플리케이션 계층의 모델이다. 입니다!
CQRS에서 많이 사용하는 패턴이죵

Copy link
Member Author

Choose a reason for hiding this comment

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

오호 조회 성능을 높이기 위해 QueryDSL의 @QueryProjection을 사용한다면 DTO(ReadModel)가 out-adapter-persistence 계층에서부터 생성되기 때문에 이를 application 계층에서 도메인으로 변환할 수도, 변환할 이유도 없겠네요.

ReadModel은 조회의 애플리케이션 계층의 모델이다

@jihwan2da 공감합니다! application 계층에서 도메인을 반환하는 것은 여러 제약사항이 있을 수 있을거 같네요. application 계층에서 dto를 반환하는 것에 동의합니다. 하지만, 한가지 고민되는 사항이 있는데요. 현재 알림 목록 조회 응답으로 알림 생성 날짜도 함께 전달해야합니다. 피그마를 보면 '14 oct' 이런 식으로 알림 생성 날짜를 나타내도록 되어 있어요. 날짜 형식 변환은 View에 매우 종속적인 로직이라고 생각해요. 따라서 application 계층에서는 DB로부터 조회한 날짜를 dto로 전달하고, presentation 계층에서 날짜를 파싱하는 구조는 어떤가요? 정리하자면, application, presentation 계층에서 각각 dto를 갖고 있고, presentation 계층에서 자신이 원하는 포맷으로 데이터를 변환해 사용하자는 것입니다!

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,28 @@ import gloddy.notification.Notification
import gloddy.notification.NotificationType
import gloddy.notification.UserId

data class NotificationResponse(
data class NotificationGetResponse(
val notifications: List<NotificationDto>
)

data class NotificationDto(
val userId: UserId,
val redirectId: Long,
val title: String,
val content: String,
val type: NotificationType
)

fun List<Notification>.toResponse(): NotificationResponse =
NotificationResponse(
fun List<Notification>.toResponse(): NotificationGetResponse =
NotificationGetResponse(
this.map { it.toDto() }.toList()
)

fun Notification.toDto(): NotificationDto =
NotificationDto(
userId = this.userId,
redirectId = this.redirectId,
title = this.title,
content = this.content,
type = this.type
)
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ class NotificationEntity(
@field:DynamoDBAttribute(attributeName = "redirect_id")
var redirectId: String = "",

@field:DynamoDBAttribute(attributeName = "title")
var title: String = "",

@field:DynamoDBAttribute(attributeName = "content")
var content: String = "",

Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,26 @@
package gloddy.dynamodb.notification

import gloddy.notification.Notification
import gloddy.notification.NotificationId
import gloddy.notification.UserId


fun NotificationEntity.toDomain(): Notification =
Notification(
id = NotificationId(this.id),
userId = UserId(this.userId.toLong()),
redirectId = this.redirectId.toLong(),
title = this.title,
content = this.content,
type = this.type!!
)

fun Notification.toEntity(): NotificationEntity =
NotificationEntity(
id = this.id.value,
userId = this.userId.value.toString(),
redirectId = this.redirectId.toString(),
title = this.title,
content = this.content,
type = this.type
)
5 changes: 0 additions & 5 deletions push/src/main/kotlin/gloddy/NotificationPushService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,13 @@ package gloddy
import gloddy.command.PushCommand
import gloddy.dynamodb.fcmToken.FCMTokenQueryAdapter
import gloddy.fcmToken.FirebaseToken
import gloddy.notification.Notification
import gloddy.notification.NotificationType
import org.springframework.stereotype.Component

@Component
class NotificationPushService(
private val pushClient: PushClient,
private val fcmTokenQueryAdapter: FCMTokenQueryAdapter
) {
companion object {
const val PUSH_TITLE = "Gloddy"
}

fun push(pushCommand: PushCommand) {
val fcmToken = fcmTokenQueryAdapter.get(pushCommand.userId)
Expand Down
27 changes: 27 additions & 0 deletions push/src/main/kotlin/gloddy/config/AsyncConfiguration.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package gloddy.config

import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.scheduling.annotation.AsyncConfigurer
import org.springframework.scheduling.annotation.EnableAsync
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor
import java.util.concurrent.Executor

@Configuration
@EnableAsync
class AsyncConfiguration : AsyncConfigurer {

companion object {
private const val PUSH_THREAD_NAME_PREFIX = "PUSH-ASYNC-THREAD-"
}

@Bean(name = ["PUSH-EVENT-ASYNC-EXECUTOR"])
override fun getAsyncExecutor(): Executor {
val executor = ThreadPoolTaskExecutor()
executor.setThreadNamePrefix(PUSH_THREAD_NAME_PREFIX)
executor.corePoolSize = 10
executor.maxPoolSize = 10
executor.initialize()
return executor
Copy link
Member Author

@twoosky twoosky Nov 29, 2023

Choose a reason for hiding this comment

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

기존에는 corePoolSize = 2, maxPoolSize = 10, queueCapacity = 5 로 스레드 풀을 설정했습니다. corePoolSize는 스레드 풀에 항상 대기중인 스레드 개수입니다. queueCapacity는 대기 가능한 요청의 수입니다. 만약, queueCapacity만큼 요청이 대기하는 경우 새로운 스레드가 생성됩니다. (maxPoolSize에 도달할 때까지)

기존 설정의 경우 QueueCapacity 초과에 의한 TaskRejectedException이 발생할 수 있습니다. TaskRejectedException은 maxPoolSize와 QueueCapacity 까지 초과된 요청이 들어오는 경우 발생하는 예외입니다. 이전 설정에서는 동시에 15개보다 많은 요청이 들어오는 경우 Exception이 발생합니다. 이를 해결하기 위해 queueCapacity 설정을 default로 변경하고, corePoolSize를 늘렸습니다. queueCapacity default값은 Integer.max_value 입니다. 즉, 새로운 스레드를 생성하지 않고, 스레드 풀에 사용 가능한 스레드가 반환될 때까지 대기합니다.

Copy link
Member Author

@twoosky twoosky Nov 29, 2023

Choose a reason for hiding this comment

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

@async 는 기본적으로 비동기 작업을 새로운 스레드를 매번 생성해서 수행합니다. 이 경우, 수많은 요청이 들어왔을 때 매번 스레드를 생성하여 OutOfMemoryError가 발생할 수 있고, 스레드의 개수가 많아지면 context switching 비용이 증가합니다. 따라서, getAsyncExecutor()을 통해 스레드 풀 사이즈를 설정했습니다. Push 이벤트 처리는 PUSH-ASYNC-THREAD-{ThreadId} 이름의 스레드가 새로 할당되어 작업합니다.

Copy link
Member

Choose a reason for hiding this comment

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

queueCapacity만큼 요청이 대기하는 경우 새로운 스레드가 생성됩니다. (maxPoolSize에 도달할 때까지)

저는 이부분이 이해가 안가요!! 제가 알기론

  1. 요청이 들어오면 corePool에 있는 스레드를 활용
  2. corePoolSize 만큼 스레드를 사용하고 있다면 작업 큐 사이즈 만큼 요청을 대기
  3. 작업 큐의 사이즈가 다 찼으면 max 스레드를 활용
    이라고 알고 있습니다.

그리고 queueCapacity를 무한으로 가져가면 maxPool이 의미가 없어지는거 아닌가요?? 무한 큐이기 때문에 큐가 다 쌓일 일이 없어서max Thream Pool을 활용하지 않을 것 같습니다!

Copy link
Member Author

@twoosky twoosky Dec 8, 2023

Choose a reason for hiding this comment

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

지환님 말씀이 맞아요! 제가 헷갈리게 써놨네요.

queueCapacity를 무한으로 가져가면 maxPool이 의미가 없어지는거 아닌가요??

이것도 맞아요! maxPoolSize가 의미가 없기 때문에 corePoolSize와 maxPoolSize를 같게 설정해놨습니다. 그리고, queueCapacity를 Integer.max_value로 설정한 이유는 알림 푸시에 필요한 스레드의 개수를 정하지 못했기 때문이예요 🤯 설정해놓은 maxPoolSize와 queueCapacity를 초과하는 경우 TaskRejectedException이 발생할 수 있기 때문에 일단은 queueCapacity를 최대로 잡았습니다. 또한, 현재 푸시 실패에 대한 재전송 로직이 없기 때문에 exception이 발생하지 않는 것이 우선이라고 생각했어요. 이 부분은 추후 성능 테스트를 해보면서 스레드 풀을 설정하면 좋을거 같아요!

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import gloddy.NotificationPushService
import gloddy.command.toGroupingPushCommand
import gloddy.notification.event.NotificationCreateEvent
import org.springframework.context.event.EventListener
import org.springframework.scheduling.annotation.Async
import org.springframework.stereotype.Component

@Component
class NotificationEventListener(
private val notificationPushService: NotificationPushService
) {

@Async("PUSH-EVENT-ASYNC-EXECUTOR")
@EventListener
fun consume(event: NotificationCreateEvent) {
notificationPushService.push(event.toGroupingPushCommand())
Expand Down