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

Jump to bottom

FIX: Element sequence when using asyncLopInsert api. #633

Merged
merged 1 commit into from
Jun 30, 2023
Merged

FIX: Element sequence when using asyncLopInsert api. #633

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
6 changes: 5 additions & 1 deletion src/main/java/net/spy/memcached/collection/CollectionPipedInsert.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,11 @@ public ByteBuffer getAsciiCommand() {
CollectionCreate.makeCreateClause(attribute, cd.getFlags()) : "";
for (int i = this.nextOpIndex; i < eSize; i++) {
byte[] each = encodedList.get(i);
setArguments(bb, COMMAND, key, index, each.length,
int eIndex = index;
if (index >= 0) {
eIndex += i;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

int eIndex = index; // element index
if (eIndex >= 0) {
  eIndex += i;
}
setArguments(..., eIndex, ...);

setArguments(bb, COMMAND, key, eIndex, each.length,
createOption, (i < eSize - 1) ? PIPE : "");
bb.put(each);
bb.put(CRLF);
Expand Down
47 changes: 47 additions & 0 deletions src/test/manual/net/spy/memcached/bulkoperation/PipeInsertTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,53 @@ public void testLopPipeInsert() {
}
}

public void testLopPipeInsertIndex() {
int elementCount = 3;
List<Object> middleElements = new ArrayList<Object>(elementCount);
List<Object> headerElements = new ArrayList<Object>(elementCount);
List<Object> footerElements = new ArrayList<Object>(elementCount);

for (int i = 0; i < elementCount; i++) {
middleElements.add("middleValue" + i);
headerElements.add("headerValue" + i);
footerElements.add("footerValue" + i);
}

try {
CollectionAttributes attr = new CollectionAttributes();
mc.asyncLopInsert(KEY, 0, "FirstValue", attr).get(5000L, TimeUnit.MILLISECONDS);
mc.asyncLopInsert(KEY, -1, "LastValue", attr).get(5000L, TimeUnit.MILLISECONDS);

CollectionFuture<Map<Integer, CollectionOperationStatus>> future1 =
mc.asyncLopPipedInsertBulk(KEY, 1, middleElements, attr);
CollectionFuture<Map<Integer, CollectionOperationStatus>> future2 =
mc.asyncLopPipedInsertBulk(KEY, 0, headerElements, attr);
CollectionFuture<Map<Integer, CollectionOperationStatus>> future3 =
mc.asyncLopPipedInsertBulk(KEY, -1, footerElements, attr);

Copy link
Collaborator

Choose a reason for hiding this comment

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

empty line 1개 제거합니다.

Map<Integer, CollectionOperationStatus> map1
= future1.get(5000L, TimeUnit.MILLISECONDS);
Map<Integer, CollectionOperationStatus> map2
= future2.get(5000L, TimeUnit.MILLISECONDS);
Map<Integer, CollectionOperationStatus> map3
= future3.get(5000L, TimeUnit.MILLISECONDS);
Assert.assertEquals(map1.size() + map2.size() + map3.size(), 0);

List<Object> list = mc.asyncLopGet(KEY, 0, 9999, false, false).get();
Assert.assertEquals((elementCount * 3) + 2, list.size());

int offset = 0;
Assert.assertEquals(headerElements, list.subList(offset, offset + elementCount));
offset += (elementCount + 1);
Assert.assertEquals(middleElements, list.subList(offset, offset + elementCount));
offset += (elementCount + 1);
Assert.assertEquals(footerElements, list.subList(offset, offset + elementCount));
} catch (Exception e) {
e.printStackTrace();
Assert.fail(e.getMessage());
}
}

public void testMopPipeInsert() {
int elementCount = 5000;

Expand Down
12 changes: 6 additions & 6 deletions src/test/manual/net/spy/memcached/emptycollection/PipedBulkInsertListWithAttrTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public void testInsertWithAttribute() {
attr.setMaxCount(3333);

List<Object> valueList = new ArrayList<Object>();
for (int i = 1; i < 11; i++) {
for (int i = 1; i <= 10; i++) {
valueList.add(i);
}

Expand All @@ -74,7 +74,7 @@ public void testInsertWithAttribute() {
// check values
List<Object> list2 = mc.asyncLopGet(KEY, 0, 10, false, false).get();
for (int i = 0; i < list2.size(); i++) {
Assert.assertEquals(10 - i, list2.get(i));
Copy link
Collaborator

@jhpark816 jhpark816 Jun 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

기존 테스트를 보니, 순서가 반대인 것을 인지하고 있었던 것 같습니다.
그런데, 이상하다고 생각하지 않았던 이유는 무엇인지?

c client는 어떤가요?

Copy link
Collaborator Author

@brido4125 brido4125 Jun 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

기존 테스트를 보니, 순서가 반대인 것을 인지하고 있었던 것 같습니다.
그런데, 이상하다고 생각하지 않았던 이유는 무엇인지?

죄송합니다. 해당 PR을 올리기 전에는 위 테스트를 확인하지 않았습니다. 다음부터 테스트 코드도 신경쓰겠습니다.

c client는 어떤가요?

C 클라이언트는 구조가 파악이 아직 안되어, 시간이 좀 소요될것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

c client는 어떤가요?

https://github.com/naver/arcus-c-client/blob/master/docs/04-list-API.md#list-element-일괄-삽입

c client는 element의 개수만큼 index를 받고 있습니다. 따라서 사용자가 입력되는 순서를 결정할 수 있습니다.

/* [A, E]에 [B, C, D]를 insert하는 경우 */

// indexes = {1, 1, 1}
[A,E] -> [A,D,C,B,E]

// indexes = {1, 2, 3}
[A,E] -> [A,B,C,D,E]

Assert.assertEquals(i + 1, list2.get(i));
}

// check expire time
Expand All @@ -96,7 +96,7 @@ public void testInsertWithDefaultAttribute() {
CollectionAttributes attr = new CollectionAttributes();

List<Object> valueList = new ArrayList<Object>();
for (int i = 1; i < 11; i++) {
for (int i = 1; i <= 10; i++) {
valueList.add(i);
}

Expand All @@ -114,7 +114,7 @@ public void testInsertWithDefaultAttribute() {
// check values
List<Object> list2 = mc.asyncLopGet(KEY, 0, 10, false, false).get();
for (int i = 0; i < list2.size(); i++) {
Assert.assertEquals(10 - i, list2.get(i));
Assert.assertEquals(i + 1, list2.get(i));
}
} catch (Exception e) {
e.printStackTrace();
Expand All @@ -128,7 +128,7 @@ public void testInsertWithoutAttributeCreate() {
Assert.assertNull(mc.asyncGetAttr(KEY).get());

List<Object> valueList = new ArrayList<Object>();
for (int i = 1; i < 11; i++) {
for (int i = 1; i <= 10; i++) {
valueList.add(i);
}

Expand All @@ -147,7 +147,7 @@ public void testInsertWithoutAttributeCreate() {
// check values
List<Object> list2 = mc.asyncLopGet(KEY, 0, 10, false, false).get();
for (int i = 0; i < list2.size(); i++) {
Assert.assertEquals(10 - i, list2.get(i));
Assert.assertEquals(i + 1, list2.get(i));
}
} catch (Exception e) {
e.printStackTrace();
Expand Down
Loading