jam2in / arcus-java-client

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

setupResend() 이상한 부분 검증 #25

Closed jhpark816 closed 4 years ago

jhpark816 commented 8 years ago

아래는 setupResend() 코드입니다..

    public final void setupResend(boolean cancelWrite, String cause) {
        // First, reset the current write op, or cancel it if we should
        // be authenticating
        Operation op=getCurrentWriteOp();
        if((cancelWrite || shouldAuth) && op != null) {
            op.cancel(cause);
        } else if(op != null) {
            ByteBuffer buf=op.getBuffer();
            if(buf != null) {
                buf.reset();
            } else {
                getLogger().info("No buffer for current write op, removing");
                removeCurrentWriteOp();
            }
        }
        // Now cancel all the pending read operations.  Might be better to
        // to requeue them.
        while(hasReadOp()) {
            op=removeCurrentReadOp();
            if (op != getCurrentWriteOp()) {
                getLogger().warn("Discarding partially completed op: %s", op);
                op.cancel(cause);
            }
        }

        while((cancelWrite || shouldAuth) && hasWriteOp()) {
            op=removeCurrentWriteOp();
            getLogger().warn("Discarding partially completed op: %s", op);
            op.cancel(cause);
        }

        getWbuf().clear();
        getRbuf().clear();
        toWrite=0;
    }

cancelWrite = true인 경우,

이상한 부분은current write op에 대해 2회 cancel 처리이며, 1회 cancel 처리로 변경되어야 할 것 같습니다.

추가로, cancel 처리하는 op에 대해 buffer != null이더라도 reset 처리는 필요 없는 거죠 ?

cancelWrite = false인 경우,

이상한 부분은 위의 2번째 처리 로직이며, 그 op를 제거하지 않아야 할 것 같습니다. 오히려, 그 op의 initialize() 메소드를 호출해야 하지 않는 지요 ?

whchoi83 commented 8 years ago

1회 cancel 하도록 하는 내용은 좀 더 살펴봐야 할 것 같습니다. 일단 setupResend 는 최신 spymemcached 도 동일한 코드로 되어 있습니다.

두 번째 reset 과 관련된 내용은... write Queue 에 있으면서 null 인 경우는 없다고 보는게 맞기 때문에 remove 가 맞을 것 같은데 좀 더 살펴봐야 할 것 같습니다.

현재는 shouldAuth 와 관련된 코드를 포함해 좀 더 봐야 알 것 같습니다.