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.

Sign up for GitHub

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

Jump to bottom

[LIME-71 ] 투표 참여 API 데드락 & 동시성 이슈 해결 #75

Merged
merged 4 commits into from
Apr 1, 2024
Merged

[LIME-71 ] 투표 참여 API 데드락 & 동시성 이슈 해결 #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0bef2c8
fix: 투표 참여 API 데드락 해결을 위해 외래키 분리
Yiseull Mar 21, 2024
bafd101
test: 외래키 분리로 인한 테스트 수정
Yiseull Mar 21, 2024
d445993
test: 투표 통합 테스트 보완
Yiseull Mar 21, 2024
7a3f34e
fix: Lettuce 락을 이용해 투표 참여 API 동시성 이슈 해결
Yiseull Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions lime-api/src/main/java/com/programmers/lime/domains/vote/application/VoteLockManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.programmers.lime.domains.vote.application;

import org.springframework.stereotype.Component;

import com.programmers.lime.domains.vote.domain.Vote;
import com.programmers.lime.domains.vote.implementation.VoteManager;
import com.programmers.lime.redis.vote.VoteRedisManager;

import lombok.RequiredArgsConstructor;

@Component
@RequiredArgsConstructor
public class VoteLockManager {

private final VoteManager voteManager;
private final VoteRedisManager voteRedisManager;

public void participate(
final Vote vote,
final Long memberId,
final Long itemId
) throws InterruptedException
{
while (Boolean.FALSE.equals(voteRedisManager.lock(String.valueOf(vote.getId())))) {
Thread.sleep(10);
Comment on lines +24 to +25
Copy link
Member

Choose a reason for hiding this comment

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

10ms마다 검사하는 방법이 가장 효율적이었던건지 궁금해서 질문드립니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

제가 생각할 때는 10이 가장 적절한 것 같습니다!

락을 얻기 위해 계속 시도하면 레디스에 부하를 주기 때문에 Thread.sleep()으로 약간의 시간차를 두고 락 획득을 시도하도록 하였습니다. 다만 이 시간을 어떻게 설정해야 될까 고민을 많이 했고, 100, 50, 10 이렇게 3가지를 고려했습니다. 아무래도 처리 로직이 간단하기 때문에 처리 속도도 빠릅니다. 따라서 tps량이 가장 많은 10으로 선택을 했고, 레디스가 그 정도 부하는 충분히 버틸 수 있다고 생각했습니다! 10으로도 문제 없이 처리할 수 있는데 100과 50으로 시간을 설정하는 것은 오히려 응답 시간을 지연시킬 수 있다고 생각했습니다.

}

try {
voteManager.participate(vote, memberId, itemId);
} finally {
voteRedisManager.unlock(String.valueOf(vote.getId()));
}
}
}
15 changes: 11 additions & 4 deletions lime-api/src/main/java/com/programmers/lime/domains/vote/application/VoteService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
import com.programmers.lime.redis.vote.VoteRedisManager;

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

@Slf4j
@Service
@RequiredArgsConstructor
public class VoteService {
Expand All @@ -46,6 +48,7 @@ public class VoteService {
private final VoteManager voteManager;
private final VoteRemover voteRemover;
private final VoterReader voterReader;
private final VoteLockManager voteLockManager;
private final MemberUtils memberUtils;
private final ItemReader itemReader;
private final VoteRedisManager voteRedisManager;
Expand Down Expand Up @@ -85,11 +88,15 @@ private void participate(
final Long memberId,
final Long itemId
) {
voterReader.find(vote, memberId)
voterReader.find(vote.getId(), memberId)
.ifPresentOrElse(
voter -> voteManager.reParticipate(itemId, voter),
() -> {
voteManager.participate(vote, memberId, itemId);
try {
voteLockManager.participate(vote, memberId, itemId);
} catch (InterruptedException e) {
log.info("투표 참여 중 락 획득 실패, voteId={}, memberId={}", vote.getId(), memberId);
}
eventPublisher.publishEvent(new RankingUpdateEvent(String.valueOf(vote.getHobby()), vote.isVoting(), getVoteRedis(vote)));
}
);
Expand All @@ -99,7 +106,7 @@ public void cancelVote(final Long voteId) {
final Long memberId = memberUtils.getCurrentMemberId();
final Vote vote = voteReader.read(voteId);

voteManager.cancel(vote, memberId);
voteManager.cancel(voteId, memberId);
eventPublisher.publishEvent(new RankingDecreasePopularityEvent(String.valueOf(vote.getHobby()), getVoteRedis(vote)));
}

Expand Down Expand Up @@ -162,7 +169,7 @@ public VoteGetByKeywordServiceResponse getVotesByKeyword(
parameters,
null
);
final long totalVoteCount = voteReader.countByKeyword(keyword);
final long totalVoteCount = voteReader.countByKeyword(keyword); // 키워드를 가진 아이템 쿼리 두 번 나감 -> 리팩토링

return new VoteGetByKeywordServiceResponse(cursorSummary, totalVoteCount);
}
Expand Down
68 changes: 41 additions & 27 deletions lime-api/src/test/java/com/programmers/lime/domains/vote/application/VoteServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.mockito.BDDMockito.*;

import java.time.LocalDateTime;
import java.util.Optional;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
Expand All @@ -28,6 +29,7 @@
import com.programmers.lime.domains.vote.domain.setup.VoteSetUp;
import com.programmers.lime.domains.vote.domain.setup.VoterSetUp;
import com.programmers.lime.domains.vote.implementation.VoteReader;
import com.programmers.lime.domains.vote.implementation.VoterReader;
import com.programmers.lime.domains.vote.model.VoteDetailInfo;
import com.programmers.lime.domains.vote.model.VoteSortCondition;
import com.programmers.lime.domains.vote.model.VoteStatusCondition;
Expand All @@ -51,6 +53,9 @@ class VoteServiceTest extends IntegrationTest {
@Autowired
private VoteReader voteReader;

@Autowired
private VoterReader voterReader;

@Autowired
private ItemSetup itemSetup;

Expand Down Expand Up @@ -150,15 +155,17 @@ class ParticipateVote {
void participateVoteTest() {
// given
final Long itemId = vote.getItem1Id();
final Long memberId = 1L;

given(memberUtils.getCurrentMemberId())
.willReturn(1L);
.willReturn(memberId);

// when
voteService.participateVote(voteId, itemId);

// then
assertThat(vote.getVoters()).hasSize(1);
final Voter voter = voterReader.read(voteId, memberId);
assertThat(voter.getItemId()).isEqualTo(itemId);
}

@Test
Expand All @@ -167,7 +174,7 @@ void reParticipateVoteTest() {
// given
final Long memberId = 1L;
final Long selectedItemId = vote.getItem1Id();
final Voter voter = voterSetup.saveOne(vote, memberId, selectedItemId);
voterSetup.saveOne(voteId, memberId, selectedItemId);
final Long reSelectedItemId = vote.getItem2Id();

given(memberUtils.getCurrentMemberId())
Expand All @@ -177,7 +184,7 @@ void reParticipateVoteTest() {
voteService.participateVote(voteId, reSelectedItemId);

// then
assertThat(vote.getVoters()).hasSize(1);
final Voter voter = voterReader.read(voteId, memberId);
assertThat(voter.getItemId()).isEqualTo(reSelectedItemId);
}

Expand Down Expand Up @@ -217,7 +224,7 @@ void participateVoteWithNotExistItemTest() {
void cancelVoteTest() {
// given
final Long memberId = 1L;
voterSetup.saveOne(vote, memberId, vote.getItem1Id());
voterSetup.saveOne(voteId, memberId, vote.getItem1Id());

given(memberUtils.getCurrentMemberId())
.willReturn(memberId);
Expand All @@ -226,7 +233,8 @@ void cancelVoteTest() {
voteService.cancelVote(voteId);

// then
assertThat(vote.getVoters()).isEmpty();
final Optional<Voter> voter = voterReader.find(voteId, memberId);
assertThat(voter.isEmpty()).isTrue();
}

@Nested
Expand Down Expand Up @@ -303,7 +311,7 @@ void readVoteWithParticipatedTest() {
// given
final Long memberId = 1L;
final Long selectedItemId = vote.getItem1Id();
voterSetup.saveOne(vote, memberId, selectedItemId);
voterSetup.saveOne(voteId, memberId, selectedItemId);

given(memberUtils.getCurrentMemberId())
.willReturn(memberId);
Expand All @@ -319,13 +327,13 @@ void readVoteWithParticipatedTest() {
@Nested
class GetVotesByCursor {

Vote vote2;
Vote vote3;
Long vote2Id = 2L;
Long vote3Id = 3L;

@BeforeEach
void setUp() {
vote2 = voteSetup.saveOne(2L, item1.getId(), item2.getId());
vote3 = voteSetup.saveOne(3L, item1.getId(), item2.getId());
voteSetup.saveOne(vote2Id, item1.getId(), item2.getId());
voteSetup.saveOne(vote3Id, item1.getId(), item2.getId());
}

@Test
Expand All @@ -345,9 +353,9 @@ void getVotesByCursorWithRecentTest() {

// then
assertThat(result.summaries()).hasSize(3);
assertThat(result.summaries().get(0).voteInfo().id()).isEqualTo(vote3.getId());
assertThat(result.summaries().get(1).voteInfo().id()).isEqualTo(vote2.getId());
assertThat(result.summaries().get(2).voteInfo().id()).isEqualTo(vote.getId());
assertThat(result.summaries().get(0).voteInfo().id()).isEqualTo(vote3Id);
assertThat(result.summaries().get(1).voteInfo().id()).isEqualTo(vote2Id);
assertThat(result.summaries().get(2).voteInfo().id()).isEqualTo(voteId);
}

@Test
Expand All @@ -357,9 +365,9 @@ void getVotesByCursorWithPopularTest() {
final Hobby hobby = Hobby.BASKETBALL;
final VoteSortCondition sortCondition = VoteSortCondition.POPULARITY;

voterSetup.saveOne(vote2, 1L, vote.getItem1Id());
voterSetup.saveOne(vote2, 2L, vote.getItem1Id());
voterSetup.saveOne(vote3, 1L, vote.getItem1Id());
voterSetup.saveOne(vote2Id, 1L, vote.getItem1Id());
voterSetup.saveOne(vote2Id, 2L, vote.getItem1Id());
voterSetup.saveOne(vote3Id, 1L, vote.getItem1Id());

// when
final CursorSummary<VoteSummary> result = voteService.getVotesByCursor(
Expand All @@ -371,9 +379,12 @@ void getVotesByCursorWithPopularTest() {

// then
assertThat(result.summaries()).hasSize(3);
assertThat(result.summaries().get(0).voteInfo().id()).isEqualTo(vote2.getId());
assertThat(result.summaries().get(1).voteInfo().id()).isEqualTo(vote3.getId());
assertThat(result.summaries().get(2).voteInfo().id()).isEqualTo(vote.getId());
assertThat(result.summaries().get(0).voteInfo().id()).isEqualTo(vote2Id);
assertThat(result.summaries().get(0).voteInfo().participants()).isEqualTo(2);
assertThat(result.summaries().get(1).voteInfo().id()).isEqualTo(vote3Id);
assertThat(result.summaries().get(1).voteInfo().participants()).isEqualTo(1);
assertThat(result.summaries().get(2).voteInfo().id()).isEqualTo(voteId);
assertThat(result.summaries().get(2).voteInfo().participants()).isEqualTo(0);
}

@Test
Expand All @@ -393,9 +404,9 @@ void getVotesByCursorWithClosedTest() {

// then
assertThat(result.summaries()).hasSize(3);
assertThat(result.summaries().get(0).voteInfo().id()).isEqualTo(vote.getId());
assertThat(result.summaries().get(1).voteInfo().id()).isEqualTo(vote2.getId());
assertThat(result.summaries().get(2).voteInfo().id()).isEqualTo(vote3.getId());
assertThat(result.summaries().get(0).voteInfo().id()).isEqualTo(voteId);
assertThat(result.summaries().get(1).voteInfo().id()).isEqualTo(vote2Id);
assertThat(result.summaries().get(2).voteInfo().id()).isEqualTo(vote3Id);
}

@Test
Expand All @@ -419,6 +430,9 @@ void getVotesByCursorWithPostedTest() {

// then
assertThat(result.summaries()).hasSize(3);
assertThat(result.summaries().get(0).voteInfo().id()).isEqualTo(vote3Id);
assertThat(result.summaries().get(1).voteInfo().id()).isEqualTo(vote2Id);
assertThat(result.summaries().get(2).voteInfo().id()).isEqualTo(voteId);
}

@Test
Expand All @@ -429,8 +443,8 @@ void getVotesByCursorWithParticipatedTest() {
final VoteStatusCondition statusCondition = VoteStatusCondition.PARTICIPATED;
final Long memberId = 1L;

voterSetup.saveOne(vote2, memberId, vote.getItem1Id());
voterSetup.saveOne(vote3, memberId, vote.getItem1Id());
voterSetup.saveOne(vote2Id, memberId, vote.getItem1Id());
voterSetup.saveOne(vote3Id, memberId, vote.getItem1Id());

given(memberUtils.getCurrentMemberId())
.willReturn(memberId);
Expand All @@ -445,8 +459,8 @@ void getVotesByCursorWithParticipatedTest() {

// then
assertThat(result.summaries()).hasSize(2);
assertThat(result.summaries().get(0).voteInfo().id()).isEqualTo(vote3.getId());
assertThat(result.summaries().get(1).voteInfo().id()).isEqualTo(vote2.getId());
assertThat(result.summaries().get(0).voteInfo().id()).isEqualTo(vote3Id);
assertThat(result.summaries().get(1).voteInfo().id()).isEqualTo(vote2Id);
}

@Test
Expand Down
17 changes: 2 additions & 15 deletions lime-domain/src/main/java/com/programmers/lime/domains/vote/domain/Vote.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package com.programmers.lime.domains.vote.domain;

import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

import com.programmers.lime.common.model.Hobby;
Expand All @@ -11,7 +9,6 @@
import com.programmers.lime.error.BusinessException;
import com.programmers.lime.error.ErrorCode;

import jakarta.persistence.CascadeType;
import jakarta.persistence.Column;
import jakarta.persistence.Embedded;
import jakarta.persistence.Entity;
Expand All @@ -20,7 +17,6 @@
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.OneToMany;
import jakarta.persistence.Table;
import lombok.AccessLevel;
import lombok.Builder;
Expand Down Expand Up @@ -65,9 +61,6 @@ public class Vote extends BaseEntity {
@Column(name = "maximum_participants", nullable = false)
private int maximumParticipants;

@OneToMany(mappedBy = "vote", cascade = CascadeType.ALL)
private final List<Voter> voters = new ArrayList<>();

@Builder
private Vote(
final Long memberId,
Expand Down Expand Up @@ -102,12 +95,6 @@ public boolean containsItem(final Long itemId) {
return item1Id.equals(itemId) || item2Id.equals(itemId);
}

public void addVoter(final Voter voter) {
if (!this.voters.contains(voter)) {
this.voters.add(voter);
}
}

public boolean isOwner(final Long memberId) {
return this.memberId.equals(memberId);
}
Expand All @@ -121,7 +108,7 @@ public void close(final LocalDateTime now) {
this.endTime = now;
}

public boolean reachMaximumParticipants() {
return this.maximumParticipants == this.voters.size();
public boolean reachMaximumParticipants(final int participants) {
return this.maximumParticipants <= participants;
}
}
13 changes: 4 additions & 9 deletions lime-domain/src/main/java/com/programmers/lime/domains/vote/domain/Voter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.Table;
import lombok.AccessLevel;
import lombok.Getter;
Expand All @@ -28,9 +25,8 @@ public class Voter extends BaseEntity {
@Column(name = "id")
private Long id;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "vote_id", nullable = false)
private Vote vote;
@Column(name = "vote_id", nullable = false)
private Long voteId;

@Column(name = "member_id", nullable = false)
private Long memberId;
Expand All @@ -39,17 +35,16 @@ public class Voter extends BaseEntity {
private Long itemId;

public Voter(
final Vote vote,
final Long voteId,
final Long memberId,
final Long itemId
) {
this.vote = Objects.requireNonNull(vote);
this.voteId = Objects.requireNonNull(voteId);
this.memberId = Objects.requireNonNull(memberId);
this.itemId = Objects.requireNonNull(itemId);
}

public void participate(final Long itemId) {
this.itemId = itemId;
this.vote.addVoter(this);
}
}
Loading
Loading