-
Notifications
You must be signed in to change notification settings - Fork 2
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
The head ref may contain hidden characters: "118-fea-\uC870\uD68C-\uC131\uB2A5-\uCD5C\uC801\uD654-\uBC0F-\uC778\uAE30-\uAC8C\uB354\uB9C1-\uC870\uD68C"
Conversation
Test Results89 tests 89 ✅ 17s ⏱️ Results for commit 0f5c312. ♻️ This comment has been updated with latest results. |
There was a problem hiding this 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분마다로 둬도 첫페이지를 캐싱하는 속도가 빠르니까 정합성이 아주 박살날 것 같지는 않습니다
그럼 만약 새로운 게시글이 너무 빠르게 추가되면 어떡하냐 라고 생각할 수도 있는데 캐싱이라는 것 자체가 자주 조회되고 변화가 적은 데이터에 대해서 효율적이기 때문에 그런 경우엔 캐싱을 배제하는 것이 더 맞는 접근방식이 아닐까 싶습니다.
의견을 종합하자면 구현 방식도 좋고 열심히 잘 작성한 코드 같지만 전체적으로 코드가 복잡해지고 관리해야하는 데이터 리소스가 많기 때문에 성능적인 면에서도 효율이 조금 떨어지지 않을까 생각됩니다.
데이터를 캐싱하고 최신 데이터 정합성을 맞추려는 접근 자체는 매우 좋다고 생각되지만 그렇게 하기 위해 너무 많은 코드와 자원이 필요하지 않나 생각합니다!
redisTemplate.delete(zSetKey); | ||
redisTemplate.delete(zSetPfKey); |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파라미터가 많을땐 줄바꿈을 해주면 가독성이 더 좋아질 듯 합니다~
There was a problem hiding this 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); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마지막 데이터 빼고 새로운 데이터를 ZSet에 넣는 로직인 것 같아요!! 다만 Redis 안에 데이터수가 pageable size 보다 적을 때에는 마지막 데이터를 삭제하면 안될 것 같아요!
Quality Gate failedFailed conditions See analysis details on SonarQube Cloud Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE |
#️⃣연관된 이슈
📝작업 내용
포트폴리오 전체 조회 같은 경우 첫 페이지에 유저가 몰릴 가능성이 높을 뿐만 아니라 첫 페이지는 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
포트폴리오
스크린샷 (선택)
💬리뷰 요구사항(선택)