FIX: Add operation to temp writeQ when auth is needed

[uhm0311]

🔗 Related Issue

• delayed switchover 처리 도중, old master의 연산을 new master로 이관하는 과정에서 new master가 마침 재연결이 완료되어 auth 연산을 해야 할 가능성이 있다.
• 이러한 경우 old master에게 이관받은 연산을 처리하기 전에 auth 연산을 먼저 처리해야 하는데, 기존에는 이관 받은 연산을 writeQ에 그대로 넣고 있었다.
• 이렇게 이관할 경우, auth 연산은 inputQ에 들어가고 이관 받은 연산은 writeQ에 들어가므로 auth 연산을 먼저 처리할 수 없을 가능성이 생긴다.
• 따라서 new master가 auth를 해야 하는 상황일 때에는 old master에게 이관 받은 연산을 곧바로 writeQ에 넣지 않고 임시 큐에 넣어둔 후, auth가 끝났을 때 임시 큐에서 writeQ로 옮길 필요가 있다.

⌨️ What I did

• MemcachedNode 클래스에 authBlockedWriteQ 필드를 추가합니다.
  • authBlockedWriteQ 필드는 IO Thread가 혼자서 사용할 필드이므로 동시성 제어가 필요하지 않습니다.
• MemcachedNode#setupForAuth()에서 authBlockedWriteQ를 초기화합니다.
• MemcachedNode#addOpToWriteQ()에서 auth가 필요한 경우에는 authBlockedWriteQ에 연산을 넣습니다.
  • old master의 연산을 new master로 이관할 때에 addOpToWriteQ()가 호출됩니다.
• switchover 상황에서 연산을 이관하는 경우 외에 addOpToWriteQ()가 호출되는 경우는 다음과 같습니다.
  • migration으로 인해 NOT_MY_KEY 응답을 받았을 때 : 연산 이관이라는 관점에서 old master의 연산을 new master로 이관하는 경우와 동일합니다.
  • 연결 수립 직후 version 연산을 보낼 때 : version 연산이 auth 연산 처리 이후로 미뤄지게 되지만, 미뤄지더라도 문제가 없는 연산입니다.
• MemcachedNode#authComplete()에서 auth가 성공한 경우 authBlockedWriteQ의 연산을 writeQ로 옮깁니다.
  • auth가 실패했다면 authBlockedWriteQ의 연산을 모두 캔슬합니다.
• IO Thread의 handleWrites() 로직을 수행하지 않고 건너 뛰는 조건을 수정합니다.
  • AS-IS : delayed switchover를 해야 하는 경우 절대로 handleWrites() 로직을 수행하지 않음
  • TO-Be : delayed switchover를 해야 하더라도, auth를 수행해야 하는 상태라면 handleWrites() 로직을 수행함

[jhpark816]

@oliviarla @brido4125 리뷰 바랍니다.

[uhm0311]

@jhpark816 @oliviarla @brido4125

임시 큐를 사용하는 방안으로 변경했습니다.
첫번째 코멘트에 PR의 설명을 수정해두었습니다.
추가 설명이 필요하시다면 말씀해주세요.

[oliviarla]

old master의 연산을 new master로 이관하는 과정에서 new master가 마침 재연결이 완료되어 auth 연산을 해야 할 가능성이 있다.

이 상황에 대해 조금 더 구체적으로 설명해주실 수 있나요?

그리고, 방안을 변경한 이유가 궁금합니다. 단순히 코드 변경이 적어서인지 아니면 다른 이점이 있는지요?

[uhm0311]

방안을 변경한 이유가 궁금합니다. 단순히 코드 변경이 적어서인지 아니면 다른 이점이 있는지요?

방안을 변경하는 것이 코드 변경이 적고 기존 틀에 더 잘 맞는 것 같아서 변경하였습니다.

---

이 상황에 대해 조금 더 구체적으로 설명해주실 수 있나요?

예를 들면 아래와 같은 경우인데, 조금 복잡할 수 있습니다. 필요하다면 오프라인으로 더 설명해드리겠습니다.

• old master : ZK로부터 이벤트를 받아서 switchover를 해야 하지만, readQ에 비멱등성 연산이 있어서 delayed switchover로 분류됨
• new master : Socket IO 수행 도중 Exception이 발생하여 1초 뒤 재연결을 시도하기로 함
• old master : readQ에서 먼저 처리되는 연산들이 get과 같은 read 성격의 연산이라서 정상 응답을 받음
• new master : 1초가 지나서 재연결이 완료됨
• old master : readQ에 있던 write 성격의 연산 응답으로 REPL_SLAVE 응답을 받았고, 이에 따라 old master에 남아 있는 연산을 new master로 이관해야 함

위와 같은 상황에서 new master의 writeQ에 곧바로 연산을 이관시키는 경우, new master가 원래 수행해야 하는 auth 연산은 inputQ에 들어가게 됩니다.
이렇게 되면 old master에게 이관 받은 연산들은 auth가 아직 수행되지 않아서 실패하게 됩니다.
이렇게 실패하지 않도록 하기 위해, auth를 먼저 수행할 수 있도록 writeQ 대신 임시 큐를 사용하여 auth 연산만 inputQ - writeQ를 거쳐서 연산이 처리될 수 있도록 하려는 목적입니다.

[brido4125]

authBlockedWriteQ 만드는 대신 기존 write 큐를 덱으로 써서,
auth 연산이 필요한 경우 제일 앞단에 객체 넣어주는건 구현적으로 불가능한가요?

[uhm0311]

authBlockedWriteQ 만드는 대신 기존 write 큐를 덱으로 써서,
auth 연산이 필요한 경우 제일 앞단에 객체 넣어주는건 구현적으로 불가능한가요?

가능은 하지만 조금 복잡한 구현이 추가로 필요합니다.
기존 MemcachedNode#fillWriteBuffer()에서는 writeQ의 연산을 하나만 꺼내서 전송하는 것이 아니라, 하나를 전송하고도 더 전송할 수 있으면 계속 꺼내서 전송합니다.
따라서 auth 연산이 필요함 && 맨 앞의 연산이 auth 연산임 조건을 동시에 만족할 때에만 writeQ에서 꺼내서 전송하도록 만드는 로직이 추가되어야 합니다.

[brido4125]

전 현재처럼 기존 write 큐에 있는 연산을 옮겼다가, Auth 완료하고 나서 다시 옮겨 놓은 연산들 수행하는 것보다 덱 사용하는 구현이 직관적이지 않나 싶긴합니다.

이 부분은 @jhpark816 이나 @uhm0311 님은 어떻게 생각하시나요?

[uhm0311]

Deque를 사용하려면 아래의 작업이 먼저, 혹은 동시에 수행되어야 합니다.

• AuthThread 제거 : 동시성 제어를 위해 AuthThread가 writeQ에 직접 넣을 수 없으므로, writeQ 맨 앞에 직접 넣으려면 AuthThread를 먼저 제거해야 함

[jhpark816]

리뷰 완료

[jhpark816]

리뷰 완료

[jhpark816]

@oliviarla @brido4125
수정 사항을 확인 바랍니다.