FIX: Infinity waiting with get method in BulkGetFuture. #654
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
uhm0311
approved these changes
Aug 11, 2023
jhpark816
reviewed
Aug 11, 2023
| @@ -32,6 +32,8 @@ | |||
| import net.spy.memcached.ops.Operation; | |||
| import net.spy.memcached.ops.OperationState; | |||
|
|
|||
| import static net.spy.memcached.DefaultConnectionFactory.DEFAULT_OPERATION_TIMEOUT; | |||
There was a problem hiding this comment.
여기서 DefaultConnectionFactory 관련 import 하는 것은 적절하지 않은 것 같습니다.
| @@ -72,7 +74,7 @@ public boolean cancel(boolean ign) { | |||
| @Override | |||
| public Map<String, T> get() throws InterruptedException, ExecutionException { | |||
| try { | |||
| return get(Long.MAX_VALUE, TimeUnit.MILLISECONDS); | |||
| return get(DEFAULT_OPERATION_TIMEOUT * ops.size(), TimeUnit.MILLISECONDS); | |||
There was a problem hiding this comment.
Future 인터페이스의 get() 메소드는 무한히 대기 의미를 가지므로,
Long.MAX_VALUE 값도 충분히 의미 있어 보입니다.
SW 버그가 없다면 제 시간에 처리되거나 cancel 처리됩니다.
SW 버그가 존재할 경우를 대비하여 적절한 timeout 설정하는 것이 필요한 것 같습니다.
비동기 처리이믈 ops 크기만큼 곱한 timeout 사용할 필요는 없고,
connection factory를 통해 설정된 operationTimeout 값을 그대로 사용하는 것이 좋겠습니다.
따라서, 다른 Future 구현 로직과 같이 BulkGetFuture 객체 생성 시에 인자로 넣어주시죠.
| @@ -104,7 +106,7 @@ public Map<String, T> get(long duration, TimeUnit units) | |||
| Collection<Operation> timedoutOps = new HashSet<Operation>(); | |||
| Map<String, T> ret = internalGet(duration, units, timedoutOps); | |||
| if (timedoutOps.size() > 0) { | |||
| this.timeout = true; | |||
| this.isTimeout = true; | |||
brido4125
force-pushed
the
fix/BulkGetFuture
branch
from
bbe2cfb
to
bd773c6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
관련 이슈
#636
변경 사항
기존에는 무한 대기하던 시간을
(BulkGetFuture가 가진 Ops의 크기 * DefaultTimeout) 으로 변경하였습니다.
합리적으로 생각해보면 하나의 Op가 처리될때 700L로 timeout이 설정됩니다.
여러 op가 있을 경우는 ops의 크기만큼 timeout을 가지면
가장 단순한 접근으로 해결할 수 있다고 생각합니다.
timeout 설정에 대해 다른 생각이 있으시면 코멘트 부탁드립니다.