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.

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

Api: πŸ› N+1 문제 κ°œμ„  ν…ŒμŠ€νŠΈμ˜ 간헐적 μ‹€νŒ¨λ¬Έμ œ μˆ˜μ • #122

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

asn6878
Copy link
Collaborator

@asn6878 asn6878 commented Jul 4, 2024

μž‘μ—… 이유

CI ν™˜κ²½μ—μ„œ SpendingSearchServiceTest.java의 간헐적 ν…ŒμŠ€νŠΈ μ‹€νŒ¨κ°€ ν™•μΈλ˜μ–΄ 이슈 체크 및 μˆ˜μ •μ˜ ν•„μš”μ„± λŒ€λ‘.


μž‘μ—… 사항

μ—λŸ¬ λ°œμƒ 원인

image
원인은 일단 μ™Έλž˜ν‚€ μ œμ•½μ‘°κ±΄μ˜ μœ„λ°˜μ΄μ—ˆμŠ΅λ‹ˆλ‹€.

Spending 객체가 SpendingCustomCategory ν…Œμ΄λΈ”μ— μ‘΄μž¬ν•˜μ§€ μ•ŠλŠ” idλ₯Ό spending_custom_categoryν•„λ“œμ˜ κ°’μœΌλ‘œ κ°€μ§€κ²Œ λ˜λ©΄μ„œ μœ„λ°˜λ˜μ—ˆλ˜ κ²ƒμ΄μ—ˆμŠ΅λ‹ˆλ‹€.

문제 λ˜λŠ” λΆ€λΆ„

μ½”λ“œλ₯Ό λ‹€ λ•Œλ € λ„£μ–΄μ„œ μ„€λͺ…ν•˜λ‹€κ°€ λ„ˆλ¬΄ λ³΅μž‘ν•΄μ„œ κ°„λ‹¨ν•˜κ²Œ μ€„μ˜€μŠ΅λ‹ˆλ‹€.

  • SpendingSearchServiceTest.javaμ—μ„œ ν…ŒμŠ€νŠΈλ₯Ό μœ„ν•΄ μ»€μŠ€ν…€ μΉ΄ν…Œκ³ λ¦¬λ₯Ό κ°€μ§€λŠ” Spending 객체듀을 bulkInsertν•˜λŠ” 과정이 μ‘΄μž¬ν•˜λŠ”λ°, 이λ₯Ό μˆ˜ν–‰ν•˜κΈ° μœ„ν•΄μ„œλŠ” λ¨Όμ € μ»€μŠ€ν…€ μΉ΄ν…Œκ³ λ¦¬λ“€μ„ DB에 생성해 μ£Όμ–΄μ•Ό 함.
  • 이 λ•Œλ¬Έμ— SpendingCustomCategoryFixture μ—μ„œ μ»€μŠ€ν…€ μΉ΄ν…Œκ³ λ¦¬λ₯Ό capacity 개 만큼 DB에 bulkInsert 함.
  • κ·Έ ν›„, capacity 만큼의 Spending듀을 DB에 μ €μž₯함. μ΄λ•Œ CustomCategory ν•„λ“œμ— λ“€μ–΄κ°ˆ id(μ™Έλž˜ν‚€) 값은 {1~capacity}μ‚¬μ΄μ—μ„œ 랜덀으둜 λ½‘μ•˜μŒ. 1 + FLOOR(RAND() * %d)

1~capacity μ‚¬μ΄μ˜ κ°’λ“€μ—μ„œ 랜덀으둜 λ½‘λŠ” 과정이 무엇인가 잘λͺ» 된 것 같은데, μ œκ°€ 직접 찍어본 κ²°κ³Ό 잘λͺ»λ  μˆ˜κ°€ μ—†μŠ΅λ‹ˆλ‹€...
λΆ„λͺ…νžˆ capacity 개의 μ»€μŠ€ν…€ μΉ΄ν…Œκ³ λ¦¬λ“€μ΄ μ €μž₯되고, Spending 객체듀을 μƒμ„±ν• λ•ŒλŠ” 1~capacity μ‚¬μ΄μ˜ idλ₯Ό μ‚¬μš©ν•˜λŠ”λ° μ—¬κΈ°μ„œ μ—†λŠ” idλ₯Ό μ°Έμ‘°ν•  일이 μ—†λŠ”λ° 말이죠.. (μ•„λ§ˆ 이건 μ €λ²ˆμ— μ–ΈκΈ‰ν•œ 병렬싀행과 무엇인가 관련이 μžˆλŠ” 것 κ°™μŠ΅λ‹ˆλ‹€..)

κ²°κ΅­ κ·Έλž˜μ„œ μ €λŠ” κΈ°μ‘΄ μ½”λ“œμ˜ λ¬Έμ œμ μ„ 찾지 λͺ»ν–ˆμŠ΅λ‹ˆλ‹€....만!
일단은 전에 @psychology50 λ‹˜μ΄ ν•œλ²ˆ μ–ΈκΈ‰ν•΄μ£Όμ…¨λ˜ 것 처럼 랜덀이 ν•„μš”μ—†λŠ” 곳에 ꡳ이 λžœλ€μ„ 넣은것이 화근이라고 생각해
fetchν…ŒμŠ€νŠΈμ˜ λͺ©μ μ„±μ„ μœ μ§€ν•¨κ³Ό λ™μ‹œμ— 랜덀으둜 customCategoryIdλ₯Ό λ„£μ–΄μ£ΌλŠ” 뢀뢄을 λ³€κ²½ν•΄ μ™Έλž˜ν‚€ μ œμ•½μ‘°κ±΄μ΄ μœ„λ°˜λ  여지λ₯Ό μ™„μ „νžˆ μ œκ±°ν–ˆμŠ΅λ‹ˆλ‹€.

그에따라 bulkInsertSpendingλ©”μ†Œλ“œ μ‚¬μš©ν•˜λŠ” 방법이 λ³€κ²½λ˜μ—ˆκ³ , μ΄λŠ” javaDoc으둜 λͺ…μ‹œν•΄ λ‘μ—ˆμŠ΅λ‹ˆλ‹€.


리뷰어가 μ€‘μ μ μœΌλ‘œ 확인해야 ν•˜λŠ” λΆ€λΆ„

SpendingFixture.javaμ—μ„œ bulkInsertSpending λ©”μ†Œλ“œλ§Œ λ΄μ£Όμ‹œλ©΄ λ©λ‹ˆλ‹€.

일단 이것도 κ°€μž₯ 크게 μ˜μ‹¬λ˜λŠ” 뢀뢄을 μ œκ±°ν•œ 것일 뿐이지, λ‘œμ»¬μ—μ„œλŠ” μ‹€νŒ¨ν•˜λŠ”μΌ 없이 계속 μ„±κ³΅ν•΄μ„œ 병합 ν›„ μ§€μΌœ 보아야 ν•  λ“― ν•©λ‹ˆλ‹€.... 😒

#119 PR은 λ³Έ PR merge이후에 mergeν•˜κ² μŠ΅λ‹ˆλ‹€~~


λ°œκ²¬ν•œ 이슈

X

@asn6878 asn6878 added the bug κΈ΄κΈ‰ν•˜κ³ , μ€‘μš”λ„κ°€ 높은 이슈 label Jul 4, 2024
@asn6878 asn6878 self-assigned this Jul 4, 2024
Copy link
Member

@psychology50 psychology50 left a comment

Choose a reason for hiding this comment

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

저도 이런 ν˜„μƒμ΄ λ°œμƒν•˜λŠ” λͺ…ν™•ν•œ μ΄μœ λŠ” λͺ¨λ₯΄μ§€λ§Œ, λΉ„μŠ·ν•œ κ²½ν—˜μ„ ν•œ 적은 μžˆμŠ΅λ‹ˆλ‹€.
μ•„λ¬΄λž˜λ„ λ³‘λ ¬λ‘œ μž‘μ—…λ“€μ„ μˆ˜ν–‰ν•˜λ©΄μ„œ pkκ°€ μ˜ˆμƒμΉ˜ λͺ»ν•œ 지점뢀터 μ‹œμž‘ν•œλ‹€λ˜κ°€, μ—¬λŸ¬κ°€μ§€ λ¬Έμ œκ°€ μžˆλŠ” κ±° κ°™μ•„μš”...^^

μ—¬νŠΌ ν…ŒμŠ€νŠΈν•  λ•Œ λžœλ€κ°’μ„ λ„£λŠ” 건 μ„œλ‘œ μ£Όμ˜ν•˜λŠ” 게 쒋을 κ±° κ°™μŠ΅λ‹ˆλ‹€.

@asn6878 asn6878 merged commit adbaeed into dev Jul 4, 2024
1 check passed
@asn6878 asn6878 deleted the fix/fetch-test branch July 4, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug κΈ΄κΈ‰ν•˜κ³ , μ€‘μš”λ„κ°€ 높은 이슈
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants