jam2in / arcus-java-client

Arcus Java client
Apache License 2.0
0 stars 0 forks source link

optimize() 코드 검증 #28

Closed jhpark816 closed 8 years ago

jhpark816 commented 8 years ago

연속된 2개 이상의 연산의 get 연산이어야 optimize 할 수 있을 건데요..

그러면, 처음에 writeQ.remove()한 operation은 처리되지 않을 것 같은 데, 버그 아닌가요 ?

    protected void optimize() {
        // make sure there are at least two get operations in a row before
        // attempting to optimize them.
        if(writeQ.peek() instanceof GetOperation) {
            optimizedOp=writeQ.remove();
            if(writeQ.peek() instanceof GetOperation) {
                OptimizedGetImpl og=new OptimizedGetImpl(
                        (GetOperation)optimizedOp);
                optimizedOp=og;

                while(writeQ.peek() instanceof GetOperation) {
                    GetOperationImpl o=(GetOperationImpl) writeQ.remove();
                    if(!o.isCancelled()) {
                        og.addOperation(o);
                    }
                }

                // Initialize the new mega get
                optimizedOp.initialize();
                assert optimizedOp.getState() == OperationState.WRITING;
                ProxyCallback pcb=(ProxyCallback) og.getCallback();
                getLogger().debug("Set up %s with %s keys and %s callbacks",
                    this, pcb.numKeys(), pcb.numCallbacks());
            }
        }
    }
jhpark816 commented 8 years ago

writeQ.remove()한 operation을 optimizedOp가 가리키고 있으므로, 이 코드는 정상이네요.

본 이슈는 close 합니다.

whchoi83 commented 8 years ago

optimize 코드 설명을 작성하고 있었는데, 확인하셨으니 다행입니다.

참고 1 최신 spymemcached 에도 현재까지는 동일한 코드로 되어 있습니다.

  1. https://github.com/naver/arcus-java-client/blob/master/docs/02-arcus-java-client.md 내용에

setShouldOptimize(boolean o)

최적화 로직 사용여부를 결정한다. 최적화 로직은 Operation Queue에 순서대로 있는 get 연산들을 multi-get과 get 연산으로 조합형으로 한꺼번에 수행하게 된다. Arcus에서는 optimize 로직 사용을 권장하지 않는다.

로 되어있는데 Optimize 기능은 사용하지 않는 것은 어떨까 합니다. ConnectionFactoryBuilder 에서 본 옵션을 제거하거나, deprecated 라고 명시하는 정도면 괜찮지 않을까 합니다.

jhpark816 commented 8 years ago

예.. optimize 코드는 일단 사용하지 않는 것으로 하긴 해야 합니다. default 값은 disable 상태이어야 하고, 본 옵션의 설정 함수는 두지만, not supported 정도로 처리해 두면 좋겠다는 의견입니다.

이유는 항상 writing 해당할 operation으로 optimizedOp가 항상 포함되어 있어야 하는 데. 이게 일관되게 포함되지 않았던 코드가 일부 있어서, 경우에 따라선 오류가 있을 것 같습니다.

나중에, 이 기능을 제대로 사용할 만큼의 상황이 되면, 그 때 enable 시키도록 설정 함수를 제대로 제공하면 될 것 같습니다.