유저 활동 카운터의 Lost Update 방지 (@Version + 격리 재시도) (#251)

* 유저 활동 카운터(createdCourse 등)의 Lost Update 방지

RunnectUser.createdCourse/createdRecord/createdScrap/createdPublicCourse가
순수 in-memory ++ 로만 구현돼 있어, 동시 요청이 같은 유저 row를 커밋 전에
읽으면 한쪽 증가분이 조용히 유실될 수 있었다(로컬 Postgres 대상 재현
테스트로 실증, 다음 커밋 참고).

RunnectUser에 @Version 낙관적 락을 추가하고, 카운터 증가 + 스탬프 지급을
메인 트랜잭션(코스/기록/스크랩 생성)에서 분리된 REQUIRES_NEW 트랜잭션
(UserStampService.recordActivityAndAwardStamp)으로 격리했다 — 낙관적 락
충돌이 코스/기록/스크랩 생성 자체까지 롤백시키지 않게 하기 위함이다.
충돌 시에는 OptimisticLockRetrier가 지터를 두고 재시도한다.

* 유저 카운터 Lost Update 재현/수정 검증 테스트 추가

로컬 Postgres에 대해 실제 트랜잭션 두 개가 커밋 전 상태를 서로 보지
못하는 상황을 재현해, @Version 도입 전에는 조용한 데이터 손상
(assertion 실패)으로, 도입 후에는 명시적 충돌 감지
(ObjectOptimisticLockingFailureException)로 바뀌었음을 검증한다.

두 번째 테스트는 실제 프로덕션 경로(OptimisticLockRetrier +
recordActivityAndAwardStamp)를 스레드 10개로 동시 호출해, 재시도로
충돌이 모두 해소되고 최종 카운트가 정확히 맞는지(호출부에는 예외가
전파되지 않는지) 검증한다.

* 스탬프/카운터 갱신이 메인 저장 롤백과 무관하게 커밋되던 문제 수정

CodeRabbit 리뷰 지적: recordActivityAndAwardStamp()가 REQUIRES_NEW라 호출
즉시 별도 트랜잭션으로 독립 커밋되는데, 이 호출이 메인 엔티티 저장보다
먼저(Scrap) 또는 저장 직후지만 같은 트랜잭션 커밋 전에(Course/Record) 실행되고
있었다. 메인 저장이 실패해 트랜잭션이 롤백돼도 이미 커밋된 스탬프/카운터는
되돌아가지 않는다.

실제로 재현: 같은 유저가 같은 코스를 동시에 스크랩하면 유니크 제약으로 한쪽만
저장에 성공하는데(#252), 수정 전에는 실패한 쪽도 스탬프/카운터가 반영돼
createdScrap이 실제 스크랩 개수(1)보다 많은 2로 기록됨을 테스트로 확인.

OptimisticLockRetrier에 runAfterCommit()을 추가해, 메인 트랜잭션이 실제로
커밋된 뒤에만(TransactionSynchronization.afterCommit) 재시도 로직이 실행되게
했다 — 이 프로젝트에 이미 있던 랭킹(Redis) 갱신의 afterCommit 패턴과 동일한
구조. 재시도가 모두 소진되는 극단적인 경우엔 메인 작업 자체는 이미 성공했으므로
예외를 던지지 않고 로그로만 남긴다.

* 스크랩 동시성 테스트가 로컬 시드 데이터에 우연히 의존하던 문제 수정

이 테스트는 PublicCourseRepository를 목으로 대체하면서도 (user_id,
public_course_id) FK 제약을 만족하려면 실제 course/public_course row가
DB에 있어야 한다는 걸 놓치고, 로컬 개발 DB에 우연히 남아있던 더미 데이터
(id=1)를 그대로 가정하고 작성했었다.

PR #251 CI에서 재현: CI는 매번 새로 뜨는 빈 DB라 그 row가 아예 없어서,
두 스레드 모두 진짜 유니크 제약 위반이 아니라 FK 제약 위반으로 실패했다.
그런데 둘 다 같은 DataIntegrityViolationException → ConflictException
경로를 타기 때문에 겉보기엔 "정상 통과"처럼 보였다 — 실제로는 검증하려던
레이스와 무관한 이유로 우연히 같은 예외 타입이 나온 것뿐이었다. 진단 로그로
capturedException/스레드별 결과/실제 저장된 scrap 개수/public_course 실존
여부를 직접 찍어보고서야 확인했다.

이제 @beforeeach에서 필요한 course/public_course row를 테스트가 직접
INSERT(네이티브 SQL)해서 어떤 환경에서 실행해도 동일하게 동작하도록 했다.
(로컬 Postgres 컨테이너에 postgis 패키지가 아예 설치돼 있지 않았던 것도
같이 확인해 postgresql-15-postgis-3를 설치했다 — 기존에 문서화된 "PostGIS
공유 라이브러리 결함"의 실제 원인이었다.)

* CI Postgres 이미지를 PostGIS 포함 이미지로 교체

Course.path가 PostGIS geometry 컬럼인데 CI는 순정 postgres:15를 써서
course 테이블 생성 자체가 매번 실패하고 있었다. Hibernate가 ddl-auto
실패를 조용히 로그만 남기고 넘어가는 바람에 지금까지 드러나지 않았을
뿐, CI에 course 테이블이 존재한 적이 없었다 — 실제 Course/PublicCourse
row가 필요 없던 테스트만 우연히 계속 통과해온 것이었다.

ScrapConcurrencyTest에 실제 course/public_course row를 직접 만들도록
고치는 과정에서 발견했다. prod/staging과 동일한 postgis/postgis 이미지로
바꿔 CI가 실제 스키마로 검증하도록 한다.

---------

Co-authored-by: 나미 <dnska6657@gmail.com>

Author: unam98

.github/workflows/prod-ci.yml

@@ -14,9 +14,14 @@ jobs:
 
     # 테스트(contextLoads 등)는 실제 prod DB/Redis가 아니라 CI 안에서 띄우는
     # 임시 컨테이너를 바라보게 한다 (원격 prod 환경에 직접 의존하지 않도록).
+    # Course.path가 PostGIS geometry 컬럼이라 순정 postgres 이미지로는 그 컬럼이 있는
+    # 테이블(course) 생성 자체가 실패한다(Hibernate가 ddl-auto 실패를 조용히 무시해서
+    # 지금까지 드러나지 않았을 뿐, course 테이블이 CI에 존재한 적이 없었다 — 실제 Course row가
+    # 필요 없던 테스트만 우연히 계속 통과해온 것). prod/staging과 동일하게 PostGIS가 포함된
+    # 이미지를 써서 CI가 실제 환경과 같은 스키마로 검증하도록 한다.
     services:
       postgres:
-        image: postgres:15
+        image: postgis/postgis:15-3.4
         env:
           POSTGRES_DB: runnect
           POSTGRES_USER: runnect

src/main/java/org/runnect/server/common/module/concurrency/OptimisticLockRetrier.java

@@ -0,0 +1,78 @@
+package org.runnect.server.common.module.concurrency;
+
+import java.util.concurrent.ThreadLocalRandom;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.orm.ObjectOptimisticLockingFailureException;
+import org.springframework.stereotype.Component;
+import org.springframework.transaction.support.TransactionSynchronization;
+import org.springframework.transaction.support.TransactionSynchronizationManager;
+
+/**
+ * 낙관적 락(@Version) 충돌로 실패한 작업을 정해진 횟수만큼 재시도한다.
+ * action은 반드시 REQUIRES_NEW 등으로 독립된 트랜잭션 단위여야 한다 — 그래야 각 시도가
+ * 완전히 새로운 트랜잭션에서 최신 상태를 다시 읽고 재시도할 수 있다.
+ *
+ * 재시도 사이에 짧은 지터를 둔다 — 충돌한 모든 스레드가 즉시 동시에 재시도하면 서로
+ * 다시 충돌할 확률이 높아지는 thundering herd 현상을 완화하기 위함이다.
+ */
+@Slf4j
+@Component
+public class OptimisticLockRetrier {
+
+    private static final int MAX_ATTEMPTS = 8;
+    private static final int BASE_BACKOFF_MILLIS = 15;
+
+    public void runWithRetry(Runnable action) {
+        for (int attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) {
+            try {
+                action.run();
+                return;
+            } catch (ObjectOptimisticLockingFailureException e) {
+                if (attempt == MAX_ATTEMPTS) {
+                    throw e;
+                }
+                backoff(attempt);
+            }
+        }
+    }
+
+    /**
+     * action을 지금 실행하지 않고, 호출 시점에 진행 중인 트랜잭션이 실제로 커밋된 뒤에만
+     * 실행되도록 예약한다. action 자체는 REQUIRES_NEW라 호출 시점(=원래의 runWithRetry)에
+     * 즉시 별도 트랜잭션으로 독립 커밋되는데, 그 시점이 메인 트랜잭션의 저장보다 앞서면
+     * 메인 저장이 나중에 실패해 롤백돼도 이 action은 이미 커밋돼 있어 되돌릴 수 없다.
+     * afterCommit으로 미루면 메인 트랜잭션이 실제로 성공했을 때만 실행된다. 진행 중인
+     * 트랜잭션이 없는 채로 호출되면(동기화 비활성) 예약할 커밋 시점 자체가 없으므로
+     * 그 자리에서 바로 실행한다.
+     *
+     * 재시도가 모두 소진되는 경우(극단적인 지속 경합)는 메인 작업 자체는 이미 성공한
+     * 뒤이므로, 그 실패를 클라이언트 응답의 실패로 만들지 않고 로그로만 남긴다.
+     */
+    public void runAfterCommit(Runnable action) {
+        if (!TransactionSynchronizationManager.isSynchronizationActive()) {
+            runWithRetry(action);
+            return;
+        }
+
+        TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
+            @Override
+            public void afterCommit() {
+                try {
+                    runWithRetry(action);
+                } catch (ObjectOptimisticLockingFailureException e) {
+                    log.error("커밋 후 재시도가 모두 소진되어 활동 카운터/스탬프 반영에 실패함", e);
+                }
+            }
+        });
+    }
+
+    private void backoff(int attempt) {
+        int maxJitterMillis = BASE_BACKOFF_MILLIS * attempt;
+        long sleepMillis = ThreadLocalRandom.current().nextLong(maxJitterMillis + 1);
+        try {
+            Thread.sleep(sleepMillis);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+        }
+    }
+}

src/main/java/org/runnect/server/course/service/CourseService.java

@@ -9,6 +9,7 @@
 import org.locationtech.jts.geom.LineString;
 import org.runnect.server.common.dto.DepartureResponse;
 import org.runnect.server.common.exception.NotFoundException;
+import org.runnect.server.common.module.concurrency.OptimisticLockRetrier;
 import org.runnect.server.common.module.convert.CoordinatePathConverter;
 import org.runnect.server.common.module.convert.DepartureConverter;
 import org.runnect.server.course.dto.request.CourseCreateRequestDto;
@@ -39,6 +40,7 @@ public class CourseService {
     private final PublicCourseRepository publicCourseRepository;
     private final UserRepository userRepository;
     private final UserStampService userStampService;
+    private final OptimisticLockRetrier optimisticLockRetrier;
 
     @Transactional
     public CourseCreateResponseDto createCourse(Long userId, CourseCreateRequestDto requestDto,
@@ -65,8 +67,9 @@ public CourseCreateResponseDto createCourse(Long userId, CourseCreateRequestDto
             .build();
 
         Course saved = courseRepository.save(course);
-        user.updateCreatedCourse();
-        userStampService.createStampByUser(user, StampType.c);
+        optimisticLockRetrier.runAfterCommit(
+            () -> userStampService.recordActivityAndAwardStamp(user.getId(), StampType.c)
+        );
 
         return CourseCreateResponseDto.of(saved.getId(), saved.getCreatedAt());
     }

src/main/java/org/runnect/server/record/service/RecordService.java

@@ -9,6 +9,7 @@
 import org.runnect.server.common.constant.ErrorStatus;
 import org.runnect.server.common.exception.NotFoundException;
 import org.runnect.server.common.exception.PermissionDeniedException;
+import org.runnect.server.common.module.concurrency.OptimisticLockRetrier;
 import org.runnect.server.course.entity.Course;
 import org.runnect.server.course.repository.CourseRepository;
 import org.runnect.server.publicCourse.entity.PublicCourse;
@@ -52,6 +53,7 @@ public class RecordService {
     private final UserStampService userStampService;
     private final RecordHealthDataRepository recordHealthDataRepository;
     private final RecordRankingService recordRankingService;
+    private final OptimisticLockRetrier optimisticLockRetrier;
 
     @Transactional
     public CreateRecordResponseDto createRecord(Long userId, CreateRecordRequestDto request) {
@@ -91,8 +93,9 @@ public CreateRecordResponseDto createRecord(Long userId, CreateRecordRequestDto
             registerRankingUpdateAfterCommit(publicCourse.getId(), userId, record.getId(), time);
         }
 
-        user.updateCreatedRecord();
-        userStampService.createStampByUser(user, StampType.r);
+        optimisticLockRetrier.runAfterCommit(
+            () -> userStampService.recordActivityAndAwardStamp(userId, StampType.r)
+        );
 
         CreateRecordDto recordDto = new CreateRecordDto(record.getId(), record.getCreatedAt().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSS")));
 

src/main/java/org/runnect/server/scrap/service/ScrapService.java

@@ -6,6 +6,7 @@
 import org.runnect.server.common.constant.ErrorStatus;
 import org.runnect.server.common.exception.ConflictException;
 import org.runnect.server.common.exception.NotFoundException;
+import org.runnect.server.common.module.concurrency.OptimisticLockRetrier;
 import org.runnect.server.publicCourse.entity.PublicCourse;
 import org.runnect.server.publicCourse.repository.PublicCourseRepository;
 import org.runnect.server.scrap.dto.request.CreateAndDeleteScrapRequestDto;
@@ -29,6 +30,7 @@ public class ScrapService {
     private final UserRepository userRepository;
     private final PublicCourseRepository publicCourseRepository;
     private final UserStampService userStampService;
+    private final OptimisticLockRetrier optimisticLockRetrier;
 
     @Transactional
     public CreateAndDeleteScrapResponseDto createAndDeleteScrap(Long userId, CreateAndDeleteScrapRequestDto request) {
@@ -46,9 +48,6 @@ public CreateAndDeleteScrapResponseDto createAndDeleteScrap(Long userId, CreateA
                         .runnectUser(user)
                         .build();
 
-                user.updateCreatedScrap();
-                userStampService.createStampByUser(user, StampType.s);
-
                 // 동시에 같은 코스를 스크랩하는 요청이 겹치면 둘 다 "기존 스크랩 없음"을 보고
                 // 각자 저장을 시도할 수 있다 — (user_id, public_course_id) 유니크 제약으로 DB가
                 // 하나는 거부하는데, 그 예외를 그대로 두면 500으로 샌다(HealthService의 기존
@@ -60,6 +59,10 @@ public CreateAndDeleteScrapResponseDto createAndDeleteScrap(Long userId, CreateA
                         ErrorStatus.ALREADY_EXIST_SCRAP_EXCEPTION,
                         ErrorStatus.ALREADY_EXIST_SCRAP_EXCEPTION.getMessage());
                 }
+
+                optimisticLockRetrier.runAfterCommit(
+                    () -> userStampService.recordActivityAndAwardStamp(userId, StampType.s)
+                );
             } else {
                 // 기존 스크랩한 내역이 있을 때
                 scrap.updateScrapTF(true);

src/main/java/org/runnect/server/user/entity/RunnectUser.java

@@ -12,6 +12,7 @@
 import javax.persistence.OneToMany;
 import javax.persistence.Table;
 import javax.persistence.UniqueConstraint;
+import javax.persistence.Version;
 import lombok.AccessLevel;
 import lombok.Builder;
 import lombok.Getter;
@@ -67,6 +68,14 @@ public class RunnectUser extends AuditingTimeEntity {
     @Column(nullable = false)
     private Long createdScrap;
 
+    /**
+     * 카운터 필드(createdCourse 등)를 다른 트랜잭션이 동시에 읽고 수정하면 Lost Update가
+     * 발생할 수 있어 낙관적 락을 건다. 충돌 시 UserStampService.recordActivityAndAwardStamp가
+     * 별도 트랜잭션에서 재시도한다.
+     */
+    @Version
+    private Long version;
+
     @OneToMany(mappedBy = "runnectUser")
     private List<Course> courses = new ArrayList<>();
 

src/main/java/org/runnect/server/user/service/UserStampService.java

@@ -11,6 +11,7 @@
 import org.runnect.server.user.repository.UserRepository;
 import org.runnect.server.user.repository.UserStampRepository;
 import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
 
 @Service
@@ -20,6 +21,45 @@ public class UserStampService {
     private final UserStampRepository userStampRepository;
     private final UserRepository userRepository;
 
+    /**
+     * 활동 카운터 증가 + 스탬프 지급을, 이 활동을 유발한 메인 트랜잭션(코스/기록/스크랩 생성)과
+     * 분리된 별도 트랜잭션에서 수행한다. RunnectUser의 카운터 필드에는 낙관적 락(@Version)이
+     * 걸려있어, 동시에 같은 유저의 카운터를 갱신하는 다른 요청과 충돌하면
+     * ObjectOptimisticLockingFailureException이 발생한다 — 메인 트랜잭션(코스/기록/스크랩 생성
+     * 자체)까지 롤백시키지 않기 위해 REQUIRES_NEW로 격리했다. 충돌 시 재시도는 호출부가
+     * OptimisticLockRetrier로 감싸서 처리한다.
+     */
+    @Transactional(propagation = Propagation.REQUIRES_NEW)
+    public void recordActivityAndAwardStamp(Long userId, StampType stampType) {
+        RunnectUser user = userRepository.findById(userId)
+            .orElseThrow(() -> new NotFoundUserException(NOT_FOUND_USER_EXCEPTION,
+                NOT_FOUND_USER_EXCEPTION.getMessage()));
+
+        incrementActivityCount(user, stampType);
+        createStampByUser(user, stampType);
+
+        // 여기서 명시적으로 flush해서, 버전 충돌이 이 메서드 안에서 즉시 드러나게 한다
+        // (그래야 REQUIRES_NEW 트랜잭션 경계를 벗어나기 전에 예외가 호출부로 전파된다).
+        userRepository.saveAndFlush(user);
+    }
+
+    private void incrementActivityCount(RunnectUser user, StampType stampType) {
+        switch (stampType) {
+            case c:
+                user.updateCreatedCourse();
+                break;
+            case s:
+                user.updateCreatedScrap();
+                break;
+            case r:
+                user.updateCreatedRecord();
+                break;
+            case u:
+                user.updateCreatedPublicCourse();
+                break;
+        }
+    }
+
     @Transactional
     public void createStampByUser(RunnectUser user, StampType stampType) {
         Long activityCount = getActivityCount(user, stampType);

src/test/java/org/runnect/server/course/service/CourseServiceTest.java

@@ -3,6 +3,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -24,6 +25,7 @@
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.runnect.server.common.exception.BadRequestException;
 import org.runnect.server.common.exception.NotFoundException;
+import org.runnect.server.common.module.concurrency.OptimisticLockRetrier;
 import org.runnect.server.common.module.convert.CoordinateDto;
 import org.runnect.server.common.module.convert.CoordinatePathConverter;
 import org.runnect.server.course.dto.request.CourseCreateRequestDto;
@@ -55,13 +57,20 @@ class CourseServiceTest {
     private UserRepository userRepository;
     @Mock
     private UserStampService userStampService;
+    @Mock
+    private OptimisticLockRetrier optimisticLockRetrier;
 
     private CourseService courseService;
 
     @BeforeEach
     void setUp() {
         courseService = new CourseService(courseRepository, publicCourseRepository, userRepository,
-            userStampService);
+            userStampService, optimisticLockRetrier);
+        org.mockito.Mockito.lenient().doAnswer(invocation -> {
+            Runnable action = invocation.getArgument(0);
+            action.run();
+            return null;
+        }).when(optimisticLockRetrier).runAfterCommit(any());
     }
 
     private RunnectUser buildUser(Long id) {
@@ -130,8 +139,7 @@ class CreateCourse {
 
             assertThat(response.getId()).isEqualTo(100L);
             assertThat(response.getCreatedAt()).isEqualTo(LocalDateTime.of(2026, 1, 1, 0, 0));
-            assertThat(user.getCreatedCourse()).isEqualTo(1L);
-            verify(userStampService).createStampByUser(user, StampType.c);
+            verify(userStampService).recordActivityAndAwardStamp(1L, StampType.c);
 
             ArgumentCaptor<Course> captor = ArgumentCaptor.forClass(Course.class);
             verify(courseRepository).save(captor.capture());

src/test/java/org/runnect/server/record/service/RecordServiceTest.java

@@ -24,6 +24,7 @@
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.runnect.server.common.exception.NotFoundException;
 import org.runnect.server.common.exception.PermissionDeniedException;
+import org.runnect.server.common.module.concurrency.OptimisticLockRetrier;
 import org.runnect.server.course.entity.Course;
 import org.runnect.server.course.repository.CourseRepository;
 import org.runnect.server.health.entity.RecordHealthData;
@@ -68,13 +69,21 @@ class RecordServiceTest {
     private RecordHealthDataRepository recordHealthDataRepository;
     @Mock
     private org.runnect.server.ranking.service.RecordRankingService recordRankingService;
+    @Mock
+    private OptimisticLockRetrier optimisticLockRetrier;
 
     private RecordService recordService;
 
     @BeforeEach
     void setUp() {
         recordService = new RecordService(recordRepository, userRepository, courseRepository,
-            publicCourseRepository, userStampService, recordHealthDataRepository, recordRankingService);
+            publicCourseRepository, userStampService, recordHealthDataRepository, recordRankingService,
+            optimisticLockRetrier);
+        org.mockito.Mockito.lenient().doAnswer(invocation -> {
+            Runnable action = invocation.getArgument(0);
+            action.run();
+            return null;
+        }).when(optimisticLockRetrier).runAfterCommit(any());
     }
 
     private RunnectUser buildUser(Long id) {
@@ -163,8 +172,7 @@ class CreateRecord {
             CreateRecordResponseDto response = recordService.createRecord(1L, request);
 
             assertThat(response.getRecord().getId()).isEqualTo(100L);
-            assertThat(user.getCreatedRecord()).isEqualTo(1L);
-            verify(userStampService).createStampByUser(user, StampType.r);
+            verify(userStampService).recordActivityAndAwardStamp(1L, StampType.r);
             verify(publicCourseRepository, never()).findById(any());
             verify(recordRankingService, never()).updateBestRecord(anyLong(), anyLong(), anyLong(), any());
         }