-
Notifications
You must be signed in to change notification settings - Fork 2
Feat/trade core #150
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
Feat/trade core #150
Conversation
local용 gitignore추가
단일 종목에 대해 여러 주문들이 동시에 들어온다고 하더라도, 체결은 종목마다 단일 스레드에서 처리되도록 수정했습니다.
동시성 이슈를 비관적 락 + repeatable_read로 대응한 버전입니다.
주문이 user 패키지에 정의된 repository를 참조하도록 변경했습니다. lock, timeout등의 공통로직들이 적용되어야 하기 때문에 통합시키는것이 좋다고 판단했습니다.
동시성 테스트를 추가하면서 테스트 내용을 더 잘 분석할 수 있도록 로그 설정과 base 테스트를 수정했습니다.
read_committed 로 변경
- 데이터 정합성(무결성) 테스트 필요
Junh-b
left a comment
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.
수정내용 확인했습니다.
account, wallet 관련 로직도 더 깔끔해졌고, 동시성 이슈도 실마리가 보여서 이번 pr로 큰산 하나를 넘은 것 같네요
gitignore, build.gradle 등 수정 내역중 일부는 dev에 합쳐지지 않는 것이 좋을 것 같아 리뷰를 남겨드렸으니 확인 부탁드립니다.
나머지 작은 수정사항들은 제가 별도 브랜치에서 수정토록 하겠습니다.
src/main/java/com/cleanengine/coin/user/info/application/AccountService.java
Show resolved
Hide resolved
...java/com/cleanengine/coin/order/adapter/out/persistentce/account/OrderAccountRepository.java
Outdated
Show resolved
Hide resolved
109an94
left a comment
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.
티커별로 단일 쓰레드 처리하는 거 배워갑니다
그리고 dev로 병합했는데 아까 tradequeuemanager에 import 바꾼거 commit하면 안됐는데 해버려서.. 여기 충돌난거 같습니다 ㅜㅜ @caniro
TradeQueueManager를 아예 삭제해서 상관없습니다 ㅋㅋ 리뷰 감사합니다 |
테스트 편의를 위해 잠시 추가했었던 설정 수정내용을 원래대로 복원했습니다.
각 영역에서 별도의 목적으로 사용중이던 Repository 클래스를 제거하고, 통일된 Repository를 사용하도록 변경했습니다.
# Conflicts: # src/main/java/com/cleanengine/coin/trade/application/TradeQueueManager.java

✨ 작업내용
🐞 이슈사항