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

FIX: Call arcus_server_check_for_update() in memcached_exist_by_key() #240 #241

Merged
merged 1 commit into from
Mar 15, 2022

Conversation

uhm0311
Copy link
Collaborator

@uhm0311 uhm0311 commented Mar 11, 2022

  • memcached_exist_by_key() 함수에서 arcus_server_check_for_update() 함수를 호출하도록 변경했습니다.
  • memcached_exist_by_key() 함수에서 기존에는 add operation을 이용해 key가 저장되어 있는지 여부를 확인했습니다. add operation 대신 getattr operation을 사용하도록 변경했습니다.
  • memcached_exist_by_key() 함수가 기존에 ascii protocol과 binary protocol을 모두 지원해주는 함수였습니다. getattr operation으로 변경하면서 binary protocol을 사용할 때도 정상적인 동작을 지원해주기 위해 binary protocol을 사용하는 getattr operation 구현을 추가했습니다.
  • FIX: Remove reading extra request message when client want to know collection attributes using binary protocol arcus-memcached#626 PR과 함께 리뷰해주시기 바랍니다.

@jhpark816
Copy link
Contributor

리뷰 완료

  • CHECK_FOR_UPDATE_IN_MEMCACHED_EXIST 태그 대신에
    add 명령 대신에 getattr 명령을 사용하도록 수정한 코드에 대해
    LIBMEMCACHED_WITH_ZK_INTEGRATION 태그로 표현하도록 합시다.
  • ascii_exist() 함수만 수정하고, binary_exist() 함수는 그대로 둡시다.
    • binrary protocol은 arcus에서 현재 제대로 지원하지 않기 때문입니다.
  • arcus_server_check_for_update() 호출하는 부분도
    LIBMEMCACHED_WITH_ZK_INTEGRATION 태그로 표현합시다.

@uhm0311 uhm0311 force-pushed the uhm0311/check_update_call_exist branch from 3132756 to 8fe7e52 Compare March 15, 2022 04:46
@uhm0311
Copy link
Collaborator Author

uhm0311 commented Mar 15, 2022

@jhpark816

리뷰 반영했습니다.

@jhpark816 jhpark816 merged commit 222d6cf into naver:develop Mar 15, 2022
@uhm0311 uhm0311 deleted the uhm0311/check_update_call_exist branch March 15, 2022 05:57
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.

2 participants