-
Notifications
You must be signed in to change notification settings - Fork 0
☀️ Core > Redis 코어 모듈 + 도메인별 캐싱 전략 수립 #106
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
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.
👍 이번에도 귀감이 되는 작업 감사합니다.
@silberbullet 님께서 항상 좋은 영감을 주는 작업을 많이 해 주시는 것 같습니다.
Properties 초기화 및 전처리, 방어적인 안전 코딩, 유연한 설계, 깔끔한 테스트 케이스 분류 등 눈에 띄는 작업이 많았습니다. 👍
아직 논의가 진행 중이었던 작업들도 포함되어 있기 때문에, 아직 approve는 할 수 없지만 발견해 둔 수정 사항을 먼저 리뷰로 남겨 두었습니다.
참고해서 조금 더 작업 부탁드리겠습니다. 💪
String host, | ||
List<Integer> ports, |
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.
❗ host-port가 짝을 이루어 제공되어야 합니다.
- 1 host 고정이 아닐 수 있습니다.
- 1 host : N ports 관계가 아닐 수 있습니다.
- 입력받는 아이디어는 다양할 것 같습니다. 떠오르는 예시들은 다음과 같습니다.
app.redis: nodes: - redis-node1:6379 - redis-node2:6379 - redis-node3:6379
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.
👍 좋은 아이디어 감사합니다. 다음과 같이 수정하였습니다.
Redis application.yml 은 다음과 같이 설정을 해야합니다.
app:
redis:
use-cluster-mode: true
nodes:
- localhost:70000
- localhost:7001
- localhost:7002
@ConfigurationProperties("app.redis") | ||
public record RedisProperties( | ||
Boolean useClusterMode, | ||
String host, | ||
List<Integer> ports, | ||
RedisCacheProperties cache |
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.
🤔 커스텀 속성을 spring.data.redis
속성보다 선호해 봅니다.
커스텀 속성은 자유도가 높은 만큼 우리가 원하는 제어를 추가하기 좋아 보입니다. 단점은 지금처럼 새로 properties 클래스를 추가해 핸들링한다는 점이지만, 좋은 커스텀 시도라고 생각합니다.
작업해 주신 내용도 많은 참고가 되는 것 같습니다. 👍
📝 Additional Notes
About spring.data.redis
SpringBoot에서 redis 사용 시 일부 기능이 spring.data.redis
속성을 참조합니다.
간단한 리서치 결과, 다음 기능이 spring.data.redis
에 의존하는 것 같습니다.
- ConnectionFactory/Template 자동 생성
:spring.data.redis
기반으로 Lettuce/Jedis ConnectionFactory와RedisTemplate
이 자동 등록 되는 부분 - Redis 기반 캐시(Cache)·세션(Session) 자동 구성
:spring.cache.redis
또는spring.session.redis
구성을 통해 RedisCacheManager나 SessionRepository가 자동 등록 되는 부분 - Reactive Redis 자동 설정
: WebFlux 환경에서 ReactiveRedisConnectionFactory/ReactiveRedisTemplate이 자동으로 설정되는 부분 - Actuator의 Health 체크
:/actuator/health/redis
같은 Redis 헬스 인디케이터가 활성화되는 부분
이 항목들은 대부분 자동 설정에 대한 것이고, 이중 일부는 자유도를 위해 수동으로 설정하는 사례도 많으며, 작업량이 그렇게 많지 않습니다.
spring.data.redis
속성에서 Cluster mode 확인하기
만약 spring.data.redis.*
속성을 사용한다면 'cluster mode 사용 중' 여부를 체크하는 속성을 제공하지 않으므로 다음 속성을 통해 간접적으로 클러스터 사용 여부를 확인합니다.
spring.data.redis:
cluster:
nodes: # 이 속성이 null이거나 비어 있다면 standalone 모드
- redis-node-1:6379
- redis-node-2:6379
- redis-node-3:6379
log.info("Redis Connection Mode is Standalone"); | ||
|
||
if (ports.size() > 1) { | ||
throw new RuntimeException("Redis Connection Mode has more than one port"); |
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.
💬 오류 메시지를 수정할 수 있어 보입니다.
"More than one Redis node is configured in standalone mode."
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.
오류 메시지 부분 수정하였습니다!
if (host == null || host.isBlank()) { | ||
host = "127.0.0.1"; | ||
log.warn("redis host is null"); | ||
} |
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.
💊 기본값은 루프백 IP 명시도 좋지만, localhost로 설정하는 건 어떨까요?
더 유연하게 처리할 수 있습니다.
- IPv4 및 IPv6 호환
- 환경에 따라
localhost
를 다른 타깃에 연결
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.
🤔 저는 유연성 보다는 안정성을 선택했습니다.
로컬 hosts 파일의 잘못된 설정이나, Docker/컨테이너 환경에서 발생할 수 있는 DNS 해석 문제 등을 고려했을 때, 명시적인
루프백 주소(127.0.0.1) 사용`이 사용자 입장에서는 더 안정적일 수 있다고 판단했습니다.
다만, @merge-simpson 님의 의견과 코어 모듈의 방향성을 고려했을 때, 유연성을 중시하는 방향이 더 적절하다고 판단하여 해당 부분은 수정 반영하였습니다. 🙇
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.
127.0.0.1과 localhost가 도커 컨테이너에서 다르게 실행되나요?
예전 프로젝트 진행할때 저도 springboot를 docker로 실행시켰었는데,
localhost, 127.0.0.1 모두 동작안해서 host.internal.docker로 작성했었습니다.
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.
도커 컨테이너 내에서 IP 가 다른걸로 알고 있습니다. 도커 컨테이너와 본인 사용자 PC 까지 요청을 연결 시켜주는 부분이 도커 브릿지
라는 걸로 알고 있습니다. 추후에 더 자세히 찾아보고 공유드리겠습니다!
String host, | ||
List<Integer> ports, |
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.
👍 저는 List<Integer>
대신 int[]
선언을 선호했지만, 최근 List<Integer>
가 낫다고 느끼고 있습니다.
비교하면 다음과 같습니다.
List<Integer>
- 장점: 유지보수성 (디버깅이 편리합니다.)
- 단점: 불필요한 박싱(boxing) 등 추가적인 오버헤드
int[]
- 장점: 불필요 오버헤드가 없음.
- 단점: 컬렉션 API, toString 등 유연한 핸들링에 제약
이 오버헤드가 비율로는 꽤 큰 차이여서 굳이 List
및 Integer
(래퍼클래스)가 주는 장점이 크지 않다면 int[]
를 사용하는 주의였습니다. 하지만 편리한 작업과 디버깅을 위해, 간단한 속성 설정에도 List<Integer>
를 허용하는 게 좋아 보입니다. 👍
public record DomainCacheProperties( | ||
long ttl, | ||
Boolean disableNull, | ||
String prefix | ||
) { | ||
public DomainCacheProperties { | ||
// Default 1 minute | ||
if (ttl == 0L) { | ||
ttl = 60L; | ||
} |
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.
❗ TTL(Time-To-Live) 설정은 '만료 없음'도 가능해야 할 것 같습니다.
long
→Long
(권장)- TTL
null
체크 제외 (타입이Long
일 시) - TTL 음수 체크
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.
좋은 지적 감사합니다 👍
하기와 같이 수정하였습니다
// Default 1 minute
if (ttl == null) {
ttl = 60L;
}else if(ttl < 0) {
throw new RuntimeException("TTL is set to 'no expiration'. The entry will not expire.");
}
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.
comment를 수정했습니다!
null 체크라고 써 뒀는데, null 체크 제외라고 수정했습니다.
null일 때 '만료 없음'으로 인식할 수 있어 보입니다.
(또는 별도 타입을 사용)
별도 타입 사용 시
Ex: Expiration 타입 (redis)
public Expiration ttlAsExpiration() {
if (ttl == null) {
return Expiration.persistence();
}
return Expiration.seconds(ttl);
}
🚧 주의사항
- 외부 라이브러리에 의존하는 타입 사용 시 종속성 발생
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.
수정 완료 하였습니다.
ttl null 체크는 제외 했으며, null 아닐 시에만 시간을 cacheManger에서 시간을 설정하도록 수정하였습니다! 👍
if(domainCacheProperties.ttl() != null){
config = config.entryTtl(Duration.ofSeconds(domainCacheProperties.ttl()));
}
if (prefix != null) { | ||
prefix = prefix.strip(); | ||
} |
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.
💊 String
은 null
대신 빈 문자열을 다루는 게 안전합니다.
이렇게 수정하는 건 어떨까요?
- if (prefix != null) {
- prefix = prefix.strip();
+ if (prefix == null) {
+ prefix = "";
}
+
+ prefix = prefix.strip();
- Null Pointer Exception 방지
- 빈 문자열도 의미 전달 충분
참고: 이펙티브 자바
문자열은""
, 각종 컬렉션도 빈 리스트, 빈 세트, 빈 맵 등을 권장.
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.
👍 LGTM입니다 !
발생할 수 있는 여러 오류에 대한 처리, 클래스를 통한 설정 파일 구조화 등 코드를 통해 많이 배웠습니다! 고생 많으셨습니다!
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.
👍 쉽지 않은 작업임에도, 전체적으로 깔끔하고 유연하게 설계해주셔 감사합니다!
각 도메인(모듈) 별로 redis 설정하는 것을 이렇게 유연하게 관리할 수 있다니 놀라웠습니다..!
redis core 모듈 설계에서도 필요에 따라 redis-api / template / cache 로 나누어서 설계하신 부분도 인상 깊었습니다!
또한 테스트 코드를 통해서 application.yml에서 redis 설정을 어떻게 해야 하는지 엿볼 수 있어 좋았습니다!
한 가지 아쉬운 점은 새로운 Redis 라이브러리를 사용하다보니 익숙하지 않은 필드나 메서드들이 많아, 단순한 코드이지만 한 번에 이해하는 데 어려움을 겪었던 것 같습니다..😭 간단한 주석이 추가된다면, 추후에 코드를 다시 리뷰할 때 조금 더 빠르게 파악할 수 있지 않을까 합니다!
예시
- DomainCacheProperties의 disableNull, prefix 필드의 역할
- config = config.computePrefixWith(cacheName -> domainCacheProperties.prefix());
PR에 작성된 모듈이나 주요 클래스 대한 설명도 주석으로 같이 남긴다면 코드 이해에 도움이 될 것 같습니다!
@wch-os 님 리뷰 감사드립니다! 👍 확실히 @wch-os 님 의견 덕분에 유의미한 전달을 위해 주석 추가에 깊은 고찰을 하게 됐습니다.
잠깐의 캐싱 기능 관련된 변수의 설명을 드리자면
|
Issues
Description
driven-redis-adapter
모듈과monolith
모듈에게 적용 할 수 있도록 합니다.✅
redis
코어 모듈 구성 및 다이어그램redis-api
redisConnectionFactory
RedisConnectionFactory
redis-api
redisProperties
RedisProperties
application.yml
설정을 매핑하여 도메인별 TTL, null 캐싱, prefix 등을 구성할 수 있는 커스텀 설정 객체입니다.redis-template
redisTemplate
RedisTemplate<String, Object>
redis-template
stringRedisTemplate
StringRedisTemplate
redis-cache
cacheManager
RedisCacheManager
✅ Redis application.yml 예시
Review Points
article
,views
등)에 대해 TTL, prefix, null 캐싱 여부를application.yml
로 설정할 수 있도록 하여 운영 환경에서 유연하게 캐시 정책을 조정할 수 있습니다.RedisCacheManager
를 통해 각 도메인에 대해 별도의RedisCacheConfiguration
을 동적으로 적용합니다.How Has This Been Tested?
Additional Notes