FIX: Handling abnormal swithover from zk. #867

brido4125 · 2025-01-08T08:11:59Z

🔗 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 · 2025-01-08T08:49:20Z

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

brido4125 · 2025-01-09T00:58:51Z

@uhm0311 @oliviarla

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

uhm0311 · 2025-01-09T08:51:55Z

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

brido4125 · 2025-01-16T07:57:00Z

@uhm0311 @oliviarla

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

src/main/java/net/spy/memcached/MemcachedConnection.java

src/main/java/net/spy/memcached/MemcachedReplicaGroup.java

src/main/java/net/spy/memcached/MemcachedConnection.java

src/main/java/net/spy/memcached/MemcachedReplicaGroup.java

src/main/java/net/spy/memcached/MemcachedConnection.java

jhpark816

리뷰 완료

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

src/main/java/net/spy/memcached/MemcachedConnection.java

jhpark816 · 2025-01-21T04:36:12Z

@brido4125

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

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

jhpark816 · 2025-01-21T07:23:00Z

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

jhpark816 · 2025-01-23T01:42:54Z

@uhm0311 빠른 리뷰 바랍니다.

uhm0311 · 2025-01-23T02:55:37Z

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

src/main/java/net/spy/memcached/MemcachedConnection.java

brido4125 · 2025-01-23T04:12:07Z

src/main/java/net/spy/memcached/MemcachedConnection.java

@@ -485,13 +485,32 @@ private void updateReplConnections(List<InetSocketAddress> addrs) throws IOExcep
            // move operation slave -> master.
            taskList.add(new MoveOperationTask(
                oldSlaveNode, oldMasterNode, false));
+            // clear the masterCandidate if the removed slave is the masterCandidate.
+            if (oldGroup.getMasterCandidate() == oldSlaveNode) {


oldMaster와 newMaster가 동일하고
oldSlave들 중 masterCandidate가 있는 경우
이를 clear 하여 null로 설정시켜 줍니다.

PR 코멘트의 (M,X)와 (M,X,X) 인 경우를 처리시켜줍니다.

이 경우로 변경된 이유는 아래 코멘트를 참고해주시길 바랍니다.
#867 (comment)

그럼 그 이후의 처리는 어떻게 되나요?

M S1 S2 상태에서 M에게 다시 SWITCHOVER 응답을 받은 경우

M S1 S2 상태에서 ZK로부터 S1 M S2를 받은 경우

다른 그룹의 변경에 의해 M S1 S2 상태가 발생하는 경우
현재 로직은 changedGroup에 추가되지 않아서
변경이 생기지 않은 그룹은 아무런 로직을 수행하지 않습니다.

M S1 S2 상태에서 M에게 다시 SWITCHOVER 응답을 받은 경우

이미 masterCandidate가 설정되어 있는 상태라서 재설정 하지 않고
swtichover 작업이 진행 예정

M S1 S2 상태에서 ZK로부터 S1 M S2를 받은 경우

정상적인 switchover 로직 수행

저는 아래와 같이 이해했는데 혹시 아니면 말씀해주세요

SWITCHOVER 응답 -> 다른 그룹의 변경에 의해 동일한 형상(M S1 S2) zk 이벤트 발생 -> 다시 SWITCHOVER 응답 or (S1 M S2) Switchover 이벤트 발생

저는 아래와 같이 이해했는데 혹시 아니면 말씀해주세요

맞습니다.

현재 로직은 changedGroup에 추가되지 않아서
변경이 생기지 않은 그룹은 아무런 로직을 수행하지 않습니다.

#867 (comment)

이 코멘트에는 changedGroup을 구하는 로직이 제거되는 것도 염두할 것처럼 보였는데 당장은 염두하지 않는 건가요?

이미 masterCandidate가 설정되어 있는 상태라서 재설정 하지 않고
swtichover 작업이 진행 예정

490라인에서는 clearMasterCandidate() 메소드를 호출하는데, 이후에 언제 어디에서 masterCandidate가 다시 설정되나요?

정상적인 switchover 로직 수행

Delayed switchover 인가요? 혹은 즉시 switchover 인가요?

이 코멘트에는 changedGroup을 구하는 로직이 제거되는 것도 염두할 것처럼 보였는데 당장은 염두하지 않는 건가요?

넵, 염두하지 않는다면 490라인이 호출되지 않습니다.
우선은 염두하지 않고 추후에 changedGroup을 리팩토링할때 고려할 생각입니다.

Delayed switchover 인가요? 혹은 즉시 switchover 인가요?

서버 응답을 이미 받았기 때문에 즉시 swtichover 입니다.

서버 응답을 이미 받았기 때문에 즉시 swtichover 입니다.

ZK로부터 새로운 캐시 리스트를 받았을 때 이미 캐시 서버로부터 응답을 받은 경우와 받지 않은 경우로 나뉘게 되는데요.
이미 캐시 서버 응답을 받은 경우에는 즉시 swtichover일 것이고, 캐시 서버 응답을 아직 받지 않고 ZK의 캐시 리스트를 먼저 받은 경우에는 기존의 Delayed switchover으로 충분히 처리가 되나요?

이미 masterCandidate가 설정되어 있는 상태라서 재설정 하지 않고
swtichover 작업이 진행 예정

490라인에서는 clearMasterCandidate() 메소드를 호출하는데, 이후에 언제 어디에서 masterCandidate가 다시 설정되나요?

이 부분도 질문했는데, 못보고 지나친 것 같습니다.

오프라인으로 말씀해주신 궁금한 케이스에 대한 답변입니다.
아래 코멘트의 2번 경우에서는 masterCandidate가 clear 되지 않습니다.
왜냐하면 해당 oldSlave는 newSlaves에 포함되어 있기 때문입니다.

#867 (comment)

jhpark816

일부 리뷰

jhpark816 · 2025-01-26T07:21:27Z

src/main/java/net/spy/memcached/MemcachedConnection.java

+              oldGroup.setMasterCandidateByAddr(newMasterAddr.getIPPort());
+              taskList.add(new MoveOperationTask(
+                      oldMasterCandidate, oldGroup.getMasterCandidate(), false));
+            }


@brido4125
아래 경우에서 masterCandidate 설정은 clear되어야 합니다.
이에 대한 처리가 있는 지를 확인해 주세요.

상태 ROLE1 ROLE2 ROLE3 처리로직

초기상태 M S1 S2

SwitchOver 응답 후 상태 M S1(masterCandidate) S2

비정상5 X X M Fail-Over 로직이 처리

jhpark816 · 2025-01-26T07:22:14Z

src/main/java/net/spy/memcached/MemcachedConnection.java

            changeRoleGroups.add(oldGroup);
+            if (!newMasterAddr.isSameAddress(
+                    ((ArcusReplNodeAddress) oldMasterCandidate.getSocketAddress()))) {


아래 형태로 비교해도 되지 않나요?

if (oldMasterCandidate != newMasterAddr) {

brido4125 force-pushed the fix/abnormalSwithcover branch from 1286a78 to 0e988d2 Compare January 8, 2025 08:14

jhpark816 requested review from uhm0311 and oliviarla January 8, 2025 08:49

brido4125 marked this pull request as draft January 16, 2025 05:46

brido4125 self-assigned this Jan 16, 2025

brido4125 marked this pull request as ready for review January 16, 2025 07:52

uhm0311 requested changes Jan 16, 2025

View reviewed changes

oliviarla reviewed Jan 16, 2025

View reviewed changes

src/main/java/net/spy/memcached/MemcachedConnection.java Outdated Show resolved Hide resolved

brido4125 marked this pull request as draft January 17, 2025 01:28

brido4125 force-pushed the fix/abnormalSwithcover branch from 0e988d2 to 4657886 Compare January 17, 2025 02:02

brido4125 marked this pull request as ready for review January 17, 2025 02:16

brido4125 force-pushed the fix/abnormalSwithcover branch from 4657886 to ce2f9fc Compare January 17, 2025 02:18

uhm0311 reviewed Jan 17, 2025

View reviewed changes

src/main/java/net/spy/memcached/MemcachedConnection.java Show resolved Hide resolved

uhm0311 reviewed Jan 17, 2025

View reviewed changes

src/main/java/net/spy/memcached/MemcachedConnection.java Outdated Show resolved Hide resolved

uhm0311 previously approved these changes Jan 17, 2025

View reviewed changes

jhpark816 reviewed Jan 20, 2025

View reviewed changes

src/main/java/net/spy/memcached/MemcachedConnection.java Outdated Show resolved Hide resolved

brido4125 dismissed uhm0311’s stale review via 542c8c4 January 20, 2025 05:39

brido4125 force-pushed the fix/abnormalSwithcover branch from ce2f9fc to 542c8c4 Compare January 20, 2025 05:39

brido4125 marked this pull request as draft January 20, 2025 05:45

brido4125 marked this pull request as ready for review January 20, 2025 05:46

FIX: Handling abnormal swithover from zk.

3cad10c

brido4125 force-pushed the fix/abnormalSwithcover branch from 542c8c4 to 3cad10c Compare January 21, 2025 06:53

brido4125 requested a review from jhpark816 January 21, 2025 07:01

oliviarla approved these changes Jan 22, 2025

View reviewed changes

brido4125 commented Jan 23, 2025

View reviewed changes

src/main/java/net/spy/memcached/MemcachedConnection.java Show resolved Hide resolved

brido4125 commented Jan 23, 2025

View reviewed changes

uhm0311 approved these changes Jan 24, 2025

View reviewed changes

jhpark816 reviewed Jan 26, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: Handling abnormal swithover from zk. #867

FIX: Handling abnormal swithover from zk. #867

brido4125 commented Jan 8, 2025 •

edited by jhpark816

Loading

jhpark816 commented Jan 8, 2025

brido4125 commented Jan 9, 2025

uhm0311 commented Jan 9, 2025

brido4125 commented Jan 16, 2025

jhpark816 left a comment

jhpark816 commented Jan 21, 2025

jhpark816 commented Jan 21, 2025

jhpark816 commented Jan 23, 2025

uhm0311 commented Jan 23, 2025

brido4125 Jan 23, 2025

uhm0311 Jan 23, 2025

brido4125 Jan 23, 2025

uhm0311 Jan 23, 2025 •

edited

Loading

brido4125 Jan 23, 2025

uhm0311 Jan 23, 2025 •

edited

Loading

brido4125 Jan 23, 2025

jhpark816 left a comment

jhpark816 Jan 26, 2025

jhpark816 Jan 26, 2025 •

edited

Loading

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

FIX: Handling abnormal swithover from zk. #867

Are you sure you want to change the base?

FIX: Handling abnormal swithover from zk. #867

Conversation

brido4125 commented Jan 8, 2025 • edited by jhpark816 Loading

🔗 Related Issue

이중화

삼중화

이중화

삼중화

⌨️ What I did

jhpark816 commented Jan 8, 2025

brido4125 commented Jan 9, 2025

uhm0311 commented Jan 9, 2025

brido4125 commented Jan 16, 2025

jhpark816 left a comment

Choose a reason for hiding this comment

jhpark816 commented Jan 21, 2025

jhpark816 commented Jan 21, 2025

jhpark816 commented Jan 23, 2025

uhm0311 commented Jan 23, 2025

brido4125 Jan 23, 2025

Choose a reason for hiding this comment

uhm0311 Jan 23, 2025

Choose a reason for hiding this comment

brido4125 Jan 23, 2025

Choose a reason for hiding this comment

uhm0311 Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

brido4125 Jan 23, 2025

Choose a reason for hiding this comment

uhm0311 Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

brido4125 Jan 23, 2025

Choose a reason for hiding this comment

jhpark816 left a comment

Choose a reason for hiding this comment

jhpark816 Jan 26, 2025

Choose a reason for hiding this comment

jhpark816 Jan 26, 2025 • edited Loading

Choose a reason for hiding this comment

brido4125 commented Jan 8, 2025 •

edited by jhpark816

Loading

uhm0311 Jan 23, 2025 •

edited

Loading

uhm0311 Jan 23, 2025 •

edited

Loading

jhpark816 Jan 26, 2025 •

edited

Loading