Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Feat/#137 MultiQueue를 이용하여 Controller에서 오는 로그 요청의 부하 감소 #140

Merged
merged 8 commits into from
Aug 29, 2024
Merged

Feat/#137 MultiQueue를 이용하여 Controller에서 오는 로그 요청의 부하 감소 #140

merged 8 commits into from
Aug 29, 2024

Conversation

miiiinju1
Copy link
Member

@miiiinju1 miiiinju1 commented Aug 29, 2024 •
edited
Loading

🚀 작업 내용

  • 현재 아키텍처에서, 싱글 스레드 - 큐 - 싱글스레드 쌍을 수평확장할 수 있도록 구현했습니다.
  • 현재 한 클래스 내에 모두 섞여있는데, 추후 리팩토링이 시급합니다.
  • RPS가 500정도로 개선되었고, 초당 약 50000개의 로그 처리가 가능합니다.

📸 이슈 번호

👀 Focus Commits [Optional]

  • 커밋해시: 내용

✍ 궁금한 점

  • 중점적으로 봐줄 내용
  • 변수명 괜찮나요
  • 로직이 좀 더럽나요
  • 가독성이 좀 그런가요?

@miiiinju1 miiiinju1 self-assigned this Aug 29, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 29, 2024 •
edited
Loading

Risk Level 3 - /home/runner/work/Team5-Guys/Team5-Guys/logbat/src/main/java/info/logbat/domain/log/queue/ReentrantLogQueue.java

The commented-out @primary annotation may lead to confusion. If it's not needed, consider removing it to improve code clarity. Additionally, ensure that the locking mechanism is robust to avoid potential deadlocks.

Risk Level 3 - /home/runner/work/Team5-Guys/Team5-Guys/logbat/src/main/java/info/logbat/domain/log/repository/AsyncLogRepository.java

The deprecated methods should ideally be removed or replaced with a proper implementation. Leaving them in the codebase can lead to confusion and misuse. Consider implementing a proper asynchronous save mechanism instead.

Risk Level 4 - /home/runner/work/Team5-Guys/Team5-Guys/logbat/src/main/java/info/logbat/domain/log/repository/AsyncMultiProcessor.java

The constructor initializes multiple threads and queues, which can lead to resource exhaustion if not managed properly. Ensure that the number of queues and threads is configurable and monitored. Additionally, consider adding error handling in the produce method to manage potential failures.

🛠️🔍⚠️

Powered by Code Review GPT

@tidavid1
refactor: AsyncMultiProcessor 리팩토링
cb59300 
- 내부 로직 일원화 및 린팅을 적용했습니다.
- MultiProcessor -> AsyncMultiProcessor 클래스 명 변경을 진행했습니다.
@tidavid1
chore: 불필요한 메서드 구현 제거
c6eaf91 
- EventConsume<T> 구현을 제거했습니다.
tidavid1
tidavid1 approved these changes Aug 29, 2024
View reviewed changes
Copy link
Member

@tidavid1 tidavid1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻 조습니다. 찬찬히 리팩토링 하면 좋을거같아요

LuizyHub
LuizyHub approved these changes Aug 29, 2024
View reviewed changes
Copy link
Member

@LuizyHub LuizyHub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multie level 큐 좋습니다!
같이 더 테스트 해봐요

@tidavid1 tidavid1 merged commit e3857a3 into dev Aug 29, 2024
1 check passed
@tidavid1 tidavid1 deleted the feat/#137 branch August 29, 2024 02:19
LuizyHub pushed a commit that referenced this pull request Aug 29, 2024
@miiiinju1 @tidavid1 @LuizyHub
Feat/#137 MultiQueue를 이용하여 Controller에서 오는 로그 요청의 부하 감소 (#140)
eebd78b 
* chore: 임시로 ReentrantLogQueue에서 Primary 제거

* feat: MultiProcessor 추가

- 임시로 여러 큐를 가지는 MultiProcessor를 추가함

* chore: default queue개수 조정

* feat: flatterQueue 싱글 스레드풀 추가 및 queue count default를 3 설정

* chore: default queue사이즈 설정

* refactor: AsyncMultiProcessor 리팩토링

- 내부 로직 일원화 및 린팅을 적용했습니다.
- MultiProcessor -> AsyncMultiProcessor 클래스 명 변경을 진행했습니다.

* chore: 불필요한 메서드 구현 제거

- EventConsume<T> 구현을 제거했습니다.

* chore: 의존성 정리, 사용하지 않는 Component 제거

---------

Co-authored-by: KyungMin Lee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tidavid1 tidavid1 tidavid1 approved these changes

@LuizyHub LuizyHub LuizyHub approved these changes

Assignees

@miiiinju1 miiiinju1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@miiiinju1 @tidavid1 @LuizyHub