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

REFACTOR: methods in MemcachedConnection invoked with updateReplConnection. #829

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

Conversation

brido4125
Copy link
Collaborator

@brido4125 brido4125 commented Oct 8, 2024

🔗 Related Issue

https://github.com/jam2in/arcus-works/issues/598

⌨️ What I did

MemcachedConnection에 존재하는 아래 4가지 메서드들을
리팩토링 하고 위치를 ArcusReplNodeAddress로 옮겼습니다.

  • findChangedGroups
  • findAddrsOfChangedGroups
  • getAddrsFromNodes
  • getSlaveAddrsFromGroupAddrs

getAddrsFromNodes과 getSlaveAddrsFromGroupAddrs의
경우 내부적으로 사용되는 HashSet의 크기가 최대 2개입니다.
(두 메서드 모두 slave 노드들을 다루기 때문)

그래서 HashSet의 해시 테이블 부하계수를 디폴트(0.75)로 사용해도 상관 없을 것 같아
두 메서드의 구현을 좀 더 간결하게 변경하였습니다.

@brido4125 brido4125 marked this pull request as draft October 8, 2024 06:45
@brido4125 brido4125 marked this pull request as ready for review October 8, 2024 07:22
@brido4125 brido4125 force-pushed the internal/replGroups branch 2 times, most recently from 07b6ee1 to ce95a3a Compare October 8, 2024 08:30
@brido4125 brido4125 self-assigned this Oct 10, 2024
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.

그룹과 관련된 메서드들은 ArcusReplNodeAddress 클래스 대신 MemcachedReplicaGroup 클래스로 이동시키는 건 어떤가요?
ArcusReplNodeAddress는 복제 캐시 노드 각각의 정보를 담는 클래스이고, 그룹 단위로 관리하는 클래스는 MemcachedReplicaGroup이기 때문에, MemcachedReplicaGroup 클래스를 통해 변경된 그룹과 그에 해당하는 캐시 노드 주소들을 찾는 것이 자연스러운 것 같습니다.

src/main/java/net/spy/memcached/ArcusReplNodeAddress.java Outdated Show resolved Hide resolved
src/main/java/net/spy/memcached/ArcusReplNodeAddress.java Outdated Show resolved Hide resolved
@brido4125 brido4125 force-pushed the internal/replGroups branch 2 times, most recently from 40bf635 to e699a66 Compare October 14, 2024 02:50
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.

3 participants