Skip to content

캐싱을 통한 조회 속도 향상#48

Open
seonghoo1217 wants to merge 14 commits into
devfrom
feat/#46
Open

캐싱을 통한 조회 속도 향상#48
seonghoo1217 wants to merge 14 commits into
devfrom
feat/#46

Conversation

@seonghoo1217

Copy link
Copy Markdown
Member

Closes #46

상세 내용

  • 캐싱을 이용하여 조회 속도를 향상시켰습니다.

비고

  • 회원쪽 로직은 캐싱을 처리하지 않는게 맞다 생각하여 제외하였습니다.

@seonghoo1217 seonghoo1217 self-assigned this Dec 7, 2023
@Gongmeda

Gongmeda commented Dec 7, 2023

Copy link
Copy Markdown
Member

회원쪽 로직에 캐싱이 필요없다고 생각한 이유가 뭔가요?

@Gongmeda Gongmeda self-requested a review December 7, 2023 13:31
@seonghoo1217

Copy link
Copy Markdown
Member Author

우선 캐싱의 목적에 대해 고려를 해야한다고 생각합니다.

  1. 자주 호출되는 API인가?
  2. 조회속도향상을 고려할 필요가 있는가?

이 두가지 논점을 고려하였을때 로그인을 위한 캐싱처리는 목적에 맞지 않는다고 생각하며, existsByEmail() 메서드를 이용하여 이미 존재하는 이메일인지 판단하는 로직 또한 마찬가지의 이유라고 생각합니다.

부가적으로 로컬 캐시 환경을 사용하는 현재시점에 별도의 캐시스토리지를 쓰지않는 이상 민감한 정보인 email이나 password가 저장되는게 부담이라고 생각됩니다.

하지만 자신조회 API의 경우 memberId에 대해서 캐싱처리하는게 좋을까요?

@Gongmeda

Copy link
Copy Markdown
Member

우선 캐싱의 목적에 대해 고려를 해야한다고 생각합니다.

  1. 자주 호출되는 API인가?
  2. 조회속도향상을 고려할 필요가 있는가?

이 두가지 논점을 고려하였을때 로그인을 위한 캐싱처리는 목적에 맞지 않는다고 생각하며, existsByEmail() 메서드를 이용하여 이미 존재하는 이메일인지 판단하는 로직 또한 마찬가지의 이유라고 생각합니다.

부가적으로 로컬 캐시 환경을 사용하는 현재시점에 별도의 캐시스토리지를 쓰지않는 이상 민감한 정보인 email이나 password가 저장되는게 부담이라고 생각됩니다.

하지만 자신조회 API의 경우 memberId에 대해서 캐싱처리하는게 좋을까요?

넵. GET /members/me API에만 캐싱 추가하면 될 것 같아요.

- 회원쪽 API중 캐싱 대상이 될 유일한 API이기에 처리
@seonghoo1217

Copy link
Copy Markdown
Member Author

추가 처리하였습니다. Approve 부탁드립니다

Gongmeda
Gongmeda previously approved these changes Dec 10, 2023

@Gongmeda Gongmeda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

통합 테스트 코드가 깨지는데, 이 부분도 확인해주세요

Comment thread src/main/java/ddip/me/ddipbe/application/EventCommandService.java Outdated
public CacheManager cacheManager() {
ConcurrentMapCacheManager cacheManager = new ConcurrentMapCacheManager();
cacheManager.setAllowNullValues(false);
cacheManager.setCacheNames(List.of("events", "members", "successRecords"));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

캐시명을 여기에 명시적으로 기입하는 이유가 뭔가요?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

SimpleCacheManager를 만들어 처리하여도 되지만, 명시적으로 캐싱 대상을 기입하는 것이 좋다고 생각하였습니다

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

그게 좋은 이유가 뭔가요?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ConcurrentMapCacheManagerSimpleCacheManager의 차이점이 뭔가요? SimpleCacheManager를 고려하는 이유가 뭔지 궁금합니다.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

추가로, cacheManager.setCacheNames 를 적용했을 때의 효과와 이점이 뭔가요?

}

@GetMapping("me")
@Cacheable(value ="events" ,key = "#memberId + #pageReq.page() + #pageReq.size() + (#open == null ? false : true ).toString()")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

페이징 쿼리에 포함되어 있는 이벤트의 정보가 수정되었을 때 어떻게 되는지 확인해보셨나요?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

페이징 단위로 캐싱이 새로되는 점에서 오히려 조회 속도에 이점이 없네요.
Open 상태와만 조합하여 사용하는 로직으로 변경하였습니다

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

캐싱이 새로 되는게 문제가 아니라 데이터 변경이 안되는게 문제 아닌가요?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

아니요 오히려 memberId로만 Key를 설정할경우 데이터 변경이 일어나지않습니다.

  • key값이 유일하게 memberId 만 걸려 있을 경우 동일 데이터를 캐시에서 반환하기에 페이징을 조합하여 캐싱을 구성하였습니다

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

말씀드렸다시피, '페이지에 대한 캐시 Evict을 어떻게 구분해서 하냐'의 문제입니다

}

@DeleteMapping("{uuid}")
@Cacheable(value = "events", key = "#uuid")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

DELETE API에 캐싱 처리를 한 이유가 궁금합니다

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

오히려 기존에 캐싱된 이벤트의 캐시를 날려야 하는게 아닌가요?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Evict으로 수정하였습니다

}

@PostMapping("{uuid}/form")
@Cacheable(value = "successRecords", key = "#uuid + #token")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

POST API에도 캐싱처리를 한 이유가 궁금합니다

}

@GetMapping("{uuid}/success")
@Cacheable(value = "events", key = "#uuid")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/{uuid} API랑 캐시키가 겹쳐서 응답이 이상해지는 문제가 있습니다

- 페이징 API의 경우 캐싱시 최신 데이터 반영이 되지않음
- 이를 개선 하고자 수정시 동기화 로직등을 고려하였으나 오버헤드 우려
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

조회 API Caching을 통한 최적화

2 participants