-
Notifications
You must be signed in to change notification settings - Fork 47
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
FIX: getElement logic in BTreeStoreAndGetFuture. #662
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰 완료
throw new RuntimeException(e); | ||
} catch (ExecutionException e) { | ||
throw new RuntimeException(e); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드를 가장 상위에 두면 좋겠습니다.
OperationTimeoutException 을 던저야 하는 지도 검토 바랍니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
호출되는 get() 메서드 내에서
CheckedOperationTimeoutException
를 던지고
이를 TimeOutException에서 catch 하여
RuntimeException을 생성하기에
새롭게 생성하여 Throw할 필요는 없다고 생각합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try 문이 가장 앞애 나오개 합시다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@uhm0311 exception 부분 리뷰해 주세요.
참고로, OperationFuture.get() 코드에서도
명시적인 OperationTimeoutException 던지는 것 같습니다.
public T get() throws InterruptedException, ExecutionException {
try {
return get(timeout, TimeUnit.MILLISECONDS);
} catch (TimeoutException e) {
throw new OperationTimeoutException(e);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brido4125
TimeoutException에 한해서 다음과 같이 합시다.
...
} catch (TimeoutException e) {
throw new OperationTimeoutException(e);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brido4125
나도 같은 의견이니, 이렇게 수정해 주시죠.
04ffb09
to
19b8f23
Compare
19b8f23
to
0398941
Compare
Motivation
기존 BTreeStoreAndGetFuture의 getElement 동작은
곧바로 element 변수를 리턴하는 로직인데
이는 Future의 get()이 완료되지 않을 경우
리턴되는 결과가 부정확하다.
즉, 응용의 Worker Thread가 Blocking되어 정확한 결과값을 리턴받도록 변경해야한다.
getElement() 내부적으로 get()을 호출하여
정확한 element를 리턴하도록 변경한다.