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

INTERNAL: Fix an overwrite issue for the big key #338

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ing-eoking
Copy link
Collaborator

@ing-eoking ing-eoking commented Jan 16, 2025

🔗 Related Issue

⌨️ What I did

  • fetch 연산에서 키가 MEMCACHED_MAX_KEY보다 클 경우, 덮어쓰기가 발생하지 않도록 방지합니다. 만약 덮어쓰기가 발생하면 이를 로그에 기록합니다.
    • OVERWRITE가 발생한 경우, 사용자는 키 길이만으로 이를 충분히 감지할 수 있을 것으로 보입니다.
  • Binary 프로토콜 관련 부분도 수정했습니다.
    • Binary 프로토콜에서는 키 길이를 알 수 있기 때문에, MEMCACHED_MAX_KEY만큼 데이터를 저장한 후 나머지 데이터는 임시 버퍼로 flush 처리했습니다.

@ing-eoking ing-eoking marked this pull request as draft January 16, 2025 01:14
@ing-eoking ing-eoking force-pushed the bigkey branch 4 times, most recently from e7b429e to 5092df5 Compare January 16, 2025 03:21
@ing-eoking ing-eoking marked this pull request as ready for review January 16, 2025 03:24
@ing-eoking
Copy link
Collaborator Author

ing-eoking commented Jan 16, 2025

@jhpark816

수정완료되었습니다.

CI 테스트가 실패하여 잠시 draft로 변경하겠습니다.

@ing-eoking ing-eoking marked this pull request as draft January 16, 2025 06:20
@ing-eoking ing-eoking marked this pull request as ready for review January 16, 2025 08:49
Copy link
Contributor

@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.

일부 리뷰

libmemcached/response.cc Show resolved Hide resolved
@jhpark816 jhpark816 requested a review from namsic January 17, 2025 04:13
@jhpark816
Copy link
Contributor

@namsic 리뷰 부탁합니다.

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