INTERNAL: Retry authentication on failure without reconnecting

[uhm0311]

🔗 Related Issue

• https://github.com/jam2in/arcus-works/issues/754

⌨️ What I did

• auth가 실패했을 때, 재연결을 하지 않고 auth를 재시도합니다.
• SaslClient 객체 내부의 상태를 초기화하기 위해, auth가 실패하면 SaslClient 객체를 새로 생성합니다.
• 후속 PR로 NOT_SUPPORTED 처리를 반영할 예정입니다.

[jhpark816]

@oliviarla @brido4125
정확한 리뷰가 필요합니다.

[brido4125]

연결에 실패할 때 마다, 매번 Sasl Client 객체를 새롭게 생성해 줘야하나요?
연결이 실패한 경우, latch를 통해 buildOperation으로 생성된 sasl operation만 새롭게 보내보고
그 결과를 통해 연결의 성공/실패 여부만 판단하면 안되나요?
(현재 jdk 단의 SaslClient 객체 자체에 대한 이해도가 없는 상태임을 고려해주세요)

[uhm0311]

SaslClient 내부 상태가 몇 단계로 나뉘어져 있는데, 그 단계가 최초 상태로 돌아가지 않으면 재시도가 불가능합니다.
이는 서버와 클라이언트가 서로 주고 받는 값들을 기반으로 새로운 값을 계산해야 하는데, AUTH_ERROR 응답을 받는 경우 SCRAM-256 알고리즘 입장에서 Valid한 값을 받지 않았으므로 다음 값 계산이 불가능하기 때문입니다.

[oliviarla]

SaslClient#dispose 메서드에서 state를 SEND_CLIENT_FIRST_MESSAGE로 초기화하는 방법은 어떤가요?

dispose 메서드의 주석에 Invoking this method invalidates the SaslClient instance. This method is idempotent. 라고 쓰여있고 일반적으로 password를 제거하는 동작을 수행하고 있긴 합니다.

[uhm0311]

dispose() 메소드의 역할은 소멸자의 역할에 더 가깝습니다.
더 이상 SaslClient 객체를 사용하지 않을 때에 호출하는 용도에 더 가깝습니다.

[brido4125]

연관 이슈에 보면 아래 내용이 있는데요,

AuthThread 내에서 AUTH_ERROR 응답을 받은 경우, TCP 연결을 해제하지 않고 auth를 재시도한다. 그 외의 예외 상황(Timeout, inactive node, ERROR 응답 수신 등)에서는 TCP 연결을 해제한 후 다시 연결을 하도록 한다.

현재 구현은 "SASL_OK"가 아닌 모든 경우에
재연결 하지 않고 auth를 재시도 하고 있는 구현이지 않나요?

Timeout, inactive node, ERROR 등의 상황에는 기존로직처럼
연결 해제 후, 재연결하도록 해야할것 같아요

[uhm0311]

@brido4125

Timeout은 기존에 latch.await()에 타임아웃 값을 주고 있지 않아서, 현재는 걸러낼 수 없습니다.
타임아웃 값을 주고 Timeout 처리를 하는 것은 다음 PR로 반영하겠습니다.

그 외의 에러들은 IO Thread의 handleIO() 로직 내에서 알아서 연결이 해제되기 때문에 AuthThread에서 따로 무언가 해줄 것이 없습니다.

[jhpark816]

@uhm0311
본 PR 의미를 이해하기 위해 질문합니다.
어떤 priorStatus 일 경우에 buildOperation() 다시 수행하여 auth 요청을 보내게 되나요?

[jhpark816]

그리고, 어떤 priorStatus 인 경우에 재수행하게 되나요?

[uhm0311]

SASL_CONTINUE 응답을 받은 경우 buildOperation()을 다시 수행하고 다음 auth 과정을 진행합니다.
AUTH_ERROR 응답을 받은 경우에는 auth 과정을 처음부터 재시도합니다.

[jhpark816]

현재 busy 하게 재수행하는 데, 일정 시간 대기후에 재수행해야 하지 않는가요?
그리고 그 동안에 들어오는 요청들에 대한 처리도 있어야 할 것 같습니다.

[uhm0311]

latch.await() 후에 Thread.sleep(100)이 있기 때문에 0.1초에 한 번씩 수행하게 됩니다.

[jhpark816]

개념적으로는 연결을 끊을 필요가 없다는 것인데요.
1초간 요청을 보낼 수 없는 상태이므로,
구현의 편의성 관점에서는 연결을 끊고 1초 후에 재연결을 시도하게 구현하여도 될 것 같습니다.
어떤 구현이 나은가요?

[jhpark816]

Auth 관련 수정을 하나의 commit으로 보는 것이 좋을 수 있겠다는 생각이 듭니다.

[uhm0311]

아래와 같이 정리해주시길 바랍니다.

• AUTH_ERROR 응답을 받았을 떄의 동작 : ...
• CLIENT_ERROR, SERVER_ERROR, ERROR 응답을 받았을 때의 동작 : ...

[jhpark816]

아래와 같이 정리해주시길 바랍니다.

• AUTH_ERROR 응답을 받았을 떄의 동작 : ...
• CLIENT_ERROR, SERVER_ERROR, ERROR 응답을 받았을 때의 동작 : ...

Client에서의 auth 연산에 대해 잘 알고 있으므로,
위의 경우에 어떻게 처리해야 하는 지를 스스로 정리할 수 있을 것입니다.
이슈에 정리해 주시죠.

[uhm0311]

auth 요청에서 ERROR 응답을 받을 경우, delayed reconnect 처리하면 좋겠습니다.
auth 요청이 실패한 상태에서 reconnect 하기 전의 상태에서 들어오는 모든 요청은 "authentication timeout"가 아닌 "authentication failed"로 cancel하면 좋겠습니다.
재연결하여 auth 요청을 보내는 경우에, 다른 요청들은 대기시키고, authentication timeout으로 캔슬하면 됩니다.
auth 실패가 명확한 경우는 항상 1초가 대기하게 만들지 않고 "authentication failed"로 cancel하는 것이 좋습니다.

auth 요청 시에 ERROR 응답이 와도 1초 대기가 필요합니다.

앞서 잘못 판단했습니다.
ERROR 응답이 오면 즉시 재수행하는 것이 맞습니다.

위와 같은 코멘트들 때문에 어떤 동작을 원하시는 것인지 상당히 헷갈리는 상태입니다.
대표님이 원하시는 동작이 있을테니, 이를 알려달라는 것입니다.
제가 생각하는 동작을 알려드리는 것은 쉽게 수행할 수 있는 작업이지만, 대표님이 원하시는 동작이 무엇인지 이해할 수 없으니 다시 정리해주셨으면 좋겠습니다.