FIX: Handling abnormal swithover from zk.

[brido4125]

🔗 Related Issue

캐시 서버로부터의 응답을 통해 master candidate를 설정하고 난 뒤,
zk로부터 비정상적인 switchover를 감지하는 경우에 대한 처리이다.

이중화

상태	ROLE1	ROLE2	처리로직
초기상태	M	S
SwitchOver 응답 후 상태	M	S(masterCandidate)
정상 상태 (1st ZK 변경)	S	S	Invalid Group 처리
정상 상태 (2nd ZK 변경)	S	M	switch-over 로직이 처리
비정상1	X	M	Fail-Over 로직이 처리
비정상2	M	X	처리필요
비정상3	X	X	그룹 제거(기존 로직이 처리)

삼중화

상태	ROLE1	ROLE2	ROLE3	처리로직
초기상태	M	S1	S2
SwitchOver 응답 후 상태	M	S1(masterCandidate)	S2
정상 상태 (1st ZK 변경)	S3	S1	S2	Invalid Group 처리
정상 상태 (2nd ZK 변경)	S3	M	S2	switch-over 로직이 처리
비정상1	S3	M	X	Switch-Over 로직이 처리
비정상2	X	M	S2	Fail-Over 로직이 처리
비정상3	S3	X	M	처리필요
비정상4	X	M	X	Fail-Over 로직이 처리
비정상5	X	X	M	Fail-Over 로직이 처리
비정상6	M	X	X	처리필요
비정상7	X	X	X	그룹 제거(기존 로직이 처리)

참고로 아래와 같은 케이스들은 zk 시퀀스 넘버에 의한
Repl Role 선정 방식에 의해 발생되지 않습니다.

ROLE1	ROLE2	ROLE3
M	X	S

위 표를 참고하여 발생 가능한 비정상 경우들은 아래와 같다.

이중화

• M S : 초기 상태
• M S(master-candidate) : 서버로부터 응답을 받은 후 상태
• M X : new Master가 될 노드가 떨어지고 old Master가 newMaster인 상태 - 비정상
• S M : 정상적인 경우

삼중화

아래 케이스와 함께 이중화의 비정상 케이스처럼
old Master가 newMaster이고 slave가 모두 떨어진 케이스

• M S1 S2: 초기 상태
• M S1(master-candidate) S2: 서버로부터 응답을 받은 후 상태
• S X M : new Master가 될 노드가 떨어지고 2번 Slave가 새로운 New master가 된 상태 - 비정상
• S M S : 정상적인 경우

⌨️ What I did

비정상 이중화인 경우는 oldMaster와 newMaster가 동일하기 때문에 아래 로직을 따른다.

arcus-java-client/src/main/java/net/spy/memcached/MemcachedConnection.java

Line 470 in f60ed48

if (oldMasterAddr.isSameAddress(newMasterAddr)) {

비정상 삼중화인 경우는 oldSlaves 내에 newMaster가 있고
newSlaves 내에 oldMaster가 존재하기에 기존의 Switchover 로직을 따른다.

arcus-java-client/src/main/java/net/spy/memcached/MemcachedConnection.java

Lines 490 to 491 in daf6334

	} else if (oldSlaveAddrs.contains(newMasterAddr)) {
	if (newSlaveAddrs.contains(oldMasterAddr)) {

두 경우 모두 cache server 응답에 의해 설정된
master candidate에 쌓인 ops 객체들을 oldmaster로 옮겨줘야한다.

이중화의 경우 master candidate를 null로 설정해서 master로 op가 처리되도록 한다.

삼중화는 zk로부터 받은 master를 설정하여
최신 master에 op가 처리되도록 한다.

[jhpark816]

@brido4125
리뷰어가 PR 설명을 이해할 수 있는 지를 확인하고
이해가 어렵다면, 먼저 설명하고 리뷰하는 것이 좋겠습니다.

[brido4125]

@uhm0311 @oliviarla

혹시 오프라인 설명이 필요하다면 편하게 말해주세요

[uhm0311]

설명 한 번 해주세요.
내일 해도 됩니다.

[brido4125]

@uhm0311 @oliviarla

PR 코멘트 업데이트 했습니다.

[jhpark816]

리뷰 완료

다른 경우도 확인하여 안전한 방식으로 구현합시다.

[jhpark816]

@brido4125

다른 경우도 확인하여 안전한 방식으로 구현합시다.

위의 코멘트에 대해서도 검토가 되었나요?

[jhpark816]

@uhm0311 @oliviarla
리뷰 다시 진행해 주면 됩니다.

[jhpark816]

@uhm0311 빠른 리뷰 바랍니다.

[uhm0311]

코드가 많이 간략화되었는데, 각 경우를 어떤 원리로 커버할 수 있는지 설명해주었으면 합니다.

[jhpark816]

일부 리뷰

[jhpark816]

리뷰 완료

[jhpark816]

slave 제거할 시에 masterCandidate clear하는 것이 좋겠습니다.

        for (MemcachedNode oldSlaveNode : oldSlaveNodes) {
          removeNodes.add(oldSlaveNode);
          // move operation slave -> master.
          taskList.add(new MoveOperationTask(
              oldSlaveNode, newMasterNode, false));
          // clear the masterCandidate if the removed slave is the masterCandidate.
          if (oldGroup.getMasterCandidate() == oldSlaveNode) {
            oldGroup.clearMasterCandidate();
          }
        }

[jhpark816]

기존 merged PR에 문제가 있어 코멘트 남깁니다.
확인하고, 문제가 있다면 이를 해결하는 PR을 다시 보내주세요.

[oliviarla]

@brido4125 본 PR 코멘트는 다 반영된 상태인거죠?
어떤 PR에서 해결되었는지 코멘트 달아두면 좋겠습니다.

[brido4125]

#867 (review) 코멘트

• https://github.com/jam2in/arcus-works/issues/686#issuecomment-2646920539
• INTERNAL: Remove cancelNoneIdempotent arg in MoveOperationTask #878
  참고