[FIX] 행사 종료 시, 출석 체크가 자동으로 종료되지 않는 오류를 수정합니다. #250
Conversation
- 로그 메세지 수정
- ProgramQuitService 로 스케줄러 이동
- programDate에 대한 설명 추가
- key,value,score 를 사용하는 곳에서 받을 수 있도록 수정
- 스케줄러 추가 - Key 관리
Walkthrough이번 PR은 코드 주석 추가, 서비스 클래스 리팩토링, 새로운 지연 큐 클래스 도입, 불필요한 스케줄러 및 Redis 지연 큐 클래스 삭제, 이벤트 리스너 수정 등의 변경사항을 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as 스케줄러(@Scheduled)
participant PQuit as ProgramQuitService
participant DQueue as DelayedQueue
participant Publisher as ApplicationEventPublisher
participant Listener as EndAttendModeEventListener
Scheduler->>PQuit: trigger quitAttend()
PQuit->>DQueue: getReadyTasks(key, currentTime)
DQueue-->>PQuit: 작업 ID 집합 반환
PQuit->>Publisher: publish(EndAttendModeEvent)
Publisher->>Listener: 전달된 이벤트
Listener->>Listener: 프로세스 및 로그 기록 (출석 상태 갱신)
Assessment against linked issues
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
eeos/src/main/java/com/blackcompany/eeos/program/application/model/ProgramModel.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/support/DelayedQueue.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/support/ProgramAttendScheduler.java(0 hunks)eeos/src/main/java/com/blackcompany/eeos/program/persistence/RedisDelayedQueue.java(0 hunks)eeos/src/main/java/com/blackcompany/eeos/target/application/event/EndAttendModeEventListener.java(1 hunks)
💤 Files with no reviewable changes (2)
- eeos/src/main/java/com/blackcompany/eeos/program/application/support/ProgramAttendScheduler.java
- eeos/src/main/java/com/blackcompany/eeos/program/persistence/RedisDelayedQueue.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (5)
eeos/src/main/java/com/blackcompany/eeos/program/application/model/ProgramModel.java (1)
43-43: 필드 의미를 명확히 밝힌 점은 좋습니다.
해당 주석을 통해programDate가 행사 종료 시간을 의미함을 분명히 드러내어 가독성이 향상되었습니다. 별도의 문제가 없어 보입니다.eeos/src/main/java/com/blackcompany/eeos/target/application/event/EndAttendModeEventListener.java (1)
24-35: 비동기 트랜잭션 처리 시 부분 실패 시나리오를 고려해 주세요.
@Async와@Transactional(propagation = Propagation.REQUIRES_NEW)조합으로 여러programId를 순회 처리할 때, 일부 ID 처리 중 예외가 발생하면 전체 트랜잭션이 롤백되어 나머지 ID에 대한 처리도 무효화될 수 있습니다. 이것이 의도된 동작인지 확인이 필요합니다.eeos/src/main/java/com/blackcompany/eeos/program/application/support/DelayedQueue.java (1)
1-27: RedisTemplate 직렬화 설정을 확인해 주세요.
addTask와getReadyTasks모두Object타입을 직렬화/역직렬화할 때 오류가 발생하지 않도록, RedisTemplate에 적절한 Serializer가 설정되어 있는지 점검하는 것이 좋습니다.eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java (2)
29-33: 행사 종료 시간을 점수로 사용해 DelayedQueue를 관리하는 구현이 합리적입니다.
programDate를 초 단위로 환산해 Redis Sorted Set 점수로 사용하는 방식은 로직이 분명하고 유지보수에도 유리합니다.
35-45: 일정 주기 스케줄링과 이벤트 발행 구조가 깔끔합니다.
@Transactional과@Scheduled를 조합해 매일 자정에 자동으로 종료 로직을 수행하고, 이벤트를 발행함으로써 코드 가독성과 업무 분리가 명확해졌습니다.
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java
Outdated
Show resolved
Hide resolved
- 캐스팅이 안되는 에러 발생을 방지
- 조회하는 명령과 삭제하는 명령을 분리하여, 사용하는 곳에서 조합할 수 있도록 수정
- 출석 체크 자동 종료가 완료된 경우에만 삭제하도록 수정
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java (3)
32-37: 시간 단위 변환에 대한 설명이 필요합니다.프로그램 날짜를 초 단위로 변환하는 로직이 있습니다. 밀리초를 초로 변환하는 이유(Redis의 score 정밀도와 관련된 것으로 보임)에 대한 주석이 있으면 코드의 의도가 더 명확해질 것입니다.
// programDate 를 score 로 사용 -long programDate = model.getProgramDate().getTime() / 1000; +// Redis sorted set의 score는 double 형식이므로 정밀도를 위해 밀리초를 초 단위로 변환 +long programDate = model.getProgramDate().getTime() / 1000;
58-70: 각 프로그램 ID별 예외 처리를 고려해보세요.현재 구현은 하나의 프로그램 처리 중 예외가 발생하면 나머지 프로그램을 처리하지 않게 됩니다. 각 프로그램별로 개별 try-catch 블록을 사용하면 일부 프로그램에서 오류가 발생해도 나머지 프로그램은 계속 처리할 수 있습니다.
private Set<Long> doQuit(Set<Long> programIds) { 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); + try { + log.info("출석 체크 자동 종료 (programId : {})", id); + + programRepository.changeAttendMode(id, ProgramAttendMode.END); + attendRepository.updateAttendStatusByProgramId( + id, AttendStatus.NONRESPONSE, AttendStatus.ABSENT); + + completedIds.add(id); + } catch (Exception e) { + log.error("프로그램 ID {} 처리 중 오류 발생: {}", id, e.getMessage()); + } } return completedIds; }
72-78: ID 파싱 시 예외 처리를 고려해보세요.
Long.parseLong(id.toString())에서 ID가 숫자 형식이 아닌 경우NumberFormatException이 발생할 수 있습니다. 예외 처리를 추가하여 더 안정적인 코드로 만드는 것을 고려해보세요.private Set<Long> getReadyTasks(long programDate) { Set<Long> jobs = delayedQueue.getReadyTasks(KEY, (double) programDate).stream() - .map(id -> Long.parseLong(id.toString())) + .map(id -> { + try { + return Long.parseLong(id.toString()); + } catch (NumberFormatException e) { + log.warn("유효하지 않은 프로그램 ID 형식: {}", id); + return null; + } + }) + .filter(id -> id != null) .collect(Collectors.toSet()); return jobs; }eeos/src/main/java/com/blackcompany/eeos/program/application/support/DelayedQueue.java (5)
1-12: 클래스 구조와 목적에 대한 문서화가 필요합니다.클래스가 잘 구조화되어 있지만, 이 지연 큐의 목적과 Redis Sorted Set을 어떻게 활용하는지에 대한 Javadoc 문서화가 있으면 더 좋을 것 같습니다. 특히 이 클래스가 다른 도메인에서도 사용될 수 있는 추상화된 지원 클래스이므로 더욱 명확한 문서화가 필요합니다.
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; +/** + * Redis Sorted Set을 활용한 지연 큐 구현체입니다. + * 지정된 시간에 실행되어야 하는 작업들을 스케줄링하는 데 사용됩니다. + * 각 작업은 key, value, score(실행 시간) 형태로 저장되며, + * 도메인별로 서로 다른 key를 사용하여 작업을 관리합니다. + */ @Repository @RequiredArgsConstructor public class DelayedQueue { private final RedisTemplate<String, Object> redisTemplate;
14-16: 매개변수 검증과 메서드 문서화가 필요합니다.지연 큐에 작업을 추가하는 중요한 메서드이므로, 매개변수의 유효성 검증과 메서드의 목적을 설명하는 Javadoc을 추가하면 좋을 것 같습니다.
+/** + * 지연 큐에 새로운 작업을 추가합니다. + * @param key 작업을 식별하는 키 + * @param value 실행할 작업 객체 + * @param score 작업이 실행될 시간(Unix 타임스탬프 형식) + */ public void addTask(String key, Object value, double score) { + if (key == null || value == null) { + throw new IllegalArgumentException("Key와 value는 null이 될 수 없습니다."); + } redisTemplate.opsForZSet().add(key, value, score); }
18-22: 메서드 개선 및 문서화가 필요합니다.임시 변수를 사용하지 않고 직접 결과를 반환하면 코드가 더 간결해질 것 같습니다. 또한 메서드의 목적과 동작을 설명하는 Javadoc을 추가하면 좋을 것 같습니다.
+/** + * 지정된 시간(score)까지 실행 준비가 된 작업들을 조회합니다. + * 0부터 주어진 score까지의 범위에 있는 모든 작업을 반환합니다. + * 이 메서드는 작업을 조회만 하고 큐에서 제거하지 않습니다. + * + * @param key 작업을 식별하는 키 + * @param score 현재 시간(Unix 타임스탬프 형식) + * @return 실행 준비가 된 작업 객체들의 집합 + */ public Set<Object> getReadyTasks(String key, double score) { - Set<Object> tasks = redisTemplate.opsForZSet().rangeByScore(key, 0, score); - - return tasks; + return redisTemplate.opsForZSet().rangeByScore(key, 0, score); }
24-26: 매개변수 검증과 메서드 문서화가 필요합니다.시간(score) 기준으로 작업을 제거하는 메서드의 목적과 동작에 대한 문서화가 필요합니다.
+/** + * 지정된 시간(score)까지의 작업들을 큐에서 제거합니다. + * 0부터 주어진 score까지의 범위에 있는 모든 작업이 제거됩니다. + * + * @param key 작업을 식별하는 키 + * @param score 제거할 작업의 최대 시간(Unix 타임스탬프 형식) + */ public void removeByScore(String key, double score) { + if (key == null) { + throw new IllegalArgumentException("Key는 null이 될 수 없습니다."); + } redisTemplate.opsForZSet().removeRangeByScore(key, 0, score); }
28-30: 매개변수 검증과 메서드 문서화가 필요합니다.값을 기준으로 작업을 제거하는 메서드의 목적과 동작에 대한 문서화가 필요합니다.
+/** + * 지정된 값에 해당하는 작업들을 큐에서 제거합니다. + * + * @param key 작업을 식별하는 키 + * @param value 제거할 작업 객체들(여러 개 지정 가능) + */ public void removeByValue(String key, Object... value) { + if (key == null || value == null) { + throw new IllegalArgumentException("Key와 value는 null이 될 수 없습니다."); + } redisTemplate.opsForZSet().remove(key, value); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramService.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/support/DelayedQueue.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/usecase/ProgramQuitUsecase.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/target/application/event/EndAttendModeEventListener.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java (2)
Learnt from: rlajm1203
PR: JNU-econovation/EEOS-BE#250
File: eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java:47-55
Timestamp: 2025-03-31T04:04:35.284Z
Learning: DateConverter.toEpochSecond() 메서드는 long 값이 아닌 Timestamp 객체를 반환하므로 .getTime()을 호출하여 long 타입의 값을 얻어야 합니다.
Learnt from: rlajm1203
PR: JNU-econovation/EEOS-BE#250
File: eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java:47-55
Timestamp: 2025-03-31T04:04:35.284Z
Learning: DateConverter.toEpochSecond(LocalDate) 메소드는 long 값이 아닌 Timestamp 객체를 반환하므로, epoch 시간을 밀리초 단위의 long 값으로 변환하려면 .getTime()을 호출해야 합니다.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (8)
eeos/src/main/java/com/blackcompany/eeos/program/application/usecase/ProgramQuitUsecase.java (1)
7-7: 메소드 이름 변경이 더 명확합니다!메소드 이름이
pushQuitAttendJob에서reserveQuitProgram으로 변경되었습니다. 새 이름이 메소드의 기능을 더 명확하게 표현하며 코드 가독성이 향상되었습니다.eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramService.java (1)
93-93: 인터페이스 변경에 맞게 메소드 호출이 업데이트되었습니다.
ProgramQuitUsecase인터페이스의 메소드 이름 변경에 맞게 호출 코드가 올바르게 업데이트되었습니다.eeos/src/main/java/com/blackcompany/eeos/target/application/event/EndAttendModeEventListener.java (3)
25-26: 이벤트 리스너 타입이 변경되었습니다.
@TransactionalEventListener에서@EventListener로 변경되었습니다. 이제 이벤트가 트랜잭션 완료 후가 아닌 발생 즉시 처리됩니다.@Transactional(propagation = Propagation.REQUIRES_NEW)어노테이션이 있어 트랜잭션 관리는 여전히 안전합니다.
27-37: 이벤트 처리 로직이 개선되었습니다.로그 메시지가 추가되고 코드 구조가 더 명확해졌습니다. 프로그램 ID 목록을 반복하면서 각각에 대해 출석 모드를 변경하고 출석 상태를 업데이트하는 방식이 잘 구현되어 있습니다.
39-39: 로그 메시지가 잘 추가되었습니다.종료할 프로그램이 없는 경우에도 적절한 로그 메시지를 출력하여 모니터링이 용이해졌습니다.
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java (3)
3-30: 의존성 및 필드 변경이 잘 구현되었습니다.
RedisDelayedQueue에서 추상화된DelayedQueue인터페이스로 변경하여 유연성이 향상되었습니다. 상수KEY를 통해 큐 키의 일관성을 유지할 수 있게 되었습니다. 필요한 의존성이 잘 추가되었습니다.
39-56: 스케줄링된 작업이 잘 구현되었습니다.매일 자정에 실행되는 스케줄링 작업이 추가되었습니다. 기존 스케줄러가 삭제되고 이 클래스에서 직접 처리하도록 변경된 것은 좋은 개선입니다. 예외 처리와 로깅도 적절하게 구현되어 있습니다.
이전 리뷰 코멘트에서 확인된 바와 같이
DateConverter.toEpochSecond()가 Timestamp를 반환하므로.getTime()을 호출하는 것이 맞습니다.
80-82: 완료된 작업 제거 메소드가 잘 구현되었습니다.완료된 작업을 큐에서 제거하는 메소드가 잘 구현되어 있습니다. 이를 통해 처리 완료된 작업이 불필요하게 저장되지 않게 되었습니다.
eeos/src/main/java/com/blackcompany/eeos/program/application/support/DelayedQueue.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java (3)
39-56: 출석 체크 자동 종료 기능이 스케줄링으로 개선되었습니다.매일 자정에 실행되도록 @scheduled 애노테이션이 적용되었고, 트랜잭션 처리를 위한 @transactional 애노테이션이 추가되었습니다. 에러 처리 로직이 포함되어 안정성이 향상되었습니다.
다만, 에러가 발생했을 때 예외를 다시 던지는 것이 적절한지 검토해볼 필요가 있습니다. 스케줄링된 작업에서는 예외를 던지면 다음 작업 실행에 영향을 줄 수 있습니다.
- } catch (Exception e) { - log.error("행사 자동 종료 중 에러가 발생하였습니다. {}", e.getMessage()); - throw e; - } + } catch (Exception e) { + log.error("행사 자동 종료 중 에러가 발생하였습니다.", e); + }
79-85: getReadyTasks 메서드의 반환 방식을 개선할 수 있습니다.스트림 처리 결과를 중간 변수에 할당한 후 바로 반환하는 패턴입니다. 불필요한 할당을 제거하여 코드를 더 간결하게 만들 수 있습니다.
- 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 Set<Long> getReadyTasks(long programDate) { + return delayedQueue.getReadyTasks(KEY, (double) programDate).stream() + .map(id -> Long.parseLong(id.toString())) + .collect(Collectors.toSet()); + }
32-89: 전체적인 구조 개선에 대한 제안DelayedQueue를 사용한 작업 예약 및 실행 패턴이 잘 구현되었습니다. 몇 가지 개선 사항을 제안합니다:
예외 처리: quitAttend 메서드에서 발생할 수 있는 구체적인 예외 유형(IllegalArgumentException, IOException 등)을 개별적으로 처리하면 더 정확한 오류 진단이 가능합니다.
시간 단위 일관성: reserveQuitProgram과 quitAttend 메서드에서 시간을 초 단위로 일관되게 사용하도록 수정하세요.
로깅 강화: 중요한 지점에서 더 상세한 로깅을 추가하면 문제 추적이 용이해집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java (2)
Learnt from: rlajm1203
PR: JNU-econovation/EEOS-BE#250
File: eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java:47-55
Timestamp: 2025-03-31T04:04:35.284Z
Learning: DateConverter.toEpochSecond() 메서드는 long 값이 아닌 Timestamp 객체를 반환하므로 .getTime()을 호출하여 long 타입의 값을 얻어야 합니다.
Learnt from: rlajm1203
PR: JNU-econovation/EEOS-BE#250
File: eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java:47-55
Timestamp: 2025-03-31T04:04:35.284Z
Learning: DateConverter.toEpochSecond(LocalDate) 메소드는 long 값이 아닌 Timestamp 객체를 반환하므로, epoch 시간을 밀리초 단위의 long 값으로 변환하려면 .getTime()을 호출해야 합니다.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (5)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java (5)
3-20: 임포트 구문이 정리되고 필요한 클래스가 추가되었습니다.새로운 기능 구현을 위해 DelayedQueue, AttendStatus, LocalDate, Set 등이 추가되었고, @slf4j 및 @scheduled 애노테이션을 사용하기 위한 임포트가 적절히 추가되었습니다.
21-21: 로깅 기능 추가 및 의존성 변경이 적절합니다.기존 RedisDelayedQueue에서 추상화된 DelayedQueue로 변경되었고, AttendRepository가 추가되었습니다. 상수 KEY를 사용하여 큐의 식별자를 명시적으로 정의한 것이 좋습니다.
Also applies to: 26-29
32-37: 메서드 이름이 더 명확해졌으며 구현이 단순화되었습니다.기존
pushQuitAttendJob에서reserveQuitProgram으로 메서드 이름이 변경되어 기능을 더 명확하게 표현합니다. 시간을 초 단위로 변환하는 부분이 추가되었습니다.
58-77: 출석 체크 종료 처리 로직이 잘 구현되었습니다.출석 체크 종료 시 ProgramAttendMode를 END로 변경하고, 무응답(NONRESPONSE) 상태의 출석을 결석(ABSENT)으로 업데이트하는 로직이 구현되어 있습니다. 빈 리스트 처리와 로깅이 적절히 포함되어 있습니다.
87-89: removeCompleteTask 메서드가 간결하게 구현되었습니다.완료된 작업을 큐에서 제거하는 기능이 단순하고 명확하게 구현되었습니다.
| long programDate = DateConverter.toEpochSecond(LocalDate.now()).getTime(); | ||
|
|
There was a problem hiding this comment.
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.
| long programDate = DateConverter.toEpochSecond(LocalDate.now()).getTime(); | |
| long programDate = DateConverter.toEpochSecond(LocalDate.now()).getTime() / 1000; |
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java
Show resolved
Hide resolved
| "출석 체크 종료 Transaction committed: {}", | ||
| TransactionSynchronizationManager.isActualTransactionActive()); | ||
| @EventListener(EndAttendModeEvent.class) | ||
| public void handle(EndAttendModeEvent event) { |
There was a problem hiding this comment.
p3.
이제 EndAttendModeEvent 클래스를 더 이상 사용하지 않는 거죠?
이벤트 방식을 변경한 이유가 궁금한데, 혹시 설명해주실 수 있을까요? 😊
There was a problem hiding this comment.
처음에 이벤트로 처리했던 이유는, 종료할 행사 대상을 조회하는 클래스와, 실제로 행사 종료 작업을 수행하는 클래스의 결합도를 낮추기 위해서 적용했습니다.
하지만 생각해보니, 굳이 그럴 이유가 없다는 생각이 들었습니다. (하나의 같은 연결된 작업이라고 생각했습니다.)
그리고 행사를 자동으로 종료해야 하는데, 작업 중 에러가 발생할 경우도 있습니다.
이전에는 큐에서 꺼내는 즉시 삭제했는데, 이렇게 될 경우 자동으로 종료하는 작업을 수행하다가 예외가 발생하면 종료해야 할 행사의 id 정보는 어디서도 찾을 수 없게 됩니다.
그래서 성공적으로 종료된 행사만 Queue에서 제거하도록 하는 방안이 좋을 것 같아 동기로 처리하여 완료된 programId 를 받을 수 있도록 하였습니다.
| delayedQueue.addTask(KEY, model.getId(), programDate); | ||
| } | ||
|
|
||
| @Transactional |
There was a problem hiding this comment.
p3.
현재 구조에서는 행사 자동 종료 중 하나에서 에러가 발생하면, 상위에서 트랜잭션이 관리되기 때문에 모든 행사의 자동 종료가 중단될 가능성이 있을 것 같아요.
하지만 한 프로그램의 자동 종료가 실패했다고 해서 다른 프로그램까지 롤백되거나 시도조차 못 할 이유는 없다고 생각해요.
그래서 개별 단위로 트랜잭션을 관리하는 방식도 좋은 대안이 될 것 같아요!
| } | ||
|
|
||
| @Transactional | ||
| @Scheduled(cron = "0 0 0 * * *") |
There was a problem hiding this comment.
p3.
스케줄링을 사용할 경우, 두 가지 방식이 가능할 것 같아요.
- 큐에 종료 시간을 저장해두고, 스케줄러가 실행될 때 해당 시간이 지났는지 확인한 후 종료 처리를 수행하는 방법
- 스케줄링 시점에 직접 repository 를 조회하여 종료 시간이 지났지만 아직 종료되지 않은 항목들을 찾아 처리하는 방법
GPT에게 물어보니, 데이터가 많을 경우에는 큐 방식이 더 적절하다고 추천하더라고요. 😆
전 간단하게 생각해서 2번을 생각했습니다..ㅎ(이런 방식도 있을 수 있겠다~ 공유입니다!)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java (1)
79-85: getReadyTasks 메서드에서 불필요한 타입 변환이 있습니다.타입 변환 과정에서 불필요한
.toString()호출이 있습니다. DelayedQueue의 getReadyTasks 메서드가 이미 Long 타입의 Set을 반환한다면 이 변환은 생략할 수 있습니다.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; + return delayedQueue.getReadyTasks(KEY, (double) programDate); }만약 DelayedQueue가 다른 타입을 반환한다면, 더 간결한 방식으로 변환할 수 있습니다:
private Set<Long> getReadyTasks(long programDate) { return delayedQueue.getReadyTasks(KEY, (double) programDate).stream() .map(id -> Long.valueOf(id.toString())) .collect(Collectors.toSet()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java (2)
Learnt from: rlajm1203
PR: JNU-econovation/EEOS-BE#250
File: eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java:47-55
Timestamp: 2025-03-31T04:04:35.284Z
Learning: DateConverter.toEpochSecond() 메서드는 long 값이 아닌 Timestamp 객체를 반환하므로 .getTime()을 호출하여 long 타입의 값을 얻어야 합니다.
Learnt from: rlajm1203
PR: JNU-econovation/EEOS-BE#250
File: eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java:47-55
Timestamp: 2025-03-31T04:04:35.284Z
Learning: DateConverter.toEpochSecond(LocalDate) 메소드는 long 값이 아닌 Timestamp 객체를 반환하므로, epoch 시간을 밀리초 단위의 long 값으로 변환하려면 .getTime()을 호출해야 합니다.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (7)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java (7)
26-29: 필드 주입 및 상수 선언이 적절합니다.DelayedQueue로의 전환과 AttendRepository 추가가 잘 이루어졌습니다. KEY 상수를 명시적으로 선언한 것도 유지보수성 측면에서 좋은 접근입니다.
31-37: 예약 작업 추가 로직이 간결하게 작성되었습니다.프로그램 종료 예약 로직이 간결하게 구현되었습니다. ProgramModel에서 직접 날짜와 ID를 추출하여 지연 큐에 추가하는 방식이 명확합니다.
39-56: 트랜잭션 범위에 대한 고려가 필요합니다.현재 구조에서는 하나의 트랜잭션으로 모든 종료 작업을 처리하고 있습니다. 이로 인해 한 프로그램의 종료 과정에서 오류가 발생하면 다른 모든 프로그램의 종료 작업이 롤백될 수 있습니다.
각 프로그램 종료를 개별 트랜잭션으로 처리하는 방식을 고려해보세요:
- @Transactional @Scheduled(cron = "0 0 0 * * *") public void quitAttend() { log.info("출석 체크 자동 종료 시작"); try { long programDate = DateConverter.toEpochSecond(LocalDate.now()).getTime(); Set<Long> jobs = getReadyTasks(programDate); Set<Long> completedIds = doQuit(jobs); removeCompleteTask(completedIds); } catch (Exception e) { log.error("행사 자동 종료 중 에러가 발생하였습니다. {}", e.getMessage()); - throw e; + // 전체 작업을 중단하지 않고 로깅만 수행 } }그리고 doQuit 메서드에서 각 작업을 개별 트랜잭션으로 처리:
@Transactional(propagation = Propagation.REQUIRES_NEW) private void processProgram(Long id) { try { programRepository.changeAttendMode(id, ProgramAttendMode.END); attendRepository.updateAttendStatusByProgramId( id, AttendStatus.NONRESPONSE, AttendStatus.ABSENT); } catch (Exception e) { log.error("프로그램 ID: {} 종료 중 오류 발생: {}", id, e.getMessage()); // 개별 프로그램 오류는 다른 프로그램에 영향을 주지 않음 } }
44-44: DateConverter 사용이 올바르게 수정되었습니다만, 시간 단위 일관성 확인이 필요합니다.이전 리뷰 피드백을 반영하여
DateConverter.toEpochSecond(LocalDate.now()).getTime()을 사용하고 있습니다. 그러나 reserveQuitProgram 메서드와 quitAttend 메서드 간에 시간 단위의 일관성을 확인해야 합니다.현재 코드에서 reserveQuitProgram의 programDate와 quitAttend의 programDate는 모두 밀리초 단위를 사용하는 것으로 보입니다. 하지만 이전 코드에서는 초 단위 변환(1000으로 나누기)이 있었습니다. DelayedQueue 인터페이스의 구현체가 어떤 시간 단위를 기대하는지 확인해보세요.
58-77: doQuit 메서드가 효율적으로 구현되었습니다.프로그램 종료 및 출석 상태 업데이트 로직이 잘 구현되었습니다. 빈 세트에 대한 처리도 적절히 추가된 점이 좋습니다.
87-89: removeCompleteTask 메서드가 적절히 구현되었습니다.완료된 작업을 큐에서 제거하는 로직이 간결하게 구현되었습니다.
79-79: 불필요한 쿼리 실행 가능성이 있습니다.이전 리뷰에서 지적된 바와 같이, programDate만을 기준으로 프로그램을 조회하면 이미 종료된 상태의 프로그램도 다시 가져와서 END 상태로 변경하는 불필요한 쿼리가 발생할 수 있습니다.
📌 관련 이슈
closes #181
문제 상황
불참으로 변경문제 원인
서버 로그
✒️ 작업 내용
이전에 Redis Sorted Set을 활용한 지연 큐를 사용하여 구현된 부분을 수정하였습니다.
예약 작업이 필요한 다른 도메인에서도 사용할 수 있도록 추상화 하였습니다.
이때 사용할 key 는 delayed 큐를 사용하는 곳에서 관리해야 합니다.
또한 저장하는 모든 Task 관련 Value 는 Object로 감싸므로, value 역시 사용하는 곳에서 다운 캐스팅 해야 합니다.
토글
수정된 Delayed 큐를 사용하여, 행사는 종료되었지만 출석 체크 모드가 켜진 행사를 자동으로 종료하고 미응답을 불참으로 변경합니다.
AttendMode
ATTEND->ENDAttendStatus
NON_RESPONSE->ABSENT토글
3.31 변경 사항
💬 REVIEWER에게 요구사항 💬
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Chores