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

feat: 반려동물 식품 로컬 캐시 적용 #540

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

parkmuhyeun
Copy link
Member

📄 Summary

Done

  • PetFoodQueryRepositoryImpl 쿼리 불필요한 join 및 group by 제거
  • 반려동물 식품 캐싱 적용 및 테스트

반려동물 식품 캐싱 적용

반려동물 식품 같은 경우도 조회는 매우 자주 일어나지만 수정, 삽입이 거의 일어나지 않기 때문에 캐싱하기에 적합하다고 생각합니다. 그래서 적용해보았는데요.

image

캐시가 Hit 되는 경우 쿼리가 2번에서 0번

image

image

시간도 2초에서 0.01초로 개선되었습니다.
(필터링 API는 현재 String으로 받고 있는 파라미터가 있어서 데이터가 많으면 불필요한 조인 테이블이 발생해서 2초정도 걸리는데요. 이걸 Id로 받도록 고치면 애초에 2초가 아니고 0.2~5초 대로 감소하긴 합니다. 하지만 그렇게 되는경우 클라이언트쪽도 고쳐야되기 때문에 제외, 이 경우에도 0.5 -> 0.01이기 때문에 유의미)

해당 @Cacheable의 key 같은 경우 기본키 전략이 아닌 customKeyGenerator를 따로 만들어서 사용했습니다. 스프링은 SimpleKeyGenerator에 의해 Cache Key를 생성하는데요.

image image image
  • 파라미터가 없는 경우 empty
  • 1개인 경우 해당 파라미터
  • 여러개 인경우 모든 파라미터의 hashcode 값을 이용해 하나의 복합키로 만듭니다.

반려동물 식품 필터링 API에는 여러가지 파라미터들이 들어오는데요. 기본키로 생성 하는 경우 위의 설명대로 복합키로 만들어 key로 사용하기 때문에 테스트할 때 캐시를 어떻게 꺼내 사용할 방법을 찾지 못해서 결국 customGenerator를 등록해서 사용했습니다. 테스트 할 때 더 좋은 방법이 있으면 말씀해주세요!

🙋🏻 More

현재 최대값을 10000개 만료 시간을 3초로 임시로 설정해놨는데 얼마로 하는게 좋을지 의논해보면 좋을 것 같습니다~

  • maximumSize: 캐시에 포함할 수 있는 최대 엔트리 수를 지정합니다.
  • expireAfterWrite: 항목이 생성된 후 또는 해당 값을 가장 최근에 바뀐 후 특정 기간이 지나면 각 항목이 캐시에서 자동으로 제거되도록 지정합니다.

Copy link
Collaborator

@kyY00n kyY00n left a comment

Choose a reason for hiding this comment

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

잘 봤어요 ㅎㅎ customGenerator 가 단순해서 쉽게 이해했어요.
근데 성능에는 차이가 있나 궁금하긴 하네요.
제 생각 남겼으니 답글주세요!

@@ -50,7 +54,14 @@

class PetFoodQueryServiceTest extends ServiceTest {

private static final int size = 20;
private static final int SIZE = 20;
Copy link
Collaborator

Choose a reason for hiding this comment

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

중요하지 않은 변경사항은 따로 커밋하면 좋은 것 같아요!
저도 이제 연습해보고 있는건데, 히스토리를 관리하는 센스가 중요한 것 같더라구요.
나중에 읽는 사람(높은 확률로 나 자신)한테 문서보다 훨씬 좋은 설명이기 때문에..
무민도 저와 함께 중요하고 관련있는 변경사항들이 히스토리에 같이 모여있을 수 있게 노력해봅시당

Comment on lines +81 to +82
@Autowired
private CacheManager cacheManager;
Copy link
Collaborator

Choose a reason for hiding this comment

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

저라면, 이 부분은 따로 테스트를 했을 것 같아요.
무민은 캐시 속을 검증하기 위해 customGenerator를 사용했지만 관점을 달리해서.. 캐시를 사용하는 서비스만 테스트 해볼 수 있지 않을까요?

그런 관점에서 mock test를 하면.. 캐시 매니저 구현에는 의존하지 않을 것 같아요 (사실 simpleGenerator 로직에 문제가 없었던 건 아니니까...! 바꿀 필요 없어짐)

그럼 reposiroty 구현체를 스터빙할 수 있고 (mockito 사용하면) 다음처럼 테스트할 수 있어요.

  • 캐시 히트 시 Repository 호출하지 않음을 테스트
  • 캐시 미스 시 Repository 호출 테스트
  • 업데이트 시 Repository 호출 테스트

이런 식으로, 서비스 행위 테스트가 되겠죵..

단점은 .. 시간이란 비용과.. 목테스트를 최소화하는 우리 프로젝트의 별종케이스가 되는 것.. 그리고 캐시 클래스 자체의 기능은 테스트 하지 않는다는거? -> 근데 이건 스프링이 제공하는 거라 학습테스트나 다름 없을 것 같아요.
아무튼 제 생각은 그렇슴니다

무민이 질문해줘서 재밌게 읽었어요 . 감사합니다

Copy link
Collaborator

Choose a reason for hiding this comment

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

나 왜 이렇게
.. <- 이거 많이 써요?
원래 그랬나

@kyY00n
Copy link
Collaborator

kyY00n commented Sep 6, 2024

이거 왜 아직도 이렇게 있어요

@wonyongChoi05
Copy link
Collaborator

ㅋㅋㅋㅋㅋㅋㅋㅋㅋ

@parkmuhyeun
Copy link
Member Author

이거 뭐임?

@parkmuhyeun
Copy link
Member Author

ㅖ?

@kyY00n
Copy link
Collaborator

kyY00n commented Sep 6, 2024

기억은 나요?

@parkmuhyeun
Copy link
Member Author

이 사람 죽었습니다

Copy link
Collaborator

@HyeryongChoi HyeryongChoi left a comment

Choose a reason for hiding this comment

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

나도 리뷰해줄래

고생하셨어요~ 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
5 participants