현재 유플러스 유레카 과정에서 유플러스 측에서 제시해준 요구사항을 바탕으로 자녀에게 개인화 맞춤 도서를 추천해주는 서비스를 개발하고 있다.
3주간의 개발 기간 중 첫 1주이다.
그 중 MBTI 관련 설문의 답변, 선호 장르, 선호 주제어를 고른 답변을 받아 결과를 계산해 저장하는 로직을 작성한 상태였는데, 현업자분들이 피드백을 주셨다.
그 피드백을 바탕으로 개선 전과 개선 후를 비교해보려고 한다.
피드백
- 현재 SurveyFacade에 설문과 관련된 동작들이 작성되어 있고, 그 부분이 클래스 단위 Transaction으로 묶여 있다.
- 트랜잭션이 필요하지 않은 일종의 계산 로직 같은 부분들은 트랜잭션에 묶일 필요가 없어 분리해서 개선을 할 필요가 있다.
- 서비스 클래스를 만들면서 해당 메서드가 서비스 레이어에 있어도 되는지, 그리고 해당 서비스에 존재해도 되는지 고민해볼 필요가 있다.
- Facade 패턴을 사용했다는 것은 그 부분은 다른 서비스를 조립하는 곳인데 테스트가 반드시 필요한 지 생각해볼 필요가 있다.
- 테스트 가능하지만 조립된 서비스들에 의존이 되고 만약 서비스가 바뀌면 그 테스트도 바뀌어야 한다.
- 테스트 코드 작성의 의미를 생각해보면 좋을 것 같다.
- 정적 팩토리 메서드를 적극적으로 활용해 코드 가독성, 편의성을 늘릴 수 있다.
위와 같은 피드백들을 해주셨다.
이 코드를 작성하면서 이 메서드의 위치가 여기가 맞는 지 등의 고민을 매우 많이 했었는데 질문을 하지 않았음에도 코드를 보시고 이 부분을 피드백 해주셔서 좋았다.
그래서 피드백을 바탕으로 코드를 개선해보려고 한다. 특히 기존에 신경을 많이 쓰지 않았던 트랜잭션 분리에 신경을 써서 개선을 해보고 개선 전과 개선 후의 차이를 보고자 한다.
기존 코드는 아래와 같다.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
@Slf4j
@Service
@RequiredArgsConstructor
@Transactional
public class SurveyFacadeImpl implements SurveyFacade {
private final MBTIService mbtiService;
private final ChildPersonalityHistoryService historyService;
private final ChildProfileService childProfileService;
@Override
public void submitSurvey(SurveyResultRequestDto requestDto, Long childProfileId) {
log.info("Submit survey Input childProfileId: {}", childProfileId);
ChildProfile childProfile = childProfileService.getChildProfile(childProfileId);
/*
진단 시 누적 점수는 초기화된다.
설문 답변을 통해 MBTI 점수를 계산한다.
기존 누적 MBTI 점수를 초기화하고 계산한 MBTI 점수를 누적 MBTI 점수에 update 한다.
*/
MBTIScore mbtiScore = mbtiService.calculateMBTIScore(requestDto.getAnswers());
childProfileService.resetCumulativeMBTIScore(childProfileId);
childProfileService.updateCumulativeMBTIScore(childProfileId, mbtiScore);
historyService.deleteDiagnosisHistory(childProfileId);
historyService.createHistory(childProfileId, mbtiScore,HistoryCreatedType.DIAGNOSIS);
/*
진단 시 누적 점수는 초기화된다.
설문 답변을 통해 장르/주제어 누적 점수를 update한다.
누적 점수를 바탕으로 선호 장르/주제어를 저장한다.
*/
childProfileService.resetFavoriteScores(childProfileId);
updateFavoriteGenres(childProfile, requestDto.getFavoriteGenres());
updateFavoriteTopics(childProfile, requestDto.getFavoriteTopics());
}
@Override
public SurveyResultDto getSurveyResult(Long childProfileId) {
log.info("Get survey result ChildProfile ID: {}", childProfileId);
ChildProfile childProfile = childProfileService.getChildProfile(childProfileId);
ChildPersonalityHistory latestHistory = historyService.getLatestHistory(childProfileId);
MBTIPercentageDto mbtiResult = mbtiService.calculatePercentages(latestHistory.getMbtiScore());
// 트랜잭션으로 인해 점수를 update하는 부분과 분리
List<Genre> preferredGenres = determinePreferredGenres(childProfile.getGenreScores());
List<Topic> preferredTopics = determinePreferredTopics(childProfile.getTopicScores());
for (Genre genre : preferredGenres) {
FavoriteGenre favoriteGenre = FavoriteGenre.builder()
.genre(genre)
.build();
latestHistory.addFavoriteGenre(favoriteGenre);
}
for (Topic topic : preferredTopics) {
FavoriteTopic favoriteTopic = FavoriteTopic.builder()
.topic(topic)
.build();
latestHistory.addFavoriteTopic(favoriteTopic);
}
List<FavoriteDto> favoriteGenresDto = preferredGenres.stream()
.map(genre -> new FavoriteDto(genre.getName(), genre.getImage()))
.toList();
List<FavoriteDto> favoriteTopicsDto = preferredTopics.stream()
.map(topic -> new FavoriteDto(topic.getName(), topic.getImage()))
.toList();
return SurveyResultDto.builder()
.IPercent(mbtiResult.getIPercent())
.EPercent(mbtiResult.getEPercent())
.SPercent(mbtiResult.getSPercent())
.NPercent(mbtiResult.getNPercent())
.TPercent(mbtiResult.getTPercent())
.FPercent(mbtiResult.getFPercent())
.JPercent(mbtiResult.getJPercent())
.PPercent(mbtiResult.getPPercent())
.mbtiResult(MbtiDto.fromEntity(latestHistory.getMbtiScore().getMbti()))
.favoriteGenres(favoriteGenresDto)
.favoriteTopics(favoriteTopicsDto)
.build();
}
@Override
public void reSurvey(Long childProfileId) {
log.info("Resurvey - Delete DiagnosisHistory ChildProfile ID: {}", childProfileId);
historyService.deleteDiagnosisHistory(childProfileId);
}
/*
선호 장르/주제어 관련 로직의 위치가 여기가 맞는지 고민 필요.
*/
private void updateFavoriteGenres(ChildProfile childProfile, List<Long> favoriteGenreIds) {
log.info("Updating GenreScores ChildProfile ID: {}", childProfile.getId());
// 선택한 장르에 5점 부여
for (Long genreId : favoriteGenreIds) {
GenreScore genreScore = childProfile.getGenreScores().stream()
.filter(gs -> gs.getGenre().getId().equals(genreId))
.findFirst()
.orElseThrow(() -> new EntityNotFoundException(genreId));
genreScore.updateScore(5.0);
}
log.info("GenreScores updated ChildProfile ID: {}", childProfile.getId());
}
private void updateFavoriteTopics(ChildProfile childProfile, List<Long> favoriteTopicIds) {
log.info("Updating TopicScores ChildProfile ID: {}", childProfile.getId());
// 선택한 주제어에 5점 부여
for (Long topicId : favoriteTopicIds) {
TopicScore topicScore = childProfile.getTopicScores().stream()
.filter(ts -> ts.getTopic().getId().equals(topicId))
.findFirst()
.orElseThrow(() -> new EntityNotFoundException(topicId));
topicScore.updateScore(5.0);
}
log.info("TopicScores updated ChildProfile ID: {}", childProfile.getId());
}
private List<Genre> determinePreferredGenres(List<GenreScore> genreScores) {
log.info("set preferred genres");
double averageScore = genreScores.stream()
.mapToDouble(GenreScore::getScore)
.average()
.orElse(0.0);
log.debug("Average GenreScore: {}", averageScore);
List<Genre> preferredGenres = genreScores.stream()
.filter(gs -> gs.getScore() > averageScore)
.map(GenreScore::getGenre)
.toList();
return preferredGenres;
}
private List<Topic> determinePreferredTopics(List<TopicScore> topicScores) {
log.info("set preferred topics");
double averageScore = topicScores.stream()
.mapToDouble(TopicScore::getScore)
.average()
.orElse(0.0);
log.debug("Average TopicScore: {}", averageScore);
List<Topic> preferredTopics = topicScores.stream()
.filter(ts -> ts.getScore() > averageScore)
.map(TopicScore::getTopic)
.toList();
return preferredTopics;
}
}
그리고 기존 코드를 바탕으로 관련 API를 묶어 100번 실행해본 결과 위와 같은 결과가 나왔다.
개선
만약 계산하는 부분을 메서드로 분리하고 그 부분에는 트랜잭션을 적용하지 않은 다음 트랜잭션이 필요한 메서드에 그 계산 메서드를 포함했을 경우 어떻게 될까?
-> 계산 메서드도 기존 트랜잭션 컨텍스트 내에서 실행되지만, 트랜잭션 관리가 필요하지 않기 때문에 트랜잭션 오버헤드가 발생하지 않는다.
이 부분을 인지하고 코드를 개선해보면 좋을 것 같다.
개선 1
단순하게 현재 구현된 코드에서 Transactional을 클래스 단위가 아닌 메서드 단위로 붙여보았다.
당연하게도 전혀 개선이 이루어지지 않았다.
Transactional은 기본 전파 속성이 Required이고, 따로 전파 옵션을 설정해주지 않았기 때문에 클래스에 적용하던 메서드에 적용하던 결국 같이 호출된다면 이미 존재하는 기존 트랜잭션에 같이 참여하게 되어 동작은 동일할 것이기 때문이다.
다만 읽기만 하는 메서드의 경우 readOnly 속성을 붙였을 때 기존 트랜잭션에 참여하는 것은 동일하지만 readOnly 설정이 반영되어 그 차이는 날 수 있다.
개선 2
우선 클래스 단위로 적용되어 있던 모든 @Transactional 애노테이션을 메서드 단위로 적용했다. 조회만 하는 부분은 readOnly 옵션을 붙이고, 필요가 없는 메서드는 애노테이션을 붙이지 않았다.
1
2
3
4
5
6
personalityScoreService.resetCumulativeMBTIScore(childProfileId);
personalityScoreService.updateCumulativeMBTIScore(childProfileId, mbtiScore);
personalityScoreService.resetFavoriteScores(childProfileId);
personalityScoreService.updateFavoriteGenresScore(childProfile, requestDto.getFavoriteGenres());
personalityScoreService.updateFavoriteTopicsScore(childProfile, requestDto.getFavoriteTopics());
그리고 위와 같이 기존에 SurveyFacade에 있던 메서드들의 위치를 그 메서드들이 존재해야 될 것 같다고 생각하는 Service 클래스로 이동했다.
1
public MBTIScore calculateMBTIScore(List<MBTISurveyAnswerDto> answers) { ... }
단순 계산 로직만 있는 부분은 Transactional 애노테이션을 적용하지 않도록 수정했다.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
private SurveyResultDto getSurveyResult(MBTIScore mbtiScore, MBTI mbti, List<Genre> preferredGenres, List<Topic> preferredTopics) {
MBTIPercentageDto mbtiResult = mbtiService.calculatePercentages(mbtiScore);
List<FavoriteDto> favoriteGenresDto = preferredGenres.stream()
.map(genre -> new FavoriteDto(genre.getName(), genre.getImage()))
.toList();
List<FavoriteDto> favoriteTopicsDto = preferredTopics.stream()
.map(topic -> new FavoriteDto(topic.getName(), topic.getImage()))
.toList();
return SurveyResultDto.builder()
.IPercent(mbtiResult.getIPercent())
.EPercent(mbtiResult.getEPercent())
.SPercent(mbtiResult.getSPercent())
.NPercent(mbtiResult.getNPercent())
.TPercent(mbtiResult.getTPercent())
.FPercent(mbtiResult.getFPercent())
.JPercent(mbtiResult.getJPercent())
.PPercent(mbtiResult.getPPercent())
.mbtiResult(MbtiDto.fromEntity(mbti))
.favoriteGenres(favoriteGenresDto)
.favoriteTopics(favoriteTopicsDto)
.build(); //필요 X
}
그리고 MBTIScore mbtiScore, MBTI mbti 이 부분들은 기존에 history에서 getter를 통해 가져오는 방식으로 구현했었는데, 이 부분을 트랜잭션이 적용된 메서드에서 가져오고 그 값을 넘겨주는 방식으로 구현했다.
history에서 가져오게 되는 경우 mbtiScore나 mbti를 가져올 때 지연로딩으로 가져오게 되기 때문에 트랜잭션이 필요했다.
하지만 그 부분만 트랜잭션이 필요하고 나머지는 단순 계산, DTO 변환 로직이기 때문에 그 부분때문에 트랜잭션을 적용하기에는 낭비가 되는 부분이 있었다.
따라서 트랜잭션이 적용된 메서드에서 직접 값을 가져와서 그 값을 넘기는 방식으로 변경하여 트랜잭션 작업이 필요없는 메서드를 만들었다. 트랜잭션이 적용되지 않아도 되는 메서드이기 때문에 필요에 의해 private으로 접근자도 바꾸어 주었다.
최종적으로 설문 결과를 제출하는 API, 결과 조회를 하는 API 두 API로 분리되어 있던 API 구조를 결과를 제출하고 바로 결과를 조회하도록 하는 API로 통합하였다.
그 결과 성능이 크게 개선된 것을 볼 수 있다.
하지만 내 생각보다 너무 큰 차이가 발생했다. 2개로 분리되어 있던 API를 통합하면서 개선이 된건지, 트랜잭션이 필요없는 부분들을 분리해서 개선이 된건지 잘 모르겠다는 생각이 들었다.
그래서 API를 분리하더라도 수행되는 로직은 기존과 같기 때문에 아무런 로직을 수행하지 않는 API를 추가해서 테스트를 해보았는데 위 결과가 나왔다.
100번 수행 기준 약 4초정도의 개선이 있는 것으로 보이는데, 이 결과를 보아 트랜잭션의 범위를 최적화했다고 볼 수 있을 것 같다.
이 정도 차이는 불필요한 커넥션 점유 시간을 줄인 결과라고 보고 만약 요청이 더 많았다면 100번 기준의 작은 성능 차이도 유의미하다고 볼 수 있을 것 같다는 생각이 든다.
정리
성능 차이가 크게 없음에도 분리해야 하는 이유는 아래와 같이 정리할 수 있을 것 같다.
- 아주 찰나의 시간이라고 하더라도, 커넥션을 의미없이 점유하는 시간이 늘어나게 되므로 트랜잭션의 범위에서 빼는 것이 좋다.
- 커넥션을 소유하는 시간이 길어질 수록 나중에는 커넥션을 얻기 위해 대기하는 상황이 발생할 수도 있기 때문이다.
- 트랜잭션이 필요한 작업과 필요 없는 작업을 명확하게 분리해 코드의 책임을 분명하게 할 수 있다.
- 이는 코드 가독성 및 유지보수성에 도움이 될 수 있다.
피드백을 통해 처음 이 부분을 인지하고 개선해보았는데 실제로 개선된 결과를 보니 트랜잭션 분리가 얼마나 중요한 지 알 수 있었다.
트랜잭션 분리라는 부분에서 나는 기존에 어떤 서비스가 존재할 때 다른 외부 API나 로깅같은 부분이 같은 트랜잭션에 묶이면 메인 서비스 로직과 관련 없는 로깅같은 부분들이 에러가 날 때 롤백할 필요없는 메인 서비스 로직까지 롤백되는 것을 고려하여 분리하는 경우만 생각했다.
그런데 이렇게 DB 커넥션과 관련없는 로직을 분리하는 것도 성능에 영향을 끼친다는 것을 알았다. 피드백을 통해 이런 부분에서도 성능을 개선할 수 있음을 깨달았고 앞으로 트랜잭션의 적용 범위에 대해서 많이 신경써야겠다는 생각을 했다. 피드백이 정말 좋다.