-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
검색을 위한 ElasticSearch 설정 및 장소 검색 성능 개선 #149
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지나가다 리뷰남겨요~
고려할거 같은데, 동일한 검색 요청에 대해 캐시를 적용하는것도 고려해보면 괜찮을듯.
backend/src/search/search.service.ts
Outdated
for (const place of places) { | ||
const data = { | ||
...place, | ||
location: { | ||
lat: place.latitude, | ||
lon: place.longitude, | ||
}, | ||
}; | ||
delete data.latitude; | ||
delete data.longitude; | ||
|
||
await this.indexData(this.PLACE_INDEX, data); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 직렬로 처리되고 있는지 확인해보세요. 병렬처리를 해도 된다면 promise.all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
임시로 작성한거라 지금은 삭제한 코드입니다! 이외에도 동기화할 때 비슷한 로직이 쓰일 것 같은데 참고하겠습니다!
backend/src/search/search.service.ts
Outdated
}); | ||
|
||
// 결과가 없으면 prefix 검색 (name, formattedAddress) | ||
if (result.hits?.hits.length === 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이부분 너무 긴데. 여기서부터는 분리해서 다른 함수에서 처리해도 될 듯?
backend/src/search/search.service.ts
Outdated
this.logger.info(`Indexing 완료: ${endTime}`); | ||
this.logger.info( | ||
`소요 시간: ${new Date(endTime).getTime() - new Date(startTime).getTime()}ms`, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로깅하는 코드가 중간 중간 있어서 어떻게 할까? 고민이 될 듯.
지금은 많지 않지만 더 로그가 많아지면 별도 함수로 나눠도 될 듯하고요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로깅하는 코드가 중간 중간 있어서 어떻게 할까? 고민이 될 듯. 지금은 많지 않지만 더 로그가 많아지면 별도 함수로 나눠도 될 듯하고요.
소요 시간 같은 경우는 AOP 방식으로 구현하면 될 것 같습니다. 다만, 특정상황에 로그를 찍어야하는 경우는 해당 함수에서 로그를 찍는 방법밖에 잘 모르겠는데 혹시 다른 방법이 있나요?
동음이의어 -> 동의어 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고했습니다!
eb65b29
to
5a77fd6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GOOD
@@ -0,0 +1,35 @@ | |||
# Production 환경 ES 및 Kibana 설정 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
루트에 docs 디렉토리를 따로 만들어도 좋을 것 같아요.
index: ElasticSearchConfig.PLACE_INDEX, | ||
from, | ||
size, | ||
query: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p2. 쿼리를 객체 형태로 사용하는 것 같은데, 분리하고 조합해서 사용하면 더 보기 좋을 것 같아요.
ex) should : [ NAME_MATCH ... ]
}); | ||
} | ||
|
||
private async tokenizeQuery(query: string): Promise<string[]> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q. 인덱스에서 analyzer 타입 설정 한 것 같은데, 이건 용도가 다른가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인덱스 파일에 설정한 분석기는 삽입시 사용되고,
이건 검색 키워드에 사용되는 걸까요?
그 후에,
토큰 -> 토큰 으로 매칭해서 검색이 이루어지나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인덱스에서 설정한 분석기는 어떤 필드들에서 쓸 수 있게 정의를 한거고
"name": {
"type": "text",
"fields": {
"keyword": {
"type": "keyword"
}
},
"analyzer": "nori_with_synonym"
},
이 처럼 필드마다 분석기를 따로 둘 수 있습니다!
이건 검색 키워드에 사용되는 걸까요?
받아온 검색 키워드를 토큰화하는데 사용합니다!
그 후에,
토큰 -> 토큰 으로 매칭해서 검색이 이루어지나요?
넵! 코드 보시면 토큰들로 검색하는 경우도 있고 검색 키워드로 검색하는 경우도 있어요!
async search( | ||
@Query('query') query: string, | ||
@Query('lat') lat?: number, | ||
@Query('lon') lon?: number, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p3. 줄이더라도 long 이 나을 것 같아요
await this.elasticSearchQuery.searchPlaceWithPrefix(query); | ||
result = prefixSearched.hits?.hits || []; | ||
} | ||
return { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q. 동기화하는 함수는 여기서 제공하지 않나요?
즉시 동기화 함수 제공 , 시간 지표나 메시지 큐를 통해 주기적 동기화 모두 지원할 예정인가요?
즉시 동기화 하지 않는다면, Mysql 검색 결과 (시간으로 필터링) 와 병합하기도 하는 것 같더라구요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동기화라는 것이 MySQL과의 데이터 동기화를 말씀하신 걸로 이해했습니다!
데이터 동기화에 대해서는 팀원들과 한번 이야기 해 봐야 할 것 같아요. 제가 생각한 것을 말씀드리면
우선 초기 데이터는 MySQL
, ES
에 bulk.
이후
방법 1 : 데이터 추가, 수정, 삭제 -> 그떄 그때 MySQL
, ES
둘다 수정
방법 2 : 데이터 추가만 MySQL
, ES
둘다 수정 + 수정, 삭제의 경우를 고려하여 업데이트 배치
고려사항 : Place
의 경우 사용자는 추가만 할 수 있고 수정, 삭제는 Admin
이 할 수 있음.
좋은 의견 있으시면 알려주세요! 같이 정하고 작업해야 될 것 같아서 우선 작업 안 했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 2번 생각했습니다!
추가했는데 바로 검색이 안되면 불편할 것 같고,
삭제했는데 검색이 되더라도, 상세 보기에서 404 페이지를 보여주면 될 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 2번 생각했습니다!
추가했는데 바로 검색이 안되면 불편할 것 같고, 삭제했는데 검색이 되더라도, 상세 보기에서 404 페이지를 보여주면 될 것 같아요.
넵 pr이 너무 커져서 다음 pr에서 작업해보겠습니다!
포스트맨에서 environment에 따라 배포 url / localhost 로 쉽게 전환할 수 있어요 |
5a77fd6
to
4188823
Compare
2f84222
to
10b536c
Compare
📄 Summary
ElasticSearch
설정 및 장소 검색 성능 개선🙋🏻 More
ES
와Kibana
를 도커 컨테이너로 생성place index
자동생성 스크립트ES
,Kibana
는 인증 과정 제거 (배포용은Docker
등 추가 설정 필요)241120 추가
ES
,Kibana
배포환경 설정docker-compose
이용search service
내부es query
를 클래스 분리search.dailyroad.site
배포완료사용방법
docker compose up -d
로es
와kibana
컨테이너 실행GET : {{localhost}}/search?query=서울특별시
쿼리에 넣어서 검색추후 할 일
mysql
데이터 동기화 api 삭제 예정 (현재는 편의를 위해 작성)place
추가, 수정, 삭제에 따른 동기화 Batch 로직 작성 필요🕰️ Actual Time of Completion
close #138