Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REFACTOR: remove unused getMaxPipeCount. #853

Merged
merged 1 commit into from
Dec 12, 2024
Merged

REFACTOR: remove unused getMaxPipeCount. #853

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions src/main/java/net/spy/memcached/ArcusClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1162,11 +1162,6 @@ public Future<Map<String, OperationStatus>> asyncDeleteBulk(String... key) {
return asyncDeleteBulk(Arrays.asList(key));
}

@Override
public int getMaxPipedItemCount() {
return CollectionPipedInsert.MAX_PIPED_ITEM_COUNT;
}

@Override
public CollectionFuture<Boolean> asyncBopCreate(String key,
ElementValueType valueType,
Expand Down
7 changes: 0 additions & 7 deletions src/main/java/net/spy/memcached/ArcusClientIF.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,13 +440,6 @@ <T> Future<Map<String, CollectionOperationStatus>> asyncSopInsertBulk(
Future<Map<String, CollectionOperationStatus>> asyncSopInsertBulk(
List<String> keyList, Object value, CollectionAttributes attributesForCreate);

/**
* Get maximum possible piped bulk insert item count.
*
* @return Get maximum possible piped bulk insert item count.
*/
int getMaxPipedItemCount();

/**
* Create an empty b+ tree
*
Expand Down
6 changes: 0 additions & 6 deletions src/main/java/net/spy/memcached/ArcusClientPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import net.spy.memcached.collection.BTreeOrder;
import net.spy.memcached.collection.ByteArrayBKey;
import net.spy.memcached.collection.CollectionAttributes;
import net.spy.memcached.collection.CollectionPipedInsert;
import net.spy.memcached.collection.Element;
import net.spy.memcached.collection.ElementFlagFilter;
import net.spy.memcached.collection.ElementFlagUpdate;
Expand Down Expand Up @@ -627,11 +626,6 @@ public Future<Map<String, CollectionOperationStatus>> asyncSopInsertBulk(
attributesForCreate);
}

@Override
public int getMaxPipedItemCount() {
return CollectionPipedInsert.MAX_PIPED_ITEM_COUNT;
}

@Override
public CollectionFuture<Boolean> asyncBopCreate(String key,
ElementValueType valueType,
Expand Down
7 changes: 4 additions & 3 deletions src/test/java/net/spy/memcached/ArcusTimeoutTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.concurrent.TimeoutException;

import net.spy.memcached.collection.CollectionAttributes;
import net.spy.memcached.collection.CollectionPipedInsert;
import net.spy.memcached.collection.ElementFlagFilter;
import net.spy.memcached.collection.SMGetElement;
import net.spy.memcached.collection.SMGetMode;
Expand Down Expand Up @@ -141,7 +142,7 @@ void testBulkDeleteTimeoutUsingSingleThread() {
@Test
void testSopPipedInsertBulkTimeout() {
String key = "testTimeout";
int valueCount = mc.getMaxPipedItemCount();
int valueCount = CollectionPipedInsert.MAX_PIPED_ITEM_COUNT;
Object[] valueList = new Object[valueCount];
for (int i = 0; i < valueList.length; i++) {
valueList[i] = "MyValue" + i;
Expand Down Expand Up @@ -198,7 +199,7 @@ void testBopPipedInsertBulkTimeout() {
String key = "MyBopKey";
String value = "MyValue";

int bkeySize = mc.getMaxPipedItemCount();
int bkeySize = CollectionPipedInsert.MAX_PIPED_ITEM_COUNT;
Map<Long, Object> bkeys = new TreeMap<>();
for (int i = 0; i < bkeySize; i++) {
bkeys.put((long) i, value);
Expand Down Expand Up @@ -296,7 +297,7 @@ void testMopInsertBulkMultipleTimeout() {
String key = "MyMopKey";
String value = "MyValue";

int elementSize = mc.getMaxPipedItemCount();
int elementSize = CollectionPipedInsert.MAX_PIPED_ITEM_COUNT;
Map<String, Object> elements = new TreeMap<>();
for (int i = 0; i < elementSize; i++) {
elements.put(String.valueOf(i), value);
Expand Down
3 changes: 2 additions & 1 deletion src/test/manual/net/spy/memcached/bulkoperation/SopInsertBulkMultipleValueTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import net.spy.memcached.collection.BaseIntegrationTest;
import net.spy.memcached.collection.CollectionAttributes;
import net.spy.memcached.collection.CollectionPipedInsert;
import net.spy.memcached.ops.CollectionOperationStatus;

import org.junit.jupiter.api.Test;
Expand All @@ -41,7 +42,7 @@ void testInsertAndGet() {
String key = "testInsertAndGet";
String prefix = "MyValue";

int valueCount = mc.getMaxPipedItemCount();
int valueCount = CollectionPipedInsert.MAX_PIPED_ITEM_COUNT;
Object[] valueList = new Object[valueCount];
for (int i = 0; i < valueList.length; i++) {
valueList[i] = String.format("%s%d", prefix, i);
Expand Down
3 changes: 2 additions & 1 deletion src/test/manual/net/spy/memcached/collection/list/LopBulkAPITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import net.spy.memcached.collection.BaseIntegrationTest;
import net.spy.memcached.collection.CollectionAttributes;
import net.spy.memcached.collection.CollectionOverflowAction;
import net.spy.memcached.collection.CollectionPipedInsert;
import net.spy.memcached.internal.CollectionFuture;
import net.spy.memcached.ops.CollectionOperationStatus;

Expand All @@ -42,7 +43,7 @@ class LopBulkAPITest extends BaseIntegrationTest {
private final List<Object> valueList = new ArrayList<>();

private int getValueCount() {
return mc.getMaxPipedItemCount();
return CollectionPipedInsert.MAX_PIPED_ITEM_COUNT;
}

@BeforeEach
Expand Down
3 changes: 2 additions & 1 deletion src/test/manual/net/spy/memcached/collection/map/MopBulkAPITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import net.spy.memcached.collection.BaseIntegrationTest;
import net.spy.memcached.collection.CollectionAttributes;
import net.spy.memcached.collection.CollectionOverflowAction;
import net.spy.memcached.collection.CollectionPipedInsert;
import net.spy.memcached.internal.CollectionFuture;
import net.spy.memcached.ops.CollectionOperationStatus;

Expand All @@ -41,7 +42,7 @@ class MopBulkAPITest extends BaseIntegrationTest {
private final Map<String, Object> updateMap = new HashMap<>();

private int getValueCount() {
return mc.getMaxPipedItemCount();
return CollectionPipedInsert.MAX_PIPED_ITEM_COUNT;
Copy link
Collaborator

Choose a reason for hiding this comment

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

CollectionPipedInsert.MAX_PIPED_ITEM_COUNT 외에도
CollectionPipedUpdate.MAX_PIPED_ITEM_COUNT 값이 있습니다.

테스트 로직에서 두 값 중 어떤 값을 사용해야 하는 지도 검토해 보시죠.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • CollectionPipedInsert.MAX_PIPED_ITEM_COUNT
  • CollectionPipedUpdate.MAX_PIPED_ITEM_COUNT
  • SetPipedExist.MAX_PIPED_ITEM_COUNT

셋 다 동일하게 500으로 설정되어있습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

셋 다 동일하게 500으로 설정되어있습니다.

위의 이유가
CollectionPipedInsert.MAX_PIPED_ITEM_COUNT 값을 모든 경우에서 사용하는 이유가 되지 못합니다.

언제가 CollectionPipedUpdate.MAX_PIPED_ITEM_COUNT 값이 700으로 변경될 수도 있습니다.
이 값이 변경되더라도, 테스트 코드는 정상적으로 동작해야 합니다.
따라서, 코드 로직을 확인하고 그 로직에 맞는 값을 사용해야 합니다.

코드를 모두 확인해 보았고,
CollectionPipedInsert.MAX_PIPED_ITEM_COUNT 값을 사용하는 것이 맞다라고 하면,
충분한 설명이 될 수 있을 것입니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 값이 변경되더라도, 테스트 코드는 정상적으로 동작해야 합니다. 따라서, 코드 로직을 확인하고 그 로직에 맞는 값을 사용해야 합니다.

pipe max count 값이 변경 시, 고려할 수 있는 두 가지 경우입니다.

  • 캐시서버도 값과 함께 변경 -> 테스트 코드 성공 (말씀하신 정상적인 동작을 수행)
  • 값만 변경 -> 테스트 코드 실패로 인한 변경 감지 가능

현재 구현 상의 Insert / Update MAX_PIPED_ITEM_COUNT의 시멘틱을 안다면
두 가지 값의 변경은 함께 이루어질 것이라고 예상합니다.

언젠가 CollectionPipedUpdate.MAX_PIPED_ITEM_COUNT 값이
700으로 변경된다면
CollectionPipedInsert.MAX_PIPED_ITEM_COUNT도 함께
700으로 변경될 가능성이 더 높지 않나요?

결론적으로 변수의 의미와 내부 로직을 살펴보니 Insert의 MAX_PIPED_ITEM_COUNT를 사용해도 상관 없다고 생각합니다.

}

@BeforeEach
Expand Down
3 changes: 2 additions & 1 deletion src/test/manual/net/spy/memcached/collection/set/SopBulkAPITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import net.spy.memcached.collection.BaseIntegrationTest;
import net.spy.memcached.collection.CollectionAttributes;
import net.spy.memcached.collection.CollectionPipedInsert;
import net.spy.memcached.internal.CollectionFuture;
import net.spy.memcached.ops.CollectionOperationStatus;

Expand All @@ -42,7 +43,7 @@ class SopBulkAPITest extends BaseIntegrationTest {
private final List<Object> valueList = new ArrayList<>();

private int getValueCount() {
return mc.getMaxPipedItemCount();
return CollectionPipedInsert.MAX_PIPED_ITEM_COUNT;
}

@BeforeEach
Expand Down
5 changes: 3 additions & 2 deletions src/test/manual/net/spy/memcached/collection/set/SopPipedExistTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import net.spy.memcached.collection.CollectionAttributes;
import net.spy.memcached.collection.CollectionResponse;
import net.spy.memcached.collection.ElementValueType;
import net.spy.memcached.collection.SetPipedExist;
import net.spy.memcached.internal.CollectionFuture;

import org.junit.jupiter.api.AfterEach;
Expand Down Expand Up @@ -145,7 +146,7 @@ void testMaxPipedExist() {
List<Object> findValues = new ArrayList<>();

// insert items
for (int i = 0; i < mc.getMaxPipedItemCount(); i++) {
for (int i = 0; i < SetPipedExist.MAX_PIPED_ITEM_COUNT; i++) {
findValues.add("VALUE" + i);

if (i / 2 == 0) {
Expand All @@ -163,7 +164,7 @@ void testMaxPipedExist() {

assertTrue(future.getOperationStatus().isSuccess());

for (int i = 0; i < mc.getMaxPipedItemCount(); i++) {
for (int i = 0; i < SetPipedExist.MAX_PIPED_ITEM_COUNT; i++) {
if (i / 2 == 0) {
assertFalse(map.get("VALUE" + i));
} else {
Expand Down