장소 저장할 때 ES에도 저장 #164

koomchang · 2024-11-21T05:35:46Z

📄 Summary

장소 저장할 때 ES에도 저장

🙋🏻 More

typeorm-transactional 모듈 설치
@nestjs/schedule 모듈 설치

처음에는 장소저장할 때 ES에도 저장을 같이하고 Transactional로 묶었다.

class PlaceService {
    
    @Transactional
    async addPlace() {
        const savedPlace = await this.placeRepository.save(place)
        await this.searchService.save(place)
        return {id: savedPlace.id}

그런데 MySQL에 저장이 되었는데 ES에 저장이 되지 않았다고 롤백하는 건 문제가 있다고 생각했다.
사용자 입장에서 생각해보면

새로운 장소를 등록했다.
ES에 저장이 되지 않아 롤백되었다. (500에러인데 서버에서 처리해야 하는게 아닌가? + ES 다운되면 장소 추가가 아예 불가해짐)
사용자는 처리되지 않았다는 메세지를 받는다.

그래서 다시 생각을 해본 것이
MySQL에 저장하는 것만 트랜잭션을 걸고 (사실 저장로직만 있어서 지금은 트랜잭션 어노테이션 없어도 될 것 같긴함)
ES에 저장하는 이벤트만 발생을 시키고 id를 반환하는 식으로 구현했다.

class PlaceService {

    @Transactional
    async addPlace() {
        const savedPlace = await this.placeRepository.save(place);
        this.eventEmitter.emit('place.created', savedPlace);
        return { id: savedPlace.id };

이런식으로 하면 사용자는 ES의 상태에 상관없이 장소추가를 잘할 수 있고 서비스 운영에도 문제가 되지 않을 것이다.

추가적으로 이렇게 하면서 생각이 든 것이 장소 수정, 삭제 하는 경우에도 이런 식으로 이벤트 호출을 하면 굳이 배치를 쓰지 않아도 될 것이라 생각했다. 그래서 구현은 안 했는데 팀원분들의 생각이 궁금하니 코멘트 남겨주세요

241123 추가

생성, 수정, 삭제 2시간 내의 장소들을 검색하여 1시간 마다 ES와 동기화 (bulk, upsert) 할 수 있도록 구성했습니다. 실제 데이터가 잘 동기화되는 것을 확인했습니다.

🕰️ Actual Time of Completion

6

close #163

Miensoap

추가적으로 이렇게 하면서 생각이 든 것이 장소 수정, 삭제 하는 경우에도 이런 식으로 이벤트 호출을 하면 굳이 배치를 쓰지 않아도 될 것이라 생각했다.

-> 얼마나 자주 수정/삭제 되는지에 따라 다를 것 같아요.

ES나 네트워크 상황에 따른 실패는 배치로 처리해야 하지 않을까요?

Miensoap · 2024-11-21T06:32:28Z

backend/src/app.module.ts

+      },
+    }),
+    EventEmitterModule.forRoot({
+      maxListeners: 15,


q. 15는 기본값인가요?
따로 결정했다면 이유가 있나요?

테스트 실패 로그보니 이 모듈 의존성이던데, 테스트 모듈에도 추가해야할 것 같아요.

기본값이 10인데, 큰 이유는 없습니다. 값을 어느정도로 조절해야할지는 잘 모르겠습니다.

Miensoap · 2024-11-21T06:33:24Z

backend/src/app.module.ts

+      async dataSourceFactory(options) {
+        const dataSource = new DataSource(options);
+        await dataSource.initialize();
+        return addTransactionalDataSource(dataSource);


q. 테스트에서 datasource 를 오버라이드해서 사용하는데,
이 설정도 따로 적용해야 할까요?

수정했습니다!

Miensoap · 2024-11-21T06:42:35Z

backend/src/search/search.service.ts

+        document: data,
+      });
+    } catch (e) {
+      this.logger.error(


p1. ES 나 네트워크 상황에 따른 실패상황은 이후 재시도 해야 하지 않을까요?

그러러면, 이벤트를 처리하고 끝나는 것이 아니라,
배치 작업에서 구분할 수 있게 비동기적으로 성공/실패 여부를 저장해야 할 것 같아요.

검색에 안나와서 같은 장소에 대해 등록 요청이 다시 들어온다면?

이런 상황도 고려해야겠네요!

맞습니다.

이벤트 성공/실패 여부에 따른 작업이 배치로 이루어지거나 슬랙 알림등 오면 좋을 것 같아요. 혹시 에러로그로 해두면 이 부분을 슬랙 알림을 보낼 수 있게 설정 가능한가요?

https://elastalert.readthedocs.io/en/latest/

특정 패턴에 알림을 보내게 설정할 수 있는 것 같아요.

실패 원인이 데이터 문제인 경우도 있을까요?
ES 통신 문제라면 그쪽을 모니터링해 알림을 받는게 낫지 않을까요?
ex) health check

Miensoap

테스트 안되는 거 잘 해결하셨네요!
수고하셨습니다~

q. 트랜잭션에서 Elasticsearch 에 저장하는 로직은 제외했다고 하셨는데,
제외하지 않으면 저 @Transactioanl 데코레이터가 ES 에 저장하는 것도 롤백시킬 수 있나요?

저 라이브러리 설정 dataSource 에 적용하는 것 같아서요.

Miensoap · 2024-11-23T08:50:54Z

backend/src/course/guards/CoursePermissionGuard.ts

@@ -11,8 +11,8 @@ export class CoursePermissionGuard implements CanActivate {
    const courseId = Number(request.params.id);
    const userId = Number(request.user.userId);

-    const course = await this.courseService.getCourseOwnerId(courseId);
-    if (course !== userId) {
+    const courseOwnerId = await this.courseService.getCourseOwnerId(courseId);


p2. 항상 함께 사용될 것 같은데 인증 가드를 상속해 사용하면 어떨까요?
불필요한 컨트롤러 인자와 가드 사용 코드를 줄일 수 있을 것 같아요.

참고 : AdminGuard

q. 밑에서 다시 사용하나요?
변수로 분리하신 이유가 있나요?

q. 밑에서 다시 사용하나요? 변수로 분리하신 이유가 있나요?

가독성 고려했습니다!

Miensoap · 2024-11-23T08:56:38Z

backend/src/batch/batch.service.ts

+    this.logger.debug(`syncPlacesToElastic() 배치 시작`);
+    try {
+      this.logger.debug(`동기화 작업을 시작합니다.`);
+      const places = await this.placeRepository.findUpdatedPlacesForTwoHours();


q. 수동으로 동기화 한 것 포함해서 모두 동기화하나요?

많지 않아서 괜찮을 것 같습니다~

만약 수동으로 잘 저장되는 데이터 수가 아주 많아서,
중복으로 동기화하는 비용이 유의미하게 커지는 지점이 어디쯤일지 계산해보면 좋을 것 같아요.
-> 그렇다면 실패했다는 표시를 어떻게 해서 주기적 동기화에 포함할 것인지?

q. 수동으로 동기화 한 것 포함해서 모두 동기화하나요?

많지 않아서 괜찮을 것 같습니다~

넵! 현재는 그런 상황이고 만약 syncedAt 같은 칼럼을 추가하면 최근에 싱크된 데이터는 빼고 동기화할 수 있을 것 같네요!

만약 수동으로 잘 저장되는 데이터 수가 아주 많아서, 중복으로 동기화하는 비용이 유의미하게 커지는 지점이 어디쯤일지 계산해보면 좋을 것 같아요. -> 그렇다면 실패했다는 표시를 어떻게 해서 주기적 동기화에 포함할 것인지?

맞습니다! 같이 고민해봐야 될 부분인 것 같네요

Miensoap · 2024-11-23T08:58:58Z

backend/src/place/place.module.ts

  controllers: [PlaceController],
-  providers: [PlaceService, PlaceRepository],
+  providers: [PlaceService, PlaceRepository, SearchService, PinoLogger],


q. 이거 테스트 때문에 로거 추가하신건가요?

저도 비슷하게 글로벌 모듈로 이미 등록된 ConfigModule 을 다시 import 했는데요,
혹시 다른 방법 없을까요?

q. 이거 테스트 때문에 로거 추가하신건가요?

저도 비슷하게 글로벌 모듈로 이미 등록된 ConfigModule 을 다시 import 했는데요, 혹시 다른 방법 없을까요?

테스트 때문에 추가했습니다.
저도 비슷한 고민을 했는데 테스트뿐만 아니라, 일반 로직이 들어있는 service들에서도 DI로 logger를 받는게 좀 어색한 것 같더라구요. 로거를 싱글톤으로 이용할 수 있으면 좋지 않을까 고민했습니다.

기존의 nest 에서 제공하는 LoggerModule 구조를 따라간건데,
그것도 고민해봐야겠네용

Miensoap · 2024-11-23T09:02:35Z

backend/src/search/dto/ESPlaceSaveDTO.ts

+    readonly rating: number,
+    readonly location: {
+      lat: number;
+      lon: number;


p3. lng 은 어떨까요?
보통 이렇게 줄이는 것 같아서요.

저는 아예 안줄이는 것도 좋다고 생각합니다

Miensoap · 2024-11-23T09:06:07Z

backend/src/search/search.service.ts

+    }
+  }
+
+  async searchPlace(
    query: string,
    lat?: number,
    long?: number,


lon, long, lng , longitude 중 하나로 모든 코드를 통일하는 게 좋을 것 같아요.

Miensoap · 2024-11-23T09:07:40Z

backend/src/search/search.service.ts

+    });
+
+    this.logger.debug(
+      `Elasticsearch에 동기화된 장소의 갯수: ${response.items.length}`,


디버그 로깅이 꼼꼼해서 좋네요 ㅎㅎ

Miensoap · 2024-11-23T09:12:47Z

backend/test/place/place.service.test.ts

    container = await new MySqlContainer().withReuse().start();
    dataSource = await initDataSource(container);

    const module: TestingModule = await Test.createTestingModule({
+      imports: [
+        LoggerModule.forRoot({


q. 로거에 대한 의존성이 생기니까 테스트에도 영향이 가네요.
분리할 수 있는 방법이 없을까요?

logger.info() 등을 빈 함수로 모킹하는 테스트 유틸 모듈을 사용할 수 있을까요?

q. 로거에 대한 의존성이 생기니까 테스트에도 영향이 가네요. 분리할 수 있는 방법이 없을까요?

logger.info() 등을 빈 함수로 모킹하는 테스트 유틸 모듈을 사용할 수 있을까요?

같이 고민해봐요..! 글로벌에서 테스트 모듈을 사용하는 것도 하나의 방법일 것 같고..
테스팅용 모듈을 만드는 비용이 크지 않다면 전체적인 의존성을 다 주입해도 괜찮지 않을까하는 생각도 해봤습니다!

koomchang · 2024-11-23T09:19:27Z

테스트 안되는 거 잘 해결하셨네요! 수고하셨습니다~

q. 트랜잭션에서 Elasticsearch 에 저장하는 로직은 제외했다고 하셨는데, 제외하지 않으면 저 @Transactioanl 데코레이터가 ES 에 저장하는 것도 롤백시킬 수 있나요?

저 라이브러리 설정 dataSource 에 적용하는 것 같아서요.

저번에 테스트 했을 때 mysql에 데이터 저장 되고, ES에서 터지면 mysql이 롤백되는 건 확인했습니다. 다만, ES는 NoSQL이다보니 트랜잭션 개념이 없어서, 롤백 개념이 없습니다. 쿼리가 실행되면 되돌릴 수 없습니다.

koomchang added BE 백엔드 관련 이슈입니다 🌱 기능추가 새로운 기능 요청입니다 ⚙️ 리팩터링 코드 퀄리티를 높입니다 labels Nov 21, 2024

koomchang added this to the week4 milestone Nov 21, 2024

koomchang self-assigned this Nov 21, 2024

koomchang requested review from Miensoap, 1119wj and hyohyo12 as code owners November 21, 2024 05:35

koomchang linked an issue Nov 21, 2024 that may be closed by this pull request

장소 데이터 추가, 수정, 삭제에 따른 배치 처리를 한다 #163

Closed

2 tasks

Miensoap requested changes Nov 21, 2024

View reviewed changes

Miensoap changed the title ~~코스 저장할 때 ES에도 저장~~ 장소 저장할 때 ES에도 저장 Nov 21, 2024

koomchang force-pushed the feature/#163 branch from f2aed99 to 4fb85cd Compare November 22, 2024 14:57

koomchang requested a review from Miensoap November 23, 2024 08:23

Miensoap approved these changes Nov 23, 2024

View reviewed changes

1119wj approved these changes Nov 24, 2024

View reviewed changes

koomchang added 14 commits November 24, 2024 23:44

config: typeorm-transactional 모듈 설치 #163

b938e05

config: nestjs에 typeorm-transactional 이용을 위한 환경 설정 #163

108a13a

feat: es에 장소 저장할 수 있는 기능추가 #163

5f56fe0

feat: 장소 추가할 때 MySQL, ES에 모두 저장되도록 실패할 시 트랜잭션 적용까지 처리 #163

0d91f7d

config: NestJS EventEmitter 모듈 설치 #163

68f9947

feat: 장소 저장할 때 ES는 event 호출로 분리 #163

35079ed

fix: es 장소 저장 반환값 수정 #163

255aacc

test: 테스트 환경에서 DataSource 트랜잭션 설정 #163

6d17f6a

fix: 의존성 때문에 테스트 실패하는 버그 수정 #163

c3c3c10

fix: 테스트에서 로거를 확인 못하는 현상 수정 #163

cee82da

feat: nestjs schedule 모듈 설치 #163

2bd74aa

feat: 생성, 수정, 삭제 2시간 이내에 변경된 데이터들을 1시간 마다 한번씩 ES에 동기화하는 배치 구성 #163

adb4d87

feat: es 에 장소 저장 실패하는 경우 에러 로그 추가 #163

833cfec

refactor: 불분명한 의미의 변수명 수정 #163

16dacf6

koomchang added 2 commits November 24, 2024 23:47

feat: 코스 사용자 확인 권한에서 admin은 모두 통과할 수 있도록 설정 #163

5613a4a

refactor: lat, lon 네이밍을 줄이지 않고 사용하도록 변경 #163

aee978c

koomchang force-pushed the feature/#163 branch from 1ade903 to aee978c Compare November 24, 2024 14:47

config: yarn.lock 업데이트 #163

33a1c63

koomchang merged commit 2a36c81 into develop Nov 24, 2024
2 checks passed

koomchang deleted the feature/#163 branch November 24, 2024 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

장소 저장할 때 ES에도 저장 #164

장소 저장할 때 ES에도 저장 #164

koomchang commented Nov 21, 2024 •

edited

Loading

Miensoap left a comment

Miensoap Nov 21, 2024

koomchang Nov 21, 2024

Miensoap Nov 21, 2024

koomchang Nov 22, 2024 •

edited

Loading

Miensoap Nov 21, 2024

koomchang Nov 21, 2024

Miensoap Nov 21, 2024 •

edited

Loading

Miensoap left a comment

Miensoap Nov 23, 2024

Miensoap Nov 23, 2024

koomchang Nov 23, 2024

Miensoap Nov 23, 2024

Miensoap Nov 23, 2024

koomchang Nov 23, 2024

Miensoap Nov 23, 2024

koomchang Nov 23, 2024

Miensoap Nov 23, 2024

Miensoap Nov 23, 2024

Miensoap Nov 23, 2024

Miensoap Nov 23, 2024

Miensoap Nov 23, 2024

koomchang Nov 23, 2024

koomchang commented Nov 23, 2024

장소 저장할 때 ES에도 저장 #164

장소 저장할 때 ES에도 저장 #164

Conversation

koomchang commented Nov 21, 2024 • edited Loading

📄 Summary

🙋🏻 More

241123 추가

🕰️ Actual Time of Completion

Miensoap left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koomchang Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Miensoap Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Miensoap left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koomchang commented Nov 23, 2024

koomchang commented Nov 21, 2024 •

edited

Loading

koomchang Nov 22, 2024 •

edited

Loading

Miensoap Nov 21, 2024 •

edited

Loading