Skip to content
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

CLEANUP: Remove TranscoderService from asyncGet() #699

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

uhm0311
Copy link
Collaborator

@uhm0311 uhm0311 commented Dec 19, 2023

TranscoderService는 별도의 Thread Pool에서 비동기적으로 decode를 수행합니다.
TranscoderService의 Thread Pool은 coreSize 1, maxSize 10으로 되어 있습니다.

CPU 리소스가 부족하거나, 동시에 decode 해야 할 데이터가 너무 많이 쌓이면 TranscoderService의 Thread Pool이 제시간 내에 decode를 수행하지 못할 수 있습니다.
따라서 TranscoderService를 통해 decode 하는 것이 아니라, 응용의 Worker Thread가 get 연산의 결과를 가져가려고 하는 순간에 decode를 수행하도록 변경했습니다.

변경 사항으로 인한 기대 효과는 get 연산의 성능 향상 및 CPU 리소스 부족 혹은 동시에 decode 해야 하는 대상이 너무 많아서 생기는 Timeout 오류 빈도 감소입니다.

@@ -0,0 +1,31 @@
package net.spy.memcached.internal;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal 패키지 내에 result라고 디렉토리 하나 더 만드는게 좋을것 같습니다.
추후 smgetResult 또한 해당 패키지에서 관리할 수 있습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다.

import net.spy.memcached.CachedData;
import net.spy.memcached.transcoders.Transcoder;

public final class GetResult<T> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가적으로 SMGetResult의 경우 내부적으로 OperationStatus를 필드로 가집니다.
만약 XxxResult 객체를 도입하기로 한다면, OperationStatus를 Future에서 관리할지, Result로 옮길지 통일해야한다고 봅니다.

Copy link
Collaborator Author

@uhm0311 uhm0311 Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetFuture 클래스가 OperationFuture 객체를 필드로 갖는 현재 구조에서는 어느 쪽으로도 통일하기가 힘듭니다.
해당 사항은 별도의 PR로 처리해야 할 것 같습니다.

Copy link
Collaborator

@brido4125 brido4125 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추후에 머지 되면 제가 multi get 연산에 대한 작업 진행해도 될까요?

@uhm0311
Copy link
Collaborator Author

uhm0311 commented Dec 19, 2023

@brido4125

네 그렇게 하세요

@uhm0311
Copy link
Collaborator Author

uhm0311 commented Dec 20, 2023

#701

위 PR의 변동 사항으로 인해 생긴 충돌을 제거했습니다.

@jhpark816 jhpark816 merged commit fb61e7a into naver:develop Dec 20, 2023
1 check passed
@uhm0311 uhm0311 deleted the uhm0311/develop5 branch December 20, 2023 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants