-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor/338 : Rate Limiter 토큰 대기방식 개선 #422
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
Changes from 13 commits
780ef58
fe27fab
2b66a85
1ae5ad6
602493d
78c3000
28beb39
5954fd0
142f305
df00655
9a5c4c6
a1c9f9f
d959733
f44108d
2a0f547
839f75b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| package com.example.cs25batch.adapter; | ||
|
|
||
| import jakarta.annotation.Nullable; | ||
| import java.time.Duration; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.data.redis.connection.stream.Consumer; | ||
| import org.springframework.data.redis.connection.stream.MapRecord; | ||
| import org.springframework.data.redis.connection.stream.ReadOffset; | ||
| import org.springframework.data.redis.connection.stream.RecordId; | ||
| import org.springframework.data.redis.connection.stream.StreamOffset; | ||
| import org.springframework.data.redis.connection.stream.StreamReadOptions; | ||
| import org.springframework.data.redis.core.StringRedisTemplate; | ||
|
|
||
| @RequiredArgsConstructor | ||
| public class RedisStreamsClient { | ||
|
|
||
| private final StringRedisTemplate redisTemplate; | ||
| private final String stream; | ||
| private final String group; | ||
| private final String consumer; | ||
|
|
||
| @Nullable | ||
| public MapRecord<String, Object, Object> readWithConsumerGroup(Duration blockTimeout) { | ||
|
|
||
| StreamReadOptions options = StreamReadOptions.empty().count(1).block(blockTimeout); | ||
|
|
||
| List<MapRecord<String, Object, Object>> records = redisTemplate.opsForStream().read( | ||
| Consumer.from(group, consumer), | ||
| options, | ||
| StreamOffset.create(stream, ReadOffset.lastConsumed()) | ||
| ); | ||
|
|
||
| return (records == null || records.isEmpty()) ? null : records.get(0); | ||
| } | ||
|
|
||
| public void ack(String recordId) { | ||
| redisTemplate.opsForStream().acknowledge(stream, group, RecordId.of(recordId)); | ||
| } | ||
|
|
||
| public void del(String recordId) { | ||
| redisTemplate.opsForStream().delete(stream, RecordId.of(recordId)); | ||
| } | ||
|
|
||
| public void ackAndDel(String recordId) { | ||
| RecordId id = RecordId.of(recordId); | ||
| redisTemplate.opsForStream().acknowledge(stream, group, id); | ||
| redisTemplate.opsForStream().delete(stream, id); | ||
| } | ||
|
|
||
| public void addDlq(String dlqStream, Map<String, String> message){ | ||
| redisTemplate.opsForStream().add(dlqStream, message); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package com.example.cs25batch.config; | ||
|
|
||
| import com.example.cs25batch.adapter.RedisStreamsClient; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.data.redis.core.StringRedisTemplate; | ||
|
|
||
| @Configuration | ||
| public class RedisStreamsConfig { | ||
| @Bean | ||
| public RedisStreamsClient quizEmailStreamsClient(StringRedisTemplate redisTemplate) { | ||
| return new RedisStreamsClient( | ||
| redisTemplate, | ||
| "quiz-email-stream", // stream 이름 | ||
| "mail-consumer-group", // group | ||
| "mail-worker" // consumer | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,11 @@ | ||
| package com.example.cs25batch.sender; | ||
|
|
||
| import com.example.cs25batch.batch.dto.MailDto; | ||
| import io.github.bucket4j.Bucket; | ||
|
|
||
| public interface MailSenderStrategy { | ||
| void sendQuizMail(MailDto mailDto); | ||
|
|
||
| Bucket getBucket(); | ||
| boolean tryConsume(Long num); | ||
|
|
||
| void acquirePermitOrWait(); | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,10 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.example.cs25batch.batch.service.SesMailService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.github.bucket4j.Bandwidth; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.github.bucket4j.Bucket; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import io.github.bucket4j.ConsumptionProbe; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.concurrent.ThreadLocalRandom; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.concurrent.TimeUnit; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.concurrent.locks.LockSupport; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.stereotype.Component; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -18,7 +22,7 @@ public class SesMailSenderStrategy implements MailSenderStrategy{ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .addLimit( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Bandwidth.builder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .capacity(14) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .refillGreedy(7, Duration.ofMillis(500)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .refillGreedy(14, Duration.ofMillis(1000)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .build() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -29,7 +33,21 @@ public void sendQuizMail(MailDto mailDto) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public Bucket getBucket() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return bucket; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public boolean tryConsume(Long num){ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return bucket.tryConsume(num); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public void acquirePermitOrWait(){ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while (true) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ConsumptionProbe probe = bucket.tryConsumeAndReturnRemaining(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (probe.isConsumed()) return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| long nanos = probe.getNanosToWaitForRefill(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| long jitter = TimeUnit.MILLISECONDS.toNanos( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ThreadLocalRandom.current().nextInt(0, 50) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LockSupport.parkNanos(Math.min(nanos + jitter, TimeUnit.SECONDS.toNanos(1))); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+41
to
+51
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion acquirePermitOrWait에서도 인터럽트 신호 처리 추가 권장 JavaMail 전략과 동일한 이유로 인터럽트 플래그를 확인하여 루프를 탈출하세요. @Override
public void acquirePermitOrWait(){
while (true) {
+ if (Thread.currentThread().isInterrupted()) {
+ return;
+ }
ConsumptionProbe probe = bucket.tryConsumeAndReturnRemaining(1);
if (probe.isConsumed()) return;
long nanos = probe.getNanosToWaitForRefill();
long jitter = TimeUnit.MILLISECONDS.toNanos(
ThreadLocalRandom.current().nextInt(0, 50)
);
LockSupport.parkNanos(Math.min(nanos + jitter, TimeUnit.SECONDS.toNanos(1)));
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
🛠️ Refactor suggestion
Reader에서 토큰을 선점하면 ‘메시지 없음’에도 토큰이 소모됩니다 — 토큰 획득 호출 제거 권장(Writer로 이동)
현재 acquirePermitOrWait는 토큰을 실제로 소비합니다(tryConsumeAndReturnRemaining 사용). 메시지가 없거나 빈 레코드일 때도 처리량을 깎는 문제가 생깁니다. 토큰 획득은 MailWriter에서 실제 발송 직전에 수행하세요.
MailWriter 측 반영은 별도 코멘트의 diff를 참고하세요.
📝 Committable suggestion
🤖 Prompt for AI Agents