naver / arcus-c-client

ARCUS C client
https://github.com/naver/arcus
Apache License 2.0
13 stars 16 forks source link

memcached_get_by_key() 함수 마지막에 dummy fetch 필요성 검토. #155

Open jhpark816 opened 3 years ago

jhpark816 commented 3 years ago

memcached_get_by_key() 함수의 마지막에 아래와 같은 dummy fetch 코드가 있다. 필요성을 검토해 보고, 필요가 없다면 제거합시다.

  size_t dummy_length;
  uint32_t dummy_flags;
  memcached_return_t dummy_error;

  char *dummy_value= memcached_fetch(ptr, NULL, NULL,
                                     &dummy_length, &dummy_flags,
                                     &dummy_error);
  assert_msg(dummy_value == 0, "memcached_fetch() returned additional values beyond the single get it expected");
  assert_msg(dummy_length == 0, "memcached_fetch() returned additional values beyond the single get it expected");
  assert_msg(ptr->query_id >= query_id +1, "Programmer error, the query_id was not incremented.");
  //assert_msg(ptr->query_id == query_id +1, "Programmer error, the query_id was not incremented.");
jhpark816 commented 2 years ago

dummy fetch 기능이 불필요하여 제거하였습니다.

jhpark816 commented 2 years ago

dummy fetch 제거하면 unit test 실패합니다. 이에 관한 @uhm0311 코멘트는 아래와 같습니다.


commit: https://github.com/naver/arcus-c-client/commit/7b5cf920f1efea242ca5e7ab684a6b739043bedd 해당 커밋으로 유닛 테스트 실패가 생겼습니다.

tests/mem_functions.cc:4309: Assertion "no_msg == 0" failed, in noreply_test
    Testing noreply                 [ failed ]

해당 테스트 코드는 다음과 같습니다.

int no_msg=0;
for (uint32_t x= 0; x < memcached_server_count(memc); ++x)
{
  memcached_server_instance_st instance=
    memcached_server_instance_by_position(memc, x);
  no_msg+=(int)(instance->cursor_active);
}

test_true(no_msg == 0);

아래 코드의 제거로 인해 instance->cursor_active 값이 0이 아니게 되었습니다.

char *dummy_value= memcached_fetch(ptr, NULL, NULL,
                                   &dummy_length, &dummy_flags,
                                   &dummy_error);

memcached_fetch() 함수는 다음과 같습니다.

https://github.com/naver/arcus-c-client/blob/71bcafd30601dbbc687652fff6a34aa25ca47660/libmemcached/fetch.cc#L56-L137

85번 라인에서 호출하는 memcached_fetch_result() 함수는 다음과 같습니다.

https://github.com/naver/arcus-c-client/blob/71bcafd30601dbbc687652fff6a34aa25ca47660/libmemcached/fetch.cc#L139-L248

201번 라인에서 호출하는 memcached_server_response_reset() 함수는 다음과 같습니다.

https://github.com/naver/arcus-c-client/blob/71bcafd30601dbbc687652fff6a34aa25ca47660/libmemcached/common.h#L174

따라서 memcached_fetch() 함수를 호출하는 부분은 남겨야 합니다.

jhpark816 commented 2 years ago

@uhm0311 의 추가 코멘트 입니다.


libmemcached 1.0 버전을 살펴보니, get.cc에서 dummy fetch를 제거한 뒤 문제 되는 테스트 코드를 #if 0으로 비활성화 해뒀습니다.

#if 0
    int no_msg=0;
    for (uint32_t x= 0; x < memcached_server_count(memc); ++x)
    {
      const memcached_instance_st * instance=
        memcached_server_instance_by_position(memc, x);
      no_msg+=(int)(instance->cursor_active);
    }

    test_true(no_msg == 0);
#endif

dummy fetch를 제거하는 방향을 유지하려면 같은 조치를 취해야 할 것 같습니다.