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

118 fea 조회 성능 최적화 및 인기 게더링 조회 #126

Merged

Conversation

kcsc2217
Copy link
Collaborator

@kcsc2217 kcsc2217 commented Dec 30, 2024

#️⃣연관된 이슈

ex) #이슈번호, #이슈번호

📝작업 내용

포트폴리오 전체 조회 같은 경우 첫 페이지에 유저가 몰릴 가능성이 높을 뿐만 아니라 첫 페이지는 where문에 걸리는게 없어 현재full scan으로 데이터를 가져오고 있다.
그리하여 첫 페이지는 Redis ZSet을 이용하여 캐시 작업을 하였다

@Cacheable를 사용하면 편리한데 Redis ZSET을 사용한 이유는 새로운 개더링, 포폴이 들어오실 디비와의 정합성을 맞추기 위해서 매번 @CacheEvit 작업을 하여 key를 삭제하면 다시 DB에서 재조회를 해야 하기 때문에 Redis 캐시를 사용할 이유가 없다고 판단하여 DB를 최소화로 타기 위해 ZET에 Score를 create_date로 두었다. (시간 순 역순으로 정렬 중)

https://velog.io/@kcsc2217/Cacheable-%EB%B3%B4%EB%8B%A4-%EB%8D%94-%EC%A2%8B%EC%9D%80-%EB%B0%A9%EC%95%88

게더링, 포폴 인덱스 작업 -> Work bench 에서 직접할 예정

포트폴리오

스크린샷 (선택)

💬리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?

@kcsc2217 kcsc2217 linked an issue Dec 30, 2024 that may be closed by this pull request
2 tasks
Copy link

github-actions bot commented Dec 30, 2024

Test Results

89 tests   89 ✅  17s ⏱️
18 suites   0 💤
18 files     0 ❌

Results for commit 0f5c312.

♻️ This comment has been updated with latest results.

@kcsc2217 kcsc2217 self-assigned this Dec 30, 2024
@kcsc2217 kcsc2217 added 📌 feature New feature added ⛏ fix Bug were fixed labels Dec 30, 2024
Copy link
Collaborator

@AnTaeho AnTaeho left a comment

Choose a reason for hiding this comment

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

전체적으로 좋은 것 같긴한데, 몇가지 의견을 남겨보자면

우선 수많은 이벤트들이 있는데 굳이 이벤트 처리할 필요는 없을 것 같다는 느낌이 듭니다.
이벤트 처리를 통해서 비동기로 처리하는 것이 아니라 단순한 로직을 하나 실행 시키는 느낌이기때문에
zset을 관리하는 클래스를 만들어서 메서드 호출로 관리를 해도 될 것 같다는 느낌

두번째로 첫페이지 캐싱같은 경우 어느정도의 정합성이 깨지더라도 업데이트 주기를 짧게 가져가서 정합성을 높히는 방식이 리소스 측면에서 더 좋지 않을까하는 생각이 듭니다.
ZSet을 사용하면 말한대로 순서 관리랑 score를 통해서 검색도 할 수 있지만 어차피 List 자료형을 사용하면 순서는 순서대로 보장이 되고 주기를 짧게 한 1분마다로 둬도 첫페이지를 캐싱하는 속도가 빠르니까 정합성이 아주 박살날 것 같지는 않습니다

그럼 만약 새로운 게시글이 너무 빠르게 추가되면 어떡하냐 라고 생각할 수도 있는데 캐싱이라는 것 자체가 자주 조회되고 변화가 적은 데이터에 대해서 효율적이기 때문에 그런 경우엔 캐싱을 배제하는 것이 더 맞는 접근방식이 아닐까 싶습니다.

의견을 종합하자면 구현 방식도 좋고 열심히 잘 작성한 코드 같지만 전체적으로 코드가 복잡해지고 관리해야하는 데이터 리소스가 많기 때문에 성능적인 면에서도 효율이 조금 떨어지지 않을까 생각됩니다.
데이터를 캐싱하고 최신 데이터 정합성을 맞추려는 접근 자체는 매우 좋다고 생각되지만 그렇게 하기 위해 너무 많은 코드와 자원이 필요하지 않나 생각합니다!

Comment on lines 22 to 23
redisTemplate.delete(zSetKey);
redisTemplate.delete(zSetPfKey);
Copy link
Collaborator

Choose a reason for hiding this comment

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

키 값을 공용으로 사용한다면 아예 상수를 모아두는 클래스를 만들어서 스태틱 상수로 관리하는 것이 좋아보임!
RedisConse.Z_SET_KEY , RedisConse.Z_SET_PF_KEY 이런식으로

long epochMilli = timeStamp.atZone(ZoneId.systemDefault()).toInstant().toEpochMilli();
int nanos = timeStamp.getNano();
return epochMilli + (nanos / 1000000.0);
return ((Long) timeStamp.atZone(ZoneId.systemDefault()).toInstant().toEpochMilli()).doubleValue();
Copy link
Collaborator

Choose a reason for hiding this comment

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

어차피 시간 순으로 저장되는 거면 System.currentTimeMillis() 이걸 쓰는거도 좋아보임!
정확한 시점이 중요하다기 보단 순서만 보장되면 되니께

) {

public static PortFolioResponse toDto(PortFolio portFolio){

List<String> relationUrl= checkRelationUrl(portFolio);

return new PortFolioResponse(portFolio.getPortfolioId(),portFolio.getUser().getId(), portFolio.getUser().getJobTitle(), portFolio.getUrl(),portFolio.getUser().getName() , portFolio.getUser().getBriefIntro(), portFolio.getUser().getMajorJobGroup().name(), portFolio.getUser().getMinorJobGroup().name(), portFolio.getUser().getImageUrl(),relationUrl);
return new PortFolioResponse(portFolio.getPortfolioId(),portFolio.getUser().getId(), portFolio.getUser().getJobTitle(), portFolio.getUrl(),portFolio.getUser().getName() , portFolio.getUser().getBriefIntro(), portFolio.getUser().getMajorJobGroup().name(), portFolio.getUser().getMinorJobGroup().name(), portFolio.getUser().getImageUrl(),relationUrl, portFolio.getCreateAt());
Copy link
Collaborator

Choose a reason for hiding this comment

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

파라미터가 많을땐 줄바꿈을 해주면 가독성이 더 좋아질 듯 합니다~

Copy link
Collaborator

@Leehunil Leehunil left a comment

Choose a reason for hiding this comment

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

제가 말한 부분만 변경되면 머지해도 될 것 같아요!

Set<String> keys = redisTemplate.keys(RedisConstKey_Gathering);
if(!keys.isEmpty()){
Gathering gathering = gatheringService.getGathering(gatheringId);

Copy link
Collaborator

Choose a reason for hiding this comment

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

마지막 데이터 빼고 새로운 데이터를 ZSet에 넣는 로직인 것 같아요!! 다만 Redis 안에 데이터수가 pageable size 보다 적을 때에는 마지막 데이터를 삭제하면 안될 것 같아요!

Copy link

sonarqubecloud bot commented Jan 8, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
66.2% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@kcsc2217 kcsc2217 merged commit c14e633 into develop Jan 8, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📌 feature New feature added ⛏ fix Bug were fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 조회 성능 최적화 및 인기 게더링 조회
3 participants