CompletableFuture 코드 리뷰 기록

 

안녕하세요!

얼마 전에 저희 팀 성실천재쥬니어님의 PR에 코드 리뷰를 진행하였습니다.

하나의 서비스 흐름에서 모든 인터페이스의 리턴 타입이 CompletableFuture였습니다.

먼저 비즈니스를 살펴보고 코드를 읽다보니 Fire and Forget 패턴을 CompletableFuture + AsyncHttpClient를 통해 구현하신 것 같았습니다.

(정확한 사유는 아직 여쭤보지 않아서 모릅니다. 그냥 코드 보고 추측했습니다.) 

 

충분히 사용할 수 있는 기술들이지만 해당 PR에서는 몇가지 크리티컬한 문제가 있었고,

현재 구조에서는 불필요한 것 뿐만 아니라 코드의 복잡성을 올린다고 판단하여 CompletableFuture를 모두 걷어내고 AsyncHttpClient도 SyncHttpClient으로 변경하길 요청드렸습니다.

 

왜 이러한 선택을 하게 되었는지 포스팅 해보고자 합니다. 

 

기존 서비스 코드 구조

대략적인 서비스 코드의 구조는 다음과 같습니다.

이 구조에서 포인트는 다음과 같습니다.

• 2, 3, 4작업 모두 리턴값이 CompletableFuture(Void) 이고 5번에서 allOf().join()으로 모아주고 있습니다. 
• 3, 4 작업 모두 blocking driver를 사용하는 repository 호출을 끼고 있으며
  일부분에서 CompletableFuture + AsyncHttpClient을 사용하는 Http Client Call을 진행하고 있습니다. 
  Http Call은 수십ms 응답을 보장하며, 이 client call 작업의 callback으로 blocking I/O 호출을 여러번 하고 있습니다. 

 

문제 1. 

이 구조에서 CompletableFuture는 무의미합니다. 

 

대부분이 blocking 호출이고 중간중간 AsyncHttpClient을 사용하는 부분만 non-blocking 호출입니다. 

(빨간 선으로 표시된 부분이 s1, s2, s3의 asyncHttpClient 사용 부분입니다.) 

non-blocking 작업 자체가 적을 뿐만 아니라 이 asyncHttpClient로 호출하는 AP는 수십ms 이내 응답을 보장합니다. 

 

이런 상황에서 굳이 비동기 클라이언트를 사용하고, CompletableFuture를 사용하여 코드 복잡도를 올리고 유지보수성과 testablity를 낮출 필요가 없어보입니다.

 

문제 2.

CompletableFuture는 문법 뿐만 아니라 사용방법에도 다소 높은 이해도가 필요합니다. 

 

CompletableFuture는 다양한 메서드를 제공합니다. 수십가지의 체이닝 메서드를 제공하며 크게는 then**, then**Async 두 가지 구분으로 나눌 수 있습니다.

해당 코드에서는 chain 메서드로 thenRun이 사용되었습니다. 

then** 메서드는 future의 상태가 done이라면 콜백을 등록한 스레드에서 callback이 실행되고, done상태가 아니면 future에 값을 바인딩 해주는 스레드에서 callback이 실행됩니다.

 

낙관적으로는 future에 값을 바인딩 해주는 스레드는 http call을 하므로 future가 리턴되고나서 future는 done상태가 아닐 확률이 높습니다. 

 

그러므로 future에 값을 바인딩해주는 AsyncHttpClient thread를 사용하게 되고, 이 callback안에서 blocking I/O를 수행하므로 덩달아 AsyncHttpClient thread를 잡아먹게 됩니다. 

 

최악의 상황에 요청이 폭발적으로 늘어나 데이터베이스 지연이 생겨 repository 호출이 느려진다면, AsyncHttpClient thread 고갈이 오게 되고 이는 같은 Pool 내 barrier(격벽)가 없는 상태라면 장애를 전파할 수 있습니다. 

 

 

결과

1. 현재 우리 시스템에서 서비스 코드에 완전한 non blocking 코드는 불가능한 점 
2. CompletableFuture는 사용방법에 대한 깊은 이해도가 필요하고 이 구조에서 유지보수와 테스트에 비용이 높은 것 대비 얻을 수 있는 이점은 매우 미미한 점. 
3. 수ms 를 위해 AsyncClient을 사용하는 것은 무의미한 점

위와 같은 이유로 CompletableFuture를 걷어내고 AsyncHttpClient를 SyncHttpClient 로 변경을 요청드렸습니다.

만약 fire and forget 패턴을 원하신다면 CompletableFuture + AsyncClient를 사용하는 것이 아닌 @Async를 통한 타 Executor에 위임하는 방식을 권해드렸습니다. 

 

 

---

 

이미 개발 완료된 건을 뒤집는 제안이라 너무 죄송했습니다^_ㅠ

개발 전 디자인 리뷰는 진행하는 편인데, 대략적으로 어떤 기술을 사용하여 구현할지 리뷰하는 것도 규칙으로 만들어볼까 합니다.

그래도 개발 하시면서 많은 공부가 되었을 거라 생각합니다ㅎㅎ 저는 이분 연차에 이렇게 해볼 생각 조차 못했는데, 보면서 참 많이 배우게 되는 동료입니다.