Conversation
Summary of ChangesHello @koosco, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 미디어 바이너리 데이터 조회 성능을 향상시키고 S3 스토리지 비용을 절감하기 위해 Redis 기반 캐싱 시스템을 도입합니다. 캐시 미스 시 발생하는 S3에 대한 동시 요청 폭주(cache stampede)를 방지하기 위해 분산 락 메커니즘을 구현하여 시스템 안정성을 강화했습니다. 또한, 바이너리 데이터의 효율적인 Redis 저장을 위한 전용 템플릿을 추가하여 캐싱 솔루션의 완성도를 높였습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Code Format Check ✅
|
There was a problem hiding this comment.
Code Review
이 PR은 미디어 도메인에 Redis 캐시와 분산 락을 도입하여 성능을 개선하고 cache stampede 문제를 해결하는 중요한 변경사항을 담고 있습니다. 전반적으로 포트와 어댑터 패턴을 잘 활용했고, 분산 락 구현도 견고합니다. 테스트 코드 또한 동시성 시나리오를 포함하여 잘 작성되었습니다. 다만, 몇 가지 중요한 수정이 필요한 부분이 있습니다. 가장 시급한 문제는 바이너리 데이터 캐싱에 부적절한 Redis 직렬화 설정이 사용된 점입니다. 이로 인해 캐시가 의도대로 동작하지 않을 수 있습니다. 또한, 분산 락 획득 실패 시의 fallback 로직이 여러 인스턴스 환경에서 여전히 stampede를 유발할 수 있는 잠재적 위험을 가지고 있습니다. 마지막으로, 비동기 작업에 공용 스레드 풀을 사용하는 부분에 대한 개선 제안을 포함했습니다.
src/main/kotlin/com/yapp2app/media/infra/cache/redis/RedisMediaBinaryCacheAdapter.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/yapp2app/media/application/usecase/GetImageByKeyUseCase.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/yapp2app/media/infra/cache/redis/RedisMediaBinaryCacheAdapter.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/yapp2app/media/infra/lock/redis/RedisDistributedLockAdapter.kt
Outdated
Show resolved
Hide resolved
Code Format Check ✅
|
Test ❌
|
Code Format Check ✅
|
Test ✅
|
Code Format Check ✅
|
Test ✅
|
src/main/kotlin/com/yapp2app/media/infra/cache/redis/RedisMediaBinaryCacheAdapter.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/yapp2app/media/infra/cache/redis/RedisMediaBinaryCacheAdapter.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/yapp2app/media/infra/cache/redis/RedisMediaBinaryCacheAdapter.kt
Outdated
Show resolved
Hide resolved
| private val log = LoggerFactory.getLogger(javaClass) | ||
|
|
||
| // In-memory single-flight map (same as production) | ||
| private val inFlightOperations = ConcurrentHashMap<String, CompletableFuture<Any?>>() |
There was a problem hiding this comment.
저희 분산락을 구현해야해서 동일한 WAS끼리만 중복을 막아주는 건 의미 없을것 같습니다. 어차피 Redis를 이용하여 분산락을 쓰기 떄문에 해당 Map은 불필요해보입니다!
src/main/kotlin/com/yapp2app/media/infra/cache/redis/RedisMediaBinaryCacheAdapter.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/yapp2app/media/application/usecase/GetImageByKeyUseCase.kt
Show resolved
Hide resolved
Code Format Check ✅
|
Test ✅
|
| private val log = LoggerFactory.getLogger(javaClass) | ||
|
|
||
| // In-memory single-flight map: 동일 JVM 내 동시 요청 중복 제거 | ||
| private val inFlightOperations = ConcurrentHashMap<String, CompletableFuture<Any?>>() |
There was a problem hiding this comment.
저희 분산락을 구현해야해서 동일한 WAS끼리만 중복을 막아주는 건 의미 없을것 같습니다. 어차피 Redis를 이용하여 분산락을 쓰기 떄문에 해당 Map은 불필요해보입니다!
There was a problem hiding this comment.
요거 락과는 별개로 약간 성능 최적화 관점에서 이점이 있습니다.
Thread.sleep 사용 시 주기마다 Redis 락 점유 시도를 반복하는데, CompletableFuture 사용 시 Redis 호출 없이 첫 번째 스레드의 결과를 바로 공유받는 차이가 있습니다.
트래픽이 거의 없어 큰 차이는 없을 것 같은데, 없애는게 나을까요?
Code Format Check ✅
|
Test ✅
|
summary
details
media domain redis cache를 추가
media 도메인에 redis cache adapter를 추가했습니다. 기존에 존재하던 FakeMediaBinaryCacheAdapter 대신 RedisMediaBinaryCacheAdapter를 사용합니다.
캐싱은 24시간동안 지속되며, 2시간이 남았을 때 다시 조회하면 TTL이 갱신되도록 설정하였습니다.
캐싱이 깨졌을 때 여러 건의 요청이 몰리게 되면 각 요청들이 모두 S3에 요청을 보내는 문제를 방지하고자 redis 기반 분산락을 추가하였습니다. redis 인스턴스 실행 후 캐싱된 값이 없을 때도 동일하게 적용됩니다.
binary redis template 추가
기존 Jackson Serializer이 Binary data에 대해 정상 동작하지 않는 이슈를 해결하기 위해 BinaryRedisTemplate을 추가하였습니다.
Jackson Serializer를 사용하면 ByteArray가 json으로 직렬화되고, 캐싱된 값을 꺼내오는 과정에서 Casting 오류가 발생하여 매번 S3 조회가 발생하는 문제가 발생하였습니다. Binary 전용 RedisTemplate을 추가하여 캐스팅 오류를 방지하고, byte array를 redis에서 정상적으로 꺼내오도록 수정을 반영하였습니다.
cache missing / hit log