INTERNAL: Not to cancel operations when node is removed from ZK but still alive. #749

uhm0311 · 2024-04-29T02:53:28Z

🔗 Related Issue

https://github.com/jam2in/arcus-works/issues/535

⌨️ What I did

NodeLocator에서 노드를 제거할 때, 해당 노드와의 소켓 연결이 해제되지 않았고 Operation Queue가 비어 있지 않으면 소켓 연결을 해제와 연산 캔슬을 수행하지 않고 DelayedClosingNodes로 분류합니다.
IO Thread가 DelayedClosingNodes로 분류된 노드들을 순회하며 다음의 로직을 수행합니다.
- 소켓 연결이 끊어진 노드가 있다면 연산을 모두 캔슬하고 DelayedClosingNodes에서 제거합니다.
- Operation Queue가 비어 있는 노드가 있다면 소켓 연결을 해제하고 DelayedClosingNodes에서 제거합니다.

oliviarla · 2024-04-29T10:50:44Z

handleNodesToRemove 메서드가 복제와 마이그레이션에서도 사용되는데, 해당 부분에는 영향이 없나요?

uhm0311 · 2024-04-30T00:42:19Z

EE여도 cache_list znode가 제거되었을 때 동작은 CE와 동일합니다.
alter_list znode가 제거되었을 때는 NodeLocator에서 소켓 연결을 해제하므로 영향이 없습니다.

jhpark816 · 2024-04-30T01:08:30Z

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

-      String cause = "node removed.";
-      cancelOperations(node.destroyReadQueue(false), cause);
-      cancelOperations(node.destroyWriteQueue(false), cause);
-      cancelOperations(node.destroyInputQueue(), cause);


해당 MemcachedNode와의 연결 여부에 따라 달리 처리해야 하지 않는 지 ?

그리고, 여기에서 queue에 있는 연산들은 어떻게 처리된다는 주석을 달아 두면 좋겠습니다.
위의 continue 코드는 그대로 유지하고요.

해당 MemcachedNode와의 연결 여부에 따라 달리 처리해야 하지 않는 지 ?

cache_list든 alter_list든 znode 제거 시 NodeLocator에서 항상 소켓 연결을 해제하기 때문에 달리 처리할 필요가 없습니다.

위의 continue 코드는 그대로 유지하고요.

for문 내에서 if (mgType == MigrationType.LEAVE) 이후에 수행하는 로직이 없어서 제거했습니다.
굳이 두어야 하나요?

cache_list든 alter_list든 znode 제거 시 NodeLocator에서 항상 소켓 연결을 해제하기 때문에 달리 처리할 필요가 없습니다.

cache server가 다운되어서 소켓 연결이 이미 끊어진 것이라면 어쩔 수 없지만,
NodeLocater에서 znode 제거 시에 소켓 연결을 미리 close해 버린다면,
op queue에 있는 연산들은 기존 코드대로 cancel 처리하는 것이 맞을 것입니다.
해당 연산이 계속 처리되게 하려면, 소켓 연결도 delayed close하여야 합니다.

uhm0311 · 2024-04-30T09:11:55Z

첫번째 코멘트에 커밋 수정사항을 업데이트 했습니다.

jhpark816 · 2024-04-30T12:20:52Z

@oliviarla @brido4125 먼저 리뷰 하시죠.

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

brido4125 · 2024-05-02T02:49:51Z

https://github.com/jam2in/arcus-works/issues/549

현재 PR에서는 locator의 update와 handleDelayedClosingNodes 모두 IO 쓰레드에서 순차적으로 수행합니다.

만약, 현재 진행중인 이슈처럼 locator에 대한 update 수행을 다른 스레드가 수행할 경우
해당 구현도 변경이 필요할 수도 있습니다.

추후 참고할 목적으로 해당 코멘트 남깁니다.

brido4125 · 2024-05-02T03:23:27Z

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

@@ -226,6 +227,10 @@ public void update(Collection<MemcachedNode> toAttach,
      for (MemcachedNode node : toDelete) {
        allNodes.remove(node);
        removeHash(node);
+        if (node.hasOp() && node.isActive()) {


deley되게 제거할 노드라면 현재 로케이터에서 곧바로 해당 노드를 제거해도 되나요?
deley 시켜 조금이나마 연산을 수행시키려면 로케이터에도 노드가 있어야 될 것 같습니다.

https://github.com/jam2in/arcus-works/issues/535

이슈의 설명을 다시 읽어보세요.

@brido4125

deley되게 제거할 노드라면 현재 로케이터에서 곧바로 해당 노드를 제거해도 되나요?
deley 시켜 조금이나마 연산을 수행시키려면 로케이터에도 노드가 있어야 될 것 같습니다.

위 코멘트는 어떤 의미인가요?

캐시 요청이 해시 링에 의해 분배되기 전인 상태의 요청을 처리해야하지 않나라고 생각했습니다.
하지만 분배된 후의 요청들이 본 PR의 대상이라 고려하지 않아도 됩니다.

캐시 요청이 해시 링에 의해 분배되기 전인 상태의 요청과 분배된 후의 요청이 무엇인지 설명 바랍니다.

@brido4125
캐시 요청이 해시 링에 의해 분배되기 전인 상태의 요청과 분배된 후의 요청이 무엇인지를 설명해 주세요.

분배된 후의 요청이 큐에 삽입되고 난 이후를 의미한것이고,
분배되기 전은 큐에 삽입되기 전을 의미했습니다.

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

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

jhpark816 · 2024-05-08T01:41:43Z

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

@@ -226,6 +227,10 @@ public void update(Collection<MemcachedNode> toAttach,
      for (MemcachedNode node : toDelete) {
        allNodes.remove(node);
        removeHash(node);
+        if (node.hasOp() && node.isActive()) {
+          delayedClosingNodes.add(node);


@uhm0311
질문이 있습니다.

이와 같이 해당 node가 locator의 allNodes에서 제외된 경우,
inputQ, writeQ, readQ에 있는 요청들이 처리가 될 수 있나요?

IO Thread는 NodeLocator와 관계 없이 동작하며, closeChannel() 메소드로 소켓 연결을 해제하지 않는 이상 남은 연산은 계속 처리됩니다.

@oliviarla
위의 코멘트에 관련된 코드를 확인해 주시죠.

@uhm0311 님께서 말씀하신 것 처럼 allNodes에서 제외되더라도 addedQueue에 MemcachedNode가 존재하고 있다면 inputQ에있는 Operation들이 writeQ로 넘어가고, writeQ의 연산들이 처리됩니다.

…till alive.

uhm0311 requested review from jhpark816, oliviarla and brido4125 April 29, 2024 02:53

jhpark816 reviewed Apr 30, 2024

View reviewed changes

uhm0311 force-pushed the uhm0311/develop branch from 539d0e3 to 5444ff8 Compare April 30, 2024 05:25

uhm0311 changed the title ~~INTERNAL: Not to cancel operations when node is removed from cache_list.~~ INTERNAL: Not to cancel operations when node is removed from ZK. Apr 30, 2024

uhm0311 force-pushed the uhm0311/develop branch 3 times, most recently from 56005f1 to 4955858 Compare April 30, 2024 05:39

uhm0311 changed the title ~~INTERNAL: Not to cancel operations when node is removed from ZK.~~ INTERNAL: Not to cancel operations when node is removed from ZK but still alive. Apr 30, 2024

uhm0311 force-pushed the uhm0311/develop branch from 4955858 to ec00df8 Compare April 30, 2024 06:09

uhm0311 requested a review from jhpark816 April 30, 2024 09:11

brido4125 reviewed May 2, 2024

View reviewed changes

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

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

uhm0311 force-pushed the uhm0311/develop branch 2 times, most recently from 39ff086 to a9559bb Compare May 2, 2024 01:22

brido4125 reviewed May 2, 2024

View reviewed changes

oliviarla reviewed May 2, 2024

View reviewed changes

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

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

oliviarla previously approved these changes May 2, 2024

View reviewed changes

brido4125 previously approved these changes May 3, 2024

View reviewed changes

jhpark816 reviewed May 8, 2024

View reviewed changes

uhm0311 dismissed stale reviews from brido4125 and oliviarla via 85a2073 May 8, 2024 06:51

uhm0311 force-pushed the uhm0311/develop branch from a9559bb to 85a2073 Compare May 8, 2024 06:51

INTERNAL: Not to cancel operations when node is removed from ZK but s…

d520ae8

…till alive.

uhm0311 force-pushed the uhm0311/develop branch from 85a2073 to d520ae8 Compare May 8, 2024 06:57

uhm0311 self-assigned this May 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

INTERNAL: Not to cancel operations when node is removed from ZK but still alive. #749

INTERNAL: Not to cancel operations when node is removed from ZK but still alive. #749

uhm0311 commented Apr 29, 2024 •

edited

Loading

oliviarla commented Apr 29, 2024

uhm0311 commented Apr 30, 2024 •

edited

Loading

jhpark816 Apr 30, 2024 •

edited

Loading

uhm0311 Apr 30, 2024

jhpark816 Apr 30, 2024

uhm0311 commented Apr 30, 2024

jhpark816 commented Apr 30, 2024

brido4125 commented May 2, 2024

brido4125 May 2, 2024

uhm0311 May 2, 2024

jhpark816 May 7, 2024 •

edited

Loading

brido4125 May 7, 2024

jhpark816 May 8, 2024

jhpark816 May 9, 2024

brido4125 May 9, 2024

jhpark816 May 8, 2024

uhm0311 May 8, 2024

jhpark816 May 8, 2024

oliviarla May 9, 2024

INTERNAL: Not to cancel operations when node is removed from ZK but still alive. #749

Are you sure you want to change the base?

INTERNAL: Not to cancel operations when node is removed from ZK but still alive. #749

Conversation

uhm0311 commented Apr 29, 2024 • edited Loading

🔗 Related Issue

⌨️ What I did

oliviarla commented Apr 29, 2024

uhm0311 commented Apr 30, 2024 • edited Loading

jhpark816 Apr 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uhm0311 commented Apr 30, 2024

jhpark816 commented Apr 30, 2024

brido4125 commented May 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhpark816 May 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uhm0311 commented Apr 29, 2024 •

edited

Loading

uhm0311 commented Apr 30, 2024 •

edited

Loading

jhpark816 Apr 30, 2024 •

edited

Loading

jhpark816 May 7, 2024 •

edited

Loading