[BE-196] feat: Transactional Outbox Pattern을 이용해 주차권 결과 이메일을 신뢰성있게 준실시간으로 발송되도록 로직 개선 #403

Kwon770 · 2024-10-19T02:57:12Z

주요 변경사항

Transactional email outbox 패턴을 이용해 신청 결과 이메일을 신뢰성있는 준실시간 발송되도록 수정했습니다. #402 이를 구현하기 위한 다양한 방법은 있는데, 우선은 가장 단순한 방식인 Outbox polling방식을 채택해서 구현하였습니다.

RegistrationResultEmailJob 신규 추가 : 이벤트가 시작 1분 후부터 1분 간격으로 실행되며, PENDING -> FAILED_1 -> FAILED_2 -> FAILED_3 순서대로 이메일을 우선 발송하되, 3번 이상 실패하면 전송 대상에서 제외함. (proccessQueueDataJob와 같은 라이프 사이클)
RegistrationResultEmail 도메인 추가 : Email Outbox 역할을 하지만, 단순히 Outbox를 넘어 성공 여부와 몇 번 실패했는지 등을 기록하는 이력 테이블 역할.

CREATE TABLE registration_result_email_outbox (
    email_id CHAR(26) NOT NULL,
    created_at DATETIME(6) NOT NULL,
    event_id BIGINT NOT NULL,
    receiver_email VARCHAR(255) NOT NULL,
    receiver_name VARCHAR(255) NOT NULL,
    registration_result VARCHAR(255) NOT NULL,
    registration_sequence INTEGER NOT NULL,
    transfer_status INTEGER NOT NULL,
    updated_at DATETIME(6) NOT NULL,
    PRIMARY KEY (email_id)
) ENGINE=InnoDB;

UlidGenerator 추가 : JPA AUTO_INCREMENT를 편리하지만 난독화되어 있지 않고 sequence를 사용하므로 성능 저하가 우려되는데, 이러한 단점을 해결할 수 있는 ULID를 도입.
UserReflectionEvent 일부 수정 : 신청결과가 저장되면, 이 데이터를 바탕으로 RegistrationResultEmail 테이블에 Outbox 상태로 insert.

리뷰어에게...

proccessQueueDataJob과 라이프 사이클을 함께하고 UserReflectionEvent에 의해 트리거되는 만큼, 테스트 코드 도입 전에 추가한 스케줄러 관리와 기존 Job의 수정 사항이 문제 없는지 리뷰를 받아보고자 합니다. 특히, Quartz를 통한 회생에 문제가 없는지 점검 부탁드립니다.

체크리스트

reviewers 설정
label 설정
milestone 설정

…reationEventHandler and EventUpdatedEventHandler

…sultEmailOutbox domain

…ltEmail and Add eventId in domain And Impl adaptor

…t failed transfer 3 times

…sEventHandler to trigger email outbox and Add eventId needed to save RegistrationResultEmail in UserReflectStatusEvent

…of ULID

…ds of RegistrationResultEmailJob

LJH098

작성하신 스케줄러는 이상없어 보여요~~ EamilOutbox에도 문제가 없어 보입니다.
reRegisterQueue만 수정하면 될 것 같아요

LJH098 · 2024-10-24T04:53:50Z

Ticket-Batch/src/main/java/com/jnu/ticketbatch/config/ProcessQueueDataJob.java

-                            REDIS_EVENT_ISSUE_STORE, message, ChatMessageStatus.WAITING, score);
+                            9, message, ChatMessageStatus.WAITING, score);


reRegisterQueue에서 REDIS_EVENT_ISSUE_STORE를 9로 바꾸신 이유가 무엇일까요??

reReigsterQueue 메소드의 parameter의 1번째는 String key여서 정수를 넣으면 빌드에러가 뜨는데 수정하셔야 할 것 같아요!

오잉... 팻 핑거 이슈입니다. 수정할게요.

Kwon770

로직에 문제가 없다면, 코멘트 주신 부분을 수정하고 테스트 코드를 추가 후 다시 리뷰 요청하겠습니다.

Kwon770 · 2024-10-24T08:04:44Z

Ticket-Batch/src/main/java/com/jnu/ticketbatch/config/ProcessQueueDataJob.java

-                            REDIS_EVENT_ISSUE_STORE, message, ChatMessageStatus.WAITING, score);
+                            9, message, ChatMessageStatus.WAITING, score);


오잉... 팻 핑거 이슈입니다. 수정할게요.

kssumin

Transaction Outbox Pattern이라는 새로운 방법을 알게되었습니다!역시 멋져요!!

꼭 반영 안 하셔도 됩니다!! 그냥 제 의견을 드립니다😀

kssumin · 2024-10-29T02:19:20Z

...et-Domain/src/main/java/com/jnu/ticketdomain/domains/registration/domain/TransferStatus.java

+package com.jnu.ticketdomain.domains.registration.domain;
+
+public enum TransferStatus {
+    // 이메일 전송 대상의 조회 정렬 조건이므로, 순서를 변경하지 마세요.


public TransferStatus getNextStatus() { return switch (this) { case PENDING -> FAILED_1; case FAILED_1 -> FAILED_2; case FAILED_2 -> FAILED_3; case FAILED_3, EXCLUDED, SUCCESS -> EXCLUDED; }; }

이런식으로 상태 전이 로직을 TransferStatus enum 안으로 이동시켜 응집도를 높이는 게 어떤가요??
그럼 순서 변경 이슈, 상태 전이를 잘못하는 이슈를 방지할 수 있을 것 같아요!

kssumin · 2024-10-29T02:20:06Z

.../src/main/java/com/jnu/ticketdomain/domains/registration/domain/RegistrationResultEmail.java

+            this.transferStatus = TransferStatus.SUCCESS;
+
+        } else {
+            switch (this.transferStatus) {


public void updateTransferResult(boolean isSuccess) { this.transferStatus = isSuccess ? TransferStatus.SUCCESS : this.transferStatus.getNextStatus(); }

상태 전이 로직을 TransferStatus로 이동한다면 이렇게 할 수 있을 것 같아요!

불린이면 이게 더 깔끔한걸지도 ㅇㅇ..

pyg410

굿굿 고생하셨슴다.
그냥 궁금한건데, Transactional Outbox Pattern을 단일 서버, 단일 DB에 Polling까지 같이 구현하는게 정배인가요??(진짜 모름)

제가 생각하기에 해당 패턴사용 이유는 메인 트랜잭션과 부가적인 기능의 완전한 독립 + 각 작업에 대한 신뢰성 보장이라고 느껴지는데
지금 상황에서는 메인트랜잭션과 부가적인 기능의 완전한 독립이라기에는 같은 DB의 물리적 리소스를 사용하고 있고 커넥션 부족해서 장애가 발생할 가능성도 있다고 생각이 들었슴다.
딱 2PC의 크리티컬한 단점만 커버한 느낌이 들었슴다. 아니라면 죄송함다
차라리 SOA나 MSA같은 느낌으로 별도의 서비스, db로 빼는게 낫지 않을까 조심스레 생각해봅니다

Execute 메서드에서는 for문 안에 update문이 있는 로직으로 작성되었더라구요
살짝 걱정되는 부분은 해당 로직이 예를들어 300건의 이메일을 보내야 하는 경우 보내야하는 이메일 개수와 선형적으로 비례해서 connection을 사용하게되는건데 음 이렇게 보낸다면 차라리 서버까지는 하나 더 못두더라도, 간단한 sqlite같은 경량 db하나 두고 별도의 connection을 사용하게 하는게 더 낫지않을까.. 라는 생각이 들었슴다.
별도의 db구성이 힘들다면 bulk update라두..

그냥 참고만해주시고 수정하실필요없이 반영하시면 될것같슴다

BlackBean99

수고하셨습니다.

BlackBean99 · 2024-12-02T13:12:18Z

Ticket-Batch/src/main/java/com/jnu/ticketbatch/config/RegistrationResultEmailJob.java

+        } catch (Exception e) {
+            log.error("RegistrationResultEmailJob Exception: {}", e.getMessage());
+        }


for 내에서 이메일을 날리다 실패한 경우는 검증 후 outbox에 다시 넣는것은 어떤가요? // 이메일 발송을 실패한 원인이 메일 서버 한계때문일수도 있어서요.

BlackBean99 · 2024-12-02T13:12:56Z

Ticket-Batch/src/main/java/com/jnu/ticketbatch/job/EventRegisterJob.java

+    public static final String EVENT_ID = "eventId";
+    public static final String GROUP = "group1";
+    public static final String ASIA_SEOUL = "Asia/Seoul";


public static으로 선언할거면 constant 클래스에 이어서 넣으면 통일성이 있을지도?

BlackBean99 · 2024-12-02T13:13:57Z

.../src/main/java/com/jnu/ticketdomain/domains/registration/domain/RegistrationResultEmail.java

+            this.transferStatus = TransferStatus.SUCCESS;
+
+        } else {
+            switch (this.transferStatus) {


불린이면 이게 더 깔끔한걸지도 ㅇㅇ..

BlackBean99 · 2024-12-02T13:19:37Z

Ticket-Domain/src/main/resources/application-domain.yml

+    hibernate:
+      ddl-auto: create-drop


이거 주석 안치고 이렇게 해도 되나요?

BlackBean99 · 2024-12-02T13:20:12Z

...icketdomain/domains/registration/repository/RegistrationResultEmailRepositoryCustomImpl.java

+        return queryFactory
+                .selectFrom(registrationResultEmail)
+                .where(
+                        registrationResultEmail.eventId.eq(eventId),
+                        registrationResultEmail.transferStatus.in(
+                                TransferStatus.PENDING,
+                                TransferStatus.FAILED_1,
+                                TransferStatus.FAILED_2,
+                                TransferStatus.FAILED_3))
+                .orderBy(
+                        registrationResultEmail.transferStatus.asc(),
+                        registrationResultEmail.emailId.asc())
+                .limit(fetchSize) // PENDING -> FAILED 순으로 정렬 (transferStatus는 EnumType.ORDINAL)
+                .fetch();


이런 코드 스타일 오랜만에 보내요. dsl을 쓰고싶은데 참으신건가요 풉킥풉킥

Kwon770 added 6 commits October 17, 2024 23:13

feat: Declare registrationResultEmailJob and append trigger at EventC…

88fc36c

…reationEventHandler and EventUpdatedEventHandler

feat: Impl RegistrationResultEmailJob execute and Add RegistarationRe…

8c3e909

…sultEmailOutbox domain

feat: Rename RegistrationResultEmailOutbox domain to RegistrationResu…

a3bab2c

…ltEmail and Add eventId in domain And Impl adaptor

feat: Add detailed step of TransferStatus to exclude email outbox tha…

e0d7872

…t failed transfer 3 times

feat: add deleting job code by cancelScheduledJob() of EventUpdateJob

6a8b391

feat: Add the code saving RegistrationResultEmail in UserReflectStatu…

bfaaa1d

…sEventHandler to trigger email outbox and Add eventId needed to save RegistrationResultEmail in UserReflectStatusEvent

Kwon770 added 기능 새로운 기능 추가, 가벼운 수정 리팩터링 코드 퀄리티 개선을 위한 작업 성능개선 성능 개선 관련 작업 labels Oct 19, 2024

Kwon770 requested review from BlackBean99, pyg410, kssumin and LJH098 October 19, 2024 02:57

Kwon770 self-assigned this Oct 19, 2024

Kwon770 added 3 commits October 19, 2024 12:02

refactor: Change email_id column type to CHAR(26), which is the type …

64e0f4a

…of ULID

chore: Add comment in TransferStatis ENUM

2e297e8

refactor: Change RequiredArgsConstructor DI to Autowired for the fiel…

72b6e2e

…ds of RegistrationResultEmailJob

LJH098 requested changes Oct 24, 2024

View reviewed changes

Kwon770 commented Oct 24, 2024

View reviewed changes

kssumin approved these changes Oct 29, 2024

View reviewed changes

fix: lint and fix fat finger issue

411b77c

pyg410 approved these changes Nov 13, 2024

View reviewed changes

BlackBean99 approved these changes Dec 2, 2024

View reviewed changes

		REDIS_EVENT_ISSUE_STORE, message, ChatMessageStatus.WAITING, score);
		9, message, ChatMessageStatus.WAITING, score);

[BE-196] feat: Transactional Outbox Pattern을 이용해 주차권 결과 이메일을 신뢰성있게 준실시간으로 발송되도록 로직 개선 #403

Are you sure you want to change the base?

[BE-196] feat: Transactional Outbox Pattern을 이용해 주차권 결과 이메일을 신뢰성있게 준실시간으로 발송되도록 로직 개선 #403

Uh oh!

Conversation

Kwon770 commented Oct 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

주요 변경사항

리뷰어에게...

관련 이슈

체크리스트

Uh oh!

LJH098 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kwon770 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kssumin left a comment

Choose a reason for hiding this comment

Uh oh!

kssumin Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kssumin Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pyg410 left a comment

Choose a reason for hiding this comment

Uh oh!

BlackBean99 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kwon770 commented Oct 19, 2024 •

edited

Loading

kssumin Oct 29, 2024 •

edited

Loading

kssumin Oct 29, 2024 •

edited

Loading