Review #151
Open
Review #151
Conversation
This file contains 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
yoowonyoung
reviewed
Sep 3, 2024
| @@ -15,6 +15,8 @@ | |||
| @AllArgsConstructor | |||
| public class ScrapId { | |||
|
|
|||
| // 현재 복합키를 이용해서 scrapId를 구현했습니다. | |||
| // 실무에서 복합키를 사용하는지, 사용한다면 사용하는 경우가 궁금합니다. | |||
There was a problem hiding this comment.
실무에서도 복합키를 사용 해요. 한가지 예시로 Envers를 이용한 데이터 변경 이력을 관리할때 인데요, Envers를 사용하게 되면 이력테이블의 키는 원본테이블의 키와 이력 테이블의 rev 로 복합키를 구성해서 사용 합니다~
| @@ -21,6 +21,7 @@ public class RestaurantRepositoryCustomImpl implements RestaurantRepositoryCusto | |||
|
|
|||
| private final JPAQueryFactory queryFactory; | |||
|
|
|||
| // querydsl gradle 후 Q 클래스들이 제가 사용하려는 클래스들만이 아니라 모든 클래스들에 대한 Q 클래스가 생성되는데 다른 방법은 없는 것인지 궁금합니다! | |||
There was a problem hiding this comment.
질문의 요지는 @Entity 어노테이션이 붙은 클래스중 queryDsl로 쓰고싶은 특정 클래스들만 q클래스가 만들어지게 하고 싶다~ 가 맞을까요? 그렇다면 queryDsl의 Annotation Processor 의 필터링 규칙을 수정하면 되는데, 혹시 왜 q클래스가 만들어지는것을 막고 싶은것인지가 궁금하네요. queryDsl가 동작하려면 필요한것이 q클래스이기 때문에 이를 막는다는것은 뭔가 사용 방법이 잘못된것은 아닌지 점검 해볼 필요가 있어 보입니다
| @@ -41,6 +41,7 @@ public WaitingEntity getWaitingEntity(Long waitingId) { | |||
| .orElseThrow(() -> new WaitingException("웨이팅이 존재하지 않습니다.")); | |||
| } | |||
|
|
|||
| // 다른 사이트의 코드들을 참고해보면 서비스 메소드들에 트랜잭션을 붙이지 않은 메소드들도 많던데 트랜잭션을 붙여야 하는 경우와 붙히지 않는 경우의 차이가 궁금합니다 | |||
There was a problem hiding this comment.
- 이건 정말 Case By Case라고 할 수 있어요. 클래스 레벨에
@Transactional어노테이션을 붙여놓고 메서드 레벨에서는 생략 하는 경우도 있고, 반대로 메서드 레벨에서 모두 붙여서 사용 하는 경우도 있죠 - 예시를 들어주신것처럼 특정 메서드에서 트랜잭션의 격리 레벨을 변경할때도 붙여야 하고, 트랜잭션의 Read Only 설정을 바꿀때도 붙여줘야 하죠. 정확히 어떤 상황에서 메서드, 클래스 레벨에 붙일것인지에 따라서 모두 다르다고 볼수있습니다
- 단, 아예 어노테이션을 생략하는 경우는 트랜잭션이 관리 되지 않기 때문에 이것은 꼭 설정 하는게 좋다고 생각이 드네요
| // 식당 프리뷰(홈 화면)를 조회할 때마다 DB에서 리뷰, 스크랩 데이터를 조회하고, 업데이트를 하면 서버의 부하가 클 것이라 예상하였습니다. | ||
| // 그렇기 때문에 조회할 때마다 업데이트가 아닌 스케줄러를 이용해서 1시간 마다 업데이트하는 방식으로 진행하고 있습니다. | ||
| // 이 방식보다 더 효율적이고 좋은 방안이 있을까요? | ||
| // 또한 홈 화면의 식당 프리뷰 조회 시마다 DB의 데이터를 계속 조회하는 것이 아닌 다른 좋은 방안이 있을까요? 떠오르는 것은 캐시를 활용하면 될 것 같습니다. |
There was a problem hiding this comment.
- 서버에 부하가 예상되어서 미리 계산을 해두는 방식을 떠올린것은 좋은 아이디어입니다! 다만, 정말로 서버에 부하가 걸리는지, 응답시간은 얼마나 걸리는지 측정을 해본것일까요?
- 리뷰수, 스크랩수를 가져오는것은 각각 리뷰 테이블, 스크랩 테이블에 식당ID로 인덱스가 생성 되어 있다면 실제론 오래걸리지 않고 또 부하도 심하지 않아요. 직접 쿼리를 작성 해보고 쿼리 플랜을 조회함으로써 성능이 얼마나 걸릴지 조회 해보고 판단하는 습관을 가지면 좋을것같습니다
- 만약 정말 오래 걸린다면, 주기적으로 계산하는 방법 이외에도 조회시 일단 기존에 있던 값으로 반환을 하며 백그라운드에서 비동기로 값을 업데이트 해주는 방식도 있어요. 미리 계산된 값을 주기 때문에 응답시간엔 영향이 없고, 또 다음번 조회시에는 업데이트 된 값을 받아볼 수 있게 됩니다. 이렇게 하면 모든 데이터를 업데이트 하지 않고 자주 조회되는것만 업데이트 해줄수 있죠
- Redis를 사용하는것은 좋은 대안이 될 수 있어요. 단, 캐시 구현체로 Redis가 아닌 Caffein Cache와 같은 로컬 캐시를 사용할수도 있습니다. Redis를 구축하는데 시간이 오래 걸릴것 같다면 로컬 캐시를 고려 해보는것도 좋은 대안이 될거에요
- 특히 캐시를 구현 한다면 만료 정책을 어떻게 정할것인지도 중요 한데요. 일반적인 TTL을 사용하는 방법도 있고 Loading Cache로 주기적으로 업데이트 시켜주는 방법도 있어요. 위의 식당 프리뷰에서 리뷰, 스크랩 수를 계산하는것에도 캐시를 사용 한다면 스케줄러를 통한 주기적 업데이트 대신 Loading Cache로 주기적으로 업데이트 시키는것도 방법이죠
| @@ -1,6 +1,6 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <configuration> | |||
|
|
|||
| <!-- ec2 서버에서 실행할 때 로그 파일을 따로 만들기 위해 logback-spring.xml 파일을 작성했는데 디렉터님께서 말씀하신 로그 파일이 이런 형식으로 작성하는 것이 맞는지 궁금합니다.--> | |||
There was a problem hiding this comment.
- 일단 logback은 작성하시는 방법은 appender도 있고, rollingPolicy도 있고 잘 작성 한것으로 보여요
- 하지만 지금은 Spring Profile별로 다른 logback 파일이 존재하는것이 아닌 하나의 파일로 합쳐져있는것으로 보이네요. 이런 경우 프로파일별로 관리 하기에 복잡해져서 logback파일 자체를 프로파일별로 독립 시키는것을 추천합니다
| @@ -2,6 +2,9 @@ | |||
|
|
|||
| public class CatchLineException extends RuntimeException { | |||
|
|
|||
| // Custom Exception의 최상위 Exception으로 이 Exception을 이용하고 있습니다. | |||
| // 다른 Custom Exception은 모두 이 Exception을 상속받고 있습니다. | |||
| // Exception에 추가로 넣어야 할 정보와 실무에서는 Custom Exception을 어떻게 사용하고 계신지 궁금합니다. | |||
There was a problem hiding this comment.
자주 발생하는 Exception의 종류를 구분 해주는 ErrorType 같은걸 Enum으로 선언해서, 각 Enum들이 기본 Message를 가지게 하는 방법도 있구요, 생성자 오버로딩을 다양하게 해서 ErrorType만 받는다던지, ErrorType과 message를 같이 받는다던지, 혹은 다른 API or 라이브러리를 사용하다가 발생하는 Exception에 대응하기 위해 Throwable 도 파라미터로 받는 방법도 있습니다~
|
|
||
| // TODO: 코드리뷰 | ||
|
|
||
|
|
There was a problem hiding this comment.
- 비즈니스 로직에 따라서 다양한 필터들은 존재할 수 있지만, 이 사이에 중복되는 로직이 많다면 뭔가 정책 설계가 잘못된것은 아닌지 돌아볼 필요는 있습니다. 사실 필터는 모든 요청 & 응답에 대해서 적용이 되는것이 기본이기 때문에 중복이 발생한다면 둘중 하나는 필요 없을수도 있단 소리거든요
- 그럼 서로 다른 부분은 어떻게 처리하느냐에 대한 고민이 남으실텐데요, 이런 경우 인터셉터를 조합해 활용하는것도 방법입니다. 공통적인 인증 관련 로직은 필터로 보내서 모든 요청에 대해서 적용되게 만들고, 특정 페이지의 경우는 인터셉터를 통해서 추가적인 로직을 적용 시키는 형식으로요
- 필터와 인터셉터는 서로 작동하는 시점과 대상, 목적이 모두 다르기 때문에 적절하게 판단 해서 어느 부분에서 필터를 쓸지 어느 부분에서 인터셉터를 쓸지 고민 해보는게 좋을것 같아요
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.