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(server): 메일전송을 비동기로 처리한다. #513
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수리 고생많았어!
메일 보낼 때, 시간이 꽤 지연돼서 은근 불편했는데 반영되고나면 그 불편함이 사라지겠지? 🤔
불편함을 해소해줄 수 있는 이슈라 체감이 클 것 같네 😃
몇몇 제안 남겨놨는데, 괜찮다 싶으면 반영해줬으면 좋겠네!
@Override | ||
public Executor getAsyncExecutor() { | ||
ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); | ||
executor.setCorePoolSize(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
응 만약 1로 두는 거면 삭제해야겠다.
executor.setCorePoolSize(1); | ||
executor.setMaxPoolSize(5); | ||
executor.setQueueCapacity(100); | ||
executor.setThreadNamePrefix("async-mail-service"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
나중에 mail 기능 말고도 비동기로 만들 기능이 있으면 이 부분은 리팩토링 대상이라고 봐야겠네?
아 ThreadNamePrefix만 그런 줄 알았는데, 수리가 코멘트 마지막에 언급한 ps. 추가로 여러 상황에서 다른 Async 설정을 사용하고 싶을 때는 어떻게 할까 찾아봤는데 이때는 @overide말고 @qualifier를 선언해서 사용한대!
음... 개인적인 생각이지만, override말고 @Qualifier
애너테이션으로 변경하면 어떨까? 🤔
추후 비동기 기능이 생길지 안생길지는 조금 의문이지만, 그때 작업하는 작업자가 수리의 마지막 코멘트를 기억한다면 다행이지만 아닌 경우에는 또 다시 관련 자료(= Qualified 관련)를 찾는데 시간을 보낼 것 같아서 🤔
변경에 큰 어려움이 있는 것이 아니라면 첫 번째 비동기 기능을 @Qualified
형식으로 만들어 두면 다음 작업자가 작업하는데 많은 도움이 될 것 같다는 생각을 해봤어!!!
(고민해야할 점이 많다면 스킵해도 무방!!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스는 AsyncConfig
인데 쓰레드 이름이 async-mail-service
인건 아닌듯!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Qualifier
로 진행한다면 어떠한 형태를 띄는지가 궁금해 ! AsyncConfigurerSupport
를 상속받는 다른 AsyncConfig
클래스가 생기는 건지 아니면, getAsyncExecutor
처럼 Executor
를 반환하는 다른 메서드가 생기는건지
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://cofs.tistory.com/319
여기보면 로키가 말한 interface implements 하면서 @Qualifier
를 사용하는 예시가 나와있네
근데 @Qualifier
사용방법이 잘못됐네... 사용하는 이유가 없는 코드긴하다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Qualifier
랑 interface를 implements 하는 방식은 별개의 얘기였어!
Qualifier는 수리 코멘트에 여러 상황에서 다른 Async 설정을 사용하고 싶을 때는 어떻게 할까 찾아봤는데 이때는 @overide말고 @qualifier를 선언해서 사용한대!
부분을 참고해서 의견 냈던거야!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 응! 나도 로키가 말한 inerface implements 하면서 /// 여기 까지가 로키 의견 ! @Qualifier
는 pr 코멘트 생각하면서 말한거였어 ㅋㅋㅋㅋ 오해할 만한 문장이었다 ㅋㅋㅋ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파즈, 로키가 의논 했던 부분 qulifier하고 상속 관련은 밑에 코멘트에 정리해놨어!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조이 AsyncConfig 이긴 하지만, 추후 다른 thread 관련 설정도 해당 config에서 하지 않을까 싶어서
mail 이라는 키워드를 넣어서 분리해봤어!
@@ -22,21 +23,22 @@ | |||
@Component | |||
@RequiredArgsConstructor | |||
public class SmtpMailConnector { | |||
// todo 메일 전송 시간이 김. 비동기로 해볼까? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오... 언행일치 👍👍 멋진데?
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 줄바꿈은 제거하면 좋을 것 같어 😃
|
||
private static final String PREFIX_SUBJECT = "[Thank You For]"; | ||
|
||
private final JavaMailSender javaMailSender; | ||
private final TemplateEngine templateEngine; | ||
|
||
|
||
@Async |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트 방법을 몰라서 직접 테스트를 못해봐서 미안해....
Baldung - @Async 관련 자료를 참고해보면 @Async
를 클래스레벨에 적용하면 클래스 내에 모든 public
메서드에만 적용된다고 해!
메서드 각각에 @Async
를 선언하는 것보다 SmtpMailConnector
클래스에 @Async
를 선언하면 어떨까? 🤗
참고자료에서 발췌
It must be applied to public methods only. (이 외에 내용도 읽어보면 좋을 것 같네!! 이미 읽어봤을지도 모르겠다 👀)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오오 조아욘!!!!!!!!! 이거는 적용했다.
|
||
@Configuration | ||
@EnableAsync | ||
public class AsyncConfig extends AsyncConfigurerSupport { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public class AsyncConfig extends AsyncConfigurerSupport { | |
public class AsyncConfig implements AsyncConfigurer { |
자바 8 이후부터는 default
메서드 덕분에 AsyncConfigurer
를 바로 구현해도 된다고 하는데 🤔
AsyncConfigurerSupport
구현체를 보면 AsyncConfigurer
랑 차이가 없는 것을 알 수 있을꺼야!(default 제외)
별다른 이점이 없다면 최상위 클래스를 상속 or 구현하는 것이 좋다고 생각해서 이렇게 변경하면 좋을 것 같은데 어떻게 생각해? 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Async
를 테스트해볼 순 없을까?
}).fail(function (e) { | ||
alert("실패!") | ||
}); | ||
await jQuery.ajax({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
index.html 삭제할까 생각중인데 혹시 고친 이유가 있어? 이거 쓰나?
비동기 테스트해보려고 한거야?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
응..비동기테스트는 직접 밖에 아직 방법을 못찾아서
나도 이 index.html 삭제하고픔.
삭제하기 전에 파노, 인치에게 프론트 개발 서버 또한 배포를 하면 어떨지에 대해서 말해볼까?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그건 따로 말하고 이건 그냥 삭제해도 될거같은데? 어차피 이거 쓰느 사람없고, 지금 api 서버에도 배포되어 있어서 아주 불편해
|
||
import java.util.UUID; | ||
|
||
@NoArgsConstructor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
없어도 되지 않아? 왜 붙였어?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아고 저건 못봤넹
executor.setCorePoolSize(1); | ||
executor.setMaxPoolSize(5); | ||
executor.setQueueCapacity(100); | ||
executor.setThreadNamePrefix("async-mail-service"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스는 AsyncConfig
인데 쓰레드 이름이 async-mail-service
인건 아닌듯!
executor.setCorePoolSize(1); | ||
executor.setMaxPoolSize(5); | ||
executor.setQueueCapacity(100); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corePoolSize, maxPoolSize, queueCapacity 는 어떤 기준으로 정했는지 궁금해
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수리 commit 내역 잘 봤어! 변경해야 할 점들은 이미 다른 팀원들이 다 말해준 것 같다.
다만, 여기 설명에서 잘못된 점이 있어서 짚고 넘어가면 좋을 것 같아서 말해.
Core/Max PoolSize 는 적혀있는 내용이 맞고,
QueueCapacity : MaxPoolSize를 초과하는 요청이 Thread 생성 요청시 해당 내용을 Queue에 저장하게 되고, 사용할 수 있는 Thread 여유 자리가 발생하면 하나씩 꺼내서 동작
이 부분이 조금 틀려. WorkingQueue 동작 방식은 먼저 CorePoolSize가 꽉 차면 해당 Queue에 집어넣게돼. 그리고 해당 Queue가 모두 꽉 차면 그때가 되어서야 MaxPoolSize 수 까지 올려가면서 스레드를 생성하게 되는거야! 좋은 예시 있는 블로그를 찾아봤는데 링크 여기 굉장히 예시가 좋다! 한 번 읽어보면 큰 도움될 것 같어!
|
||
@Override | ||
public Executor getAsyncExecutor() { | ||
ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
우리가 지난번에 했던 http 미션에서 ExecutorService
가 있고 그것을 통해서 스레드 관리를 해줄 수가 있었는데링크,
이것과 무슨 차이가 있는지 좀 찾아봤더니
Spring을 사용한다면 ThreadPoolTaskExecutor를 살펴보는 것도 좋다. 내부 구현은 ThreadPoolExecutor로 구현되어 있다. ThreadPoolExecutor 보다 조금 더 간편하며, 추가적인 return 타입도 있다.
출처 http://wonwoo.ml/index.php/post/2254
spring에서 조금 더 편하게 만든 스레드 관리 클래스였어!
내부적으로는 ThreadPoolExecutor
를 사용하고 있었고,
workingQueue의 size도 new LinkedBlockingQueue<>(queueCapacity);
를 이용하고 있는 것을 볼 수가 있는데, 저 큐는 ThreadPoolExecutor
에서도 사용하는 큐야! 스프링 별거를 다 도와주네 허허
@Override | ||
public Executor getAsyncExecutor() { | ||
ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); | ||
executor.setCorePoolSize(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
나는 이거 한 5 정도는 되어야 할 것 같다고 생각해
executor.setCorePoolSize(1); | ||
executor.setMaxPoolSize(5); | ||
executor.setQueueCapacity(100); | ||
executor.setThreadNamePrefix("async-mail-service"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Qualifier
로 진행한다면 어떠한 형태를 띄는지가 궁금해 ! AsyncConfigurerSupport
를 상속받는 다른 AsyncConfig
클래스가 생기는 건지 아니면, getAsyncExecutor
처럼 Executor
를 반환하는 다른 메서드가 생기는건지
ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); | ||
executor.setCorePoolSize(1); | ||
executor.setMaxPoolSize(5); | ||
executor.setQueueCapacity(100); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
workingQueue 에 대한 설명이 조금 틀린 것 같아서 메인 코멘트에 작성해뒀어!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오케 나도 맨 처음 PR 코멘트에 반영했으!!
@@ -22,21 +23,22 @@ | |||
@Component | |||
@RequiredArgsConstructor | |||
public class SmtpMailConnector { | |||
// todo 메일 전송 시간이 김. 비동기로 해볼까? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이거 근데 주석 작성 조이 아님 ? ㅋㅋㅋ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아... 왜 수리가 적은줄 알았지...? 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅇㅇ나임ㅋㅋ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅋㅋㅋㅋㅋㅋㅋ 조이임
|
||
import java.util.UUID; | ||
|
||
@NoArgsConstructor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
나도 Async에 대해 잘 숙지하지 못하고 써서 좀 정리 해봤어! @async에 대해AsyncConfigurerSupport를 왜 상속@Configuration
@EnableAsync
public class AsyncConfig extends AsyncConfigurerSupport {
@Override
public Executor getAsyncExecutor() {
ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
executor.setCorePoolSize(1);
executor.setMaxPoolSize(5);
executor.setQueueCapacity(100);
executor.setThreadNamePrefix("async-mail-service");
executor.initialize();
return executor;
}
}
결론
@async를 다양한 설정으로 적용하고 싶을 때?
Qualifier
@Qualifier("mailExecutor")
private final ThreadPoolTaskExecutor executor; public void sendVerificationCode(String mailAddress, String verificationCode) {
Runnable r = () -> {
Context context = new Context();
context.setVariable("code", verificationCode);
String message = templateEngine.process("mail-verification-code.html", context);
sendMail("환불 인증번호", message, mailAddress);
};
executor.execute(r);
} Async
@Component
@Async("mailExecutor")
@RequiredArgsConstructor
public class SmtpMailConnector {
...
} CorePoolSize, maxPoolSize, QueueCapacity에 대해
기준
|
내가 처음에 그래서 Mail관련 Exectutor를 Bean으로 빼는 작업을 진행했어 그리고 Thread Pool 설정 의논해 보고 싶어!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수리가 남겨둔 코멘트 전부 확인했어 내가 이해를 잘 못해서 한참 이것저것 보다보니 이제 이해했다! 😃
빈으로 등록하려는 mailExecutor
의 설정 값들이 중요할 것 같은데, 일단은 많은 리소스를 필요로할 것 같지 않다는 의견에 동의하기 때문에 파즈가 설정하자는대로 항상 크기가 고정되게 설정해보는 것도 좋을 것 같아.
추후 사용자가 많이 늘어나서 동시적으로 메일을 전송할 일이 많아진다면 maxPoolSize
를 corePoolSize보다 크게 잡아서 유동적으로 pool size가 바뀌게 하는 것이 좋을 것 같아
(이 설정 값만 결정나면 될 것 같아서 approve 할게)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이번 PR 내용은 내가 공부를 따로 못했는데 다들 좋은 내용을 쉽게 설명해줘서 이해가 잘 됐어!
그리고 난 newFixedThreadPool
찬성. 신경쓸 점이 많이 없어서 좋아.
고생했어 수리!!😀
}).fail(function (e) { | ||
alert("실패!") | ||
}); | ||
await jQuery.ajax({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그건 따로 말하고 이건 그냥 삭제해도 될거같은데? 어차피 이거 쓰느 사람없고, 지금 api 서버에도 배포되어 있어서 아주 불편해
interface나 class를 구현하는 방식은 default executor인 SimpleAsyncTaskExecutor 를 사용하지 않고 다른 executor를 사용하기 위해서 구현/상속을 한다는 것이다. 즉, 여러 executor를 사용하기 위해서는 위의 interface/class를 구현/상속 하는 형태를 띄면 안된다. -> 이거는 아닌 것 같아! default는 상속, 구현을 통해 정하고, 새로 추가하고 싶으면 Executor를 Bean으로 등록해서 Async(Bean 이름) 이런식으로 사용하는 듯! |
그리고 Index.html 이번에 비동기 테스트하면서 사용을 좀 했어가지구.. |
아하 그냥 SimpleAsyncTaskExecutor 를 바꿔줄 때에 그렇게 쓰는거구나 오케 |
block 이라는 의미는 Queue가 꽉찼을때의 삽입 시도/Queue가 비어있을 때 추출 시도를 막는다는 것이야. 많은 thread들이 해당 queue에 접근해서 데이터를 넣고 뺄 때, LinkedBlockingQueue는 따로 capacity 설정을 하지 않으면 capacity가 Integer.MAX_VALUE으로 설정돼
아래 테스트를 통과해 corethread 수가 5개로 설정하고 100개의 일을 돌렸을 때,
그리고
에서
|
확인완료~ |
메일전송 비동기로 처리하도록 Async 추가했어!
Async설정을 해주는 부분이 있는데, 이 부분을 일단 임의로 설정했어!
우리 서비스가 아직 그렇게 많은 요청이 들어오지 않아서 작게 잡아봤어!
비동기 테스트를 어떻게 할지 몰라서 일단 직접해보는 식으로 진행했는데, 개선 된 것 확인했어!
메일 전달 내용에도 문제가 없더라구!
PR올려볼게!
변경사항
AsyncConfig 추가 및 메일전송 메소드에 @async 추가
PaymentInfo NoArgsContructor 추가
index.html
ps. 추가로 여러 상황에서 다른 Async 설정을 사용하고 싶을 때는 어떻게 할까 찾아봤는데 이때는 @overide말고 @qualifier를 선언해서 사용한대!