-
Notifications
You must be signed in to change notification settings - Fork 1
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
Refactor/#34 주차장 데이터 삽입 성능개선 및 외부 api 장애 발생시 처리 로직 추가 #86
The head ref may contain hidden characters: "Refactor/#34_\uC8FC\uCC28\uC7A5_\uB370\uC774\uD130_\uC0BD\uC785_\uC131\uB2A5\uAC1C\uC120"
Changes from 12 commits
06f795f
e906096
fde58fa
2c41564
9d06db0
472db69
d3391e6
7e783ef
ff521a7
59b7bfd
90d5d67
78ababc
81e989e
dc4ae8e
017a847
7ba4298
270e6a2
4b85021
879a941
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,59 @@ | ||
package com.parkingcomestrue.external.api; | ||
|
||
public class ApiCounter { | ||
|
||
private final double MIN_TOTAL_COUNT; | ||
|
||
private double totalCount; | ||
private double errorCount; | ||
private boolean isClosed; | ||
|
||
public ApiCounter() { | ||
this.MIN_TOTAL_COUNT = 10; | ||
this.totalCount = 0; | ||
this.errorCount = 0; | ||
this.isClosed = false; | ||
} | ||
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.
|
||
|
||
public ApiCounter(double minTotalCount) { | ||
this.MIN_TOTAL_COUNT = minTotalCount; | ||
this.totalCount = 0; | ||
this.errorCount = 0; | ||
this.isClosed = false; | ||
} | ||
|
||
public synchronized void countUp() { | ||
totalCount++; | ||
} | ||
|
||
public synchronized void errorCountUp() { | ||
totalCount++; | ||
errorCount++; | ||
} | ||
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.
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. 서버 한대 상황에서 |
||
|
||
public void reset() { | ||
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. 요게 서킷 open 하는 역할인가요?? 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. 넴 초기(정상) 상태로 돌리는 용도입니다 |
||
totalCount = 0; | ||
errorCount = 0; | ||
isClosed = false; | ||
} | ||
|
||
public boolean isClosed() { | ||
return isClosed; | ||
} | ||
|
||
public void close() { | ||
isClosed = true; | ||
} | ||
|
||
public boolean isErrorRateOverThan(double errorRate) { | ||
if (totalCount < MIN_TOTAL_COUNT) { | ||
return false; | ||
} | ||
double currentErrorRate = errorCount / totalCount; | ||
return currentErrorRate >= errorRate; | ||
} | ||
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. 요 부분에서 사용하는 totalCount 를 다른 메서드에서 조작하기 때문에 syncronized 하게 실행해도 괜찮을거같네요. 예를 들어, if 문을 통과하고 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. 해당 케이스를 생각 못했네유.. |
||
|
||
public double getTotalCount() { | ||
return totalCount; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package com.parkingcomestrue.external.api; | ||
|
||
import java.util.concurrent.CompletableFuture; | ||
import java.util.concurrent.ExecutorService; | ||
import java.util.concurrent.Executors; | ||
import java.util.function.Supplier; | ||
|
||
public class AsyncApiExecutor { | ||
|
||
private static final ExecutorService executorService = Executors.newFixedThreadPool(100, (Runnable r) -> { | ||
Thread thread = new Thread(r); | ||
thread.setDaemon(true); | ||
return thread; | ||
} | ||
); | ||
|
||
public static <U> CompletableFuture<U> executeAsync(Supplier<U> supplier) { | ||
return CompletableFuture.supplyAsync(supplier::get, executorService); | ||
} | ||
} | ||
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. 해당 부분 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. 좋습니다~! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package com.parkingcomestrue.external.api; | ||
|
||
import java.lang.annotation.ElementType; | ||
import java.lang.annotation.Retention; | ||
import java.lang.annotation.RetentionPolicy; | ||
import java.lang.annotation.Target; | ||
import java.util.concurrent.TimeUnit; | ||
|
||
@Retention(RetentionPolicy.RUNTIME) | ||
@Target(ElementType.METHOD) | ||
public @interface CircuitBreaker { | ||
|
||
int minTotalCount() default 10; | ||
double errorRate() default 0.2; | ||
long resetTime() default 30; | ||
TimeUnit timeUnit() default TimeUnit.MINUTES; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
package com.parkingcomestrue.external.api; | ||
|
||
import java.util.Map; | ||
import java.util.concurrent.ConcurrentHashMap; | ||
import java.util.concurrent.Executors; | ||
import java.util.concurrent.ScheduledExecutorService; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.aspectj.lang.ProceedingJoinPoint; | ||
import org.aspectj.lang.annotation.Around; | ||
import org.aspectj.lang.annotation.Aspect; | ||
import org.springframework.stereotype.Component; | ||
|
||
@Slf4j | ||
@Aspect | ||
@Component | ||
public class CircuitBreakerAspect { | ||
|
||
private final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(10); | ||
private final Map<Object, ApiCounter> map = new ConcurrentHashMap<>(); | ||
|
||
@Around("@annotation(annotation)") | ||
public Object around(ProceedingJoinPoint proceedingJoinPoint, CircuitBreaker annotation) { | ||
ApiCounter apiCounter = getApiCounter(proceedingJoinPoint, annotation.minTotalCount()); | ||
if (apiCounter.isClosed()) { | ||
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. 제가 알기론 close 가 정상상태로 알고 있습니다. 그래서 isOpen() 이 일반적인 서킷브레이커 패턴에서 사용하는 네이밍일거같슴다 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. 오홍 그렇네요! 수정했습니다~ |
||
log.warn("현재 해당 {} API는 오류로 인해 중지되었습니다.", proceedingJoinPoint.getTarget()); | ||
return null; | ||
} | ||
try { | ||
Object result = proceedingJoinPoint.proceed(); | ||
apiCounter.countUp(); | ||
return result; | ||
} catch (Throwable e) { | ||
handleError(annotation, apiCounter); | ||
return null; | ||
} | ||
} | ||
|
||
private ApiCounter getApiCounter(ProceedingJoinPoint proceedingJoinPoint, int minTotalCount) { | ||
Object target = proceedingJoinPoint.getTarget(); | ||
if (!map.containsKey(target)) { | ||
map.put(target, new ApiCounter(minTotalCount)); | ||
} | ||
return map.get(target); | ||
} | ||
|
||
private void handleError(CircuitBreaker annotation, ApiCounter apiCounter) { | ||
apiCounter.errorCountUp(); | ||
if (apiCounter.isErrorRateOverThan(annotation.errorRate())) { | ||
apiCounter.close(); | ||
scheduler.schedule(apiCounter::reset, annotation.resetTime(), annotation.timeUnit()); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.parkingcomestrue.external.api; | ||
|
||
import com.parkingcomestrue.external.api.parkingapi.HealthCheckResponse; | ||
|
||
public interface HealthChecker { | ||
|
||
HealthCheckResponse check(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package com.parkingcomestrue.external.api.parkingapi; | ||
|
||
import lombok.Getter; | ||
|
||
@Getter | ||
public class HealthCheckResponse { | ||
|
||
boolean isHealthy; | ||
int totalSize; | ||
|
||
public HealthCheckResponse(boolean isHealthy, int totalSize) { | ||
this.isHealthy = isHealthy; | ||
this.totalSize = totalSize; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package com.parkingcomestrue.external.api.parkingapi; | ||
|
||
import com.parkingcomestrue.common.domain.parking.Parking; | ||
import com.parkingcomestrue.external.api.HealthChecker; | ||
import java.util.List; | ||
|
||
public interface ParkingApiService extends HealthChecker { | ||
|
||
default boolean offerCurrentParking() { | ||
return false; | ||
} | ||
|
||
List<Parking> read(int pageNumber, int size); | ||
|
||
int getReadSize(); | ||
} |
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.
double
인 이유가 있을까요? 정수형 같아보여서용!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.
그렇네여.. 수정했습니다!