[FEAT] elasticsearch에 경매 상태 변화 반영#95
Merged
Merged
Conversation
elasticsearch의 경매 정보가 DB의 경매 정보와 틀어지는 것을 막고자 이를 막아주는 scheduler를 구현했습니다.
AuthIntegrationTest뿐만 아니라 많은 테스트가 임시 유저 이메일로 test@test.com 쓰기 때문에 test@test.com를 가진 유저가 DB에 이미 존재할 경우 테스트가 실패하는 문제가 있었습니다. 이 commit은 이를 해결합니다.
Walkthrough별칭 기반 Elasticsearch 인덱스 초기화와 주기적 재인덱싱(잡 기록·진행 추적), 커서 기반 배치 색인, 경매 취소 시 비동기 검색 문서 갱신, 검색 분기(DB/Elasticsearch) 조정, 관련 설정·테스트가 추가됩니다. 변경 사항Elasticsearch 재인덱싱 및 별칭 관리
🎯 4 (Complex) | ⏱️ ~45 minutesPossibly related PRs
제안 레이블
제안 리뷰어
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/example/auction/common/initializer/ElasticsearchIndexInitializer.java`:
- Around line 36-74: The run method (ElasticsearchIndexInitializer.run) is
TOCTOU-vulnerable because it checks alias/index existence via aliasOps.exists()
then creates a physical index and binds the alias in separate steps; fix by
making the alias switch/create atomic or serialized: either acquire a
distributed lock (e.g., Redis/DB lock) around the critical section (wrap run's
existence check, index creation, and alias binding) or, preferably, perform an
atomic alias swap using Elasticsearch's updateAliases API in a single
AliasActions request (create the new indexName from
AuctionDocumentUtil.getNewIndexName, create the index if needed, then call
targetOps.alias(...) with a single AliasActions that adds the alias to indexName
and removes it from any other indices so the alias change is atomic); ensure
code references aliasOps, targetOps, and AuctionDocumentUtil.getNewIndexName are
updated accordingly.
In
`@src/main/java/com/example/auction/domain/auction/search/service/AuctionDocumentReindexService.java`:
- Around line 33-114: The reindexAuctionDocuments method can exit on exceptions
(during index creation, elasticsearch.bulkIndex, alias switch, or delete)
leaving the job IN_PROGRESS and still logging success; wrap the main reindex
flow in a try/catch (inside
AuctionDocumentReindexService.reindexAuctionDocuments), on any exception set
newStatus = AuctionDocumentReindexJobStatus.FAILED, call
job.updateJobStatus(newStatus) and helper.saveJob(job) before rethrowing or
returning, and only emit the final success log when newStatus == DONE (i.e.,
after helper.pointAliasAtNewIndex and old index deletion complete successfully);
ensure all paths (timeout -> FAILED and exception paths) persist the job status
via helper.saveJob and include references to operations like
elasticsearch.indexOps(...).create, elasticsearch.bulkIndex(...),
helper.pointAliasAtNewIndex and elasticsearch.indexOps(...).delete.
- Around line 108-109: pointAliasAtNewIndex(AuctionDocumentUtil.ALIAS_NAME,
newIndexName) can return an empty list; guard the deletion by checking
oldInexes.isEmpty() and skip calling
elasticsearch.indexOps(IndexCoordinates.of(...)).delete() when empty to avoid
IllegalArgumentException. Locate where oldInexes is assigned and replace the
unconditional delete call
(elasticsearch.indexOps(IndexCoordinates.of(oldInexes.toArray(new
String[0]))).delete()) with a simple if (!oldInexes.isEmpty()) { ... } around
that delete.
In
`@src/main/java/com/example/auction/domain/auction/search/service/AuctionElasticsearchService.java`:
- Around line 84-88: The current handling in AuctionElasticsearchService where
maybeDoc.isEmpty() leads to an immediate break must be changed so "not found" is
treated as a transient, retriable case instead of aborting; remove the break and
instead route the event (using the event.auctionId() and maybeDoc check) into
the existing retry/requeue flow (or enqueue a retry with backoff) and only
log/fail permanently after the final retry attempt. Ensure the code path around
maybeDoc.isEmpty() uses the service's retry mechanism (or a retry count on the
event) so deletes/cancels aren’t lost due to temporary index propagation delays.
In `@src/main/resources/db/migration/V8__create_document_reindex_job.sql`:
- Line 4: The migration column new_index_name currently allows NULL but the
entity property newIndexName is annotated nullable = false; update the
V8__create_document_reindex_job.sql DDL so the column is defined as NOT NULL
(e.g., new_index_name varchar(256) NOT NULL) to match the entity constraint and
preserve data integrity; ensure the trailing comma/statement punctuation stays
correct in the CREATE TABLE or ALTER TABLE statement so the migration applies
cleanly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 76fca3ce-e77e-409b-9fee-7cac93131546
📒 Files selected for processing (20)
src/main/java/com/example/auction/common/initializer/ElasticsearchIndexInitializer.javasrc/main/java/com/example/auction/domain/auction/repository/CustomAuctionRepository.javasrc/main/java/com/example/auction/domain/auction/repository/CustomAuctionRepositoryImpl.javasrc/main/java/com/example/auction/domain/auction/search/document/AuctionDocument.javasrc/main/java/com/example/auction/domain/auction/search/dto/AuctionCancelledDocument.javasrc/main/java/com/example/auction/domain/auction/search/entity/AuctionDocumentReindexJob.javasrc/main/java/com/example/auction/domain/auction/search/enums/AuctionDocumentReindexJobStatus.javasrc/main/java/com/example/auction/domain/auction/search/repository/AuctionDocumentReindexJobRepository.javasrc/main/java/com/example/auction/domain/auction/search/service/AuctionDocumentReindexService.javasrc/main/java/com/example/auction/domain/auction/search/service/AuctionDocumentReindexServiceHelper.javasrc/main/java/com/example/auction/domain/auction/search/service/AuctionElasticsearchService.javasrc/main/java/com/example/auction/domain/auction/search/util/AuctionDocumentUtil.javasrc/main/java/com/example/auction/domain/auction/service/AuctionService.javasrc/main/resources/application-dev.ymlsrc/main/resources/application-test.yamlsrc/main/resources/application.ymlsrc/main/resources/db/migration/V8__create_document_reindex_job.sqlsrc/test/java/com/example/auction/domain/auction/search/service/AuctionDocumentReindexServiceHelperTest.javasrc/test/java/com/example/auction/domain/auction/service/AuctionServiceCancelTest.javasrc/test/java/com/example/auction/domain/auth/AuthIntegrationTest.java
검색 조건에서 READY나 ACTIVE만 경매만 조회를 원할 경우 DB를 이용해 검색하도록 변경하였습니다. READY, ACTIVE한 경매는 중요한 경매이기 때문에 상태가 틀려서는 안됩니다. 그렇기 때문에 DB와 차이가 날 수 있는 elasticsearch가 아닌 DB에서 직접 경매 검색 결과를 가져오도록 수정하였습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/auction/common/initializer/ElasticsearchIndexInitializer.java (1)
61-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick win인덱스 생성과 alias 연결 결과를 확인하세요.
IndexOperations#create(settings, mapping)와IndexOperations#alias(AliasActions)는 모두 작업 성공 여부를 boolean으로 반환합니다. 현재 코드는 반환값을 무시하고 성공 로그를 남기고 있어, Elasticsearch가false를 반환한 경우에도 초기화가 성공한 것처럼 진행될 수 있습니다.false일 때 예외를 발생시켜야 이후 재인덱싱이나 검색 시점에 더 늦게 깨지지 않습니다.수정 예시
- targetOps.create( + boolean created = targetOps.create( docOps.createSettings(AuctionDocument.class), docOps.createMapping(AuctionDocument.class)); + if (!created) { + throw new IllegalStateException("auction index 생성에 실패했습니다: " + indexName); + } log.info("[ElasticsearchIndexInitializer] 인덱스 생성 - indexName={}", indexName); // auction-auction alias를 생성 - targetOps.alias(new AliasActions(new AliasAction.Add( + boolean aliased = targetOps.alias(new AliasActions(new AliasAction.Add( AliasActionParameters.builder() .withIndices(indexName) .withAliases(alias) - .build()))); + .build()))); + if (!aliased) { + throw new IllegalStateException("auction alias 연결에 실패했습니다: " + alias + " -> " + indexName); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/auction/common/initializer/ElasticsearchIndexInitializer.java` around lines 61 - 71, The index creation and alias application return booleans but the code ignores them; update the block around targetOps.create(docOps.createSettings(AuctionDocument.class), docOps.createMapping(AuctionDocument.class)) and targetOps.alias(...) to capture their boolean results, log failures with context (indexName, alias) and throw an exception (or rethrow) when either returns false so initialization fails fast instead of continuing on silent failure; reference targetOps, docOps, AuctionDocument, indexName, alias, IndexOperations#create and IndexOperations#alias when making the checks and error handling.
♻️ Duplicate comments (1)
src/main/java/com/example/auction/domain/auction/search/service/AuctionDocumentReindexService.java (1)
102-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick win빈 old index 목록은 삭제 호출을 건너뛰세요.
첫 실행이거나 alias가 아직 없던 경로라면
pointAliasAtNewIndex()가 빈 목록을 돌려줄 수 있습니다. 지금처럼 무조건 삭제를 호출하면 삭제 대상이 없을 때도 실패 경로를 탈 수 있으니,oldIndexes.isEmpty()가드가 필요합니다.수정 예시
List<String> oldIndexes = helper.pointAliasAtNewIndex(AuctionDocumentUtil.ALIAS_NAME, newIndexName); - elasticsearch.indexOps(IndexCoordinates.of(oldIndexes.toArray(new String[0]))).delete(); + if (!oldIndexes.isEmpty()) { + elasticsearch.indexOps(IndexCoordinates.of(oldIndexes.toArray(new String[0]))).delete(); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/auction/domain/auction/search/service/AuctionDocumentReindexService.java` around lines 102 - 103, The code calls elasticsearch.indexOps(...).delete() unconditionally after helper.pointAliasAtNewIndex(...), which can return an empty list; add a guard using oldIndexes.isEmpty() in AuctionDocumentReindexService (around the block that calls helper.pointAliasAtNewIndex(AuctionDocumentUtil.ALIAS_NAME, newIndexName)) so you only call elasticsearch.indexOps(IndexCoordinates.of(...)).delete() when oldIndexes is not empty to avoid erroneous delete calls on first run or when no alias exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/example/auction/domain/auction/search/service/AuctionDocumentReindexService.java`:
- Around line 39-48: The cleanup call helper.deleteOldReindexJobs(monthAgo) runs
before helper.saveJob(...) and outside the try, so if cleanup throws the current
run is never recorded; move the cleanup so job creation happens first and ensure
deleteOldReindexJobs is executed inside the same try/catch that surrounds the
reindex flow (or at minimum after AuctionDocumentReindexJob job =
helper.saveJob(...) and before the operation that can fail) so any exception
will follow the same failure handling path (use the existing try block around
the reindex flow that sets FAILED state via the same save/mark logic).
---
Outside diff comments:
In
`@src/main/java/com/example/auction/common/initializer/ElasticsearchIndexInitializer.java`:
- Around line 61-71: The index creation and alias application return booleans
but the code ignores them; update the block around
targetOps.create(docOps.createSettings(AuctionDocument.class),
docOps.createMapping(AuctionDocument.class)) and targetOps.alias(...) to capture
their boolean results, log failures with context (indexName, alias) and throw an
exception (or rethrow) when either returns false so initialization fails fast
instead of continuing on silent failure; reference targetOps, docOps,
AuctionDocument, indexName, alias, IndexOperations#create and
IndexOperations#alias when making the checks and error handling.
---
Duplicate comments:
In
`@src/main/java/com/example/auction/domain/auction/search/service/AuctionDocumentReindexService.java`:
- Around line 102-103: The code calls elasticsearch.indexOps(...).delete()
unconditionally after helper.pointAliasAtNewIndex(...), which can return an
empty list; add a guard using oldIndexes.isEmpty() in
AuctionDocumentReindexService (around the block that calls
helper.pointAliasAtNewIndex(AuctionDocumentUtil.ALIAS_NAME, newIndexName)) so
you only call elasticsearch.indexOps(IndexCoordinates.of(...)).delete() when
oldIndexes is not empty to avoid erroneous delete calls on first run or when no
alias exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca388a46-7c41-4824-ae21-b4a75f4bc032
📒 Files selected for processing (6)
src/main/java/com/example/auction/common/initializer/ElasticsearchIndexInitializer.javasrc/main/java/com/example/auction/domain/auction/search/exception/AuctionSearchErrorEnum.javasrc/main/java/com/example/auction/domain/auction/search/service/AuctionDocumentReindexService.javasrc/main/java/com/example/auction/domain/auction/search/service/AuctionElasticsearchService.javasrc/main/java/com/example/auction/domain/auction/search/service/AuctionSearchService.javasrc/main/resources/db/migration/V8__create_document_reindex_job.sql
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/example/auction/domain/auction/search/exception/AuctionSearchErrorEnum.java
- AuctionDocumentReindexJob 정리 예외가 job에 기록되도록 수정 - 옛 인덱스가 없을 경우 삭제 호출을 하지 않기
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 작업 내용
✅ 체크리스트 (Checklist)
💬 기타 사항
경매 취소는 상태는 이 서버가 직접 변경하기 때문에 저희 서버가 이를 반영하도록 했습니다.
Scheduler는 뭔가 과한 solution처럼 보일 수도 있지만 저희 architecture 구조상 elasticsearch와 database가 점점 달리지는 것은 어쩔 수 없는 현상이라 생각하여 추가했습니다.
또한 client가 READY혹은 ACTIVE한 경매만 보고 싶어 할 경우 Elasticsearch를 통해 검색 결과를 제공하는 것이 아닌 DB를 통해 검색 결과를 제공하도록 수정하였습니다.
왜냐하면 이 두 상태의 경매는 성능을 포기해서라도 정확한 검색 결과를 제공하는 것이 중요하다고 생각했기 때문입니다.
또 READY, 혹은 ACTIVE한 경매는 시간이 지나면 자연스럽게 DONE, NO_BID, 혹은 CANCELLED된 경매로 상태가 바뀌기 때문에 경매 검색 조회 결과 수가 자연스럽게 조절되어서 성능 면에서 크게 문제가 없을거라 생각했습니다.
이 PR이 merge 된다면 README에 있는 ERD를 수정하는 PR도 추후에 올리겠습니다.
Summary by CodeRabbit
New Features
Improvements
Configuration
Tests