Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class ProgramModel implements AbstractModel {
private Long id;
private String title;
private String content;
private Timestamp programDate;
private Timestamp programDate; // programDate 는 행사 종료 시간
private String eventStatus;
private ProgramCategory programCategory;
private String githubUrl;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,90 @@
package com.blackcompany.eeos.program.application.service;

import com.blackcompany.eeos.common.utils.DateConverter;
import com.blackcompany.eeos.program.application.model.ProgramAttendMode;
import com.blackcompany.eeos.program.application.model.ProgramModel;
import com.blackcompany.eeos.program.application.support.DelayedQueue;
import com.blackcompany.eeos.program.application.usecase.ProgramQuitUsecase;
import com.blackcompany.eeos.program.persistence.ProgramRepository;
import com.blackcompany.eeos.program.persistence.RedisDelayedQueue;
import java.time.Instant;
import com.blackcompany.eeos.target.application.model.AttendStatus;
import com.blackcompany.eeos.target.persistence.AttendRepository;
import java.time.LocalDate;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Collectors;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Slf4j
@Service
@RequiredArgsConstructor
public class ProgramQuitService implements ProgramQuitUsecase {

private final RedisDelayedQueue redisDelayedQueue;
private final DelayedQueue delayedQueue;
private final AttendRepository attendRepository;
private final ProgramRepository programRepository;
private final String KEY = "quit_program_reservation";

@Override
public void pushQuitAttendJob(ProgramModel model) {
long delayedTime = model.getProgramDate().getTime() - Instant.now().toEpochMilli();
redisDelayedQueue.addTask(model.getId(), delayedTime);
public void reserveQuitProgram(ProgramModel model) {
// programDate 를 score 로 사용
long programDate = model.getProgramDate().getTime() / 1000;

delayedQueue.addTask(KEY, model.getId(), programDate);
}

@Transactional
Copy link
Contributor

Choose a reason for hiding this comment

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

p3.
현재 구조에서는 행사 자동 종료 중 하나에서 에러가 발생하면, 상위에서 트랜잭션이 관리되기 때문에 모든 행사의 자동 종료가 중단될 가능성이 있을 것 같아요.

하지만 한 프로그램의 자동 종료가 실패했다고 해서 다른 프로그램까지 롤백되거나 시도조차 못 할 이유는 없다고 생각해요.

그래서 개별 단위로 트랜잭션을 관리하는 방식도 좋은 대안이 될 것 같아요!

@Scheduled(cron = "0 0 0 * * *")
Copy link
Contributor

Choose a reason for hiding this comment

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

p3.

스케줄링을 사용할 경우, 두 가지 방식이 가능할 것 같아요.

  1. 큐에 종료 시간을 저장해두고, 스케줄러가 실행될 때 해당 시간이 지났는지 확인한 후 종료 처리를 수행하는 방법
  2. 스케줄링 시점에 직접 repository 를 조회하여 종료 시간이 지났지만 아직 종료되지 않은 항목들을 찾아 처리하는 방법

GPT에게 물어보니, 데이터가 많을 경우에는 큐 방식이 더 적절하다고 추천하더라고요. 😆
전 간단하게 생각해서 2번을 생각했습니다..ㅎ(이런 방식도 있을 수 있겠다~ 공유입니다!)

public void quitAttend() {
log.info("출석 체크 자동 종료 시작");
try {
long programDate = DateConverter.toEpochSecond(LocalDate.now()).getTime();

Comment on lines +44 to +45
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

DateConverter 사용 시 주의가 필요합니다.

이전 리뷰에서 논의된 바와 같이, DateConverter.toEpochSecond()는 Timestamp 객체를 반환하므로 .getTime()을 호출하여 long 값을 얻는 것이 맞습니다. 다만, 변환된 시간이 초 단위인지 밀리초 단위인지 일관성을 유지해야 합니다.

reserveQuitProgram 메서드에서는 밀리초 값을 1000으로 나누어 초 단위로 변환하고 있는데, 여기서는 그대로 밀리초 값을 사용하고 있습니다. 두 메서드 간의 일관성이 필요합니다.

-		long programDate = DateConverter.toEpochSecond(LocalDate.now()).getTime();
+		long programDate = DateConverter.toEpochSecond(LocalDate.now()).getTime() / 1000;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
long programDate = DateConverter.toEpochSecond(LocalDate.now()).getTime();
long programDate = DateConverter.toEpochSecond(LocalDate.now()).getTime() / 1000;

Set<Long> jobs = getReadyTasks(programDate);

Set<Long> completedIds = doQuit(jobs);

removeCompleteTask(completedIds);

} catch (Exception e) {
log.error("행사 자동 종료 중 에러가 발생하였습니다. {}", e.getMessage());
throw e;
}
}

private Set<Long> doQuit(Set<Long> programIds) {
if (!programIds.isEmpty()) {
Set<Long> completedIds = new HashSet<>();

for (Long id : programIds) {
log.info("출석 체크 자동 종료 (programId : {})", id);

programRepository.changeAttendMode(id, ProgramAttendMode.END);
attendRepository.updateAttendStatusByProgramId(
id, AttendStatus.NONRESPONSE, AttendStatus.ABSENT);

completedIds.add(id);
}

return completedIds;
}

log.info("종료할 행사가 존재하지 않습니다.");
return new HashSet<>();
}

private Set<Long> getReadyTasks(long programDate) {
Set<Long> jobs =
delayedQueue.getReadyTasks(KEY, (double) programDate).stream()
.map(id -> Long.parseLong(id.toString()))
.collect(Collectors.toSet());
return jobs;
}

private void removeCompleteTask(Set<Long> programIds) {
delayedQueue.removeByValue(KEY, programIds);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public CommandProgramResponse create(final Long memberId, final CreateProgramReq

attendTargetService.save(saveId, request.getMembers());
presentTeamUsecase.save(saveId, request.getTeamIds());
quitUsecase.pushQuitAttendJob(model.toBuilder().id(saveId).build());
quitUsecase.reserveQuitProgram(model.toBuilder().id(saveId).build());

return responseConverter.from(saveId);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.blackcompany.eeos.program.application.support;

import java.util.Set;
import lombok.RequiredArgsConstructor;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.stereotype.Repository;

@Repository
@RequiredArgsConstructor
public class DelayedQueue {

private final RedisTemplate<String, Object> redisTemplate;

public void addTask(String key, Object value, double score) {
redisTemplate.opsForZSet().add(key, value, score);
}

public Set<Object> getReadyTasks(String key, double score) {
Set<Object> tasks = redisTemplate.opsForZSet().rangeByScore(key, 0, score);

return tasks;
}

public void removeByScore(String key, double score) {
redisTemplate.opsForZSet().removeRangeByScore(key, 0, score);
}

public void removeByValue(String key, Object... value) {
redisTemplate.opsForZSet().remove(key, value);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@

public interface ProgramQuitUsecase {

void pushQuitAttendJob(ProgramModel model);
void reserveQuitProgram(ProgramModel model);
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@
import com.blackcompany.eeos.target.persistence.AttendRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.event.EventListener;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.event.TransactionPhase;
import org.springframework.transaction.event.TransactionalEventListener;
import org.springframework.transaction.support.TransactionSynchronizationManager;

@Component
@RequiredArgsConstructor
Expand All @@ -23,21 +21,21 @@ public class EndAttendModeEventListener {
private final ProgramRepository programRepository;

@Async
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
@Transactional(propagation = Propagation.REQUIRES_NEW)
public void handleDeletedProgram(EndAttendModeEvent event) {
log.info(
"출석 체크 종료 Transaction committed: {}",
TransactionSynchronizationManager.isActualTransactionActive());
@EventListener(EndAttendModeEvent.class)
public void handle(EndAttendModeEvent event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

p3.
이제 EndAttendModeEvent 클래스를 더 이상 사용하지 않는 거죠?
이벤트 방식을 변경한 이유가 궁금한데, 혹시 설명해주실 수 있을까요? 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

처음에 이벤트로 처리했던 이유는, 종료할 행사 대상을 조회하는 클래스와, 실제로 행사 종료 작업을 수행하는 클래스의 결합도를 낮추기 위해서 적용했습니다.

하지만 생각해보니, 굳이 그럴 이유가 없다는 생각이 들었습니다. (하나의 같은 연결된 작업이라고 생각했습니다.)

그리고 행사를 자동으로 종료해야 하는데, 작업 중 에러가 발생할 경우도 있습니다.
이전에는 큐에서 꺼내는 즉시 삭제했는데, 이렇게 될 경우 자동으로 종료하는 작업을 수행하다가 예외가 발생하면 종료해야 할 행사의 id 정보는 어디서도 찾을 수 없게 됩니다.

그래서 성공적으로 종료된 행사만 Queue에서 제거하도록 하는 방안이 좋을 것 같아 동기로 처리하여 완료된 programId 를 받을 수 있도록 하였습니다.

log.info("출석 체크 자동 종료 시작");

for (Long id : event.getProgramIds()) {
programRepository.changeAttendMode(id, ProgramAttendMode.END);
attendRepository.updateAttendStatusByProgramId(
id, AttendStatus.NONRESPONSE, AttendStatus.ABSENT);
if (!event.getProgramIds().isEmpty()) {
for (Long id : event.getProgramIds()) {
log.info("출석 체크 자동 종료 (programId : {})", id);
programRepository.changeAttendMode(id, ProgramAttendMode.END);
attendRepository.updateAttendStatusByProgramId(
id, AttendStatus.NONRESPONSE, AttendStatus.ABSENT);
}
return;
}

if (event.getProgramIds().isEmpty()) {
log.info("종료할 프로그램이 없습니다.");
}
log.info("종료할 프로그램이 없습니다.");
}
}