[Feat] #627 솝탬프 조회 API 변경 #628
Conversation
- Optional로 wrapping하여 Clap을 가져옴
- 엔티티에 존재하는 빌더 메서드의 생성자를 커스텀하게 작성하도록 변경함. clapCount와 viewCount는 주입받지 않고 생성 시 0 으로 두도록 강제. - @NoArgsConstructor 의 접근 제한자를 PROTECTED로 지정, @AllArgsConstructor 는 사이드 이펙트를 완벽히 컨트롤하기 힘들 수 있겠다는 판단이 들어서 지정하지 않음
- 조회수 증감 로직을 DB의 `UPDATE` 쿼리를 통해 직접 원자적으로 처리하도록 구현함 - 조회수는 사용자의 요청마다 매우 빈번하게 발생함. 만약 매 조회 시마다 락을 걸면 성능 오버헤드가 큼. 낙관적으로 업데이트를 한다해도 조회수는 사실 중요한 지표가 아닌데, 이 조회수 증가 로직으로 인해 업데이트 등의 기능에서 실패율이 늘어날 필요가 없다고 판단함. 또한 이번에 추가된 박수 기능과 달리 조회수는 높은 정합도를 요구하지 않음. == 민감하지 않은 데이터임. 심지어는 현재 정책상으로 한 유저의 중복 조회도 별도로 방지하고 있지 않음. 따라서 락을 사용하거나 조회수 증가 시마다 버전을 업데이트해줄 필요가 없다고 생각함 - 현재 상황에서 조회수의 정합성이 깨지는 경우는 솝탬프 수정과 조회가 동시에 발생하여 race condition이 발생하는 경우인데 (조회수가 증가해도 낙관적 업데이트가 성공해서 덮어씌워지는 상황) 이 정도는 허용 가능한 범위라고 판단함. - 마지막으로 이런 판단을 내린 이유 중 하나는 솝탬프 기능의 특수성인데, 솝탬프는 현재 활동 중인 기수의 유저들만 접근 가능한 콘텐츠임. 따라서 접근 가능한 유저 수가 항시 200 명을 넘지 않기 때문에 동시성 이슈도 적을 것이라고 판단했고 성능적으로도 문제 없을 것으로 기대됨
- 총 박수 횟수 - 내 박수 횟수 - 조회수 - 내 솝탬프 여부
- Response 응답을 생성하며 내 박수 횟수와 내 솝탬프인지 여부를 주입받도록 변경
- 요청한 user가 해당 스탬프에 친 박수 수를 가져옴 - 조회수 증가 로직 추가 - 이곳에서 ServiceDto가 아니라 presentation layer에서 사용할 Dto를 반환해주도록 수정하며 수정된 Dto Mapper 로직 적용
- 클래스 단에서 트랜잭션 기본값을 사용하는게 좋지 않다고 판단했으며, 결정적으로 현재 getStampInfo 메서드는 조회 수 증가 로직에서만 트랜잭션을 걸고 싶은데(더티 체킹 범위를 줄이고자) 트랜잭션 전파가 발생하기 때문. 그리고 읽기 작업을 한 트랜잭션에서 수행할 필요가 딱히 없다고 봄
…도록 변경 - StampMain의 mapping을 서비스에서 진행
- 테스트 코드가 깨져서 id 필드를 추가함
- 이전에 생성자로 빌더를 제한한 이유는 생성 시 clapCount와 viewCount를 제한하기 위함이었는데, 현재 상황에서 테스트 코드 작성 시 문제가 발생하는 부분이 많아 클래스 레벨의 빌더로 복귀함
- mapper 책임을 컨트롤러단에서 갖도록 함. 통일성 및 layer에 맞는 dto 사용을 위함 - isMine 필드가 자바의 명명 규칙 때문에 mapstruct 에서 필드를 제대로 찾지 못하는 문제가 존재해서 default 메서드로 직접 매핑하는 로직을 추가함
|
|
||
| Optional<Stamp> findByIdAndUserId(Long id, Long userId); | ||
|
|
||
| @Modifying |
There was a problem hiding this comment.
@Modifying(clearAutomatically = true, flushAutomatically = true)
이렇게 자동 clear/flush 되도록 수정하면 좋을 것 같아요!
There was a problem hiding this comment.
이게 조금 더 안전한가요?? 조회 수 증가 쿼리로 레코드가 수정되었다고 1차 캐시를 날릴 필요가 있을지, 현재는 트랜잭션을 수정하는 레벨에서만 가져가서 상관없겠지만, 추후 트랜잭션 범위가 커졌을 때 flush를 자동으로 하면 다른 수정 쿼리로 발생한 락의 시간이 길어지진 않을지 걱정이네요..
우선 현재는 시간 이슈 + 이 방식도 좋을 것 같아서 반영하겠습니다!
There was a problem hiding this comment.
JPQL update는 벌크 업데이트라 1차 캐시를 우회해서 같은 트랜잭션/영속성 컨텍스트에 Stamp 엔티티가 이미 로드돼 있으면 stale 상태가 되고, 이후 저장 시 증가분을 덮어쓸 위험이 있기 때문에 flush/clear하는 게 안전하다고 생각해요!
퍼포먼스가 걱정되면 이 메서드를 호출하는 범위를 짧게(작은 트랜잭션) 유지하거나, 조회 트랜잭션과 분리하는 식으로 운영할 수도 있을 것 같아요~
| val stamp = stampService.findStamp(missionId, soptampUserId); | ||
| val requestUserClapCount = clapService.getUserClapCount(requestUserId, stamp.getId()); | ||
|
|
||
| stampService.increaseViewCountById(stamp.getId()); | ||
|
|
||
| return StampInfo.StampView.of( | ||
| stamp, requestUserClapCount, Objects.equals(requestUserId, soptampUserId)); |
There was a problem hiding this comment.
여기서 응답 매핑에 들어가는 stamp가 viewCount ++ 되기 전이라서 응답의 viewCount에 내 조회가 포함된 viewCount가 들어가지 않네요! 정합성이 엄청 중요한 필드는 아니니 증가 후 재조회할 필요까진 없을 것 같지만, 응답에서 viewCount + 1해서 내려주면 좋을 것 같아요!
There was a problem hiding this comment.
원래 응답 Dto가 중복으로 사용되는 부분들이 있어서 이렇게 + 1 을 해주기 좀 애매하다고 생각했는데, 아예 view 라는 네이밍으로 조회 시 사용하는 Dto를 분리해버리고 viewCount 가 + 1 된 값으로 응답하도록 반영하겠습니다!
| .updatedAt(stamp.getUpdatedAt()) | ||
| .missionId(stamp.getMissionId()) | ||
| .clapCount(stamp.getClapCount()) | ||
| .viewCount(stamp.getViewCount()) |
There was a problem hiding this comment.
SoptampFacade에 단 리뷰처럼 여기서 +1 한 값 내려주도록 하면 좋을 듯합니다!
- 기존에 upload 와 조회 시 동시에 사용되던 Dto를 분리함
…y = true 적용 - 조회 수 증가 쿼리로 레코드가 수정되면 1차 캐시가 날아가고 - 트랜잭션 범위가 커졌을 때 flush를 자동으로 하므로 다른 쿼리들의 락 점유 시간이 길어질 우려가 있음 해당 메서드의 실행 순서를 잘 고려할 것
2 participants
Related issue 🛠
Work Description ✏️
조회수 증가 로직 구현
Trouble Shooting ⚽️
Related ScreenShot 📷
Uncompleted Tasks 😅
To Reviewers 📢