Repository navigation
fix: [ALT-301] 근무 배정·대타 승인 동시 요청의 시간 충돌 직렬화 - #111
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
hodoon
left a comment
hodoon
left a comment
There was a problem hiding this comment.
리뷰했습니다. 인라인 코멘트 확인 부탁드립니다.
| WorkspaceWorker workspaceWorker = schedule.getWorkspaceWorker(); | ||
| Long userId = workspaceWorker.getUser().getId(); | ||
| List<WorkspaceShift> existingShifts = existingShiftsByUserId.getOrDefault(userId, List.of()); | ||
| List<WorkspaceShift> existingShifts = latestShiftsByUserId.getOrDefault(userId, List.of()); |
There was a problem hiding this comment.
이제 겹침 판정이 잠금 후 다시 조회한 latestShiftsByUserId만 사용해서, existingShiftsByUserId 파라미터와 Javadoc은 더 이상 의미가 없어진 것 같습니다.
호출 측 GenerateNextMonthWorkspaceShift도 buildExistingShiftsMap 사전 조회와 커밋 후 맵 병합을 그대로 수행하고 있어, 결과에 반영되지 않는 쿼리가 배치마다 한 번씩 나가고 있습니다.
Tx가 업장마다 커밋한 뒤 DB를 다시 조회하므로 앞선 업장에서 만든 근무도 그 조회에 포함됩니다. 파라미터와 사전 조회·병합 코드를 함께 정리하면 어떨까요?
| List<WorkspaceShift> findByWorkspaceAndDateRange(Workspace workspace, int year, int month); | ||
| List<WorkspaceShift> findByManagerAndDateRange(ManagerUser managerUser, Long workspaceId, int year, int month); | ||
| Optional<WorkspaceShift> findById(Long id); | ||
| Optional<WorkspaceShift> findByIdForUpdate(Long id); |
There was a problem hiding this comment.
기존 port들(SubstituteRequestQueryRepository, WorkspaceQueryRepository, PostingQueryRepository)은 비관적 잠금 조회에 findByIdWithPessimisticLock이라는 이름을 쓰고 있어서, 이름을 findByIdWithPessimisticLock / findAllByIdWithPessimisticLock으로 맞추면 일관성이 생길 것 같습니다.
또 SET LOCAL lock_timeout = '5s' 문자열이 두 어댑터에 직접 적혀 있는데, PostingQueryRepositoryImpl처럼 LOCK_TIMEOUT_STATEMENT 상수와 H2 비호환 주석을 두는 방식을 따르면 어떨까요?
겹침 판정이 사용자 잠금 후 재조회한 근무만 쓰게 되면서 existingShiftsByUserId 파라미터와 배치의 사전 조회·맵 병합이 결과에 반영되지 않는 쿼리로 남았다. 업장 Tx가 커밋 후 다시 조회하므로 앞선 업장 생성분도 판정에 포함돼 제거해도 동작은 같다. 잠금 조회 메서드는 기존 포트 관례(findByIdWithPessimisticLock)에 맞추고 lock_timeout 문은 PostingQueryRepositoryImpl과 같은 LOCK_TIMEOUT_STATEMENT 상수로 둔다.
겹침 판정이 사용자 잠금 후 재조회한 근무만 쓰게 되면서 existingShiftsByUserId 파라미터와 배치의 사전 조회·맵 병합이 결과에 반영되지 않는 쿼리로 남았다. 업장 Tx가 커밋 후 다시 조회하므로 앞선 업장 생성분도 판정에 포함돼 제거해도 동작은 같다. 잠금 조회 메서드는 기존 포트 관례(findByIdWithPessimisticLock)에 맞추고 lock_timeout 문은 PostingQueryRepositoryImpl과 같은 LOCK_TIMEOUT_STATEMENT 상수로 둔다.
사용자 잠금 직렬화로 같은 근무자 동시 요청이 잠금 대기 초과 시 429(E001)를 받을 수 있고, 잠금 후 재검사로 겹침 400(B001)과 중복 배정 409(B020)가 동시 요청에서도 확실히 나오는데 문서에 빠져 있어 클라이언트가 처리 대상을 알 수 있도록 남긴다.
동일 사용자의 겹치는 근무가 충돌 검사와 저장 사이의 동시 요청으로 모두 확정되는 문제를 고쳤습니다. 배정·근무자 교체·시간 변경·대타 승인·고정 생성이 사용자 행 잠금을 공유하고 잠금 후 최신 충돌을 검사해 같은 트랜잭션에 저장합니다.
변경
검증
JAVA_TOOL_OPTIONS=-Dapi.version=1.44 ./gradlew clean build최초 전체536개 성공(1m1s, skipped/실패0); 아래 test-only 후속의 최신 전체536개 성공(1m). 임시 Testcontainers 정리 완료.기존 별도 관찰·계약 보존
삭제한 원근무의 대타 승인으로 일정이 CONFIRMED로 부활하는 기존 동작을 PG/HTTP에서 재현했습니다. 이번 잠금 작업이 무관한 상태 정책을 바꾸지 않도록 기존 승인 및 수동3경로 삭제일정404를 보존했습니다. 부활 결함은 이 PR에서 미해결이며 ALT305에서 별도 수정했고 독립 HTTP E2E 검증 중입니다. 위 부활 관찰은 standalone ALT301 당시의 기존 동작입니다.
겹침·병합·통합
관련 작업: ALT-301
ALT305 통합을 위한 test-only 후속
findByIdForUpdate가 DELETED 행도 읽으며 상태를 변경하지 않는다는 ALT301의 실제 query 계약 검증으로 바꿨습니다.JAVA_TOOL_OPTIONS=-Dapi.version=1.44 ./gradlew clean build전체536개 통과(1m, skipped/실패0).src/main운영 diff SHA102937e5e059aedd70552fbc2f34b75c0fea7980a가 기존 HTTP E2E 코드와 동일합니다. 기존 E2E의 end-diff/sql-assertions/assertions SHA 검증도 모두 일치합니다. 이번 보완을 새 HTTP E2E 수행으로 표현하지 않습니다.