FEATURE: Add CompletableFuture BTree insert/get API by oliviarla · Pull Request #1026 · naver/arcus-java-client

oliviarla · 2025-11-26T10:29:52Z

🔗 Related Issue

bopCreate/bopInsert/bopInsertAndGetTrimmed/bopGet/bopMultiGet/bopSortMergeGet API 구현에 대한 커밋을 추가해두었습니다.

⌨️ What I did

구현 관점 특이 사항
- GenericTranscoder 대신 CFB로부터 Transcoder<Object> 타입을 가져와 Transcoder<T> 타입으로 형변환합니다.
- Collection 타입의 경우 Decode하는 로직을 별도 스레드에 위임하지 않습니다.
SMGet 관련 사항
- 기존에 없던 SMGet에서 하나의 BKey를 입력받는 API를 추가하느라, 기존 타입 생성자에 대한 변경사항이 있습니다.
- SMGetResult#mergeSMGetResults 정적 메서드에 값들을 합치는 로직을 위임합니다.
- 기본적으로 사용자가 설정한 bkey range의 범위에 따라 bkey -> btree item key 순으로 정렬됩니다. 다만 하나의 BKey를 입력받는 API의 경우 btree item key가 항상 ascending 순으로 정렬됩니다.
새로 추가된 VO 클래스들과 각 메서드의 서버 응답 별 반환 값은 New Interface 설계(Notion 문서)에 정리되어 있습니다.
가장 기본적인 메서드들만 추가하였고, bop update, bop incr 등의 API는 다음 PR로 추가할 예정입니다.

brido4125

btree api PR에 전체적으로 아래와 같은 설명 덧붙여 주세요

as-is 상태에서의 반환값 / to-be 상태에서의 반환값
as-is 상태에서의 예외 시 반환 값 / to-be 상태에서의 예외 시 반환값

위 부분이 나중에 해당 기능 릴리즈 할 때, 어차피 사용자에게도 가이드 되어야 할 내용입니다.
번거롭더라도 지금 미리 작성한다고 생각하고 해주시면 감사하겠습니다

oliviarla · 2025-12-04T03:46:36Z

@brido4125
API 메서드마다 javadoc에 반환 타입에 대한 내용을 자세히 추가해두었습니다.
as-is가 ArcusClient#asyncBop... 말씀하시는게 맞다면, 저는 이 둘을 비교하면 오히려 더 헷갈릴 것이라고 생각합니다.
따라서 현재 형태에서 반환 값이 적절한지 리뷰해주시면 감사하겠습니다. (exception이 발생하는 상황은 노션에 정리되어 있습니다.)

src/main/java/net/spy/memcached/v2/vo/SMGetResult.java

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

oliviarla · 2025-12-09T12:36:50Z

@brido4125 @uhm0311
리뷰 모두 반영해두었습니다.

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

src/main/java/net/spy/memcached/v2/InsertAndGetResult.java

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

uhm0311

리뷰입니다.

src/main/java/net/spy/memcached/v2/vo/BKey.java

src/test/java/net/spy/memcached/v2/BTreeAsyncArcusCommandsTest.java

jhpark816

일부 리뷰

jhpark816 · 2025-12-18T11:36:05Z

src/main/java/net/spy/memcached/v2/vo/BopGetArgs.java

+  public static final class Builder {
+    private ElementFlagFilter eFlagFilter;
+    private int offset = 0;
+    private int count = 50;


default 값 50은 어떤 기준으로 설정한 것인가요?

bop get/mget/smget 연산마다 달라질 것 같은데요.
bop get에서는 default가 0이어야 하고,
그 외는 count 값이 설정되어야 할 것 같습니다.

따라서, default 값 0이 맞을 것 같습니다.

bop get/mget/smget 에서 모두 오류 없이 사용 가능한 최대 값을 설정해둔 것 뿐입니다.
default 값을 0으로 두면 mget 시에 항상 50으로 재정의해두어야 하기 때문에 사용할 때 불편하지 않을까 싶었는데,
0으로 두는게 더 낫다면 변경하겠습니다.

src/main/java/net/spy/memcached/v2/vo/BopGetArgs.java

src/main/java/net/spy/memcached/v2/vo/BTreeElements.java

src/main/java/net/spy/memcached/v2/vo/SMGetElements.java

oliviarla · 2025-12-22T08:50:36Z

@jhpark816 리뷰 반영했습니다. resolve하지 않은 코멘트에 대해서는 의견 주시거나 일단 resolve 상태로 변경해주시면 됩니다.

jhpark816 · 2025-12-23T05:58:57Z

@uhm0311
리뷰 바랍니다.

src/main/java/net/spy/memcached/v2/vo/SMGetElements.java

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

jhpark816

일부 리뷰

src/main/java/net/spy/memcached/v2/vo/BTreeElements.java

src/main/java/net/spy/memcached/v2/vo/SMGetElements.java

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

jhpark816

작은 리뷰

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

jhpark816 · 2026-01-02T12:45:40Z

@uhm0311
마지막으로 리뷰를 한번 정확하게 해 주세요.

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

oliviarla · 2026-01-16T09:44:12Z

merge 알고리즘을 k-way merge 알고리즘으로 수정하였습니다.
unique 옵션을 true로 준 경우 현재까지의 merged elements 중 맨 마지막 원소와 bkey 값을 비교하여 만약 이미 해당 bkey에 대한 element가 존재한다면 추가하지 않도록 하였습니다.

jhpark816

리뷰 완료.

그 외 의견입니다.

PR 코드가 전반적으로 복잡해 보임.
용어 정리가 필요해 보임.

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

jhpark816 · 2026-01-20T02:38:06Z

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

+    for (Map.Entry<MemcachedNode, List<String>> entry : arrangedKeys) {
+      BTreeGetBulk<T> getBulk = createBTreeGetBulk(from, to, args, entry);
+      CompletableFuture<Map<String, BTreeElements<T>>> future =
+          bopMultiGetPerNode(client, getBulk).toCompletableFuture();


질문입니다.
toCompletableFuture() 구현은 어디에 있나요?

ArcusFuture가 상속받는 CompletableFuture 클래스 내부에 있습니다.

jhpark816 · 2026-01-20T02:58:58Z

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

+          : futureToKeys.entrySet()) {
+        if (entry.getKey().isCompletedExceptionally()) {
+          for (String key : entry.getValue()) {
+            results.put(key, null);


여기도 질문입니다.
이 작업을 bopMultiGetPerNode() 에서 담당할 수 있나요?

최종 결과 Map 객체를 bopMultiGetPerNode에 인자로 전달하고 해당 메서드 내부에서 응답이 오면 Map에 바로 저장하라는 말씀이신가요?

저는 현재 방식이 이해하기 쉬울 것 같습니다.

jhpark816 · 2026-01-20T03:02:12Z

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java

+        client.groupingKeys(keys, ArcusClient.SMGET_CHUNK_SIZE, APIType.BOP_SMGET);
+
+    Collection<CompletableFuture<?>> futures = new ArrayList<>();
+    List<CompletableFuture<SMGetElements<T>>> smGetFutures = new ArrayList<>();


futures와 smGetFutures 모두가 있어야 하나요? 하나만 두어도 되는지?
(같은 내용을 가지고 있어서 질문합니다.)

하나만 두도록 수정했습니다.

jhpark816 · 2026-01-20T03:17:52Z

src/main/java/net/spy/memcached/v2/vo/SMGetElements.java

+    }
+
+    return new SMGetElements<>(elements, missedKeys, trimmedKeys);
+  }


이러한 로직을 smget 처리하는 코드에 두지 않고, SMGetElements 클래스에 두고 있네요.
smget 처리 코드에 두어야 하지 않는 지 ?

이전에 @uhm0311 님께서 이쪽에 두는 것에 좋겠다는 코멘트를 받기도 했고 저도 이쪽에 두는 게 나을 것 같습니다.
AsyncArcusCommands에 이 로직을 추가하면 클래스가 너무 복잡해질 것 같습니다.

oliviarla requested review from brido4125, jhpark816 and uhm0311 November 26, 2025 10:29

oliviarla force-pushed the btreenewapi branch from a07510d to 91630d5 Compare November 26, 2025 10:32

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

oliviarla force-pushed the btreenewapi branch from 91630d5 to 310caa8 Compare December 3, 2025 10:33

This comment was marked as resolved.

Sign in to view

brido4125 reviewed Dec 4, 2025

View reviewed changes

oliviarla force-pushed the btreenewapi branch from 310caa8 to 67103e8 Compare December 4, 2025 03:44

oliviarla force-pushed the btreenewapi branch from 67103e8 to a4de747 Compare December 8, 2025 09:02

oliviarla requested review from brido4125 and uhm0311 December 8, 2025 09:02

uhm0311 requested changes Dec 8, 2025

View reviewed changes

src/main/java/net/spy/memcached/v2/vo/SMGetResult.java Outdated Show resolved Hide resolved

oliviarla force-pushed the btreenewapi branch from a4de747 to fd67320 Compare December 8, 2025 13:01

brido4125 reviewed Dec 9, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

oliviarla force-pushed the btreenewapi branch from fd67320 to 01134d3 Compare December 9, 2025 12:36

brido4125 reviewed Dec 10, 2025

View reviewed changes

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Show resolved Hide resolved

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Outdated Show resolved Hide resolved

src/main/java/net/spy/memcached/v2/InsertAndGetResult.java Outdated Show resolved Hide resolved

oliviarla force-pushed the btreenewapi branch from 01134d3 to eed5b94 Compare December 10, 2025 03:16

uhm0311 reviewed Dec 10, 2025

View reviewed changes

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Outdated Show resolved Hide resolved

uhm0311 previously approved these changes Dec 10, 2025

View reviewed changes

oliviarla requested a review from brido4125 December 10, 2025 08:13

brido4125 previously approved these changes Dec 11, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

This was referenced Dec 12, 2025

INTERNAL: Remove GenericTranscoder for AsyncArcusCommands #1028

Merged

INTERNAL: Use KeyValidator class for validating keys #1030

Merged

oliviarla requested a review from jhpark816 December 16, 2025 07:28

uhm0311 reviewed Dec 17, 2025

View reviewed changes

src/main/java/net/spy/memcached/v2/vo/BKey.java Show resolved Hide resolved

src/test/java/net/spy/memcached/v2/BTreeAsyncArcusCommandsTest.java Show resolved Hide resolved

src/test/java/net/spy/memcached/v2/BTreeAsyncArcusCommandsTest.java Show resolved Hide resolved

oliviarla force-pushed the btreenewapi branch from b5b113f to a6c92cb Compare December 18, 2025 08:49

oliviarla requested a review from uhm0311 December 18, 2025 08:49

jhpark816 reviewed Dec 18, 2025

View reviewed changes

oliviarla force-pushed the btreenewapi branch from a6c92cb to 2fa92e5 Compare December 22, 2025 08:49

oliviarla force-pushed the btreenewapi branch from 2fa92e5 to 41521bd Compare December 22, 2025 08:51

uhm0311 reviewed Dec 23, 2025

View reviewed changes

src/main/java/net/spy/memcached/v2/vo/SMGetElements.java Show resolved Hide resolved

oliviarla requested a review from uhm0311 December 24, 2025 08:05

uhm0311 reviewed Dec 26, 2025

View reviewed changes

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Show resolved Hide resolved

uhm0311 previously approved these changes Dec 26, 2025

View reviewed changes

jhpark816 reviewed Dec 27, 2025

View reviewed changes

src/main/java/net/spy/memcached/v2/vo/BTreeElements.java Outdated Show resolved Hide resolved

src/main/java/net/spy/memcached/v2/vo/SMGetElements.java Show resolved Hide resolved

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Outdated Show resolved Hide resolved

oliviarla dismissed uhm0311’s stale review via 62f6ec4 December 29, 2025 10:22

oliviarla force-pushed the btreenewapi branch from 41521bd to 62f6ec4 Compare December 29, 2025 10:22

oliviarla requested a review from jhpark816 December 30, 2025 06:33

jhpark816 reviewed Jan 2, 2026

View reviewed changes

src/main/java/net/spy/memcached/v2/AsyncArcusCommands.java Outdated Show resolved Hide resolved

uhm0311 requested changes Jan 5, 2026

View reviewed changes

oliviarla force-pushed the btreenewapi branch 2 times, most recently from d59ead4 to a9dc006 Compare January 16, 2026 09:44

uhm0311 previously approved these changes Jan 19, 2026

View reviewed changes

jhpark816 reviewed Jan 20, 2026

View reviewed changes

FEATURE: Add CompletableFuture BTree insert/get API

c679274

oliviarla dismissed uhm0311’s stale review via c679274 January 21, 2026 11:54

oliviarla force-pushed the btreenewapi branch from a9dc006 to c679274 Compare January 21, 2026 11:54

jhpark816 approved these changes Jan 21, 2026

View reviewed changes

jhpark816 merged commit 011382e into naver:develop Jan 21, 2026
4 checks passed

Conversation

oliviarla commented Nov 26, 2025 • edited by jhpark816 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Related Issue

⌨️ What I did

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as resolved.

Uh oh!

brido4125 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oliviarla commented Dec 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

oliviarla commented Dec 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

uhm0311 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oliviarla commented Dec 22, 2025

Uh oh!

jhpark816 commented Dec 23, 2025

Uh oh!

Uh oh!

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jhpark816 commented Jan 2, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oliviarla commented Jan 16, 2026

Uh oh!

jhpark816 left a comment

oliviarla commented Nov 26, 2025 •

edited by jhpark816

Loading

brido4125 left a comment •

edited

Loading

jhpark816 Jan 20, 2026 •

edited

Loading