feat: 메세지 좋아요시 동시성 이슈를 해결한다. #632

asebn1 · 2022-10-27T06:00:59Z

문제원인

시퀀스	Tx1	Tx2
1	메시지 A 찾음
2		메시지 A찾음
3	메세지 좋아요 기록 존재하지 않는가? true
4		메세지 좋아요 기록 존재하지 않는가? true
5	메세지 좋아요 저장
6		메세지 좋아요 저장
7	Tx1 Commit
8		Tx2 Commit

원인은 바로 3, 4번에 있습니다.

두 개의 트랜잭션이 검사를 했을 때 기록이 존재하지않는가? 에서 둘다 TRUE를 반환했기 때문이었습니다.

낙관적 락을 적용한 뒤 트랜잭션 변화

시퀀스	Tx1	Tx2
1	Tx1 start
2		Tx2 start
3	메시지 A 찾음, version = 0
4		메시지 A찾음, version = 0
5	메세지 좋아요 기록 존재하지 않는가? true
6		메세지 좋아요 기록 존재하지 않는가? true
7	좋아요 수 증가, 메세지 좋아요 저장
8		좋아요 수 증가, 메세지 좋아요 저장
9	version == 0 true, Tx1 Commit, version += 1
10		version == 0 false, Tx2 Rollback

메세지 자체에 낙관적 락을 걸어 중복되는 수정으로 부터 일관성을 유지할 수 있게 되었습니다.

https://jaehhh.tistory.com/148 에서 자세히 볼 수 있습니다~

kth990303

제로 수고하셨습니다.
정리를 잘해주셔서 편하게 읽을 수 있었어요!

테스트 코드가 있으면 더 좋을 거 같습니다 (https://github.com/woowacourse/jwp-hands-on/blob/solution-6-transaction/transaction/src/test/java/transaction/stage1/Stage1Test.java 여기서 코드 대략 가져와봤는데 제대로 될지는 모르겠네요 😓 )

@Test
void messageLike throws SQLException {
    final var connection = dataSource.getConnection(); // 커넥션 받기
    connection.setAutoCommit(false);  // 트랜잭션 시작

    // 스레드1: 메시지 좋아요
    new Thread(RunnableWrapper.accept(() -> {

        // 스레드2: 메시지 좋아요
    })).start();

    sleep(0.5);

    // 검증

    connection.rollback();
}

kth990303 · 2022-10-27T06:27:38Z

backend/src/main/java/com/woowacourse/naepyeon/repository/message/MessageRepositoryCustom.java


 public interface MessageRepositoryCustom {

    List<Message> findAllByRollingpaperId(final Long rollingpaperId);

    Page<WrittenMessageResponseDto> findAllByAuthorId(final Long authorId, final Pageable pageRequest);
+
+    @Lock(LockModeType.OPTIMISTIC)
+    Optional<Message> findByIdForUpdate(Long id);


Suggested change

Optional<Message> findByIdForUpdate(Long id);

Optional<Message> findByIdForUpdate(final Long id);

kth990303 · 2022-10-27T06:27:50Z

backend/src/main/java/com/woowacourse/naepyeon/repository/message/MessageRepositoryImpl.java

@@ -58,6 +60,14 @@ public Page<WrittenMessageResponseDto> findAllByAuthorId(final Long authorId, fi
        return PageableExecutionUtils.getPage(content, pageRequest, countQuery::fetchOne);
    }

+    @Override
+    public Optional<Message> findByIdForUpdate(Long id) {


Suggested change

public Optional<Message> findByIdForUpdate(Long id) {

public Optional<Message> findByIdForUpdate(final Long id) {

kth990303 · 2022-10-27T06:35:27Z

backend/src/main/java/com/woowacourse/naepyeon/service/MessageService.java

@@ -197,7 +197,7 @@ private void validateAuthor(final Long memberId, final Message message) {
    }

    public MessageLikeResponseDto likeMessage(Long memberId, Long rollingpaperId, Long messageId) {
-        final Message message = messageRepository.findById(messageId)
+        final Message message = messageRepository.findByIdForUpdate(messageId)


낙관적 락을 이용해서 version에 따라 커밋 또는 롤백하게 해주셨군요 👍

kth990303 · 2022-10-27T06:53:58Z

backend/src/main/java/com/woowacourse/naepyeon/domain/message/Message.java

+    @Version
+    private Integer version;
+


그리고 아래와 같이 uniqueConstraints로 걸면 락 없이도 가능할 것 같기도..?

@Table(name = "message_like", indexes = { @Index(name = "message_like_member_id_message_id_index", columnList = "member_id, message_id") }, uniqueConstraints = { @UniqueConstraint( name = "message_like_duplicate", columnNames = {"member_id", "message_id"} ) })

yxxnghwan

안녕하세요 제로! 낙관적 락을 적용해서 check-then-act 의 race-condition 문제를 해결해주셨네요.
근데 낙관적 락의 경우 기존에 존재하던 데이터의 경우엔 효과적으로 적용되지만, 새로 추가되는 데이터는 컨트롤할 수 없다는 단점이 있는 것 같아요.
이번 케이스에선 Message에서 likes 값을 올리는 것은 해결되지만, MessageLike데이터 로우가 추가되는 건 결국 MessageLike에 유니크 제약조건이 걸려야할 것 같은데..
데이터가 양쪽에 나눠진 반정규형이어서 양쪽으로 처리하지 않으면 안될 것 같은 느낌이 드네요!

사실 MessageLike데이터만 가지고 카운터 쿼리로 좋아요 수를 관리하면 정합성 문제가 발생하지 않고 락없이 유니크 키로만 해결될 수도 있겠지만, 매번 카운트 쿼리랑 같이 조회하는게 부담이라서 당시에 Message에 likes 값을 추가했었겠죠..?

음.. 개인적인 가장 베스트 해결방안은 레디스에 MessageLike정보를 캐시해서 저장하는게 제일 좋아보이고.. (매번 카운터 쿼리를 발생시키지 않게끔) 배치성 작업으로 주기적으로 정합성을 맞춰주는게 제일 좋을 것 같은데 추가적인 학습과 캐싱 전략을 세워야 하니 그 부분은 따로 이슈를 파서 작업하는 것이 좋겠네요!

이번 이슈에서는 제 개인적인 생각으론 나눠진 양쪽 데이터의 동시성을 모두 고려해서 낙관락과 유니크 제약조건을 함께 걸어야할 것 같은데 제로 의견은 어떠신가요?

Seungpang

제로 구현하시느라 고생하셨습니다.
프론트쪽에서 에러 발생시 로직은 한번 이야기 나눠봐야 할 것 같아요
db에 중복으로 들어가는 부분은 해결이 된것같아요!
수고하셨습니다.

Seungpang · 2022-10-28T05:23:04Z

backend/src/main/java/com/woowacourse/naepyeon/domain/message/Message.java

+    @Version
+    private Integer version;


ALTER TABLE message ADD version bigint default 0;

해당 컬럼에 flyway 스크립트를 추가해야 할 것 같아요 bigint라면 Long으로 아니면 해당 스크립트를 integer로 바꾸셔도 상관없습니다.

asebn1 · 2022-10-28T07:01:39Z

@yxxnghwan
메세지의 likes로 데이터 변경을 비교하여 낙관적 락이 적용해 롤백되는 것을 로컬환경에서 테스트 해보았습니다! 데이터 중복이 해결된 것을 확인할 수 있었어요
유니크 제약조건의 경우,
메세지 좋아요에서 메세지 이모지 종류가 추가될 경우
유니크를 member_id, message_id에 거는 것이 문제가 될 수 있어서 예외로 생각해보았어요~

naepyeon-sonarqube-1 · 2022-10-28T07:16:18Z

Analysis Details

0 Issues

0 Bugs
0 Vulnerabilities
0 Code Smells

Coverage and Duplications

83.33% Coverage (81.60% Estimated after merge)
0.00% Duplicated Code (0.00% Estimated after merge)

Project ID: woowacourse-teams_2022-nae-pyeon_AYKuzmWfelLz0D2BhgWj

View in SonarQube

yxxnghwan

낙관적 락의 경우 동시성이 발생했을 때 영속성 컨텍스트가 flush 될 때 업데이트 문이 나가면서 OptimisticLockingFailureException이 발생하는데 캐치하는 로직이 필요할 것 같아요~! 현재 상태에선 OptimisticLockingFailureException이 발생하면 컨트롤러 어드바이스에서 잡게될 것 같네요! 비즈니스 예외로 핸들링이 필요할 것 같습니다!

feat: 메세지 좋아요시 낙관적 락을 적용한다.

7ac3b31

asebn1 added 백엔드 기능 labels Oct 27, 2022

asebn1 self-assigned this Oct 27, 2022

This comment has been minimized.

Sign in to view

kth990303 approved these changes Oct 27, 2022

View reviewed changes

refactor: MessageRepo final 추가

1686bae

This comment has been minimized.

Sign in to view

kth990303 reviewed Oct 27, 2022

View reviewed changes

yxxnghwan requested changes Oct 27, 2022

View reviewed changes

Seungpang approved these changes Oct 28, 2022

View reviewed changes

refactor: message의 version을 flyway에 추가한다.

60abfd0

yxxnghwan requested changes Nov 4, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 메세지 좋아요시 동시성 이슈를 해결한다. #632

feat: 메세지 좋아요시 동시성 이슈를 해결한다. #632

asebn1 commented Oct 27, 2022 •

edited

Loading

This comment has been minimized.

kth990303 left a comment •

edited

Loading

kth990303 Oct 27, 2022

kth990303 Oct 27, 2022

kth990303 Oct 27, 2022

This comment has been minimized.

kth990303 Oct 27, 2022 •

edited

Loading

yxxnghwan left a comment

Seungpang left a comment

Seungpang Oct 28, 2022

asebn1 commented Oct 28, 2022

naepyeon-sonarqube-1 bot commented Oct 28, 2022

yxxnghwan left a comment •

edited

Loading

	Optional<Message> findByIdForUpdate(Long id);
	Optional<Message> findByIdForUpdate(final Long id);

	public Optional<Message> findByIdForUpdate(Long id) {
	public Optional<Message> findByIdForUpdate(final Long id) {

feat: 메세지 좋아요시 동시성 이슈를 해결한다. #632

Are you sure you want to change the base?

feat: 메세지 좋아요시 동시성 이슈를 해결한다. #632

Conversation

asebn1 commented Oct 27, 2022 • edited Loading

문제원인

낙관적 락을 적용한 뒤 트랜잭션 변화

This comment has been minimized.

kth990303 left a comment • edited Loading

Choose a reason for hiding this comment

kth990303 Oct 27, 2022

Choose a reason for hiding this comment

kth990303 Oct 27, 2022

Choose a reason for hiding this comment

kth990303 Oct 27, 2022

Choose a reason for hiding this comment

This comment has been minimized.

kth990303 Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

yxxnghwan left a comment

Choose a reason for hiding this comment

Seungpang left a comment

Choose a reason for hiding this comment

Seungpang Oct 28, 2022

Choose a reason for hiding this comment

asebn1 commented Oct 28, 2022

naepyeon-sonarqube-1 bot commented Oct 28, 2022

Analysis Details

0 Issues

Coverage and Duplications

yxxnghwan left a comment • edited Loading

Choose a reason for hiding this comment

asebn1 commented Oct 27, 2022 •

edited

Loading

kth990303 left a comment •

edited

Loading

kth990303 Oct 27, 2022 •

edited

Loading

yxxnghwan left a comment •

edited

Loading