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

[#76] 개인 맞춤 피드 제공 #77

Merged
merged 20 commits into from
Apr 29, 2024
Merged

[#76] 개인 맞춤 피드 제공 #77

merged 20 commits into from
Apr 29, 2024

Conversation

jzakka
Copy link
Collaborator

@jzakka jzakka commented Apr 19, 2024

사용자가 조회한 피드의 태그를 기반으로 맞춤 피드 제공

  1. 사용자 조회 피드를 redis에 저장 (ViewedFeed:{memberId} : ["1231", "33151", "32561", ...])
  2. 조회피드들의 태그들을 등장빈도 순으로 n개 추출
  3. 추출한 태그를 FrequentTags:{memberId}에 저장
  4. DB에서 해당 태그를 가지는 피드를 랜덤으로 m개 가져옴
  5. 가져온 임의의 피드들을 RecommendTags:{memberId}에 저장

@jzakka jzakka added the enhancement New feature or request label Apr 19, 2024
@jzakka jzakka requested a review from f-lab-moony April 19, 2024 02:22
@jzakka jzakka self-assigned this Apr 19, 2024
Copy link

Copy link
Collaborator

@f-lab-moony f-lab-moony left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다 ~
피드백이 구조개선에 대한 의견이 될 것 같은데, 기능상으론 현재 구조도 문제는 없어서 적용여부는 머지 후에 판단하는걸로 하시죠!

@Transactional
public void deleteFeedResponseById(Long feedId) {
jpaQueryFactory.delete(feedResponseEntity)
.where(feedResponseEntity.feedId.eq(feedId))
.execute();
}

@Transactional(readOnly = true)
public List<Long> findRandomFeedIdsByTagName(Collection<String> tagNames){
Copy link
Collaborator

Choose a reason for hiding this comment

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

랜덤을 여기서 주는게 맞을까요 ? 개인적으론 추천이기때문에 일단 고유의 랭킹순으로 피드들을 가져오고, 그 중에서 랜덤.. 예를 들면 상위 25개를 뽑고 그 중 랜덤 돌려서 상위 10개 이런 느낌이 되어야 하지 않을까 싶어요. 현재 로직은 전체에서 랜덤 돌린 결과가 나오는 거 아닌가요 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

조건을 만족하는 것 중에서 임의로 n개 추출을 하고 있습니다.

고유의 랭킹이 어떤 것인지는 잘 모르겠는데, 임의 추출 대상이 되는 그룹은 사용자가 자주 본 태그를 갖고있는 피드 만이 대상입니다.

import com.stoury.repository.RankingRepository
import com.stoury.repository.RecommendFeedsRepository
import com.stoury.repository.TagRepository
import com.stoury.domain.*
Copy link
Collaborator

Choose a reason for hiding this comment

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

임포트할때 * 는 가급적 안쓰는게 좋아요. 의도치 않은 스태틱 변수나 메서드가 잡혀서 오동작하는 경우가 있어요
실제로 저희 팀에서 장애가 났던 적도 있어서, 인텔리제이 설정에서 wildcard import 설정 꺼두시는걸 추천드려요 ~

@Getter
@NoArgsConstructor
@Table(name = "CLICK_LOG")
public class ClickLog {
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 Author

Choose a reason for hiding this comment

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

통계작업이랑 비정규화가 어떤 관계가 있는지 잘 몰라서 그러는데 그렇게 했을 때, 장점이 있나요?


@Repository
@RequiredArgsConstructor
public class ClickLogRepository {
Copy link
Collaborator

Choose a reason for hiding this comment

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

JPARepository 쓰는게 낫지 않나요 ?

@@ -56,4 +62,18 @@ public Optional<Tag> findByTagName(String tagName) {
.fetchFirst()
);
}

public List<Tag> findAllByMemberIdAndFrequency(Long memberId) {
return jpaQueryFactory
Copy link
Collaborator

Choose a reason for hiding this comment

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

ClickLog 구조랑 연결 된 얘기인데, 비정규화를 하면 이렇게 조인 많이 안하고 가져올 수 있을 것 같아요 ~
개인적으로 RDB로는 태그 수만큼 로우를 만드는것도 고려해볼만 할 것 같은데, 인덱스 잘 걸고 파티셔닝도 고려해야겠네요. 일단은 파티셔닝은 제쳐두고 생각 해보면 좋을 것 같습니다

@f-lab-moony f-lab-moony merged commit 0f89fa9 into main Apr 29, 2024
2 checks passed
@jzakka jzakka deleted the feature/76 branch April 30, 2024 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants