Conversation
yejun05011
requested review from
BYEON-YOUNGVIN,
Dayoung0402,
glaxyt and
juhui4078
Dayoung0402
left a comment
Dayoung0402
left a comment
There was a problem hiding this comment.
수고하셨습니다~! 리뷰 확인하시고 반영할 부분 반영하고 머지하시면 될 것 같아요😃
| RankingPeriod.SEMESTER, | ||
| config.getEndAt().toLocalDate().toString() | ||
| ); | ||
| LocalDate.now().getYear() + "-1" |
There was a problem hiding this comment.
LocalDate.now() 사용할 때 서버 타임존에 따라 연도 계산이 달라질 가능성이 있어 보여서, ZoneId를 명시하는 것도 고려하면 좋을 것 같아요!!
✅️LocalDate.now() -> 서버가 설정해둔 기본 시간대를 기준으로 오늘 날짜를 가져옴
✅️Year.now(ZoneId.of("Asia/Seoul")) -> 서버 시간이 뭐든 상관 없이 한국 시간 기준으로 계산
현재는 연도만 사용하고 있어서 큰 문제 없어보이지만 안정성을 조금만 더 고려하면 좋을 것 같다는 생각이 들었습니다 🤔
There was a problem hiding this comment.
LocalDate.now() 사용할 때 서버 타임존에 따라 연도 계산이 달라질 가능성이 있어 보여서, ZoneId를 명시하는 것도 고려하면 좋을 것 같아요!! ✅️LocalDate.now() -> 서버가 설정해둔 기본 시간대를 기준으로 오늘 날짜를 가져옴 ✅️Year.now(ZoneId.of("Asia/Seoul")) -> 서버 시간이 뭐든 상관 없이 한국 시간 기준으로 계산 현재는 연도만 사용하고 있어서 큰 문제 없어보이지만 안정성을 조금만 더 고려하면 좋을 것 같다는 생각이 들었습니다 🤔
타임존 관련 부분은 Asia/Seoul 기준으로 명시하도록 수정했습니다!
| public void secondSemesterSnapshot() { | ||
| snapshotJob.execute( | ||
| RankingPeriod.SEMESTER, | ||
| LocalDate.now().getYear() + "-2" |
There was a problem hiding this comment.
라인 52번째 줄에 남긴 리뷰 내용이 동일하게 적용되는 부분입니다!
|
|
||
| snapshotJob.execute( | ||
| // 1학기 - 매년 7월 1일 00:00 | ||
| @Scheduled(cron = "0 0 0 1 7 ?") |
There was a problem hiding this comment.
스케줄은 서버 재시작 시에 중복 실행될 가능성이 있어 보이는데,
동일 학기에 대해 snapshotJob.execute가 두번 실행되지 않도록 idempotency고려해보는건 어떨까요??
예시 상황)
7월 1일 00:00 에 실행되어야 하는 스케줄러 코드
그런데 6월 30일 밤에 배포함, 7월 1일 00:00 직전에 서버가 재시작됨
재시작 이후에 스케줄이 실행되고, 환경에 따라 동일 학기에 대해 한번더 실행될 가능성이 존재합니다.
현재 코드가 올라갈 서버가 1개라서 괜찮을 것 같지만, 향후 예상치 못한 상황을 대비하여 동일 periodKey에 대해 한 번만 생성되도록 로직을 보완하는 것도 좋은 경험이 될 것 같습니다 :)
There was a problem hiding this comment.
스케줄은 서버 재시작 시에 중복 실행될 가능성이 있어 보이는데, 동일 학기에 대해 snapshotJob.execute가 두번 실행되지 않도록 idempotency고려해보는건 어떨까요??
예시 상황) 7월 1일 00:00 에 실행되어야 하는 스케줄러 코드 그런데 6월 30일 밤에 배포함, 7월 1일 00:00 직전에 서버가 재시작됨
재시작 이후에 스케줄이 실행되고, 환경에 따라 동일 학기에 대해 한번더 실행될 가능성이 존재합니다.
현재 코드가 올라갈 서버가 1개라서 괜찮을 것 같지만, 향후 예상치 못한 상황을 대비하여 동일 periodKey에 대해 한 번만 생성되도록 로직을 보완하는 것도 좋은 경험이 될 것 같습니다 :)
ranking_snapshot(period, period_key)에 UNIQUE 제약을 추가하여 동일 periodKey에 대해 중복 스냅샷이 생성되지 않도록 DB 레벨에서 보장하고 있습니다! (머지 후 노션 ddl 업데이트 예정입니다.)
| config.markProcessed(); // 처리 완료 표시 | ||
| } | ||
| // 2학기 - 매년 11월 1일 00:00 | ||
| @Scheduled(cron = "0 0 0 1 11 ?") |
There was a problem hiding this comment.
라인 48번째 줄에 남긴 리뷰 내용이 동일하게 적용되는 부분입니다!
…into refactor/BE-#621/semester-snapshot-scheduler
2 participants
PR 종류
변경 사항
관련 이슈
체크리스트
상세 내용
1. ranking_config 정리
학기 스냅샷 전용 필드로 사용되던 end_at, processed를 제거하여
ranking_config를 기간 시작 기준 설정 테이블의 본래 역할로 정리하였습니다.
2. Snapshot 중복 방지 제약 추가
ranking_snapshot(period, period_key)에 유니크 제약을 추가하여
동일 기간에 스냅샷이 중복 생성되지 않도록 DB 레벨에서 보장하도록 수정하였습니다.
기타