[YS-302] refactor: 이메일 전송 최적화 #93
Conversation
- 이메일 전송 비동기 처리를 위한 코루틴 라이브러리 추가
- CoroutineDispatcherProvider 및 TransactionExecutor 인터페이스 추가 - 공통 인터페이스 분리 → 의존성 줄이고, 유지보수 용이
layer - CoroutineConfig, CoroutineDispatcherProviderImpl, TransactionExecutor 추가 - application 계층에서 비동기 실행 및 트랜잭션 쉽게 활용할 수 있도록 구현
- EmailGatewayImpl에서 비동기 메일 전송 처리 적용 - SendMatchingEmailJob에서 코루틴 기반 비동기 실행 적용 - 불필요한 블로킹 코드 제거하여 성능 최적화
- EmailService에서 비동기 트랜잭션 실행 방식 개선 - SendEmailUseCase 및 SendMatchingEmailUseCase에서 트랜잭션 적용 방식 변경 - 기존 블로킹 방식에서 완전한 비동기 실행 → API 응답 속도 향상
- EmailController에서 비동기 트랜잭션 실행 방식 반영
Walkthrough이번 변경 사항은 프로젝트에 코루틴 기반 비동기 처리 방식을 도입하는 업데이트입니다. 새로운 Gradle 의존성 추가와 함께 코루틴 디스패처, 트랜잭션 실행, 비동기 유스케이스를 위한 인터페이스 및 구현체들이 추가되었습니다. EmailService, UseCase, EmailGateway, 스케줄러, 그리고 컨트롤러 등 다양한 컴포넌트의 메서드들이 동기 코드에서 suspend 함수로 전환되어 코루틴 컨텍스트에서 실행되도록 개선되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as EmailController
participant Service as EmailService
participant Executor as TransactionExecutor
participant UseCase as SendEmailCodeUseCase
participant Gateway as EmailGateway
Controller->>Service: sendCode(request)
Service->>Executor: execute { suspend block }
Executor->>UseCase: execute(input) in coroutine
UseCase->>Gateway: sendEmail(to, subject, content)
Gateway-->>UseCase: email result
UseCase-->>Executor: result
Executor-->>Service: result
Service-->>Controller: response
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/main/kotlin/com/dobby/backend/presentation/api/controller/EmailController.kt (1)
28-33: 🛠️ Refactor suggestion이메일 전송 API의 비동기 처리가 일관되지 않습니다.
sendCode메서드는 비동기로 처리되도록 변경되었지만,verifyCode메서드는 여전히 동기 방식으로 남아있습니다. 일관성 있는 비동기 처리를 위해verifyCode메서드도 suspend 함수로 변경하는 것이 좋습니다.다음과 같이 수정을 제안합니다:
- fun verifyCode(@RequestBody @Valid emailVerificationRequest: EmailVerificationRequest) + suspend fun verifyCode(@RequestBody @Valid emailVerificationRequest: EmailVerificationRequest)
🧹 Nitpick comments (11)
src/main/kotlin/com/dobby/backend/application/service/EmailService.kt (3)
22-22: LifeCycle 고려가 필요한 CoroutineScope
Service 클래스가 Spring Bean으로 동작할 경우, 클래스 레벨에서SupervisorJob과 함께 스코프를 생성하면 Bean 해제 시점에 별도의 스코프 종료 처리가 없어 누수(leak)가 발생할 수 있습니다. 스코프 종료 시점을 명시적으로 관리하거나 상위 레벨의 스코프를 주입받는 방법을 고려해주세요.
23-28: 비동기 처리 방식의 단순화 가능성
async { ... }.await()패턴은 결국 결과를 대기한 뒤 반환하고 있으므로, 동시에 다른 로직을 진행할 목적이 아니라면withContext(dispatcherProvider.io)를 사용하는 방법도 고려해볼 수 있습니다. 코드 가독성과 유지보수에 도움이 될 수 있습니다.
36-41: 비동기 처리 방식의 단순화 가능성
async/await가 필수적이지 않다면,withContext(dispatcherProvider.io)로 직관적으로 처리하는 것이 가독성과 유지보수에 더 유리할 수 있습니다.src/main/kotlin/com/dobby/backend/application/service/TransactionExecutor.kt (1)
3-5: 트랜잭션 실행을 위한 깔끔한 인터페이스 설계입니다!제네릭 타입과 suspend 람다를 활용하여 유연한 트랜잭션 처리가 가능하도록 잘 구현되어 있습니다.
트랜잭션 타임아웃 설정이나 재시도 정책을 추가하는 것을 고려해보시면 좋을 것 같습니다. 필요하시다면 구현 예시를 제공해드릴 수 있습니다.
src/main/kotlin/com/dobby/backend/domain/gateway/email/EmailGateway.kt (1)
4-4: 이메일 게이트웨이의 비동기 처리가 잘 구현되었습니다!suspend 키워드를 통해 비동기 처리가 잘 적용되어 있습니다.
다음과 같이 예외 처리를 위한 sealed 클래스 추가를 고려해보시면 좋을 것 같습니다:
sealed class EmailResult { object Success : EmailResult() data class Failure(val error: Exception) : EmailResult() } interface EmailGateway { suspend fun sendEmail(to: String, subject: String, content: String): EmailResult }src/main/kotlin/com/dobby/backend/application/service/CoroutineDispatcherProvider.kt (1)
5-8: 디스패처 제공을 위한 깔끔한 인터페이스 설계입니다!IO와 기본 디스패처를 명확하게 분리하여 정의하였습니다.
Main 디스패처 추가를 고려해보시면 좋을 것 같습니다:
interface CoroutineDispatcherProvider { val io: CoroutineDispatcher val default: CoroutineDispatcher val main: CoroutineDispatcher // UI 관련 작업을 위한 디스패처 }src/main/kotlin/com/dobby/backend/infrastructure/config/CoroutineConfig.kt (1)
11-13: 코루틴 스코프 빈 설정이 적절합니다.SupervisorJob을 사용한 코루틴 스코프 설정이 적절하며, 자식 코루틴의 실패가 다른 코루틴에 영향을 미치지 않도록 잘 구성되어 있습니다.
빈의 이름을 더 구체적으로 지정하는 것을 고려해보세요:
@Bean - fun coroutineScope(): CoroutineScope { + fun applicationCoroutineScope(): CoroutineScope { return CoroutineScope(SupervisorJob()) }src/main/kotlin/com/dobby/backend/infrastructure/coroutine/CoroutineDispatcherProviderImpl.kt (1)
8-12: 디스패처 구현이 깔끔합니다.클린 아키텍처 원칙을 잘 따르고 있으며, 의존성 역전 원칙이 잘 적용되어 있습니다.
각 디스패처의 용도에 대한 문서화를 추가하면 좋을 것 같습니다:
@Component class CoroutineDispatcherProviderImpl : CoroutineDispatcherProvider { + /** + * IO 디스패처: 파일 I/O, 네트워크 작업, 데이터베이스 작업 등 + * 블로킹 작업에 최적화된 디스패처입니다. + */ override val io: CoroutineDispatcher = Dispatchers.IO + /** + * Default 디스패처: CPU 집약적인 작업에 최적화된 + * 디스패처입니다. + */ override val default: CoroutineDispatcher = Dispatchers.Default }src/main/kotlin/com/dobby/backend/infrastructure/gateway/email/EmailGatewayImpl.kt (2)
24-28: 예외 처리와 로깅을 개선하면 좋겠습니다.현재 구현은 모든 예외를 IllegalStateException으로 변환하고 있어, 구체적인 실패 원인을 파악하기 어렵습니다.
다음과 같이 개선하는 것을 제안드립니다:
+ private val log = LoggerFactory.getLogger(javaClass) + try { mailSender.send(message) + log.info("이메일 발송 성공: to={}, subject={}", to, subject) } catch (ex: Exception) { + log.error("이메일 발송 실패: to={}, subject={}", to, subject, ex) throw IllegalStateException("이메일 발송 실패: ${ex.message}") }
17-19: 성능 측정을 위한 메트릭 추가를 고려해보세요.PR 목표가 이메일 전송 최적화인 만큼, 성능 측정을 위한 메트릭 추가가 도움이 될 것 같습니다.
다음과 같이 메트릭을 추가하는 것을 고려해보세요:
override suspend fun sendEmail(to: String, subject: String, content: String) { + val startTime = System.currentTimeMillis() withContext(Dispatchers.IO) { // ... existing code ... + log.info("이메일 발송 소요 시간: {}ms", System.currentTimeMillis() - startTime) } }src/main/kotlin/com/dobby/backend/infrastructure/scheduler/SendMatchingEmailJob.kt (1)
42-54: 에러 처리 로직을 개선할 수 있습니다.현재 에러 처리가 각각의 이메일 전송에 대해 개별적으로 이루어지고 있습니다. 실패한 케이스에 대한 재시도 메커니즘이 없어 중요한 이메일이 전송되지 않을 수 있습니다.
다음과 같은 재시도 로직 추가를 제안합니다:
private suspend fun sendEmailWithRetry( emailInput: SendMatchingEmailUseCase.Input, maxRetries: Int = 3 ): Boolean { repeat(maxRetries) { attempt -> try { val emailOutput = emailService.sendMatchingEmail(emailInput) if (emailOutput.isSuccess) return true delay(attempt * 1000L) // 지수 백오프 } catch (e: Exception) { logger.error("Attempt ${attempt + 1} failed for ${emailInput.contactEmail}", e) } } return false }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
build.gradle.kts(1 hunks)src/main/kotlin/com/dobby/backend/application/service/CoroutineDispatcherProvider.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/service/EmailService.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/service/TransactionExecutor.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/AsyncUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt(3 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCase.kt(3 hunks)src/main/kotlin/com/dobby/backend/domain/gateway/email/EmailGateway.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/config/CoroutineConfig.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/coroutine/CoroutineDispatcherProviderImpl.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/gateway/email/EmailGatewayImpl.kt(2 hunks)src/main/kotlin/com/dobby/backend/infrastructure/scheduler/SendMatchingEmailJob.kt(2 hunks)src/main/kotlin/com/dobby/backend/presentation/api/controller/EmailController.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (11)
src/main/kotlin/com/dobby/backend/application/service/EmailService.kt (2)
8-10: 코루틴 사용을 위한 import 추가
코루틴 기능을 활용하기 위한 관련 API를 적절하게 import하셨습니다.
18-20: 추가된 의존성을 통한 구조 개선
Clean Architecture 원칙에 따라 트랜잭션 로직과 코루틴 디스패처를 분리하는 의존성 주입이 잘 이뤄졌습니다. 유지보수성 향상에 도움이 됩니다.src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCase.kt (3)
3-3: 비동기 UseCase를 위한 인터페이스 import
AsyncUseCase를 사용하기 위한 import가 정상적으로 추가되었습니다.
12-22: 클래스 어노테이션 및 비동기 UseCase 적용
@Component로 Bean 등록과 동시에AsyncUseCase를 구현하여, 비동기 실행 모델을 유스케이스 계층에 잘 반영하셨습니다.
35-35: 비동기 실행을 위한 suspend 키워드 도입
suspend함수 선언으로 코루틴 문맥 내에서 동작할 수 있게 변경된 부분이 적절해 보입니다.src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt (5)
3-5: 비동기 처리를 위한 import 추가
코루틴 디스패처, 트랜잭션 실행, 그리고AsyncUseCase를 사용하기 위한 필수 import입니다. 문제가 없어 보입니다.
13-16: 코루틴 관련 import 및 @component 적용
코루틴 함수(launch,withContext)와 Spring 빈 등록을 위한 어노테이션이 추가되어, 비동기 로직과 스프링 관리를 모두 지원하도록 구성되었습니다.
18-25: 클래스 시그니처 변경: 비동기 UseCase 채택
AsyncUseCase를 구현하면서, 트랜잭션 처리와 식별자 생성 로직까지 주입받도록 구조를 개선하셨습니다. Clean Architecture 관점에서 의존성 분리가 잘 이뤄졌습니다.
34-34: 비동기 로직을 위한 suspend 메서드
suspend fun execute로 변경되어, CoroutineScope에서 안전하게 호출할 수 있습니다.
79-79: 비동기 이메일 전송을 위한 suspend 메서드
sendVerificationEmail메서드에suspend를 적용하여, 비동기 흐름에 맞춰 안전하게 이메일 전송이 가능하도록 변경되었습니다.src/main/kotlin/com/dobby/backend/application/usecase/AsyncUseCase.kt (1)
3-5: 깔끔한 비동기 유스케이스 인터페이스 구현입니다!함수형 인터페이스를 사용하여 간결하게 정의되었고, suspend 키워드를 통해 코루틴 지원이 잘 구현되어 있습니다.
| CoroutineScope(dispatcherProvider.io).launch { | ||
| transactionExecutor.execute { | ||
| reflectVerification(input, code) | ||
| } | ||
| } | ||
| CoroutineScope(dispatcherProvider.io).launch { | ||
| sendVerificationEmail(input.univEmail, code) | ||
| } | ||
|
|
There was a problem hiding this comment.
병렬 실행 시 예외 처리 누락 가능
두 개의 launch 블록에서 각각 트랜잭션 반영과 이메일 전송을 병렬로 실행하지만, 함수는 즉시 성공 응답을 반환하므로 트랜잭션 실패나 이메일 전송 실패를 반영하지 못할 위험이 있습니다. 아래와 같이 순차 실행 혹은 예외 처리를 한 곳에서 제어하는 방식으로 개선을 고려해주세요.
@@ -39,18 +39,12 @@
CoroutineScope(dispatcherProvider.io).launch {
transactionExecutor.execute {
reflectVerification(input, code)
}
}
CoroutineScope(dispatcherProvider.io).launch {
sendVerificationEmail(input.univEmail, code)
}
-return Output(
- isSuccess = true,
- message = "학교 이메일로 코드가 전송되었습니다..."
-)
+// 제안: 트랜잭션과 이메일 전송에 대한 예외 처리를 함께 관리하기 위해 withContext로 묶어서 반환:
+return withContext(dispatcherProvider.io) {
+ transactionExecutor.execute { reflectVerification(input, code) }
+ sendVerificationEmail(input.univEmail, code)
+ Output(
+ isSuccess = true,
+ message = "학교 이메일로 코드가 전송되었습니다. 10분 이내로 인증을 완료해주세요."
+ )
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CoroutineScope(dispatcherProvider.io).launch { | |
| transactionExecutor.execute { | |
| reflectVerification(input, code) | |
| } | |
| } | |
| CoroutineScope(dispatcherProvider.io).launch { | |
| sendVerificationEmail(input.univEmail, code) | |
| } | |
| // 제안: 트랜잭션과 이메일 전송에 대한 예외 처리를 함께 관리하기 위해 withContext로 묶어서 반환: | |
| return withContext(dispatcherProvider.io) { | |
| transactionExecutor.execute { reflectVerification(input, code) } | |
| sendVerificationEmail(input.univEmail, code) | |
| Output( | |
| isSuccess = true, | |
| message = "학교 이메일로 코드가 전송되었습니다. 10분 이내로 인증을 완료해주세요." | |
| ) | |
| } |
There was a problem hiding this comment.
래빗아, API 응답 속도를 개선하기 위해 일단 API 성공응답은 빠르게 내려주고 그 다음에 실행하는 구조인데 어떻게 생각해!
There was a problem hiding this comment.
@chock-cho 아, 이제 이해했습니다! API 응답 속도를 98.66% 개선한 좋은 접근 방식이네요. 이메일 전송과 같은 비동기 작업은 즉시 응답을 반환하고 백그라운드에서 처리하는 것이 사용자 경험을 향상시킬 수 있죠. 제가 제안했던 방식은 이런 성능 최적화 이점을 상쇄할 수 있었을 것 같네요.
(｀･ω･´)ゞ
✏️ Learnings added
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt:39-47
Timestamp: 2025-02-12T01:54:34.246Z
Learning: In the email verification API, immediate response is prioritized by executing non-critical background tasks (transaction and email sending) after returning the API response, which significantly improves response times and user experience.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
CoroutineScope 를 사용해 이메일 전송을 비동기로 실행하여 API 응답 속도를 빠르게 유지한 점 좋습니다! 👍
다만, 빠른 응답을 내려주기 위해 트랜잭션과 이메일 전송이 각기 다른 CoroutineScope.launch {}에서 실행되고 있는데 제가 코루틴에 대한 이해는 부족하지만, 현재 코드에서 트랜잭션이 롤백되더라도 이메일이 전송될 가능성이 있지 않을까? 하는 걱정이 드네요. 🤔
만약 트랜잭션이 실패해서 DB 업데이트가 롤백되었는데도 이메일 전송이 이미 실행됐다면, 사용자는 인증 코드를 받았지만, 정작 DB에는 저장되지 않은 상태가 될 것 같습니다. 이렇게 되면 사용자가 받은 코드로 인증을 진행할 수 없어서 문제가 될 수도 있을 것 같은데, 혹시 이런 문제에 대해 방지가 되어있을까요??
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
좋은 의견 감사합니다! 그 생각을 못했었군요 🤔🤔
지금은 그냥 트랜잭션이 플러시되든 롤백되든 API 응답을 빨리 내려준다는 것에만 초점을 맞춘 것 같아요.
트랜잭션의 결과와 상관없이 API 응답을 내려주어야 한다는 점은 속도 개선을 위해 어쩔 수 없지만, 지수님께서 말씀하신 우려점에 저도 동의해서, 트랜잭션 롤백 시에 최대 3번의 백오프+재시도 로직을 추가하는 방법을 고려해보려고 하는데요...!
아무래도 현재는 SMTP라 실패확률이 비교적 높지만, AWS SES로 전환시에는 실패확률이 낮고 찾아보니 AWS SDK 내부적으로 재시도 로직이 탑재돼있는 부분이 있다고 하네요.
지수님의 의견을 듣고 싶습니다. 🙋♀️
There was a problem hiding this comment.
@chock-cho 네, 좋습니다! 저도 아까 코멘트를 남기면서 이 문제를 해결하려면 트랜잭션과 이메일 전송을 하나의 코루틴으로 묶어야 하는데, 그렇게 하면 결국 API 응답 속도를 빠르게 하려던 원래 목적을 달성하기 어려워질 것 같다고 생각했어요.
현재는 내부적으로 SMTP를 사용해서 이메일을 전송하는 구조라 어쩔 수 없는 딜레마일 수도 있지만, AWS SES로 전환했을 때 말씀해주신 백오프 + 재시도 로직이 들어가면 안정성이 더 높아질 것 같네요! 이 부분도 고려하면서 점진적으로 개선해 나가면 좋을 것 같습니다. 😊
| override suspend fun <T> execute(block: suspend () -> T): T { | ||
| return withContext(Dispatchers.IO) { | ||
| val deferred = coroutineScope.async { | ||
| transactionTemplate.execute { | ||
| runBlocking { block() } | ||
| } ?: throw RuntimeException("Transaction Failed") | ||
| } | ||
| deferred.await() | ||
| } | ||
| } |
There was a problem hiding this comment.
트랜잭션 실행 방식의 개선이 필요합니다.
현재 구현에는 몇 가지 중요한 문제가 있습니다:
async블록 내부에서runBlocking을 사용하면 비동기 실행의 이점이 사라집니다.- 스레드 풀 고갈의 위험이 있습니다.
- 데드락이 발생할 수 있습니다.
다음과 같이 수정하는 것을 추천드립니다:
override suspend fun <T> execute(block: suspend () -> T): T {
return withContext(Dispatchers.IO) {
val deferred = coroutineScope.async {
transactionTemplate.execute {
- runBlocking { block() }
+ runCatching { block() }.getOrNull()
} ?: throw RuntimeException("Transaction Failed")
}
deferred.await()
}
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
오잉 요렇게 되면 어떻게 되는건데? 블로킹을 실행하는 이유는 트랜잭션의 실행 보장을 위해서야.
There was a problem hiding this comment.
@chock-cho 트랜잭션의 실행은 runBlocking 없이도 보장됩니다. transactionTemplate.execute가 트랜잭션 경계를 제공하고, suspend 함수인 block()은 트랜잭션 컨텍스트 내에서 안전하게 실행됩니다.
runBlocking을 사용하면 오히려 다음과 같은 문제가 발생할 수 있습니다:
- 스레드가 블로킹되어 코루틴의 장점이 사라집니다
- async 블록 내부에서 블로킹하면 데드락이 발생할 수 있습니다
- 스레드 리소스가 낭비됩니다
트랜잭션이 실패하면 여전히 RuntimeException을 던져서 롤백이 보장됩니다.
🐰 (curious about the transaction execution approach)
✏️ Learnings added
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt:15-24
Timestamp: 2025-02-12T01:55:08.529Z
Learning: In Kotlin coroutines, transaction execution can be safely performed without `runBlocking`. The `transactionTemplate.execute` provides transaction boundaries, and suspend functions can be executed directly within the transaction context. Using `runBlocking` inside `async` blocks can lead to deadlocks and thread pool exhaustion.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
래빗이 네가 제안한 방식은 그럼 어떻게 되는건데?
개선안점을 before-after 방식으로 이야기해서 나를 설득해봐!
There was a problem hiding this comment.
@chock-cho 현재 방식과 제안된 방식을 비교해보겠습니다:
Before (현재):
coroutineScope.async {
transactionTemplate.execute {
runBlocking { block() } // 스레드 블로킹
}
}- 트랜잭션 내부에서 스레드가 블로킹됨
- 다른 코루틴들이 실행을 기다려야 함
- 스레드 풀이 빠르게 소진될 수 있음
- 데드락 위험: async 내부의 runBlocking이 같은 스레드를 기다림
After (제안):
coroutineScope.async {
transactionTemplate.execute {
block() // non-blocking 실행
}
}- 트랜잭션 경계는 여전히
transactionTemplate이 보장 - 스레드 블로킹 없이 비동기 실행
- 다른 코루틴들이 자유롭게 실행 가능
- 스레드 풀 효율적 사용
- 데드락 위험 없음
두 방식 모두 트랜잭션은 보장되지만, 제안된 방식이 코루틴의 장점을 더 잘 활용합니다.
🐰 (explaining the technical details with enthusiasm)
🧠 Learnings used
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt:15-24
Timestamp: 2025-02-12T01:55:08.749Z
Learning: In Kotlin coroutines, transaction execution can be safely performed without `runBlocking`. The `transactionTemplate.execute` provides transaction boundaries, and suspend functions can be executed directly within the transaction context. Using `runBlocking` inside `async` blocks can lead to deadlocks and thread pool exhaustion.
There was a problem hiding this comment.
@chock-cho 죄송합니다. 제가 제안한 방식에서 컴파일 에러가 발생하는 이유를 찾았습니다. TransactionTemplate이 일반 람다만 받을 수 있어서 suspend 함수를 직접 호출할 수 없네요.
대신 Spring의 TransactionalOperator를 사용하면 코루틴과 함께 트랜잭션을 자연스럽게 사용할 수 있습니다:
@Component
class TransactionCoroutineExecutor(
private val transactionalOperator: TransactionalOperator,
private val coroutineScope: CoroutineScope
): TransactionExecutor {
override suspend fun <T> execute(block: suspend () -> T): T {
return withContext(Dispatchers.IO) {
transactionalOperator.transactional(Mono.defer {
coroutineScope.async { block() }.asMono()
}).awaitSingle()
}
}
}이 방식의 장점:
runBlocking없이 suspend 함수 실행 가능- 트랜잭션 경계 보장
- 데드락 위험 없음
- 스레드 풀 효율적 사용
필요한 의존성:
implementation("org.springframework:spring-tx")
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-reactor")어떠신가요? 🤔
✏️ Learnings added
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt:15-24
Timestamp: 2025-02-12T04:05:51.738Z
Learning: When using Kotlin coroutines with Spring transactions, `TransactionalOperator` should be used instead of `TransactionTemplate`. `TransactionalOperator` provides native coroutine support through the `kotlinx-coroutines-reactor` bridge, allowing suspend functions to be executed within transaction boundaries without `runBlocking`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt:15-24
Timestamp: 2025-02-12T01:55:08.749Z
Learning: In Kotlin coroutines, transaction execution can be safely performed without `runBlocking`. The `transactionTemplate.execute` provides transaction boundaries, and suspend functions can be executed directly within the transaction context. Using `runBlocking` inside `async` blocks can lead to deadlocks and thread pool exhaustion.
There was a problem hiding this comment.
음 근데 래빗아. 우리 비즈니스 규칙과 규모에서 리액티브 프로그래밍까지 도입해야해?🤨
나는 회의적이야... 우리가 필요한건 사실
- 이메일 인증 코드 전송
- 매일 8시에 실행되는 bulk로 추천공고 모아서 전송
이거밖에없는데 리액티브 프로그래밍까지 도입하면 오버엔지니어링 아니야?
There was a problem hiding this comment.
@chock-cho 네, 말씀하신 대로입니다! 제가 제안했던 리액티브 접근방식은 현재 요구사항에 비해 과도한 것 같네요.
간단한 해결책을 제안드립니다:
@Component
class TransactionCoroutineExecutor(
private val transactionTemplate: TransactionTemplate,
private val coroutineScope: CoroutineScope
): TransactionExecutor {
override suspend fun <T> execute(block: suspend () -> T): T {
return withContext(Dispatchers.IO) {
suspendCancellableCoroutine { continuation ->
transactionTemplate.execute {
coroutineScope.launch {
continuation.resume(block())
}
null // transaction result will be provided through continuation
}
}
}
}
}이 방식은:
- 리액티브 의존성 없이 순수 코루틴만 사용
- 트랜잭션 경계 보장
- 비동기 이메일 전송 요구사항 충족
- 코드 복잡도 최소화
🐰 (thoughtfully considering the simpler approach)
⛔ Learnings removed
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt:15-24
Timestamp: 2025-02-12T04:05:51.877Z
Learning: When using Kotlin coroutines with Spring transactions, `TransactionalOperator` should be used instead of `TransactionTemplate`. `TransactionalOperator` provides native coroutine support through the `kotlinx-coroutines-reactor` bridge, allowing suspend functions to be executed within transaction boundaries without `runBlocking`.
🧠 Learnings used
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt:15-24
Timestamp: 2025-02-12T04:05:51.877Z
Learning: When using Kotlin coroutines with Spring transactions, `TransactionalOperator` should be used instead of `TransactionTemplate`. `TransactionalOperator` provides native coroutine support through the `kotlinx-coroutines-reactor` bridge, allowing suspend functions to be executed within transaction boundaries without `runBlocking`.
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt:15-24
Timestamp: 2025-02-12T01:55:08.749Z
Learning: In Kotlin coroutines, transaction execution can be safely performed without `runBlocking`. The `transactionTemplate.execute` provides transaction boundaries, and suspend functions can be executed directly within the transaction context. Using `runBlocking` inside `async` blocks can lead to deadlocks and thread pool exhaustion.
There was a problem hiding this comment.
네가 제안한 대로 하면, 트랜잭션이 비동기 작업과 함께 처리되는 시점에 발생하는 문제가 발생하잖아. 문제는 트랜잭션 경계 내에서 비동기 작업을 실행하려는 시도에서 발생하는 것인데, 트랜잭션이 아직 활성화되지 않았을 때 비동기 작업이 실행되잖아?
내 생각에는 트랜잭션을 블로킹 방식으로 유지하면서, 트랜잭션 경계 내에서 비동기 작업이 안전하게 실행되도록 하는 거 어떻니? 즉, 트랜잭션이 완료되기 전에 비동기 작업이 실행되지 않도록 해야 하는게 맞지 않니? 래빗이의 제안은 다소 위험한 부분이 있어.
There was a problem hiding this comment.
@chock-cho 네, 말씀하신 대로입니다. 제가 너무 non-blocking 실행에만 초점을 맞추다 보니 트랜잭션 안전성이라는 더 중요한 부분을 간과했네요.
현재 구현의 runBlocking은 트랜잭션 경계를 확실히 보장한다는 장점이 있습니다:
transactionTemplate.execute {
runBlocking { block() } // 트랜잭션이 활성화된 상태에서 실행 보장
}제가 제안했던 방식들은 트랜잭션 컨텍스트가 비동기 작업으로 전파되지 않을 위험이 있었네요. 현재 요구사항에서는 트랜잭션 안전성이 더 중요하므로, 기존 구현을 유지하는 것이 더 나을 것 같습니다.
✏️ Learnings added
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt:15-24
Timestamp: 2025-02-12T04:30:30.678Z
Learning: In Spring transactions with Kotlin coroutines, using `runBlocking` within `transactionTemplate.execute` ensures that the transaction context is active during the execution of suspend functions. While this approach blocks the thread, it guarantees transaction safety by preventing async operations from executing outside the transaction boundary.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt:15-24
Timestamp: 2025-02-12T01:55:08.749Z
Learning: In Kotlin coroutines, transaction execution can be safely performed without `runBlocking`. The `transactionTemplate.execute` provides transaction boundaries, and suspend functions can be executed directly within the transaction context. Using `runBlocking` inside `async` blocks can lead to deadlocks and thread pool exhaustion.
🧠 Learnings used
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/coroutine/TransactionCoroutineExecutor.kt:15-24
Timestamp: 2025-02-12T01:55:08.749Z
Learning: In Kotlin coroutines, transaction execution can be safely performed without `runBlocking`. The `transactionTemplate.execute` provides transaction boundaries, and suspend functions can be executed directly within the transaction context. Using `runBlocking` inside `async` blocks can lead to deadlocks and thread pool exhaustion.
| override fun execute(context: JobExecutionContext) { | ||
| logger.info("BulkSendMatchingEmailJob started at ${LocalDateTime.now()}") | ||
| runBlocking { | ||
| logger.info("BulkSendMatchingEmailJob started at ${LocalDateTime.now()}") | ||
|
|
||
| val input = GetMatchingExperimentPostsUseCase.Input( | ||
| requestTime = LocalDateTime.now() | ||
| ) | ||
| val output = emailService.getMatchingInfo(input) | ||
| val matchingExperimentPosts = output.matchingPosts | ||
| val input = GetMatchingExperimentPostsUseCase.Input( | ||
| requestTime = LocalDateTime.now() | ||
| ) | ||
| val output = emailService.getMatchingInfo(input) | ||
| val matchingExperimentPosts = output.matchingPosts | ||
|
|
||
| var successCount = 0 | ||
| var failureCount = 0 | ||
| var successCount = 0 | ||
| var failureCount = 0 | ||
|
|
||
| for((contactEmail, jobList) in matchingExperimentPosts) { | ||
| if(jobList.isEmpty()) continue | ||
| val emailInput = SendMatchingEmailUseCase.Input ( | ||
| contactEmail = contactEmail, | ||
| experimentPosts = jobList, | ||
| currentDateTime = LocalDateTime.now() | ||
| ) | ||
| try { | ||
| val emailOutput = emailService.sendMatchingEmail(emailInput) | ||
| if (emailOutput.isSuccess) { | ||
| successCount++ | ||
| logger.info("Email sent successfully to $contactEmail") | ||
| } else { | ||
| for ((contactEmail, jobList) in matchingExperimentPosts) { | ||
| if (jobList.isEmpty()) continue | ||
| val emailInput = SendMatchingEmailUseCase.Input( | ||
| contactEmail = contactEmail, | ||
| experimentPosts = jobList, | ||
| currentDateTime = LocalDateTime.now() | ||
| ) | ||
| try { | ||
| val emailOutput = emailService.sendMatchingEmail(emailInput) | ||
| if (emailOutput.isSuccess) { | ||
| successCount++ | ||
| logger.info("Email sent successfully to $contactEmail") | ||
| } else { | ||
| failureCount++ | ||
| logger.error("Failed to send email to $contactEmail: ${emailOutput.message}") | ||
| } | ||
| } catch (e: Exception) { | ||
| failureCount++ | ||
| logger.error("Failed to send email to $contactEmail: ${emailOutput.message}") | ||
| logger.error("Exception occurred while sending email to $contactEmail", e) | ||
| } | ||
| } catch (e: Exception) { | ||
| failureCount++ | ||
| logger.error("Exception occurred while sending email to $contactEmail", e) | ||
| } | ||
| logger.info("SendMatchingEmailJob completed. Success: $successCount, Failures: $failureCount") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
스케줄러 작업에서 runBlocking 사용 시 주의가 필요합니다.
runBlocking은 현재 스레드를 차단하여 코루틴이 완료될 때까지 대기하므로, 스케줄러의 스레드 풀에 영향을 줄 수 있습니다. 대량의 이메일을 처리할 때 성능 저하가 발생할 수 있습니다.
다음과 같은 개선을 제안합니다:
runBlocking대신CoroutineScope를 사용하여 비동기 처리:
- override fun execute(context: JobExecutionContext) {
- runBlocking {
+ override fun execute(context: JobExecutionContext) {
+ CoroutineScope(Dispatchers.IO).launch {- 배치 처리 시 코루틴의 동시성을 활용:
matchingExperimentPosts.map { (contactEmail, jobList) ->
async {
if (jobList.isEmpty()) return@async
// 이메일 전송 로직
}
}.awaitAll()There was a problem hiding this comment.
음, 그러면 db에 조회하는 건 거의 없으니까 그냥 바로 블로킹대신 CoroutineScope로 비동기 처리하는 게 좋다는 거지? 위에 나랑 래빗이가 대화한 걸 읽고도?
There was a problem hiding this comment.
@chock-cho 죄송합니다. 제 이전 제안을 수정하겠습니다. 스케줄러 Job의 경우 runBlocking이 더 적절한 선택입니다. 그 이유는:
- 모든 이메일 발송이 완료될 때까지 Job이 종료되지 않도록 보장해야 합니다
CoroutineScope.launch를 사용하면 이메일 발송이 완료되기 전에 Job이 종료될 수 있습니다- Quartz Job의 특성상 작업 완료를 보장하는 것이 중요합니다
현재 구현이 더 안전하고 적절합니다.
✏️ Learnings added
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/infrastructure/scheduler/SendMatchingEmailJob.kt:22-57
Timestamp: 2025-02-12T04:32:40.871Z
Learning: In Quartz Job implementations, use `runBlocking` instead of `CoroutineScope.launch` to ensure all coroutine operations complete before the job ends. This is especially important for tasks that need guaranteed completion, such as email sending jobs.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
- 비동기식으로 실행되는 케이스 테스트 코드 일부 제거
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCaseTest.kt (1)
64-64:⚠️ Potential issue비동기 처리 관련 테스트 케이스 추가 필요
PR의 주요 목적인 비동기 이메일 전송과 트랜잭션 처리에 대한 테스트가 누락되어 있습니다. 다음 시나리오들에 대한 테스트 추가가 필요합니다:
- 이메일 전송이 백그라운드에서 실행되는지 검증
- 트랜잭션이 비동기적으로 실행되는지 검증
- API 응답이 즉시 반환되는지 검증
다음과 같은 테스트 케이스 추가를 제안드립니다:
given("유효한 대학 이메일이 주어졌을 때") { val validEmail = "test@university.ac.kr" every { EmailUtils.isDomainExists(validEmail) } returns true every { EmailUtils.isUnivMail(validEmail) } returns true `when`("이메일 인증 코드 전송을 실행하면") { then("즉시 응답을 반환하고 이메일은 백그라운드에서 전송되어야 한다") { testScope.runTest { val startTime = System.currentTimeMillis() launch { sendEmailCodeUseCase.execute(SendEmailCodeUseCase.Input(validEmail)) } val executionTime = System.currentTimeMillis() - startTime executionTime shouldBeLessThan 100 // 즉시 응답 검증 coVerify(timeout = 1000) { transactionExecutor.execute(any()) emailGateway.sendEmail(any()) } } } } }
🧹 Nitpick comments (2)
src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCaseTest.kt (1)
38-63: 테스트 구조 개선 제안현재 테스트 케이스들이
runBlocking을 직접 사용하고 있는데, 이는 코루틴 테스트의 가독성과 유지보수성을 저하시킬 수 있습니다.다음과 같이 테스트 구조를 개선하는 것을 추천드립니다:
+class SendEmailCodeUseCaseTest : BehaviorSpec({ + val testScope = TestScope() + beforeTest { + Dispatchers.setMain(testDispatcher) + } + afterTest { + Dispatchers.resetMain() + } + given("유효하지 않은 도메인의 이메일이 주어졌을 때") { val invalidEmail = "invalid@unknown.com" every { EmailUtils.isDomainExists(invalidEmail) } returns false `when`("이메일 인증 코드 전송을 실행하면") { then("EmailDomainNotFoundException 예외가 발생해야 한다") { - shouldThrow<EmailDomainNotFoundException> { - runBlocking { sendEmailCodeUseCase.execute(SendEmailCodeUseCase.Input(invalidEmail)) } - } + testScope.runTest { + shouldThrow<EmailDomainNotFoundException> { + sendEmailCodeUseCase.execute(SendEmailCodeUseCase.Input(invalidEmail)) + } + } } } }src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCaseTest.kt (1)
95-98: 테스트 실행 로직이 코루틴에 맞게 잘 수정되었습니다!
runBlocking과coVerify를 사용한 비동기 테스트 처리가 적절합니다.테스트 가독성 향상을 위해 다음과 같이 리팩토링하는 것을 고려해보세요:
- val output = runBlocking { sendMatchingEmailUseCase.execute(input) } - - output.isSuccess shouldBe true - coVerify(exactly = 1) { emailGateway.sendEmail(contactEmail, any(), any()) } + runBlocking { + val output = sendMatchingEmailUseCase.execute(input) + output.isSuccess shouldBe true + coVerify(exactly = 1) { emailGateway.sendEmail(contactEmail, any(), any()) } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCaseTest.kt(3 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCaseTest.kt(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCaseTest.kt (1)
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt:39-47
Timestamp: 2025-02-12T01:54:34.361Z
Learning: In the email verification API, immediate response is prioritized by executing non-critical background tasks (transaction and email sending) after returning the API response, which significantly improves response times and user experience.
🔇 Additional comments (4)
src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCaseTest.kt (1)
3-4: 코루틴 관련 의존성 검증 강화 필요relaxed 모크를 사용하면 실제 동작과 다른 결과가 발생할 수 있습니다. 특히
CoroutineDispatcherProvider와TransactionExecutor의 경우 중요한 비동기 동작을 담당하므로, 명시적인 동작 검증이 필요합니다.다음과 같이 모크 설정을 개선하는 것을 추천드립니다:
-val dispatcherProvider: CoroutineDispatcherProvider = mockk(relaxed = true) -val transactionExecutor: TransactionExecutor = mockk(relaxed = true) +val testDispatcher = TestCoroutineDispatcher() +val dispatcherProvider: CoroutineDispatcherProvider = mockk { + every { io } returns testDispatcher +} +val transactionExecutor: TransactionExecutor = mockk { + coEvery { <suspend> execute(any()) } coAnswers { + firstArg<suspend () -> Unit>().invoke() + } +}Also applies to: 26-32
src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCaseTest.kt (3)
23-24: 코루틴 테스트를 위한 import 추가가 적절합니다!비동기 테스트를 위한 필수적인 라이브러리들이 잘 추가되었습니다.
159-159: 예외 처리 테스트가 코루틴에 맞게 잘 수정되었습니다!
runBlocking을 사용한 예외 발생 테스트가 적절하게 구현되었습니다.
92-92: 코루틴 mock 설정이 올바르게 변경되었습니다!
coEvery와just Runs를 사용한 mock 설정이 코루틴 테스트에 적합합니다.다른 테스트 클래스들도 같은 방식으로 변경되었는지 확인해보세요:
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Description: 다른 테스트 파일들에서 coEvery 사용 여부 확인 rg "every.*emailGateway" "src/test/kotlin"Length of output: 42
모든 테스트 파일의 모킹 설정 검증 필요
emailGateway관련해서 기존의every설정이 제거되고coEvery로 변경된 것이 직접적으로 확인되었습니다. 다만,"every.*emailGateway"검색 결과가 나오지 않은 점을 감안할 때, 다른 테스트 파일에서도 동일하게 변경되었는지 추가 수동 검증이 필요해 보입니다.
Ji-soo708
left a comment
Ji-soo708
left a comment
There was a problem hiding this comment.
QA 과정에서 이메일 전송이 지연되는 이슈가 있었는데, 이번 PR 덕분에 응답 속도가 개선되어 사용자 경험이 훨씬 좋아질 것 같아요! 🚀
다만, 이번 PR에서 코루틴 개념이 사용되었는데, 수정님과 저 둘 다 아직 공부할 부분이 있는 것 같아서 더 신중하게 리뷰해봤습니다. 제가 남긴 코멘트들을 같이 논의해 보면 좋을 것 같아요! 시간 될 때 코루틴도 같이 공부해봐요~ ☕😊
| import kotlinx.coroutines.withContext | ||
| import org.springframework.stereotype.Component | ||
|
|
||
| @Component |
There was a problem hiding this comment.
기존 UseCase 인터페이스가 아닌 AsyncUseCase를 구현하고 있어서 빈 등록을 위해 @Component를 붙여주신 거 같아요. 기존처럼 부트 스트랩 클래스에 AsyncUseCase도 직접 컴포넌스 스캔을 시켜주는 건 어떨까요?
| CoroutineScope(dispatcherProvider.io).launch { | ||
| transactionExecutor.execute { | ||
| reflectVerification(input, code) | ||
| } | ||
| } | ||
| CoroutineScope(dispatcherProvider.io).launch { | ||
| sendVerificationEmail(input.univEmail, code) | ||
| } | ||
|
|
There was a problem hiding this comment.
CoroutineScope 를 사용해 이메일 전송을 비동기로 실행하여 API 응답 속도를 빠르게 유지한 점 좋습니다! 👍
다만, 빠른 응답을 내려주기 위해 트랜잭션과 이메일 전송이 각기 다른 CoroutineScope.launch {}에서 실행되고 있는데 제가 코루틴에 대한 이해는 부족하지만, 현재 코드에서 트랜잭션이 롤백되더라도 이메일이 전송될 가능성이 있지 않을까? 하는 걱정이 드네요. 🤔
만약 트랜잭션이 실패해서 DB 업데이트가 롤백되었는데도 이메일 전송이 이미 실행됐다면, 사용자는 인증 코드를 받았지만, 정작 DB에는 저장되지 않은 상태가 될 것 같습니다. 이렇게 되면 사용자가 받은 코드로 인증을 진행할 수 없어서 문제가 될 수도 있을 것 같은데, 혹시 이런 문제에 대해 방지가 되어있을까요??
| override suspend fun sendEmail(to: String, subject: String, content: String) { | ||
| withContext(Dispatchers.IO) { | ||
| val message = SimpleMailMessage() | ||
| message.setTo(to) | ||
| message.subject = subject | ||
| message.text = content | ||
|
|
||
| try { | ||
| mailSender.send(message) | ||
| } catch (ex: Exception) { | ||
| throw IllegalStateException("이메일 발송 실패: ${ex.message}") | ||
| try { | ||
| mailSender.send(message) | ||
| } catch (ex: Exception) { | ||
| throw IllegalStateException("이메일 발송 실패: ${ex.message}") | ||
| } | ||
| } |
There was a problem hiding this comment.
위에 남긴 코멘트와 같은 맥락인데 트랜잭션은 성공해서 DB에는 인증 코드가 정상적으로 갱신되었지만, 만약 SMTP 서버가 갑자기 다운되거나 네트워크 장애가 발생하면 이메일이 전송되지 않을 가능성이 있을 것 같아요.
그래서 재시도 메커니즘을 고려해볼 필요가 있어 보이는데, 추후 AWS SES를 도입하게 된다면 이런 부분을 자동으로 처리해줄 수도 있어서 지금 당장 구현하는 것보다는 향후 SES 도입 시 같이 고민해봐도 괜찮을 것 같습니다!
| shouldThrow<EmailDomainNotFoundException> { | ||
| sendEmailCodeUseCase.execute(SendEmailCodeUseCase.Input(invalidEmail)) | ||
| runBlocking { sendEmailCodeUseCase.execute(SendEmailCodeUseCase.Input(invalidEmail)) } |
There was a problem hiding this comment.
비동기 로직을 테스트하려고 추가된 코드같아요. 그런데 실행 자체는 동기적으로 수행되는 거로 보이는데 맞을까요?
찾아보니 진짜 비동기 환경에서 테스트하려면 runTest를 많이 사용하는 거 같아요. 테스트 전용 코루틴 컨텍스트를 제공하는 runTest로 코루틴 테스트를 더욱 효과적으로 수행하는 건 어떨까요??
There was a problem hiding this comment.
넵 이것두 제가 코루틴을 공부하다가 '비동기 실행 시에는 트랜잭션 실행과 상관없이 즉시 응답인데 테스트를 어떻게 하지..?🤔' 라는 고민 과정에서 억지로 비동기 로직을 동기 로직으로 욱여넣은 건데, 흐름이 역시 부자연스럽다고 느끼긴 했어요.
제시해주신 runTest 부분에 대해 알아보고, 적용해서 다음 리뷰 요청 때 코멘트 드리도록 하겠습니다!
꼼꼼한 리뷰 감사해요! 🙇♀️
There was a problem hiding this comment.
네 좋습니다! runTest를 활용한 테스트 코드 작성이 가능하시다면, 현재 유즈케이스 테스트 코드는 유지하면서, 별도의 코루틴 전용 테스트 패키지를 만들어서 비동기 로직이 정상적으로 수행되는지 검증하는 테스트가 추가되면 더 좋을 것 같아요. 👍
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/kotlin/com/dobby/backend/DobbyBackendApplication.kt(2 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt(3 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCase.kt(3 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCaseTest.kt(1 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCaseTest.kt(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCaseTest.kt
- src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCase.kt
🧰 Additional context used
🧠 Learnings (1)
src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt (1)
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt:39-47
Timestamp: 2025-02-12T01:54:34.361Z
Learning: In the email verification API, immediate response is prioritized by executing non-critical background tasks (transaction and email sending) after returning the API response, which significantly improves response times and user experience.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (4)
src/main/kotlin/com/dobby/backend/DobbyBackendApplication.kt (1)
3-3:Details
❓ Verification inconclusive
클린 아키텍처를 준수하는 비동기 처리 구조 도입을 승인합니다!
AsyncUseCase컴포넌트 스캔 추가를 통해 비동기 유스케이스를 애플리케이션 계층에서 적절히 관리할 수 있게 되었습니다. 클린 아키텍처 원칙을 잘 준수하면서 코루틴 기반의 비동기 처리를 구현한 점이 인상적입니다.비동기 처리 구현이 완전한지 확인하기 위해 다음 사항들을 검증해주세요:
Also applies to: 15-15
🏁 Script executed:
#!/bin/bash # Description: Verify the completeness of async implementation # Check if AsyncUseCase interface exists echo "Checking AsyncUseCase interface..." fd -e kt -x grep -l "interface AsyncUseCase" # Check for suspend functions in use cases echo "Checking suspend functions in use cases..." fd -e kt -x grep -l "suspend fun" # Check for coroutine dispatchers usage echo "Checking coroutine dispatchers..." fd -e kt -x grep -l "withContext.*dispatcher"Length of output: 1265
비동기 처리 구조: 검증 및 개선 요청
AsyncUseCase인터페이스가 올바르게 정의되어 있으며, 관련된 suspend 함수들이 다양한 유스케이스에서 구현되어 있는 점은 긍정적입니다.- 다만, 검증 스크립트에서
withContext와 함께 디스패처를 명시적으로 사용하는 코드가 검색되지 않았습니다. 해당 부분이 의도된 설계라면 문제 없으나, 디스패처를 통한 스레드 관리가 필요하다면 추가 검토 후 명시적 처리 여부를 확인해 주세요.전반적으로 비동기 처리 구현은 잘 이루어졌으므로, 위의 디스패처 사용 관련 사항만 확인하면 승인할 수 있겠습니다.
src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCaseTest.kt (1)
18-24: 테스트 의존성 주입 방식이 개선되었습니다.
relaxed = true옵션을 사용하여 모든 mock 객체가 기본값을 반환하도록 설정한 것이 좋습니다.src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt (2)
82-102: 이메일 전송 재시도 로직이 잘 구현되었습니다.지수적 백오프와 함께 재시도 로직을 구현한 것이 좋습니다. 실패 시 적절한 로깅도 포함되어 있어 문제 추적이 용이할 것 같습니다.
104-108: 백오프 계산 로직이 잘 구현되었습니다.지터(jitter)를 추가하여 여러 요청이 동시에 재시도되는 것을 방지한 것이 좋습니다.
src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCaseTest.kt
Show resolved
Hide resolved
src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/kotlin/com/dobby/backend/util/RetryUtils.kt (2)
23-23: 오타를 수정해주세요.에러 메시지에 오타가 있습니다: "attmpets" -> "attempts"
- logger.error("Operation failed after $maxRetries attmpets.", ex) + logger.error("Operation failed after $maxRetries attempts.", ex)
34-36: 백오프 계산 로직을 개선해주세요.현재 구현은 지수 백오프와 지터를 잘 구현했지만, 최대 지연 시간을 제한하지 않아 너무 긴 지연이 발생할 수 있습니다.
- private fun calculateBackOff(curAttempt: Int, defaultDelay: Long, maxJitter: Long): Long { - return defaultDelay * (2.0.pow(curAttempt)).toLong() + Random.nextLong(0, maxJitter) - } + private fun calculateBackOff(curAttempt: Int, defaultDelay: Long, maxJitter: Long): Long { + val maxDelay = 30_000L // 30초 + val exponentialDelay = defaultDelay * (2.0.pow(curAttempt)).toLong() + val cappedDelay = exponentialDelay.coerceAtMost(maxDelay) + return cappedDelay + Random.nextLong(0, maxJitter) + }src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt (1)
37-47: CoroutineScope를 재사용하도록 개선해주세요.매번 새로운 CoroutineScope를 생성하는 대신, 클래스 레벨에서 scope를 정의하여 재사용하는 것이 좋습니다.
+ private val scope = CoroutineScope(dispatcherProvider.io) + override suspend fun execute(input: Input): Output { validateEmail(input.univEmail) val code = EmailUtils.generateCode() - CoroutineScope(dispatcherProvider.io).launch { + scope.launch { RetryUtils.retryWithBackOff { transactionExecutor.execute { reflectVerification(input, code) } } } - CoroutineScope(dispatcherProvider.io).launch { + scope.launch { sendVerificationEmail(input.univEmail, code) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt(3 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCase.kt(3 hunks)src/main/kotlin/com/dobby/backend/util/RetryUtils.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCase.kt (1)
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt:39-47
Timestamp: 2025-02-12T01:54:34.361Z
Learning: In the email verification API, immediate response is prioritized by executing non-critical background tasks (transaction and email sending) after returning the API response, which significantly improves response times and user experience.
src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt (1)
Learnt from: chock-cho
PR: YAPP-Github/Gradmeet-BE#93
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt:39-47
Timestamp: 2025-02-12T01:54:34.361Z
Learning: In the email verification API, immediate response is prioritized by executing non-critical background tasks (transaction and email sending) after returning the API response, which significantly improves response times and user experience.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCase.kt (1)
39-42: 구현이 잘 되었습니다!이메일 전송 실패 시 재시도 로직이 잘 구현되어 있고, 에러 처리도 적절합니다.
src/main/kotlin/com/dobby/backend/application/usecase/member/email/SendEmailCodeUseCase.kt (1)
80-86: 이메일 전송 구현이 잘 되었습니다!이메일 전송 실패 시 재시도 로직이 잘 구현되어 있고, suspend 함수로 적절하게 변경되었습니다.
Ji-soo708
left a comment
Ji-soo708
left a comment
There was a problem hiding this comment.
LGTM!
자바와는 달리 코루틴 기반으로 비동기 처리를 구현하다 보니 고려해야 할 부분이 많았네요. 백오프 로직까지 세심하게 추가하신 부분도 좋았습니다. 👍 나중에 SES 도입한다면 더욱 안정적인 이메일 백오프 처리가 가능할 것 같아요.
정말 수고 많으셨습니다!
* feat: add coroutine libraries to optimize mail sending - 이메일 전송 비동기 처리를 위한 코루틴 라이브러리 추가 * feat: add common interfaces for transaction and coroutine management - CoroutineDispatcherProvider 및 TransactionExecutor 인터페이스 추가 - 공통 인터페이스 분리 → 의존성 줄이고, 유지보수 용이 * feat: implement coroutine and transaction executor in infrastructure layer - CoroutineConfig, CoroutineDispatcherProviderImpl, TransactionExecutor 추가 - application 계층에서 비동기 실행 및 트랜잭션 쉽게 활용할 수 있도록 구현 * refact: update EmailGateway and scheduler to support async processing - EmailGatewayImpl에서 비동기 메일 전송 처리 적용 - SendMatchingEmailJob에서 코루틴 기반 비동기 실행 적용 - 불필요한 블로킹 코드 제거하여 성능 최적화 * refact: optimize async transaction handling in EmailService and UseCases - EmailService에서 비동기 트랜잭션 실행 방식 개선 - SendEmailUseCase 및 SendMatchingEmailUseCase에서 트랜잭션 적용 방식 변경 - 기존 블로킹 방식에서 완전한 비동기 실행 → API 응답 속도 향상 * refact: update EmailController to support async transaction execution - EmailController에서 비동기 트랜잭션 실행 방식 반영 * feat: fix email usecase test codes - 비동기식으로 실행되는 케이스 테스트 코드 일부 제거 * fix: include AsyncUseCase class in component scan * fix: replace runBlocking with runTest for better coroutine handling * feat: add exponential backoff and retry logic for email vericiation * fix: delete unused import packages * refact: extract retry logic into reusable utility package * refact: add autonomic retry for failed transaction while keeping email async sending
* feat: add coroutine libraries to optimize mail sending - 이메일 전송 비동기 처리를 위한 코루틴 라이브러리 추가 * feat: add common interfaces for transaction and coroutine management - CoroutineDispatcherProvider 및 TransactionExecutor 인터페이스 추가 - 공통 인터페이스 분리 → 의존성 줄이고, 유지보수 용이 * feat: implement coroutine and transaction executor in infrastructure layer - CoroutineConfig, CoroutineDispatcherProviderImpl, TransactionExecutor 추가 - application 계층에서 비동기 실행 및 트랜잭션 쉽게 활용할 수 있도록 구현 * refact: update EmailGateway and scheduler to support async processing - EmailGatewayImpl에서 비동기 메일 전송 처리 적용 - SendMatchingEmailJob에서 코루틴 기반 비동기 실행 적용 - 불필요한 블로킹 코드 제거하여 성능 최적화 * refact: optimize async transaction handling in EmailService and UseCases - EmailService에서 비동기 트랜잭션 실행 방식 개선 - SendEmailUseCase 및 SendMatchingEmailUseCase에서 트랜잭션 적용 방식 변경 - 기존 블로킹 방식에서 완전한 비동기 실행 → API 응답 속도 향상 * refact: update EmailController to support async transaction execution - EmailController에서 비동기 트랜잭션 실행 방식 반영 * feat: fix email usecase test codes - 비동기식으로 실행되는 케이스 테스트 코드 일부 제거 * fix: include AsyncUseCase class in component scan * fix: replace runBlocking with runTest for better coroutine handling * feat: add exponential backoff and retry logic for email vericiation * fix: delete unused import packages * refact: extract retry logic into reusable utility package * refact: add autonomic retry for failed transaction while keeping email async sending
* feat: add coroutine libraries to optimize mail sending - 이메일 전송 비동기 처리를 위한 코루틴 라이브러리 추가 * feat: add common interfaces for transaction and coroutine management - CoroutineDispatcherProvider 및 TransactionExecutor 인터페이스 추가 - 공통 인터페이스 분리 → 의존성 줄이고, 유지보수 용이 * feat: implement coroutine and transaction executor in infrastructure layer - CoroutineConfig, CoroutineDispatcherProviderImpl, TransactionExecutor 추가 - application 계층에서 비동기 실행 및 트랜잭션 쉽게 활용할 수 있도록 구현 * refact: update EmailGateway and scheduler to support async processing - EmailGatewayImpl에서 비동기 메일 전송 처리 적용 - SendMatchingEmailJob에서 코루틴 기반 비동기 실행 적용 - 불필요한 블로킹 코드 제거하여 성능 최적화 * refact: optimize async transaction handling in EmailService and UseCases - EmailService에서 비동기 트랜잭션 실행 방식 개선 - SendEmailUseCase 및 SendMatchingEmailUseCase에서 트랜잭션 적용 방식 변경 - 기존 블로킹 방식에서 완전한 비동기 실행 → API 응답 속도 향상 * refact: update EmailController to support async transaction execution - EmailController에서 비동기 트랜잭션 실행 방식 반영 * feat: fix email usecase test codes - 비동기식으로 실행되는 케이스 테스트 코드 일부 제거 * fix: include AsyncUseCase class in component scan * fix: replace runBlocking with runTest for better coroutine handling * feat: add exponential backoff and retry logic for email vericiation * fix: delete unused import packages * refact: extract retry logic into reusable utility package * refact: add autonomic retry for failed transaction while keeping email async sending
💡 작업 내용
TransactionExecutor및CoroutineDispatcherProvider를 활용하여 클린 아키텍처 원칙을 준수하면서도 비동기 실행이 가능하도록 리팩터링했습니다.📌 주요 변경 사항
TransactionExecutor.execute {}내부에서 실행되도록 수정launch활용)withContext(dispatcherProvider.io)를 활용하여 API 응답을 지연시키지 않도록 변경CoroutineDispatcherProvider를 Application 레이어에 인터페이스로 정의하고, Infrastructure에서 구현체를 주입하도록 변경TransactionExecutor를 활용하여 트랜잭션이 필요한 로직을 분리하여 실행✅ 셀프 체크리스트
🙋🏻 확인해주세요
기존 이메일 전송 로직 구현
과 비교하여, 크게 응답속도 개선율을 보였습니다.
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-302
Summary by CodeRabbit
Summary by CodeRabbit
새로운 기능
리팩토링
버그 수정
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-302