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

[BE] fix: 잘못된 캐시 키 정정 #1003

Merged
merged 3 commits into from
Dec 18, 2024
Merged

Conversation

nayonsoso
Copy link
Contributor

@nayonsoso nayonsoso commented Dec 13, 2024


🚀 어떤 기능을 구현했나요 ?

  • 캐시가 잘못되는 부분을 수정했습니다.

🔥 어떻게 해결했나요 ?

  • AS-IS : templateId를 기준으로 리뷰 그룹에 대한 정보를 포함한 응답을 캐싱함
    templateId가 대부분 1로 통일되어있는 지금, 리뷰 그룹이 다르더라도 캐싱된 리뷰 그룹 정보가 포함되어 응답됨
  • TO-BE : reviewGroupId 를 캐시의 키로 설정

📝 어떤 부분에 집중해서 리뷰해야 할까요?

  • 또 놓치는 부분이 있을까요?

📚 참고 자료, 할 말

Copy link

github-actions bot commented Dec 13, 2024

Test Results

157 tests   154 ✅  5s ⏱️
 58 suites    3 💤
 58 files      0 ❌

Results for commit d9386a1.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@skylar1220 skylar1220 left a comment

Choose a reason for hiding this comment

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

캐시 작업자로서 시말서 같이 올립니다🥲 TemplateReponse가 reviewGroup까지 포함하고 있는걸 놓쳤네요..

이렇게 하면 캐싱이 효과가 우리가 예상한 것과는 달라서 일단 긴급 처치라고 볼 수 있겠네요.

  • 같은 템플릿이면 모든 사용자가 캐싱된 템플릿을 불러오기를 기대
  • but, 현재는 같은 리뷰 그룹인 사용자들만 템플릿 캐싱이 됨

테드의 template 리팩토링 pr이 끝나면 그것과 함께 수정해야겠습니다..!

Copy link
Contributor

@donghoony donghoony left a comment

Choose a reason for hiding this comment

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

우선 빠르게 문제 해결을 위해 Approve합니다만, 보다 나은 캐시 적중률을 위해서는 SectionResponse 단위로 캐싱하는 것이 하나의 방법이 될 수 있습니다.

문제가 되는 상황은 TemplateResponse에 ReviewGroup 정보가 들어가기 때문이니,

  • 산초가 작성한 것처럼 templateId가 아닌 reviewGroupId로 캐싱한다 (이 경우 적중률이 떨어지며, 서로 다른 그룹에 대해서 모든 작업을 매번 진행함.)
  • SectionResponse에 대해서 캐싱
  • TemplateResponse에서 ReviewGroup 관련 내용을 삭제하고, 프론트에서 다른 방식으로 그룹 정보를 가져오도록 수정 (이미 reviewGroup API가 존재하니깐요)

과 같은 방법이 떠오르네요. 추후에 잘 바꿔봅시다

Copy link
Contributor

@Kimprodp Kimprodp left a comment

Choose a reason for hiding this comment

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

너무 임시방편 아닌가요?
구현이 단순하니 당장은 사용하지 않고, 다음에 다시 잘 확인해서 적용하는 편이 좋겠어요.

@nayonsoso
Copy link
Contributor Author

@skylar1220 @donghoony @Kimprodp

이렇게 하면 캐싱이 효과가 우리가 예상한 것과는 달라서 일단 긴급 처치라고 볼 수 있겠네요.

산초가 작성한 것처럼 templateId가 아닌 reviewGroupId로 캐싱한다 (이 경우 적중률이 떨어지며, 서로 다른 그룹에 대해서 모든 작업을 매번 진행함.)

너무 임시방편 아닌가요?

맞습니다. 임시방편이고, 적중률이 떨어집니다.

하지만 이걸 빠르게 수정하여 develop 브랜치에 머지하지 않으면,
develop cd 를 돌렸을 때 템플릿 id로 캐싱되는 문제가 또다시 발생합니다.
그리고 이 말은, release 브랜치에 지금의 develop을 머지하면 prod 서버에도 동일한 문제가 발생한다는 뜻입니다.

이 PR은 버그를 빠르게 수정하는 것을 목표로 하는 PR 입니다.
따라서 캐싱 적중률을 높이기 위한 조치는 다른 PR에서 해도 된다 생각합니다.

@donghoony
Copy link
Contributor

@nayonsoso 테드가 이야기하는 건 캐시 도입을 하지 않고, 나중에 다시 신중하게 시도하라는 의도로 보입니다~

@nayonsoso nayonsoso merged commit 788dc65 into develop Dec 18, 2024
4 checks passed
@donghoony donghoony deleted the be/hotfix/cache-key branch December 18, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants