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

[feat #116] 알림 전송 배치 작업 #142

Merged
merged 14 commits into from
Aug 28, 2024
Merged

[feat #116] 알림 전송 배치 작업 #142

merged 14 commits into from
Aug 28, 2024

Conversation

dlswns2480
Copy link
Contributor

⛏ 이슈 번호

close #116

📍 리뷰 포인트

  • 배치 비즈니스 로직

📝 참고사항(Optional)

  • 배치 - 컨텐츠 매핑 테이블 AlertContent에 컨텐츠 관련 필드들을 넣었습니다
    • 배치 id랑 컨텐츠 id만 있으면 컨텐츠 조회해서 알림 생성하는 과정이 번거로워서.. 컨텐츠 조회를 안해도 되는 방향으로 했는데 요것도 피드백 주세요..
alter table alert_content
    add column user_id bigint not null,
    add column content_thumb_nail varchar(255),
    add column title varchar(255);
  • multi job 관련 설정으로 배포 스크립트에 in-batch yml추가했고 노션에도 있습미다.
  • 리뷰하기 넘 많을 것 같아서 로직 간단 요약 첨부...

Item Reader

  • sent false, 전송시점 된 배치 알림 엔티티 loadAll
  • return AlertBatch

Item Processor

  • AlertBatch를 통해 알림 전송
  • AlertBatch sent 필드 업데이트 false → true
  • return AlertBatch

Item Writer

  • AlertBatch 전달받음(청크 단위 50)
  • AlertBatch ids로 AlertContent 전체 조회
  • Alert 생성 후 bulk insert
  • AlertContent 삭제

@dlswns2480 dlswns2480 self-assigned this Aug 26, 2024
@dlswns2480 dlswns2480 linked an issue Aug 26, 2024 that may be closed by this pull request
2 tasks
Comment on lines +17 to +25
override fun loadAllByShouldBeSentAt(now: LocalDate, pageable: Pageable): Page<AlertBatch> {
return alertBatchRepository.findAllByShouldBeSentAtAfterAndSent(now, false, pageable)
.map { it.toDomain() }
}

override fun send(alertBatch: AlertBatch) {
alertBatchRepository.findByIdOrNull(alertBatch.id)
?.sent()
}
Copy link
Member

Choose a reason for hiding this comment

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

알림 설정 (y) 로 하면 알림 등록하는 부분은.. 이전에 커밋했던가요 ..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

리뷰 이상 없으면 컨텐츠 생성, 수정작업에 해당 로직 추가하려했습니다! 지금은 없슴다ㅎ

) : ItemReader<AlertBatch> {

private var currentPage: Int = 0
private val pageSize: Int = 50
Copy link
Member

Choose a reason for hiding this comment

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

pageSIze랑 chunkSize 는 같이 관리해도 좋을 것 같아여 ! 항상 같은 값으로 설정해야하니까

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오키오키요!

Comment on lines 32 to 41
fun sendAlertStep() = batch {
step("sendAlertStep") {
chunk(50, transactionManager, fun SimpleStepBuilderDsl<AlertBatch, AlertBatch>.() {
reader(alertReader)
processor(alertProcessor)
writer(alertWriter)
transactionAttribute(DefaultTransactionAttribute(TransactionDefinition.PROPAGATION_NOT_SUPPORTED))
})
}
}
Copy link
Member

Choose a reason for hiding this comment

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

BatchDsl 최고.. 네이버 최고..

Comment on lines +12 to +23
companion object {
private const val POOL_SIZE = 3
}

@Bean("schedulerTask")
fun taskScheduler(): TaskScheduler {
val executor = ThreadPoolTaskScheduler()
executor.poolSize = POOL_SIZE
executor.threadNamePrefix = "scheduler-thread-"
executor.initialize()
return executor
}
Copy link
Member

Choose a reason for hiding this comment

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

이거 pool size 왜 3으로 둔거예요 ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cpu 코어 1이니까 2로하려다가 3으로 했습니다! 근데 2로 수정하겠습니다

Comment on lines 11 to 16
@Query(
"""
update AlertContentEntity ac set ac.deleted = true
where ac.id in :ids
"""
)
Copy link
Member

Choose a reason for hiding this comment

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

이거 전송 완료했으면 그냥 하드딜리트 해도 될듯여 ㅇㅁㅇ? 알림 테이블에도 또 남으니까..

@jimin3263
Copy link
Member

  • 배치 - 컨텐츠 매핑 테이블 AlertContent에 컨텐츠 관련 필드 추가는 알림 가는 시점 기준의 데이터로 알림 보내는거라 따로 아이디 두는게 맞는 것 같아요! 번거롭다는게 어떤 측면에서 번거롭다는걸까요?

@dlswns2480
Copy link
Contributor Author

dlswns2480 commented Aug 28, 2024

  • 배치 - 컨텐츠 매핑 테이블 AlertContent에 컨텐츠 관련 필드 추가는 알림 가는 시점 기준의 데이터로 알림 보내는거라 따로 아이디 두는게 맞는 것 같아요! 번거롭다는게 어떤 측면에서 번거롭다는걸까요?

@jimin3263 AlertContent로 컨텐츠들을 전부 조회하는 게 번거롭다는 거였는데 알림 가는 시점 기준의 데이터로 알림 보내는 것이라는 중요한 사실을 망각하고 있었습니다.. 요거 로직 수정하겠습니다! 엔티티도 기존것으로 수정하겠습니다!

@dlswns2480 dlswns2480 merged commit da72900 into develop Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

리마인드 알림 일괄 전송 기능
2 participants