INTERNAL: Refactor Future in asyncCollectionInsertBulk2 method. #644
Conversation
brido4125
force-pushed
the
refactor/bulk2InsertFuture
branch
from
47c445e
to
406b92d
Compare
| import java.util.concurrent.TimeoutException; | ||
| import java.util.concurrent.ExecutionException; | ||
|
|
||
| public class PipedBulkCollectionFuture<K, V> extends PipedCollectionFuture<K, V> { |
There was a problem hiding this comment.
Java Client에서 Piped와 Bulk는 여러 연산을 처리한다는 점에선 공통점을 갖고 있으나, 다음과 같은 차이점이 있습니다.
Piped : 동일한 키에 여러 번 연산
Bulk : 서로 다른 키에 하나 씩 연산
따라서 두 용어를 함께 쓰는 것보다, Bulk만 붙이는 것이 나을 것 같습니다.
There was a problem hiding this comment.
해당 코멘트 분석하며 BulkOperationFuture를 확인해본 결과
asyncCollectionInsertBulk2에서
리턴해주는 Type이 Future 인터페이스 타입이기 때문에
내부에서 생성하는 Future Type을 BulkOperationFuture를 사용해도
정상적으로 동작하는것을 확인했습니다.
결론적으로 새로운 Type의 Future가 아닌 기존에 존재하는 Type으로 대체하도록 구현했습니다.
| } | ||
|
|
||
| @Override | ||
| public Map<K, V> get(long duration, TimeUnit units) |
There was a problem hiding this comment.
이 메소드의 구현이 부모 클래스와 다르지 않은 것 같은데 따로 구현한 이유가 있나요?
There was a problem hiding this comment.
대부분 로직이 동일해보이지만 아래로직이 서로 다름을 확인할 수 있습니다.
else {
// continuous timeout counter will be reset only once in pipe
MemcachedConnection.opsSucceeded(ops);
}부모 클래스의 경우 piped 연산의 key 값이 동일해서
opSucceded(ops.iterator().next())로 구현됩니다.
There was a problem hiding this comment.
부모 클래스에 override 가능한 opSucceeded() 메소드를 만들고, 두 클래스 간에 차이나는 부분만 이 메소드에 구현한 다음 get()에서는 opSucceeded() 메소드를 호출하도록 하면 어떨까요?
brido4125
force-pushed
the
refactor/bulk2InsertFuture
branch
2 times, most recently
from
c4816a0
to
e6a3e97
Compare
|
|
||
| public void addOperation(Operation op) { | ||
| ops.add(op); | ||
| } |
There was a problem hiding this comment.
addOperation() 메소드를 setOperation() 메소드의 다음 위치로 옮기거나
아니면, setOperation() 메소드를 addOperation() 바로 위로 옮기면 좋겠습니다.
There was a problem hiding this comment.
그리고 사용되지 않는 메소드이지만, 아래 메소드도 추가해 두면 좋겠습니다.
public CollectionOperationStatus getOperationStatus() {
return null;
}
brido4125
force-pushed
the
refactor/bulk2InsertFuture
branch
from
e6a3e97
to
44572a9
Compare
| } | ||
|
|
||
| public CollectionOperationStatus getOperationStatus() { | ||
| return null; |
There was a problem hiding this comment.
지금 보니,
BulkOperationFuture가 외부 interface에 노출된 것이 아니네요.
외부에는 Future interface가 노출된 상태이기 때문에,
getOperationStatus() 메소드는 없어도 될 것 같습니다.
brido4125
force-pushed
the
refactor/bulk2InsertFuture
branch
from
44572a9
to
ba51c69
Compare
brido4125
force-pushed
the
refactor/bulk2InsertFuture
branch
from
ba51c69
to
4980a09
Compare
관련 이슈
https://github.com/jam2in/arcus-works/issues/403
Motivation
기존의
asyncCollectionInsertBulk2메서드는내부적으로 익명 Future 클래스를 구현하여 리턴한다.
이러한 구조를 다른 api들과 동일하게
Future rv 생성 -> 연산 결과를 rv에 전달 -> rv 리턴구조로 변경한다.
참고로
asyncCollectionInsertBulk2는여러 컬렉션에 동일한 Elem을 삽입하는 api에서 사용된다.
변경된 지점
기존에 존재하는
BulkOperationFuture를 사용하여 Future를 생성합니다.