CLEANUP: Refactored SMGetFuture. #685

uhm0311 · 2023-10-12T06:24:13Z

SMGetFuture를 추상 클래스에서 일반 클래스로 변경했습니다.
일반 클래스로 변경하면서 각 smget 메소드에서 구현했던 추상 메소드를 SMGetFuture 내로 옮겼습니다.

SMGetFuture를 통해 접근 가능한 여러 필드들은 SMGetResult 클래스를 만들어 접근하도록 했습니다.
SMGetResult 객체에 직접 참조를 통해 필드에 Write 하게 했는데, 간접 참조로 바꿔야 더 명확하다면 간접 참조로 바꾸겠습니다.

직접 참조 예시 : r.addToMergedResult(eachResult);
간접 참조 예시 : r.getMergedResult().addAll(eachResult)

SMGetFuture 내의 필드로 SMGetResult<?> smGetResult를 두었는데, SMGetResult<T>가 아닌 이유는 smget 메소드의 return type이 SMGetFuture<List<SMGetElement<G>>>이기 때문입니다.
class SMGetFuture<T>에서 T = List<SMGetElement<G>>로부터 내부의 <G>를 가져올 수 없어 SMGetResult<?> 형태의 필드를 둔 것입니다.

추가 기대 효과

또한 현재 형태의 리팩토링은 Future 클래스에서 public으로 노출하는 메소드를 getter에 한정하게 되어 다음의 이슈를 해결할 수 있는 방안 중 하나가 될 것 같습니다.

API로 노출하지 않아도 되는 public 메소드의 보호 방안 #632

brido4125 · 2023-10-16T08:29:06Z

저는 현재와 같은 구현 또는 PR의 구현 둘다 상관 없다고 생각합니다.
다만, 두 형태 중 추후 CompletableFuture 사용으로 변경될 api들의
구현을 고려하여 어떠한 구현이 더 나을지 선택되어야 한다고 봅니다.

그래서 CompletableFuture를 사용하는
ArcusFuture에 대한 설계가 윤곽이 잡히고
해당 문제에 대한 논의를 시작하는게 좋을 것 같습니다.

brido4125 · 2023-10-20T03:17:14Z

@jhpark816

현재 api에 존재하는 missedKeyList, missedkeys, resultOperationStatus 등의
다양한 필드들을 Future에서 인스턴스 생성하는 방안과
현재 PR의 방안을 비교 분석해보았습니다.

현재 PR의 방식

장점 : future를 참고하여 각종 field값에 접근할 필요가 없고, Future 생성 시 직관적으로 filed들을 주입해줄 수 있다.
단점 : 현재 구현에서는 smget() 구현이 2개인데 중복되는 Fielde들이 다량 존재해 코드 중복이 발생한다.

Future에서 field 인스턴스를 생성하는 경우

장점 : 현재 PR의 단점인 중복 코드를 방지할 수 있고, smget()을 제외한 나머지 api들이 해당 방식을 대다수 따르기에 리팩토링 공수가 줄어든다.
단점 : future을 참조하여 가져와야할 field들이 많아져 Field 값들이 생성되는곳과 사용되는 곳이 달라지게 된다. 즉, 정작 Future 내부의 메서드들에서는 해당 field를 return만 해주는 경우가 많은데 정작 field가 사용되어야할 Callback등에서 메서드 호출을 통해 접근할 수 있다. 또한 현재 구현 상 Future라는 인터페이스는 사용자에게 노출되고 캡슐화를 통해 여러 field들을 conceal하여도 노출되는 다량의 public 메서드가 생기게 된다.

사실 사용자 입장에서는 리턴받은 Future에 공개 되는 메서드들이 적은게 좀 더 편리하다고 봅니다.
그래서 현재 PR의 구현을 가져가는게 이러한 관점에서는 좋다고 생각합니다.

uhm0311 · 2023-10-20T10:30:34Z

SMGetResult라는 클래스를 만들어서 사용하는 방법도 있습니다.

public class SMGetResult<T> {
  public final int offset;
  public final int count;
  public final List<SMGetElement<T>> mergedResult = ...;
  public final List<SMGetElement<T>> mergedSubList = ...;
  public final List<OperationStatus> resultOperationStatus = ...;
  public final List<OperationStatus> failedOperationStatus = ...;
  public final List<SMGetTrimKey> mergedTrimmedKeys = ...;
  public final List<String> missedKeyList = ...;
  public final Map<String, CollectionOperationStatus> missedKeys = ...;

  ...
}

private ... smget(...) {
  final SMGetResult smgetResult = SMGetResult().builder()
          .offset(offset)
          .count(count)
          .build();

  ...
        @Override
        public void gotMissedKey(byte[] data) {
          smgetResult.missedKeyList.add(new String(data));
          OperationStatus cause = new OperationStatus(false, "UNDEFINED");
          smgetResult.missedKeys.put(new String(data), new CollectionOperationStatus(cause));
        }
  ...
  
  return SMGetFuture.builder()
          .latch(blatch)
          .ops(ops)
          .timeout(operationTimeout)
          .smgetResult(smgetResult)
          .build()
}

public class SMGetFuture<...> ... {
  ...
  
  public ... get(long duration, TimeUnit units) {
    ...
    
    if (ops.size() == 1) {
      return smgetResult.mergedResult;
    }

     return smgetResult.getSubList();
  }  

  ...

  public List<String> getMissedKeyList() {
    return smgetResult.missedKeyList;
  }

  ...
}

jhpark816 · 2023-10-20T12:03:24Z

@brido4125 @uhm0311
Future 내부에서 각 응답 결과를 받아 최종 결과를 생성할 경우,
각 연산 및 결과 생성 방식에 따라 Future가 구분되어야 합니다.
따라서, 위와 같이 결과 생성 로직이 Future 외부에 있고,
Future에 넣어주는 방식이 단일의 ArcusFuture 만드는 데 적합하지 않나 생각합니다.

@uhm0311
위의 Future 생성에서 builder 패턴은 사용하지 않는 것이 좋겠습니다.
모두 필수 인자이기에 생성자 함수로 처리하는 것이 나을 것입니다.

brido4125 · 2023-10-22T05:17:30Z

@uhm0311
api 메서드단에서 SMGetResult를 생성하여 Future에 주입해주는 방안은 좋은 방안인 것 같습니다.
본 PR에 해당 작업 진행하실껀가요?

uhm0311 · 2023-10-23T02:02:43Z

@brido4125

본 PR에 작업 진행하도록 하겠습니다.

uhm0311 · 2023-10-23T04:56:41Z

@brido4125 @jhpark816

원본 코멘트 수정했습니다. 리뷰 부탁드립니다.

src/main/java/net/spy/memcached/ArcusClient.java

jhpark816

리뷰 완료

jhpark816 · 2023-10-23T08:17:21Z

src/main/java/net/spy/memcached/ArcusClient.java

        }

        @Override
        public void gotTrimmedKey(String key, Object subkey) {
-          if (failedOperationStatus.isEmpty()) {
+          if (r.isFailedOperationStatusEmpty()) {


이 경우를 보니,
예전 코드에 있는 stopCollect Flag를 따로 두어 처리하는 것이 나을 것 같습니다.

jhpark816 · 2023-10-23T08:18:11Z

src/main/java/net/spy/memcached/ArcusClient.java

-          missedKeyList.add(key);
-          missedKeys.put(key, new CollectionOperationStatus(cause));
+          r.addToMissedKeyList(key);
+          r.putToMissedKeyMap(key, new CollectionOperationStatus(cause));


아래 하나의 메소드만 호출하고 해당 메소드 안에서 알아서 처리하면 좋겠습니다.

r.addMissedKey(key, new CollectionOperationStatus(cause));

jhpark816 · 2023-10-23T08:20:37Z

src/main/java/net/spy/memcached/ArcusClient.java

            }
-            failedOperationStatus.add(status);
+            r.addToFailedOperationStatus(status);


r.addFailedOperationStatus(status)만 호출하고 나머진 호출된 메소드 안에서 모두 처리

jhpark816 · 2023-10-23T08:21:33Z

src/main/java/net/spy/memcached/ArcusClient.java

            return;
          }

-          if (mergedResult.isEmpty()) {
+          if (r.isMergedResultEmpty()) {


r.mergeElementResult(eachResult) 만 호출하고 나머진 안에서 처리

jhpark816 · 2023-10-23T08:21:53Z

src/main/java/net/spy/memcached/ArcusClient.java

-            if (mergedTrimmedKeys.isEmpty()) {
-              mergedTrimmedKeys.addAll(eachTrimmedResult);
+            if (r.isMergedTrimmedKeysEmpty()) {
+              r.addToMergedTrimmedKeys(eachTrimmedResult);


eachTrimmedResult만 넘기고 merge 작업에 호출된 메소드 안에서 처리

jhpark816 · 2023-10-23T08:22:47Z

src/main/java/net/spy/memcached/ArcusClient.java

                pos += 1;
              }
            }
          }

          if (processedSMGetCount.get() == 0) {
-            if (!mergedTrimmedKeys.isEmpty() && count <= mergedResult.size()) {
+            if (!r.isMergedTrimmedKeysEmpty() && count <= r.sizeOfMergedTrimmedKeys()) {


processedSMGetCount.get() == 0인 경우의 처리 로직도 result 객체의 메소드로 포장하여 호출

jhpark816 · 2023-10-23T08:24:01Z

src/main/java/net/spy/memcached/internal/SMGetFuture.java

    this.ops = ops;
    this.timeout = timeout;
+    this.smGetResult = smGetResult;


this.result = result; 이면 좋겠습니다.

생성자의 인자는 ops, result, latch, timeout 순서가 좋습니다.

jhpark816 · 2023-10-23T08:25:01Z

src/main/java/net/spy/memcached/ArcusClient.java

            // merged result is empty, add all.
-            mergedResult.addAll(eachResult);
+            r.addToMergedResult(eachResult);


old smget 쪽은 이에 해당하는 smgetResultOld를 따로 정의하는 것이 낫지 않나 생각합니다.

uhm0311 · 2023-10-23T09:11:22Z

@jhpark816

Callback의 로직들을 SMGetResult 클래스로 이관했습니다.
class SMGetResultOld의 경우 class SMGetResult와 분리하는 것이 애매하다고 판단하여 우선 분리하지 않았습니다.

jhpark816 · 2023-10-31T04:57:31Z

@brido4125 리뷰를 먼저 진행 바랍니다.

brido4125 · 2023-10-31T05:04:00Z

@jhpark816

...Result 형태의 객체를 다른 api에도 적용시키는 방안으로 결정을 하신건가요?

jhpark816 · 2023-11-03T11:00:12Z

@brido4125

...Result 형태의 객체를 다른 api에도 적용시키는 방안으로 결정을 하신건가요?

아직 결정한 건 아니고, 이에 대한 의견을 먼저 받았으면 합니다.

그리고, 이러한 결정과 무관하게 smget 리팩토링을 리뷰해 주면 될 것 같습니다.

Result 객체를 사용하는 것이 좋다면, 이 방식의 구현이 적합한지 리뷰하면 됩니다.
다른 좋은 방안이 있다면 그에 대한 의견을 주면 됩니다.

src/main/java/net/spy/memcached/internal/SMGetFuture.java

jhpark816 · 2023-11-10T07:26:15Z

@oliviarla 본 PR 리뷰할 수 있을까요?

oliviarla · 2023-11-10T07:43:11Z

@jhpark816 넵 진행해보겠습니다.

oliviarla

리뷰 완료했습니다.

oliviarla · 2023-11-15T03:04:01Z

src/main/java/net/spy/memcached/ArcusClient.java


    // if processedSMGetCount is 0, then all smget is done.
    final AtomicInteger processedSMGetCount = new AtomicInteger(smGetList.size());
    final AtomicBoolean mergedTrim = new AtomicBoolean(false);
    final AtomicBoolean stopCollect = new AtomicBoolean(false);

+    final CollectionOperationStatus missedKeyCause


gotMissedKey 메서드에서만 사용되는 것 같은데 변수로 따로 두신 이유가 궁금합니다.

gotMissedKey가 호출될 때마다 매번 객체를 새로 생성할 필요가 없어서 하나 만들어두고 쓰기 위해 두었습니다.

oliviarla · 2023-11-15T05:09:38Z

src/main/java/net/spy/memcached/internal/SMGetResult.java

+import net.spy.memcached.ops.OperationStatus;
+
+public class SMGetResult<T>  {
+  private final int offset;


offset을 활용하는 방식이 Deprecated된 것 같은데, SMGetResult 클래스에는 offset을 제외한 최신 사항만 반영해두는게 나중에 제거하기 편하지 않을까요?

혹은 Deprecated된 부분은 리팩토링에서 제외하거나, Result 클래스를 두개로 분리할 수도 있을 것 같습니다.

SMGetResult 클래스를 추상 클래스로 두고, SMGetResultImpl 클래스와 SMGetResultOldImpl 클래스로 분리했습니다.

jhpark816

일부 리뷰

jhpark816 · 2023-12-18T09:14:57Z

src/main/java/net/spy/memcached/ArcusClient.java

    final String TRIMMED = "TRIMMED";
-    final String DUPLICATED = "DUPLICATED";
    final String DUPLICATED_TRIMMED = "DUPLICATED_TRIMMED";


END, DUPLICATED 사용되지 않아 제거하였네요.
TRIMMED, DUPLICATED_TRIMMED도 해당 코드에서 문자열을 직접 사용하고, 변수는 제거하도록 합시다.
이 부분은 CLEANUP 작업으로 별도 PR로 받는 것이 좋을 것 같습니다.

jhpark816 · 2023-12-18T10:38:37Z

src/main/java/net/spy/memcached/ArcusClient.java

-          missedKeyList.add(new String(data));
-          OperationStatus cause = new OperationStatus(false, "UNDEFINED");
-          missedKeys.put(new String(data), new CollectionOperationStatus(cause));
+          result.addMissedKey(new String(data), missedKeyCause);


missedKeyCause 멤버를 두는 대신에 기존처럼 아래와 같이 구현하면 좋겠습니다.
수행 효율은 떨어지지만 코드 읽기가 훨씬 수월합니다.

OperationStatus cause = new OperationStatus(false, "UNDEFINED"); result.addMissedKey(new String(data), new CollectionOperationStatus(cause));

jhpark816 · 2023-12-18T10:52:52Z

src/main/java/net/spy/memcached/ArcusClient.java

-              }
-            }
-          }
+          result.mergeSMGetElements(eachResult, isTrimmed, mergedTrim, processedSMGetCount, reverse);


대략 아래와 같은 로직으로 구현하면 좋겠습니다.

smget 수행이 성공이면,

merge 수행

smget 수행의 마지막이면, 최종 operation status 생성

smget 수행이 실패이면, (아래 로직이 맞는 지는 확인 필요)

실패 처리

if (status.isSuccess()) { result.mergeSMGetElements(eachResult, isTrimmed); if (processedSMGetCount.get() == 0) { result.makeResultOperationsStatus(); } } else { if (!stopCollect.get()) { result.addFailedOperationStatus(); stopCollect.set(true); } }

결국

mergedTrim도 SMGetResult 안에서 선언해야 하고,

processedSMGetCount는 넘기지 않도록 하고,

reverse도 SMGetResult 생성 시에 넘기면 좋을 것 같습니다.

jhpark816 · 2023-12-18T10:56:44Z

src/main/java/net/spy/memcached/ArcusClient.java

-              }
-            }
-          }
+          result.mergeSMGetElements(eachResult, eachTrimmedResult, processedSMGetCount, reverse, smgetMode);


여기도 위의 코멘트 참고하여 같은 형태로 변경하면 좋겠습니다.

uhm0311 · 2023-12-19T02:45:43Z

@jhpark816

리뷰 반영했습니다.

jhpark816

일부 리뷰

jhpark816 · 2023-12-19T03:22:59Z

src/main/java/net/spy/memcached/ArcusClient.java

-          }
+          if (status.isSuccess()) {
+            boolean isTrimmed = (TRIMMED.equals(status.getMessage()) ||
+                    DUPLICATED_TRIMMED.equals(status.getMessage()));


아래와 같이 수정하고, 위에 선언된 String 변수들을 모두 제거하면 될 것 같습니다.

boolean isTrimmed = ("TRIMMED".equals(status.getMessage()) || "DUPLICATED_TRIMMED".equals(status.getMessage()));

반영했습니다.

jhpark816 · 2023-12-19T03:25:22Z

src/main/java/net/spy/memcached/ArcusClient.java

-            = Collections.synchronizedList(new ArrayList<OperationStatus>(1));
-    final List<OperationStatus> failedOperationStatus
-            = Collections.synchronizedList(new ArrayList<OperationStatus>(1));
+    final SMGetResultImpl<T> result = new SMGetResultImpl<T>(count, reverse, smgetMode);


smget 연산의 argument가 아래와 같습니다.

기존 : offset, count

신규: count, smgetMode

이를 고려하여, count, smgetMode, reverse 순서이면 좋겠습니다.

반영했습니다.

jhpark816 · 2023-12-19T09:56:21Z

src/main/java/net/spy/memcached/internal/SMGetResult.java

+    }
+
+    return mergedSubList;
+  }


아래 메소드와 인자를 두고 subList 구하는 것이 의미에 나을 것 같습니다.

List<SMGetElement<T>> getSubList(int offset, int count)

Implementation에서 getSubList(offset, count)로 호출하거나 getSubList(0, count)로 호출하고요.

이 메소드를 다시 생각해 보니,
offset == 0인 경우는 getSubList 수행하지 않고 mergedResult를 그대로 리턴해도 되지 않나요?
즉, mergedResult.size() <= count 조건을 항상 만족할 것 같습니다. 어떤가요?

위 사항이 맞으면, getSubList()는 OldImpl에만 두면 될 것 같습니다.

아래 메소드와 인자를 두고 subList 구하는 것이 의미에 나을 것 같습니다.

그러면 SMGetFuture에 int offset, int count 필드가 필요합니다.
저는 이것이 좋은 변경이라고 생각하지 않습니다.

이 메소드를 다시 생각해 보니,
offset == 0인 경우는 getSubList 수행하지 않고 mergedResult를 그대로 리턴해도 되지 않나요?
즉, mergedResult.size() <= count 조건을 항상 만족할 것 같습니다. 어떤가요?

3개의 노드로부터 smget을 count=50을 주어 실행하여 받은 결과를 합치면 최대 150개이지 않나요?

3개의 노드로부터 smget을 count=50을 주어 실행하여 받은 결과를 합치면 최대 150개이지 않나요?

count가 50이면 최종 50개의 결과만 있으면 되므로,
mergedResult에는 항상 최대 50개의 결과만 보관하는 형태로 구현했을 것으로 생각됩니다.
구현 로직을 한번 검토해 주시죠.

if (mergedResult.size() > count) { // Remove elements that exceed the requested count. mergedResult.remove(count); }

Server 오류로 인해 주어진 count 보다 더 많은 응답을 반환하지만 않으면 맞습니다.
다만 위 코드의 if 부분을 while로 바꾸고 난 다음에 subList()의 구현을 바꾸는 것이 더 안전하다고 보는데, 어떤가요?

그러면 SMGetFuture에 int offset, int count 필드가 필요합니다.
저는 이것이 좋은 변경이라고 생각하지 않습니다.

SMGetFuture에 offset, count 필드를 두자는 것은 아니며,
구체적 예를 들면, SMGetResultImpl과 SMGetResultOldImpl에 아래의 메소드를 두고,
SMGetFuture에서 getFinalResult() 호출하자는 것입니다.

// SMGetResultImpl public List<SMGetElement<T>> getFinalResult() { return mergedResult; } // SMGETResultOldImpl public List<SMGetElement<T>> getFinalResult() { if (offset > 0 && mergedCount > 1) { return mergedResult.subList(offset, count); } return mergedResult; }

List<SMGetElement<T>> getSubList(int offset, int count)

위 형태의 메소드를 구현하는 것인 줄 알았습니다.

다만 위 코드의 if 부분을 while로 바꾸고 난 다음에 subList()의 구현을 바꾸는 것이 더 안전하다고 보는데, 어떤가요?

이 부분을 반영해도 될까요?

jhpark816 · 2023-12-19T10:07:05Z

src/main/java/net/spy/memcached/internal/SMGetResultImpl.java

+  public SMGetResultImpl(int count, boolean reverse, SMGetMode smGetMode) {
+    super(count, count, reverse);
+
+    this.smGetMode = smGetMode;


아래와 같이 count, smgetMode, reverse 데이터가 실제 사용되는 Impl 클래스에 두는 것이 읽기에 좋은 데, 어떤가요?

super(count); this.count = count; this.smGetMode = smGetMode; this.reverse = reverse;

OldImpl 경우는 아래 로직의 생성자를 가지도록 하고요.

super(offset + count); this.offset = offset; this.count = count; this.reverse = reverse;

공통 부분을 super 클래스에 두는 것은 흔한 방식입니다.
이클립스나 인텔리제이를 쓰면 this.을 입력했을 때 무엇을 참조 가능한지도 볼 수 있기 때문에 불필요하다고 봅니다.

old smget에서는 <offset, count> 쌍이 하나의 의미를 가지고,
new smget에서는 <count, smGetMode> 쌍이 하나의 의미를 가집니다.
(new smget은 향후 <count, unique> 쌍으로 변경할 예정)

이와 같이 쌍으로 표현되는 의미가 SMGetResult에 같이 표현되는 것이 아니라
단순히 중복되는 변수이다 아니다로 구분하여 super에 두는 것이
의미(semantic)을 살리지 못한 구현이라서 코멘트한 것입니다.

uhm0311 · 2023-12-19T11:28:06Z

@jhpark816

리뷰 반영했습니다.

jhpark816

리뷰 완료

jhpark816 · 2023-12-19T11:43:14Z

src/main/java/net/spy/memcached/ArcusClient.java

            }
+            result.clearMergedResult();
+            getLogger().warn("SMGetFailed. status=%s", status);


아래와 같이 new smget와 같은 로직을 가지는 것이 더 맞지 않나 생각합니다.

stopCollect.set(true); result.addFailedOperationStatus(status); getLogger().warn("SMGetFailed. status=%s", status);

jhpark816 · 2023-12-19T11:44:53Z

src/main/java/net/spy/memcached/internal/SMGetResultImpl.java

+  public void addFailedOperationStatus(OperationStatus status) {
+    failedOperationStatus.add(status);
+    mergedResult.clear();
+    mergedTrimmedKeys.clear();


아래와 같이 failedOperationStatus가 처음 등록될 경우만 mergedResult를 clear해도 되지만,
현재 코드와 같이 무조건 clear 하는 것이 괜찮은 것 같습니다. 참고로 적은 사항입니다.

if (failedOperationStatus.size() == 0) { mergedResult.clear(); mergedTrimmedKeys.clear(); } failedOperationStatus.add(status);

jhpark816 · 2023-12-20T02:39:59Z

src/main/java/net/spy/memcached/internal/SMGetResultOldImpl.java

+
+  public void clearMergedResult() {
+    mergedResult.clear();
+  }


이 메소드는 제거하고 addFailedOperationStatus()에서
mergedResult.clear(); 호출합시다.

jhpark816 · 2023-12-20T02:51:08Z

src/main/java/net/spy/memcached/internal/SMGetResultOldImpl.java

+        mergedResult.add(pos, result);
+        while (mergedResult.size() > totalResultElementCount) {
+          mergedResult.remove(totalResultElementCount);
+        }


여기 코드를 확인해 보니, 아래 코드에 의해
mergedResult 크기는 totalResultElementCount 초과할 수 없습니다.
따라서, if 문이어도 충분합니다.

if (pos >= totalResultElementCount) { addAll = false; break; }

오히려, 아래 코드에서 주어진 eachResult 크기가 totalResultElementCount 이하임을 보장할 수 없으니,
while 문으로 totalResultElementCount 초과 개수를 제거하는 코드가 들어가는 것이 안전할 것 같습니다.

if (mergedResult.isEmpty()) { // merged result is empty, add all. mergedResult.addAll(eachResult); mergedTrim.set(isTrimmed);

jhpark816 · 2023-12-20T02:54:52Z

src/main/java/net/spy/memcached/internal/SMGetResultOldImpl.java

+      } else {
+        finalResult = mergedResult;
+      }
+    }


totalResultElementCount < mergedResult.size() 조건을 만족하는 경우는 없으니, 제거해도 될 것 같습니다.

uhm0311 · 2023-12-20T03:57:23Z

@jhpark816

리뷰 반영했습니다.

jhpark816

리뷰 완료

jhpark816 · 2023-12-20T04:13:17Z

src/main/java/net/spy/memcached/internal/SMGetResultImpl.java

+  }
+
+  public void mergeSMGetElements(final List<SMGetElement<T>> eachResult,
+                                 final List<SMGetTrimKey> eachTrimmedResult) {


mergeSMGetElements 코드가 기존 코드와 동일한 지를 확인 바랍니다.
차이가 있어 보입니다.
Old Impl 쪽도 확인해 주시고요.

if (processedSMGetCount.get() == 0) {
이 부분이 없는 걸 말씀하시는 건가요?
#685 (comment)
이 코멘트에 따라 mergeSMGetElements()에서 제거하고 다른 곳으로 옮겼습니다.

jhpark816 · 2023-12-20T04:30:03Z

src/main/java/net/spy/memcached/internal/SMGetResultImpl.java

+
+  @Override
+  public void makeResultOperationStatus() {
+    if (!mergedTrimmedKeys.isEmpty() && count <= mergedTrimmedKeys.size()) {


이 코드가 기존 코드와 다른 것 같습니다.
그 외에도 차이가 있는 부분이 있는 지를 확인 바랍니다.

아. mergeSMGetElements() 메소드가 아니라 makeResultOperationStatus() 메소드였습니다.

반영했습니다.

uhm0311 · 2023-12-20T04:48:25Z

@jhpark816

SMGetResult 클래스의 위치를 별도의 패키지로 옮겼습니다.

uhm0311 requested review from jhpark816 and brido4125 October 12, 2023 06:24

uhm0311 force-pushed the uhm0311/develop branch from f380894 to 9a00017 Compare October 12, 2023 06:35

This comment was marked as outdated.

Sign in to view

uhm0311 force-pushed the uhm0311/develop branch from 9a00017 to a16f50a Compare October 23, 2023 02:33

uhm0311 marked this pull request as draft October 23, 2023 03:05

uhm0311 force-pushed the uhm0311/develop branch 3 times, most recently from b2c7acf to 0c76bac Compare October 23, 2023 04:27

uhm0311 marked this pull request as ready for review October 23, 2023 04:56

brido4125 reviewed Oct 23, 2023

View reviewed changes

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

jhpark816 reviewed Oct 23, 2023

View reviewed changes

uhm0311 force-pushed the uhm0311/develop branch from 0c76bac to 787e9db Compare October 23, 2023 09:08

uhm0311 force-pushed the uhm0311/develop branch 2 times, most recently from 02f6d8e to 2f11dd8 Compare October 24, 2023 07:16

uhm0311 force-pushed the uhm0311/develop branch from 2f11dd8 to c2b0997 Compare October 31, 2023 05:08

brido4125 reviewed Nov 9, 2023

View reviewed changes

src/main/java/net/spy/memcached/internal/SMGetFuture.java Show resolved Hide resolved

brido4125 approved these changes Nov 10, 2023

View reviewed changes

oliviarla reviewed Nov 15, 2023

View reviewed changes

uhm0311 marked this pull request as draft November 15, 2023 05:57

uhm0311 force-pushed the uhm0311/develop branch from c2b0997 to d4488ea Compare November 15, 2023 08:35

uhm0311 marked this pull request as ready for review November 15, 2023 08:35

oliviarla approved these changes Nov 29, 2023

View reviewed changes

jhpark816 reviewed Dec 18, 2023

View reviewed changes

uhm0311 force-pushed the uhm0311/develop branch from d4488ea to 77031da Compare December 19, 2023 02:20

jhpark816 reviewed Dec 19, 2023

View reviewed changes

uhm0311 force-pushed the uhm0311/develop branch 3 times, most recently from a152bdd to 6a18e79 Compare December 19, 2023 11:26

jhpark816 reviewed Dec 20, 2023

View reviewed changes

uhm0311 force-pushed the uhm0311/develop branch from 6a18e79 to 0d3b519 Compare December 20, 2023 03:57

jhpark816 reviewed Dec 20, 2023

View reviewed changes

uhm0311 force-pushed the uhm0311/develop branch from 0d3b519 to 3f66924 Compare December 20, 2023 04:28

jhpark816 reviewed Dec 20, 2023

View reviewed changes

uhm0311 force-pushed the uhm0311/develop branch from 3f66924 to 539d4c0 Compare December 20, 2023 04:35

CLEANUP: Refactored SMGetFuture.

70d3cfd

uhm0311 force-pushed the uhm0311/develop branch from 539d4c0 to 70d3cfd Compare December 20, 2023 04:47

jhpark816 merged commit d2ac677 into naver:develop Dec 20, 2023
1 check passed

uhm0311 deleted the uhm0311/develop branch December 20, 2023 05:08

CLEANUP: Refactored SMGetFuture. #685

CLEANUP: Refactored SMGetFuture. #685

Conversation

uhm0311 commented Oct 12, 2023 • edited Loading

추가 기대 효과

This comment was marked as outdated.

This comment was marked as outdated.

brido4125 commented Oct 16, 2023

brido4125 commented Oct 20, 2023

현재 PR의 방식

Future에서 field 인스턴스를 생성하는 경우

uhm0311 commented Oct 20, 2023

jhpark816 commented Oct 20, 2023

brido4125 commented Oct 22, 2023

uhm0311 commented Oct 23, 2023

uhm0311 commented Oct 23, 2023

jhpark816 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uhm0311 commented Oct 23, 2023

jhpark816 commented Oct 31, 2023

brido4125 commented Oct 31, 2023 • edited Loading

jhpark816 commented Nov 3, 2023

jhpark816 commented Nov 10, 2023

oliviarla commented Nov 10, 2023

oliviarla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhpark816 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uhm0311 commented Dec 19, 2023

jhpark816 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhpark816 Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhpark816 Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhpark816 Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

uhm0311 commented Dec 19, 2023

jhpark816 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uhm0311 commented Dec 20, 2023

jhpark816 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uhm0311 commented Oct 12, 2023 •

edited

Loading

brido4125 commented Oct 31, 2023 •

edited

Loading

jhpark816 Dec 19, 2023 •

edited

Loading

jhpark816 Dec 19, 2023 •

edited

Loading

jhpark816 Dec 19, 2023 •

edited

Loading