AI 리뷰는 코드가 아니라 설계 계약을 읽는다──Fable5에게 PooledMailKit을 리뷰하게 하다

지난 기사에서는 AI가 생성한 SMTP 송신 코드가 왜 운영 환경에서 망가지는지, 그리고 그 결함을 보완하기 위해 라이브러리를 만들었다는 내용을 다루었다.

이번에는 그 후속 이야기이다.

제작 중이던 SMTP 연결 풀 라이브러리는 이후 PooledMailKit으로서 NuGet에 공개했다.

dotnet add package PooledMailKit --version 0.1.0.7

그리고 이번에는 이 PooledMailKit을 Fable5에게 리뷰하게 했다.

결론부터 말하자면, Fable5의 리뷰는 단순한 코드 리뷰가 아니었다.

null 체크나 명명 규칙(Naming convention)에 대한 것이 아니었다.

"설계 문서에서 약속한 것"과 "구현에서 실제로 일어나는 것" 사이의 괴리를 지적해 왔다.

이는 상당히 중요한 변화라고 생각한다.

PooledMailKit이 해결하려 했던 문제

PooledMailKit은 MailKit의 SmtpClient를 매번 new / Dispose 하지 않고, SMTP 연결을 풀링(Pooling)하여 재사용하기 위한 라이브러리이다.

단, 목적은 단순한 고속화가 아니다.

이 라이브러리가 방지하고자 하는 것은 다음과 같은 운영상의 실패이다.

요청마다 SMTP 연결을 생성 및 파기
지속적인 트래픽(Sustained traffic)으로 인해 TIME_WAIT과 에페머럴 포트(Ephemeral port)를 계속 소비
송신 실패 후 망가진 SMTP 세션을 재사용
서버 측 장애 시 즉시 재연결을 반복
풀 고갈(Pool exhausted) 시 무한 대기
서버가 본문을 수락했을지도 모르는 상태에서 맹목적으로 재시도(Retry)

즉, PooledMailKit의 핵심은 "SMTP 연결을 재사용하는 것"이 아니다.

핵심은 SMTP 송신의 실패 경계(Failure boundary)를 안전하게 다루는 것이다.

SMTP에서는 실패한 타이밍에 따라 의미가 달라진다.

MAIL FROM 전에 실패했는가.

RCPT TO에서 실패했는가.

DATA 전에 실패했는가.

DATA 시작 후에 실패했는가.

혹은 DATA 종료 후에 서버가 명시적으로 거부했는가.

이 차이에 따라 자동 재시도를 해도 되는지가 달라진다.

특히 DATA 시작 후의 실패는 위험하다.

클라이언트 입장에서는 실패로 보일지라도, 서버 측에서는 메시지 본문을 받았을 가능성이 있다. 여기서 맹목적으로 재시도하면 중복 송신이 발생한다.

따라서 PooledMailKit에서는 연결 풀뿐만 아니라, 실패 분류, 송신 단계(Stage), 재시도 가능 여부, 멀티 호스트 페일오버(Multi-host failover), 메트릭스 계약(Metrics contract)을 설계 대상으로 삼았다.

Fable5에게 전달한 것

Fable5에게는 코드만 전달한 것이 아니다.

설계 문서, SMTP 세션 관리 시퀀스, 에러 분류 사양, 메트릭스 계약을 포함하여 리뷰하게 했다.

이 점이 중요하다.

코드만 보면 AI 리뷰는 표면적으로 흐르기 쉽다.

null 체크
Dispose 누락
예외 처리
명명(Naming)
테스트 부족

물론 이것들도 중요하지만, 이번에 보고 싶었던 것은 그것이 아니다.

보고 싶었던 것은 이 라이브러리가 설계상 약속하고 있는 것을 구현이 정말로 충족하고 있는가였다.

그 결과, Fable5는 4가지 중요한 괴리를 지적했다.

지적 1: 스테이지 추적(Stage tracking)이 구현되지 않음

가장 중요한 지적은 이것이었다.

설계상 PooledMailKit은 SMTP 송신 단계를 추적하고, 실패한 위치에 따라 재시도 가능 여부를 판단하게 되어 있었다.

그런데 구현에서는 실제 코드에서 SmtpStageAwareException이 생성되지 않고 있었다.

그 때문에 송신 중에 발생한 예외는 거의 모두 DataStarted로 취급되고 있었다.

이는 단순한 구현 누락이 아니다.

이 라이브러리의 핵심 가치가 죽어 있다는 지적이다.

설계 문서에서는 SMTP 4xx는 일시적 실패(Transient failure)로 정의하여, 예산 범위 내에서 재시도가 가능하다고 정의했다.

하지만 구현상 송신 중의 예외가 모두 DataStarted로 취급된다면, 4xx 역시 "DATA 시작 후의 불명확한 실패"로 취급된다.

결과적으로 재시도 가능한 일시적 실패가 재시도되지 않는다.

반대로, DATA 종료 후에 서버가 명시적으로 거부한 실패도 결과 불명으로 취급된다.

이는 메트릭스에도 영향을 미친다.

본래 '수리 거부(unacceptable)가 확정된 실패'로 집계해야 할 것이 UnknownAfterData로 취급되면, send.ambiguous.count가 부풀려진다.

운영자는 "중복 송신 리스크가 있는 실패가 늘어나고 있다"라고 판단할지도 모른다.

하지만 실제로는 서버가 명시적으로 거부하고 있을 뿐일 수도 있다.

이는 코드의 버그라기보다 관측의 왜곡이다.

그리고 운영 시스템에서 관측이 왜곡되는 것은 치명적이다.

지적 2: 메시지 기인 실패로 인해 호스트가 cooldown 상태가 됨

다음으로 중요했던 것은 호스트의 reconnect cooldown에 관한 지적이었다.

구현에서는 연결을 폐기(discard)하는 경로에서 무조건 ApplyReconnectCooldown이 호출되고 있었다.

즉, 550 수신처 거부와 같은 메시지 기인 실패에서도 호스트 전체가 cooldown 상태에 들어갈 가능성이 있었다.

이는 설계 개념으로서 잘못되었다.

550 수신처 거부는 SMTP 서버가 고장 났다는 의미가 아니다.

해당 수신처가 존재하지 않거나, 정책상 거부되었다는 의미이다.

그런데도 호스트 전체를 "당분간 새로운 연결을 해서는 안 되는" 상태로 만들면, 정상적인 SMTP 서버를 스스로 사용할 수 없게 만든다.

여기서 혼동하고 있었던 것은 다음 두 가지다.

연결을 폐기한다
호스트를 불건전(unhealthy)하다고 간주한다

이 두 가지는 별개의 개념이다.

송신에 사용한 연결을 폐기하는 판단은 있을 수 있다.

하지만 그것을 이유로 호스트 전체를 불건전하다고 간주해서는 안 된다.

특히 고트래픽(high-traffic) 환경에서는 이 문제가 PoolExhausted로 현상화될 수 있다.

부정확한 수신처가 섞인다.

연결이 폐기된다.

호스트가 cooldown 상태가 된다.

MinPool refill도 멈춘다.

풀(pool)이 줄어든다.

송신 가능한 연결이 부족해진다.

이는 SMTP 서버의 장애가 아니다.

애플리케이션 측에서 메시지 실패를 호스트 장애로 취급한 결과이다.

Fable5는 이 부분을 지적해 왔다.

이는 사람의 리뷰에서도 놓치기 쉽다.

코드상으로는 "망가진 lease를 폐기하고 cooldown한다"라는 자연스러운 처리로 보이기 때문이다.

하지만 SMTP의 의미론(semantics) 관점에서 보면 그것은 옳지 않다.

지적 3: MinPool refill 실패가 송신 실패로 이어짐

세 번째는 MinPool의 취급이다.

MinPool은 풀을 예열(warm up)해 두기 위한 메커니즘이다.

송신 가능 여부 그 자체를 결정하는 것이 아니다.

유휴(idle) 연결이 남아 있다면, MinPool refill에 실패하더라도 송신은 가능해야 한다.

그런데 구현에서는 AcquireLeaseAsync의 서두에서 EnsureMinimumPoolSizeAsync가 호출되고, 그 연결 생성 실패가 그대로 throw되는 경로가 있었다.

즉, 송신에 사용할 수 있는 유휴 연결이 남아 있어도, warm refill의 실패로 인해 acquire 전체가 실패할 가능성이 있었다.

이는 풀의 책임 분리(separation of concerns) 관점에서 잘못되었다.

MinPool은 성능과 안정성을 위한 보조 기능이다.

송신 가능한 연결이 있는지 여부와는 별개로 다루어야 한다.

본래는 다음과 같이 되어야 한다.

사용할 수 있는 유휴 연결이 있으면 반환한다
없으면 온디맨드(on-demand)로 생성한다
생성할 수 없으면 기다린다
MinPool refill은 best-effort로 수행한다

warm pool 유지에 실패했다고 해서, 이용 가능한 연결을 무시하고 송신 실패로 처리해서는 안 된다.

이 또한 코드만 보고 있으면 놓치기 쉽다.

하지만 "MinPool은 무엇을 위해 존재하는가"라는 설계 계약(design contract) 관점에서 보면 명백히 잘못되었다.

지적 4: 연결 생성 실패 시 페일오버(failover)하지 않음

네 번째는 멀티 호스트 구성에서의 페일오버이다.

설계 문서에서는 Primary의 연결 생성에 실패할 경우, cooldown에 넣고 Secondary를 시도하는 흐름이었다.

하지만 구현에서는 연결 생성 실패 시 AcquireLeaseAsync 전체가 throw되는 구조였다.

즉, 동일한 acquire 내에서 다른 호스트로 페일오버하지 않는다.

이는 멀티 호스트 대응으로서 취약하다.

사용자가 여러 SMTP 호스트를 설정했다면, 기대하는 동작은 다음과 같다.

Primary 시도
연결 생성 실패
Primary를 cooldown 상태로 전환
Secondary 시도
deadline까지 후보 호스트를 전환
모두 실패하면 acquire timeout 발생

하지만 첫 번째 연결 생성 실패 시점에 예외(throw)가 발생한다면, 다중 호스트를 사용하는 가치는 크게 떨어진다.

이는 「멀티 호스트 설정이 있다」는 것과 「멀티 호스트 페일오버 (failover)가 작동한다」는 것의 차이다.

설정할 수 있다는 것이 곧 작동한다는 뜻은 아니다.

중간 중요도이지만, 상당히 위험한 지적

Fable5의 리뷰에서는 중간 중요도로 언급되었으나, 개인적으로는 상당히 중요하다고 생각되는 지적이 있다.

송신 성공 후의 lease return 실패로 인해, 「성공한 송신」이 실패로 보고될 가능성이 있다는 지적이다.

SMTP 관점에서 송신 성공이 확정된 후, 연결을 풀(pool)로 되돌리는 처리에서 실패하더라도 그것은 송신 실패가 아니다.

여기서 예외를 호출자(caller)에게 반환하면, 호출자는 송신에 실패했다고 판단한다.

그리고 재전송을 시도할지도 모른다.

하지만 SMTP 서버는 이미 메시지를 수락한 상태다.

즉, 중복 송신이 발생한다.

이는 풀 관리상의 실패를 SMTP 송신 실패로서 호출자에게 보여주게 되는 문제이다.

이 경계는 매우 중요하다.

송신 성공 후의 뒷정리는 best-effort로 수행되어야 한다.

적어도 송신 결과를 실패로 바꾸어서는 안 된다.

여기서도 묻고 있는 것은 코드가 예외를 던지는지 여부가 아니다.

「무엇을 송신 실패로서 이용자에게 보여주어야 하는가」라는 API 계약 (API contract)이다.

Fable5가 본 것

이번에 Fable5가 본 것은 코드의 국소적인 오류가 아니었다.

그것은 설계 계약 (design contract)과 구현 (implementation) 사이의 차이점이었다.

스테이지 분류를 설계하고 있지만, 구현에서는 추적되지 않음
reconnect cooldown은 연결 생성 실패를 억제하기 위한 것인데, 메시지 실패 시에도 발동함
MinPool은 보조 기능임에도 송신 가능 여부에 영향을 미침
멀티 호스트 페일오버를 설계하고 있지만, 동일한 acquire 내에서 전환되지 않음
송신 성공 후의 뒷정리 실패가 송신 실패로 보이게 됨

이것들은 모두 코드만 봐서는 이해하기 어렵다.

왜냐하면 코드는 나름대로 잘 정돈되어 있기 때문이다.

리스(lease)의 이중 반환 방지 로직도 있다.

MaxPoolSize를 지키기 위한 pending connection creations도 있다.

weighted round-robin도 구현되어 있다.

timeout 처리도 있다.

메트릭스(metrics) 명칭도 설계에 맞추어 놓았다.

즉, 표면적인 완성도는 높다.

하지만 설계 계약과 대조해 보면 중요한 지점에서 의미가 어긋나 있다.

이것이 이번 리뷰의 본질이었다.

AI 리뷰의 가치는 어디에 있는가

이번에 알게 된 것은 Fable5가 대단하다는 단순한 이야기가 아니다.

물론 Fable5의 실력은 진짜라고 생각한다.

적어도 이번 리뷰에서는 상당히 깊은 곳까지 들여다보았다.

하지만 그뿐만이 아니다.

Fable5가 깊이 있는 리뷰를 할 수 있었던 이유는 리뷰 대상에 설계 정보가 있었기 때문이다.

코드뿐만 아니라 다음과 같은 정보가 있었다.

이 라이브러리가 무엇을 방지하기 위한 것인가
무엇을 대상 외로 하는가
SMTP 실패를 어떻게 분류하는가
DATA 시작 후의 실패를 왜 위험하다고 간주하는가
reconnect cooldown은 무엇에 대한 억제인가
메트릭스로 무엇을 관측하는가
멀티 호스트 선택은 어떻게 동작해야 하는가

이것들이 외부화되어 있었기에 AI는 구현과 대조할 수 있었다.

역설적으로 말하면, 설계 계약이 없었다면 이 리뷰는 나오지 않았을 가능성이 높다.

「이 코드를 리뷰해 줘」라고만 한다면, AI는 코드 안에 있는 정보만으로 판단한다.

그 경우 지적은 국소적일 수밖에 없다.

하지만 「이 설계 계약을 충족하는지 리뷰해 줘」라고 전달하면 AI 리뷰는 달라진다.

코드 리뷰에서 계약 리뷰 (contract review)로 변하는 것이다.

지난 글과의 연결

지난 글에서는 AI가 생성한 코드를 운용 가능한 상태로 만들려면, 인간이 목적, 범위, 품질 기준을 정의해야 한다고 썼다.

이번 결과는 그 연장선에 있다.

인간이 설계 계약을 외부화하면, AI는 그것을 사용하여 리뷰할 수 있다.

이는 AI가 자율적으로 품질을 보증한다는 이야기가 아니다.

AI 리뷰의 품질은 AI의 성능만으로 결정되지 않는다.

인간이 무엇을 전달했느냐에 따라 결정된다.

코드만 전달하면 코드 안의 내용만을 본다.

설계 계약을 전달하면 코드와 계약의 차이점을 본다.

이 차이는 매우 크다.

인간의 역할은 무엇인가

이 단계에 이르면, 인간의 역할은 더욱 명확해진다.

인간이 담당해야 할 것은 코드를 작성하는 것이 아니다.

적어도, 모든 코드를 스스로 작성하는 것은 아니다.

인간이 담당해야 할 것은 다음과 같은 판단이다.

이 라이브러리는 무엇을 방지하기 위해 존재하는가
어떤 실패를 허용하지 않는가
어떤 경계를 넘으면 위험한가
무엇을 메트릭스 (Metrics)로 관측할 것인가
무엇을 대상 외로 할 것인가
AI의 리뷰 결과를 수정 대상으로 받아들일 것인가

이것은 조정 (Adjustment)의 작업이다.

목적을 조정한다.

범위를 조정한다.

품질 기준을 조정한다.

리뷰 결과를 수용할지 여부를 판단한다.

AI는 이 조정을 지원할 수 있다.

하지만, 무엇을 지켜야 하는지를 처음에 정의하는 것은 인간이다.

이번의 결론

이번 Fable5 리뷰를 통해 알 수 있었던 점은 다음 한 문장으로 요약할 수 있다.

AI 리뷰는 코드만 읽게 하면 구현 리뷰 (Implementation Review)가 된다.

설계 계약 (Design Contract)도 읽게 하면 계약 리뷰 (Contract Review)가 된다.

그리고, 운영 가능한 소프트웨어에 필요한 것은 후자이다.

동작하는 코드를 작성하는 것은 AI에게 점점 어려워지지 않고 있다.

하지만, 동작하는 코드가 설계 계약을 충족하는지 확인하려면, 애초에 계약이 필요하다.

계약이 없다면, AI도 인간도 리뷰할 수 없다.

이번 PooledMailKit에서는 설계 문서, 에러 분류, SMTP 세션 관리, 메트릭스 계약을 외부화하였다.

그렇기에 Fable5는 그것들과 구현을 대조할 수 있었다.

이는 AI 리뷰의 실력을 보여주는 이야기인 동시에, 인간 측의 준비가 리뷰 품질을 결정한다는 이야기이기도 하다.

AI에게 리뷰를 시키려면 코드만 전달해서는 안 된다.

코드의 외부에 있는 판단을 리뷰 가능한 형태로 전달할 필요가 있다.

그때 AI 리뷰는 단순한 버그 찾기가 아니라, 설계 계약의 검사 (Inspection)가 된다.