FEATURE: Add CompletableFuture Pipeline API#1035
FEATURE: Add CompletableFuture Pipeline API#1035oliviarla wants to merge 1 commit intonaver:developfrom
Conversation
7a9e90a to
0d98bc8
Compare
src/main/java/net/spy/memcached/protocol/ascii/PipelineOperationImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/net/spy/memcached/protocol/ascii/PipelineOperationImpl.java
Show resolved
Hide resolved
d25cdb6 to
acb8d30
Compare
|
@uhm0311 @jhpark816 |
uhm0311
left a comment
There was a problem hiding this comment.
현재 코드 방향성은 괜찮아 보이고, 코드 설명 보강이 조금 더 필요해 보입니다.
73d7884 to
adb10e9
Compare
|
@uhm0311 |
2d3dee0 to
de9ab5e
Compare
de9ab5e to
d969204
Compare
| keys.add(key); | ||
| ops.add(opFact.collectionInsert(key, mkey, | ||
| mapUpsert, tc.encode(value).getData(), null)); | ||
| return this; |
There was a problem hiding this comment.
위의 mopUpsert()에서 아래와 같이 호출하면 됩니다.
return this.mopUpsert(key, mkey, value, null);| ops.add(opFact.collectionUpdate(key, bkey.toString(), bTreeUpdate, | ||
| tc.encode(value).getData(), null)); | ||
| return this; | ||
| } |
There was a problem hiding this comment.
bopUpdate() 코드는 bopDelete() 위로 옮깁시다.
| } | ||
|
|
||
| pipedBuffer.flip(); | ||
| setBuffer(pipedBuffer); |
There was a problem hiding this comment.
각 명령에 " pipe" 추가하는 로직은 최적화할 필요가 있어 보입니다. 어떤가요?
이슈라 생각한다면, 이슈로 올려두고 나중에 처리하시죠.
| SetInsert<V> setInsert = new SetInsert<>(value, null, attributes); | ||
| keys.add(key); | ||
| ops.add(opFact.collectionInsert(key, "", | ||
| setInsert, tc.encode(value).getData(), null)); |
There was a problem hiding this comment.
아래 SetExist와 SetDelete 객체를 생성할 시에 value 외에 tc를 함께 넘겨준다.
반면, SetInsert 객체 생성 시에는 value만 넘겨 생성한다.
이에 따라 value length는 얻는 코드를 포함하여 해당 operation 객체를 다루는 방식이 서로 달라서, 코드 일관성이 떨이집니다.
SetInsert 객체 생성 시에도 value 외에 tc를 함께 넘기는 방식으로 리팩토링하는 것을 검토해 주세요. 이 사항도 진행한다면 별도 이슈로 처리 바랍니다.
|
|
||
| OperationStatus status = successAll ? END : FAILED_END; | ||
| complete(status); | ||
| } else if (line.startsWith("PIPE_ERROR")) { |
There was a problem hiding this comment.
여기서는 needRedirect() 검사하지 않아도 되나요?
| private OperationStatus parseStatusLine(String line) { | ||
| boolean allDigit = line.chars().allMatch(Character::isDigit); | ||
| if (allDigit) { | ||
| return new OperationStatus(true, line, StatusCode.SUCCESS); |
There was a problem hiding this comment.
bop incr/decr 연산을 pipelinging에서 제외하는 것은 어떤지 ?
| complete(status); | ||
| } else if (line.startsWith("RESPONSE ")) { | ||
| expectingResponse = true; | ||
| responseIndex = 0; |
There was a problem hiding this comment.
Switchover에 의해 아래와 같이 재수행하는 경우를 가정합시다.
- responseIndex : 5
- ops.size() : 10
위의 경우에 RESPONSE 응답이 왔을 시에 responseIndex을 0으로 재설정하여도 되는 지 ?
| } | ||
|
|
||
| // Notify callback for single command | ||
| cb.gotStatus(ops.get(0), status); |
There was a problem hiding this comment.
Switchover에 의해 아래와 같이 재수행하는 경우를 가정합시다.
- responseIndex : 9
- ops.size() : 10
이 경우, ops.get(0) 호출하는 것도 문제가 되지 않는 지 ?
|
v2에서 pipeline 기능을 제공하는 것을 보류하게 되어 close합니다. |
🔗 Related Issue
Pipeline API를 추가합니다.
사용 방법
List<Boolean>에 각 명령에 대한 응답이 담긴다.PipelineOperationException혹은CompositeException이 발생할 수 있다.ArcusMultiFuture#getResultsWithFailures메서드로 나머지 결과를 조회할 수 있다.⌨️ What I did
ERR_NOT_FOUND / ERR_NOT_FOUND_ELEMENT / ERR_ELEMENT_EXISTS / ERR_NOTHING_TO_UPDATE실패 응답인 경우 Boolean으로 처리한다.TYPE_MISMATCH / BKEY_MISMATCH / EFLAG_MISMATCH / OVERFLOWED / OUT_OF_RANGE / UNREADABLE / NOT_SUPPORTED의 경우PipelineOperationException을 발생시킨다.OperationException을 던진다.PipelineCompositeException로 감싸서 반환한다. 이 때 현재까지의 응답을 담은 Result가 함께 담긴다.PipelineCompositeException내부의 예외들을 모두 꺼내 통합한 뒤,CompositeException또는 단일PipelineOperationException으로 반환한다.PipelineCompositeException을 통합하는 과정에서 각 노드로부터 받은 현재까지의 응답 Result 역시 통합한다.