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.

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

DOC: Cleanup doc smget documentation #726

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

namsic
Copy link
Collaborator

@namsic namsic commented Dec 20, 2023

  • jam2in/arcus-works#472

bop smget 문서를 정리합니다.
https://github.com/namsic/arcus-memcached/blob/works%23472/doc-smget/docs/ascii-protocol/ch08-command-btree-collection.md#bop-smget

  • old 인터페이스 관련 내용 제거
  • 구조 및 표현 전반적으로 재작성

Copy link
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.

리뷰 완료입니다.

docs/ascii-protocol/ch08-command-btree-collection.md Outdated Show resolved Hide resolved
docs/ascii-protocol/ch08-command-btree-collection.md Outdated Show resolved Hide resolved
- trim되지 않은 마지막 bkey 정보를 반환
- `END` or `DUPLICATED`: response의 마지막을 의미
- `END`: 조회 결과에 중복 bkey 존재하지 않음
- `DUPLICATED`: 조회 결과에 중복 bkey 존재
Copy link
Collaborator

Choose a reason for hiding this comment

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

unique 속성 여부에 상관없이 중복이면 DUPLICATED가 반환되는 건가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unique 지정된 경우는 조회 결과에 중복이 없으므로 항상 END입니다.
아래 테스트를 참고 바랍니다.

중복 bkey

bop insert bt01 1 5 create 0 0 100
arcus
CREATED_STORED
bop insert bt02 1 5 create 0 0 100
arcus
CREATED_STORED

smget unique

bop smget 9 2 0..10 5 unique
bt01 bt02
ELEMENTS 1
bt01 0 1 5 arcus
MISSED_KEYS 0
TRIMMED_KEYS 0
END

smget duplicated

bop smget 9 2 0..10 5 duplicate
bt01 bt02
ELEMENTS 2
bt01 0 1 5 arcus
bt02 0 1 5 arcus
MISSED_KEYS 0
TRIMMED_KEYS 0
DUPLICATED

### Response
#### 성공
```
ELEMENTS <ecount>
Copy link
Collaborator

Choose a reason for hiding this comment

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

라인 끝에 \r\n 안붙여도 되나요? 다른 Response Message에는 붙이는 것 같아서요..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

클라이언트를 개발하거나 문서를 확인하는 입장에서는 \r\n 개행문자를 명시적으로 포함하는 편이 더 유용할까요?
아래와 같은 이유로 제거하면 어떨까 생각했는데, 의견 주시면 좋겠습니다.

  • redis의 COMMAND 문서를 보니 각 API에서 별도로 개행 문자에 대한 표기는 하지 않는 것으로 보임
    (redis에서 multi-line 명령에 대한 문서는 찾지 못함)

  • arcus-memcached에서, single-line command는 \r\n\n 모두 처리가 가능
    (예를 들어, get key1\r\nget key1\n이 모두 동작함)

  • 개인적으로, 개행 문자를 표시하고 개행을 또 하는 것이 이상한 것 같음
    읽기는 힘들지만 set k1 0 0 5\r\narcus\r\n처럼 하는 것이 맞지 않는지

    set k1 0 0 5\r\n
    arcus\r\n
    
  • 모든 API에 적용되는 제약인데, 별도의 문서에서 한 번만 언급하고 개별 API에서는 생략하는 편이 간결하지 않은지

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 \r\n은 안붙이는게 좋아보입니다. 추후에 \r\n 자체를 아예 제거해주신다면 상관없을 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • memcached protocol 문서 참고 - https://github.com/memcached/memcached/blob/master/doc/protocol.txt
  • \r\n 문자를 추가합시다. client 개발 시에 이 정보까지 알고 있는 것이 낫습니다.
  • 개행 문자를 표시하고 개행을 하는 것은 읽기 쉽게 하기 위한 것이며, 혼돈되지 않을 것입니다.
  • 별도의 문서에 한번만 언급하는 방식도 가능하지만, 직접 명시하는 것도 직관적이라 생각해요.

| - | - |
| `TYPE_MISMATCH` | 일부 key가 B+Tree type이 아님 |
| `BKEY_MISMATCH` | B+Tree들의 bkey 유형이 서로 다름 |
| `NOT_SUPPORTED`| 지원하지 않음 |
Copy link
Collaborator

Choose a reason for hiding this comment

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

memcached 버전이 낮을 경우에 발생하는 건가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

엔진이 필요한 API를 지원하지 않는 경우입니다.
간단한 예로, default_engine이 아닌 demo_engine을 사용하면 smget이 구현되어 있지 않아 NOT_SUPPORTED 응답을 받게 됩니다.

기존 문서를 그대로 옮겨온 부분인데, 외부 사용자가 보기에 이해하기 어려울까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

부연 설명이 없으면 오해할 수 있을 것 같습니다.

| `NOT_SUPPORTED`| 지원하지 않음 |
| `CLIENT_ERROR bad command line format` | 잘못된 protocol syntax |
| `CLIENT_ERROR bad data chunk` | 중복 key가 존재하거나 `<space separated keys>` 길이가 `<lenkeys>`와 다름 |
| `CLIENT_ERROR bad value` | smget 연산의 제약 조건을 위배 |

This comment was marked as outdated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

현재 develop 기준으로 bad value 발생할 것입니다.

bop smget 14 3 0..100 0 duplicate
CLIENT_ERROR bad value

Copy link
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.

일부 리뷰한 상태이며,

설명 양식이 변경된 상태라 이에 대해 검토 후에 다시 코멘트 하겠습니다.

docs/ascii-protocol/ch08-command-btree-collection.md Outdated Show resolved Hide resolved
docs/ascii-protocol/ch08-command-btree-collection.md Outdated Show resolved Hide resolved
@namsic namsic force-pushed the works#472/doc-smget branch from 9823a0d to 06d9cbf Compare December 20, 2023 07:55
@namsic
Copy link
Collaborator Author

namsic commented Dec 20, 2023

일부 리뷰가 반영되었습니다.

@jhpark816 설명 양식이 변경된 상태라 이에 대해 검토 후에 다시 코멘트 하겠습니다.

문서 포맷과 관련된 부분은 필요한 경우 추후 별도 PR으로 검토하는 것으로 하고
현재 PR은 old interface 제거만 하는 형태로 변경하겠습니다.

@namsic namsic force-pushed the works#472/doc-smget branch from 06d9cbf to afa0b40 Compare December 20, 2023 08:51
Copy link
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.

리뷰 완료

- [duplicate|unique] - smget 동작 방식을 지정한다.
- 생략되면, 예전 smget 동작을 수행한다.
- 지정되면, 신규 smget 동작을 수행한다. duplicate는 중복 bkey를 허용하고, unique는 중복 bkey를 제거한다.
- [duplicate|unique] - duplicate는 중복 bkey를 허용하고, unique는 중복 bkey를 제거한다.
Copy link
Collaborator

Choose a reason for hiding this comment

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

new smget에서 [duplicate|unique]는 생략 가능한 것으로 표현되어 있습니다.
생략이 불가능할 것으로 생각하는 데, 이에 대해 확인 바랍니다.

missed keys에 대한 DB 조회가 offset으로 skip된 element를 가지는 경우,
응용에서 정확한 offset 처리가 불가능해지기 때문이다.
- smget 조회 조건을 만족하는 첫번째 element가 존재하는 b+tree를 missed keys로 분류한다.
따라서, 응용에서는 missed keys에 한해 백엔드 저장소인 DB에서 elements를 조회하여 최종 smget 결과에 반영할 수 있다.
Copy link
Collaborator

Choose a reason for hiding this comment

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

아래 내용으로 변경 검토 바랍니다.


  • smget 조회를 수행할 수 없는 아래 상태의 b+tree key들을 missed keys로 분류하고 이들의 elements는 smget 결과에 반영할 수 없다. 따라서, 응용에서는 missed keys에 대해 백엔드 저장소인 DB에서 elements를 조회하여 최종 smget 결과에 반영해야 한다.
    • 캐시에 존재하지 않는 key
    • 조회가 현재 허용되지 않는 key (UNREADABLE 상태)
    • 조회 조건을 만족하는 첫 번째 element가 trim되어 있는 key (OUT_OF_RANGE 상태) -

- smget 조회 조건을 만족하는 첫번째 element가 존재하는 b+tree를 missed keys로 분류한다.
따라서, 응용에서는 missed keys에 한해 백엔드 저장소인 DB에서 elements를 조회하여 최종 smget 결과에 반영할 수 있다.
- smget 조회 조건을 만족하는 두번째 이후의 element가 trim된 b+tree가 존재하면 그러한 b+tree를 trimmed keys로 분류하고 원하는 개수의 elements를 찾을 때까지 smget을 계속 진행한다.
따라서, 응용에서는 trimmed keys에 한하여 백엔드 저장소인 DB에서 trim된 elements를 조회하여 최종 smget 결과에 반영할 수 있다.
Copy link
Collaborator

Choose a reason for hiding this comment

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

아래 내용으로 변경 검토 바랍니다.


  • smget 조회 조건을 만족하는 elements를 조회하였지만 b+tree trim으로 추가의 element 조회가 불가능한 key를 trimmed keys로 분류해 두고, 해당 개수의 elements를 모두 찾을 떄까지 smget을 계속 수행한다. 따라서, 응용에서는 trimmed keys에 대하여 백엔드 저장소인 DB에서 trim된 elements를 조회하여 최종 smget 결과에 반영해야 한다.

@namsic namsic force-pushed the works#472/doc-smget branch 2 times, most recently from 27b24bd to 708a0a7 Compare February 5, 2024 03:25
@namsic
Copy link
Collaborator Author

namsic commented Feb 5, 2024

@jhpark816 리뷰 반영되었습니다.

Copy link
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.

리뷰 완료

- 캐시에 존재하지 않는 key
- 조회가 현재 허용되지 않는 key (`UNREADABLE` 상태)
- 조회 조건을 만족하는 첫 번째 element가 trim되어 있는 key (`OUT_OF_RANGE` 상태)
- smget 조회 조건을 만족하는 elements를 조회하였지만 b+tree trim으로 추가의 element 조회가 불가능한 key를 trimmed keys로 분류해 두고, 해당 개수의 elements를 모두 찾을 떄까지 smget을 계속 수행한다.
Copy link
Collaborator

Choose a reason for hiding this comment

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

다시 읽어 보니 어색하여,
"trimmed keys로 분류해 두고, 해당 개수의 elements를 모두 찾을 떄까지 smget을 계속 수행한다."
이 부분을 "trimmed keys로 분류한다."로 변경하면 좋겠습니다.

이전의 조회 결과에 이어서 추가로 조회하고자 하는 경우,
이전에 조회된 bkey 값을 바탕으로 bkey range를 재조정하여 사용할 수 있다.

```
bop smget <lenkeys> <numkeys> <bkey or "bkey range"> [<eflag_filter>] <count> [duplicate|unique]\r\n
bop smget <lenkeys> <numkeys> <bkey or "bkey range"> [<eflag_filter>] <count> <duplicate|unique>\r\n
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기에서 <duplicate|unique> 대신에 duplicate|unique로 표현해야 할 것 같습니다.

@namsic namsic force-pushed the works#472/doc-smget branch from 708a0a7 to a59f405 Compare February 5, 2024 06:34
@jhpark816 jhpark816 merged commit 416cac9 into naver:develop Feb 5, 2024
1 check passed
@namsic namsic deleted the works#472/doc-smget branch February 5, 2024 06:44
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