INTERNAL: Limit the count of elems get from set structure

[ing-eoking]

🔗 Related Issue

• jam2in/arcus-works#571

⌨️ What I did

sop get 연산에서 일부 element 조회 시 가능한 개수를 1000개로 제한합니다.

처음에는 core(memcached.c)에서 1000개 이하로 조회 개수를 제한하고 에러 메시지를 반환했으나,
이 경우, count가 1000을 초과하는 set에 대해 전체 조회가 불가능했습니다.
그래서 현재 PR은 count 정보를 가진 엔진에서 제한을 적용하고 있습니다.

[jhpark816]

처음에는 core(memcached.c)에서 1000개 이하로 조회 개수를 제한하고 에러 메시지를 반환했으나,
이 경우, count가 1000을 초과하는 set에 대해 전체 조회가 불가능했습니다.

위의 사항에 관한 코드를 보여주세요.

[ing-eoking]

위의 사항에 관한 코드를 보여주세요.

• 변경하려 했던 사항입니다.

bool delete = false;
bool drop_if_empty = false;
uint32_t count = 0;

if (! safe_strtoul(tokens[SOP_KEY_TOKEN+1].value, &count)) {
    print_invalid_command(c, tokens, ntokens);
    out_string(c, "CLIENT_ERROR bad command line format");
    return;
}
+ if (count > MAX_SGET_ELM_COUNT) {
+     out_string(c, "CLIENT_ERROR bad value");
+     return;
+ }
if (ntokens == 6) {
    if (strcmp(tokens[SOP_KEY_TOKEN+2].value, "delete")==0) {
        delete = true;
    } else if (strcmp(tokens[SOP_KEY_TOKEN+2].value, "drop")==0) {
        delete = true;
        drop_if_empty = true;
    } else {
        print_invalid_command(c, tokens, ntokens);
        out_string(c, "CLIENT_ERROR bad command line format");
        return;
    }
}

process_sop_get(c, key, nkey, count, delete, drop_if_empty);

이 경우, 유닛테스트 coll_sop_unittest.t 에서 전체 element를 조회하는 아래같은 로직이 실패하게 됩니다.

assert_sop_get("skey 50000", $flags, 50000, 0, 49999); /* fail */
assert_sop_get("skey 0",     $flags, 50000, 0, 49999); /* success */

[namsic]

sop get skey 50000과 sop get skey 0이 엔진 내부에서는 동일한 전체 조회 요청이라고 하더라도
클라이언트 입장에서는 전체 elem 개수를 명확히 알 수 있는 것이 아니기 때문에 두 요청에는 의미 상 차이가 있다고 볼 수 있습니다.

따라서 sop get skey 50000은 실패하고 sop get skey 0은 성공하는 것이 특별히 어색해보이지는 않는데요.
테스트를 수정하는 편이 오히려 자연스러운 방향으로 보입니다.

[ing-eoking]

@namsic

우선 수정되었습니다.

아래는 질문사항 입니다.

클라이언트 입장에서는 전체 elem 개수를 명확히 알 수 있는 것이 아니기 때문에 두 요청에는 의미 상 차이가 있다고 볼 수 있습니다.

클라이언트에서 getattr을 통해 전체 elem 개수를 알 수 있지 않나요?

현재 sop get은 max_count를 초과하는 값도 성공하게 끔 되어있습니다.
예를 들어 sop get MAXIMUM_MAX_COLL_SIZE 같은 명령어는 전체 조회로 봐도 되지 않을까요?

[namsic]

element가 추가되거나 제거되는 set item이 있을 때, 같은 명령이 어떨 때는 성공하고 어떨 때는 실패하는 것이 직관적이지 않습니다.

getattr 명령을 수행하여 elem 개수를 확인한 뒤에 그에 맞춰 전체 조회 요청을 보내야 할 이유가 없습니다.
또한 클라이언트가 확인한 elem 개수가 sop get 요청을 처리하는 시점의 elem 개수와 일치한다는 보장이 없습니다.

0값을 전체 조회로 정의하고 있는 상태에서, sop get key 999999 와 같은 요청을 허용하지 않아도 원하는 요구사항을 만족할 수 있습니다.
기존에 전체 조회 목적으로 count를 충분히 큰 수로 설정하여 요청을 보내는 경우가 있었다면, 인자를 0으로 주어 요청하도록 가이드하면 됩니다.

---

엔진 구현체의 특성이나 처리 능력에 따라 count 제약이 달라질 수 있다면 엔진 내부에서 처리하는 것이 자연스러울 수 있습니다.
하지만 smget 등 기존 구현에서도 인자를 core에서 확인하고 처리하고 있으므로 같은 방식으로 처리하는 것이 좋겠습니다.

[namsic]

특별히 변경 아이디어가 있는 것은 아니고, 참고 사항으로 개인 의견 공유합니다.

…MGET_…과 …SGET_이 붙어있다보니 스치듯 보았을 때
SGET => single get 또는 MGET => mop get과 같이 착각할 수 있어 보입니다.

하지만 MAX_BMGET_ELM_COUNT과 같은 기존 상수 이름을 볼 때,
현재 PR의 상수명이 일관성을 해치지 않는다는 점에서는 적절하고,
관련 코드를 확인할 때 문제가 생길 정도는 아닐 것입니다.

[jhpark816]

용어 변경합시다.

• MAX_SGET_ELM_COUNT => MAX_SOP_GET_COUNT

[jhpark816]

리뷰 완료

[jhpark816]

용어 변경합시다.

• MAX_SGET_ELM_COUNT => MAX_SOP_GET_COUNT

[jhpark816]

CLIENT_ERROR가 없는 다른 응답 문자열은 어떤 것이 있나요?

[ing-eoking]

Sop get의 경우 아래의 에러가 반환됩니다.

Response String	설명
"NOT_FOUND"	key miss
"NOT_FOUND_ELEMENT"	element miss (element가 존재하지 않는 상태임)
"TYPE_MISMATCH"	해당 item이 set collection이 아님
"UNREADABLE"	해당 item이 unreadable item임
"NOT_SUPPORTED"	지원하지 않음
"CLIENT_ERROR bad command line format"	protocol syntax 틀림
"SERVER_ERROR out of memory [writing get response]"	메모리 부족

[jhpark816]

테스트도 변경해야 겠지만, docs도 함께 변경합시다.

[ing-eoking]

수정되었습니다.