Skip to content

Conversation

@singsangssong
Copy link
Collaborator

@singsangssong singsangssong commented Aug 4, 2025

🔗 Related Issue

⌨️ What I did

memcached 서버의 gat를 java-client에도 추가합니다.
이를 통해 expire time을 재설정 가능하도록 합니다.

  • Operation 인터페이스 및 Ascii, Binary 프로토콜 구현체 추가
  • OperationFactoryGetAndTouch 추가
  • MemcachedClient에 사용자가 호출할 수 있는 gat 기능 추가
  • gat 기능을 위한 테스트 코드 추가

CI Test 실패 관련

현재 실패하고 있는 CI Test는 JDK 21, ubuntu-latest에서 Test ARCUS Client with ZK를 실패하고 있습니다.
코드에서는 AsciiClientTestgat관련 테스트를 실패하고 있습니다.
해당 테스트는 현재 도커에 릴리즈된 memcached 서버는 gat 기능이 아직 반영되지 않았기에, 테스트코드에서 gat명령을 요청해도 응답이 없는 문제입니다.

@jhpark816 jhpark816 requested review from oliviarla and uhm0311 August 4, 2025 05:25
@uhm0311

This comment was marked as resolved.

@singsangssong

This comment was marked as resolved.

@uhm0311

This comment was marked as resolved.

@oliviarla oliviarla changed the base branch from master to develop August 5, 2025 04:48
@singsangssong

This comment was marked as resolved.

@uhm0311

This comment was marked as resolved.

@oliviarla

This comment was marked as resolved.

@uhm0311

This comment was marked as resolved.

@singsangssong

This comment was marked as resolved.

@oliviarla

This comment was marked as resolved.

@jhpark816

This comment was marked as resolved.

@singsangssong

This comment was marked as resolved.

uhm0311
uhm0311 previously approved these changes Sep 9, 2025
oliviarla

This comment was marked as resolved.

oliviarla
oliviarla previously approved these changes Sep 10, 2025
@oliviarla oliviarla requested a review from jhpark816 September 10, 2025 07:26
uhm0311
uhm0311 previously approved these changes Sep 10, 2025
jhpark816

This comment was marked as resolved.

@singsangssong

This comment was marked as resolved.

@ing-eoking
Copy link
Contributor

ing-eoking commented Dec 9, 2025

Java 쪽 참고 사항

현재 클라이언트에서는 여러 키에 대한 GAT 기능은 고려하지 않고 있으며, 단일 키에 대한 GAT만 지원하면 됩니다.
기존 동작에 아래와 같은 응답 형식이 추가될 예정이라고 보시면 됩니다.

REPL_SLAVE 127.0.0.1:2181

클라이언트 쪽 변경을 최소화하면서 구현하려고 했기 때문에,
아마 handleline 함수에서 아래와 같은 로직만 추가하면 충분할 수도 있을 것 같습니다.

if (hasSwitchedOver(line)) {
      prepareSwitchover(line);
      return;
}

@uhm0311
Copy link
Collaborator

uhm0311 commented Dec 9, 2025

@ing-eoking

NOT_MY_KEY 응답도 포함이죠?

@ing-eoking
Copy link
Contributor

@uhm0311

NOT_MY_KEY 응답도 수신하고 있지만, 이는 기존 get 연산에서 반환되는 응답과 동일하므로 별도의 변화는 없다고 보시면 됩니다.

현재는 단일 키에 대해서만 gat 연산을 수행하고 있기 때문에, NOT_MY_KEY와 REPL_SLAVE/SWITCHOVER가 동시에 반환되는 경우는 없습니다.

@oliviarla
Copy link
Collaborator

@singsangssong 리뷰 반영 부탁드립니다.
궁금한 점 있으면 코멘트 달아주세요.

@singsangssong singsangssong force-pushed the task/gat branch 5 times, most recently from c9de7ca to 2eafdd5 Compare December 19, 2025 06:15
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.

최종 리뷰입니다.

@singsangssong singsangssong force-pushed the task/gat branch 2 times, most recently from e86aa2d to ee6f59b Compare December 19, 2025 06:30
oliviarla
oliviarla previously approved these changes Dec 19, 2025
@oliviarla oliviarla requested a review from jhpark816 December 19, 2025 06:31
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.

일부 리뷰

jhpark816
jhpark816 previously approved these changes Dec 19, 2025
@jhpark816 jhpark816 requested a review from uhm0311 December 19, 2025 07:31
@jhpark816
Copy link
Collaborator

@uhm0311 최종 리뷰 바랍니다.

private final String cmd;
private final Collection<String> keys;
private String currentKey = null;
protected final int exp;
Copy link
Collaborator

Choose a reason for hiding this comment

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

하위 클래스에서 직접 참조하는 부분이 이제 없으므로 private으로 선언합시다.

@jhpark816
Copy link
Collaborator

@singsangssong
Docs 내용이 빠진 것 같으니, 확인 바랍니다.

@jhpark816
Copy link
Collaborator

@oliviarla
코드만 merge 하겠습니다.

@jhpark816 jhpark816 merged commit fb8b04f into naver:develop Dec 22, 2025
2 checks passed
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.

5 participants