-
Notifications
You must be signed in to change notification settings - Fork 49
ENHANCE: Moved cache list change logic from IO thread to CacheManger thread. #649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -398,6 +398,17 @@ public void run() { | |
| waitBeforeRetryMonitorCheck(1000L - elapsed); | ||
| } | ||
| } | ||
brido4125 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아래 코멘트가 hide된 곳에 들어가서, 다시 코멘트합니다. wait() 중에 notifyAll()에 의해 깨어나더라도 다시 wait() 하는 코드가 있습니다. 즉, |
||
| // Handling cacheList changes to a MemcachedConnection resulting | ||
| // from the processing of a CacheMonitor's Watch event. | ||
| for (ArcusClient ac : getAC()) { | ||
uhm0311 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| try { | ||
| ac.getMemcachedConnection().handleCacheNodesChange(); | ||
| } catch (IOException e) { | ||
| getLogger().error("Cache List update error in ArcusClient {}, exception = {}", | ||
| ac.getName(), e.getCause()); | ||
| } | ||
| } | ||
brido4125 marked this conversation as resolved.
Show resolved
Hide resolved
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 중요한 논의가 필요한 부분입니다. 기존엔 각 client의 io thread가 동시에 hash ring update 진행하였지만 Cache Manager에 locator 하나만을 두고 이용하는 방안을 강구할 수 있나요?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이는 제가 예전에 리뷰했던 내용과 일치하며, 변경 사항이 아주 많아질 예정이기 때문에 별도의 PR로 반영한다는 대답을 들었습니다.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 언급되었던 사항이군요. 현재 PR이 기존 동작보다 hashring update 지연을 발생시킬 수 있어, Single hahsring 유지 방안을 PoC로 진행해 보는 것이 좋겠습니다. |
||
| } | ||
| } | ||
| getLogger().info("Close cache manager."); | ||
|
|
@@ -482,6 +493,10 @@ public void commandCacheListChange(List<String> children) { | |
| MemcachedConnection conn = ac.getMemcachedConnection(); | ||
| conn.setCacheNodesChange(addrs); | ||
| } | ||
|
|
||
| synchronized (this) { | ||
| notifyAll(); // awake CacheManager Thread. | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alter_list 변경 시에는 wakeup 시키지 않나요?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
processAlterListResult 콜백에 의해 commandAlterListChange가 호출되면 cacheList와 alterList의 변경을 감지하는 콜백 메서드가 별도로 구성됩니다. alterList 변경 반영의 경우 주키퍼 클라이언트의 스레드에 의해 로직이 수행됩니다. |
||
| } | ||
|
|
||
| /* ENABLE_MIGRATION if */ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.