Skip to content

FEATURE: Add CompletableFuture Admin APIs#1073

Merged
jhpark816 merged 1 commit intonaver:developfrom
f1v3-dev:feat/v2-admin
Apr 16, 2026
Merged

FEATURE: Add CompletableFuture Admin APIs#1073
jhpark816 merged 1 commit intonaver:developfrom
f1v3-dev:feat/v2-admin

Conversation

@f1v3-dev
Copy link
Copy Markdown
Collaborator

@f1v3-dev f1v3-dev commented Apr 1, 2026

🔗 Related Issue

⌨️ What I did

  • v2 API에 Admin API (flush, stats, version)를 지원합니다.
    • flush() / flush(int delay): flush_all
    • flush(String prefix) / flush(String prefix, int delay): flush_prefix
    • stats() / stats(String arg): 서버 통계 조회
    • versions(): 서버 버전 조회

@f1v3-dev f1v3-dev requested a review from oliviarla April 1, 2026 10:28
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Outdated
@f1v3-dev f1v3-dev self-assigned this Apr 2, 2026
@f1v3-dev f1v3-dev force-pushed the feat/v2-admin branch 2 times, most recently from 6fa9988 to 29bc3e5 Compare April 2, 2026 04:38
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java
@f1v3-dev f1v3-dev force-pushed the feat/v2-admin branch 3 times, most recently from 90eee8e to 97ce9bd Compare April 3, 2026 07:04
Copy link
Copy Markdown
Collaborator

@oliviarla oliviarla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 완료입니다.,

Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Outdated
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Outdated
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommandsIF.java Outdated
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Outdated
Comment thread src/test/java/net/spy/memcached/v2/AdminAsyncArcusCommandsTest.java
@f1v3-dev f1v3-dev force-pushed the feat/v2-admin branch 2 times, most recently from 664894a to b5f5324 Compare April 9, 2026 03:19
@f1v3-dev f1v3-dev requested a review from oliviarla April 9, 2026 03:38
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Outdated
oliviarla
oliviarla previously approved these changes Apr 9, 2026
}

public ArcusFuture<Boolean> flush() {
return flush(0);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@f1v3-dev
질문이 있습니다.
delay 값이 0 이면, 캐시 서버에 전달할 flush 명령에 0 이란 delay 값이 포함되나요?
0 이면 delay 값이 생략되도록 구현되어야 하는 데, 현재 어떤 상태인지 궁금합니다.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 상태는 delay 값이 0으로 함께 담겨서 요청을 보내게 됩니다.

delay = 0 인 경우에도 보내지 않도록 구현이 필요하다면, FlushOperationImpl, FlushByPrefixOperationImpl 클래스의 initialize() 메서드 부분을 수정하도록 하겠습니다.

example - FlushOperationImpl.java

  @Override
  public void initialize() {
    ByteBuffer bb = null;
    if (delay == -1) { // <-- delay == 0으로 변경하면 보내지 않음. 
      bb = ByteBuffer.wrap(FLUSH);
    } else {
      bb = ByteBuffer.allocate(32);
      bb.put(("flush_all " + delay + "\r\n").getBytes());
      ((Buffer) bb).flip();
    }
    setBuffer(bb);
  }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delay == -1 조건이 있었던 것을 보면, 기존 API에서 delay 값을 -1로 주는 경우가 있다는 것을 나타냅니다.
delay == -1 경우가 오동작하지 않으려면, delay == -1 || delay == 0 조건으로 변경해야 하죠?

Copy link
Copy Markdown
Collaborator Author

@f1v3-dev f1v3-dev Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네 맞습니다.

말씀해주신대로 기존 API에서는 -1을 "delay를 사용하지 않음" 으로 사용하고 있어서delay == -1 || delay == 0 조건으로 변경해야 합니다.

Copy link
Copy Markdown
Collaborator

@jhpark816 jhpark816 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delay == -1 || delay == 0 조건이 혼돈되므로,
기존대로 -1이란 설정을 그대로 유지하는 것은 어떤가요 ?
외부 설정은 아니고 내부 설정 값이기 때문입니다.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

의미가 모호한 매직 넘버를 없애는 방향이 좋지 않을까라고 생각합니다.

내부 값이라 하더라도 delay = -1 이라는 코드를 보는 개발자 입장에서 "-1 이라는 값이 무엇일까?" 라는 의문이 생기는 상황이 있을 것이라고 생각합니다.

반면, delay = 0 은 "딜레이가 0초, 즉 없음" 라는 의미가 직관적으로 파악할 수 있다고 생각합니다.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 값은 아직 delay 설정 자체가 없다는 의미로 쓰인 것이며, 이런 의미로 -1 값을 쓰는 경우가 일반적으로 있습니다.
delay = 0도 delay가 없음을 나타내므로, 직관적으로 이해할 수 있습니다.

-1 이든 0 이든 기존 API와 v2 API 모두에서 통일하는 것이 좋지 않나 생각합니다.
그리고, 이 경우에는 flush 명령에서 delay 값은 캐시 서버에 넘어가지 않아야 합니다.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 이든 0 이든 기존 API와 v2 API 모두에서 통일하는 것이 좋지 않나 생각합니다.
그리고, 이 경우에는 flush 명령에서 delay 값은 캐시 서버에 넘어가지 않아야 합니다.

저도 말씀해주신 의견에 동의합니다.

현재 PR은 V2 Admin API 구현에 집중하는 것이 목적이므로, delay 처리 방식 변경은 v1, v2 전체에 걸친 변경이 필요하므로 더 생각을 해본 후 별도의 PR로 진행하는 좋다고 보여집니다.

따라서, 이번 PR에서는 기존 V1 API와 동일하게 delay = -1 으로 유지하는 방향으로 진행하겠습니다.

Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java
oliviarla
oliviarla previously approved these changes Apr 14, 2026
@oliviarla oliviarla requested a review from jhpark816 April 14, 2026 05:04
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java
Copy link
Copy Markdown
Collaborator

@jhpark816 jhpark816 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 완료

Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java
Copy link
Copy Markdown
Collaborator

@jhpark816 jhpark816 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가 리뷰

Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java
Comment thread src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java
@oliviarla oliviarla requested a review from jhpark816 April 16, 2026 05:51
@jhpark816 jhpark816 merged commit 2b372e1 into naver:develop Apr 16, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants