-
Notifications
You must be signed in to change notification settings - Fork 16
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
FIX: Return the value when failed to put value got from valueLoader if wantToGetException is false. #87
Conversation
9ac68a0
to
9b7b1e5
Compare
@@ -172,8 +172,13 @@ private <T> T loadValue(String arcusKey, Callable<T> valueLoader) { | |||
T value; | |||
try { | |||
value = valueLoader.call(); | |||
} catch (Exception e) { | |||
throw new ValueRetrievalException(arcusKey, valueLoader, e); | |||
} catch (Exception originalException) { |
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.
์ฌ๊ธฐ์ catchํ์ง ๋ง๊ณ getSynchronized ๋ฉ์๋์์ Exception์ catchํ๋๊ฒ์ด ์ข์ง ์์๊น์?
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.
๊ทธ๋ฌ๋ฉด super.get(key);์์ ๋ฐ์ํ๋ Exception์ wantToGetException์ 2๋ฒ ์ ์ฉํ๊ฒ ๋์ด์ loadValue() ๋ฉ์๋์ ๋ฃ๋ ๊ฒ์ผ๋ก ๋์ฒดํ์ต๋๋ค.
RuntimeException e = new ValueRetrievalException(arcusKey, valueLoader, originalException); | ||
if (wantToGetException) { | ||
throw e; | ||
} |
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.
์ด ๊ฒฝ์ฐ๋ cache miss๋ก ์ธํด ์๋ ๋ก์ง์ ์ํํ์ฌ ๊ฒฐ๊ณผ๋ฅผ ์ป๋ ๋ถ๋ถ์
๋๋ค.
์ด ๋ถ๋ถ์์๋ wantToGetException ์ ์ฉํ์ง ์๋ ๊ฒ์ด ๋ง๋ ๊ฒ ๊ฐ์ต๋๋ค.
์ถ๊ฐ์ ์ผ๋ก, ์๋์ ์๋ putValue() ์ํ ์์ ์ฒ๋ฆฌ๊ฐ ์๋ชป๋ ๊ฒ ๊ฐ์ต๋๋ค.
- Exception ๋ฐ์์ catchํ ๊ฒฝ์ฐ, ๊ทธ exception์ throwํ์ง ์์์ผ ํฉ๋๋ค.
- ๋จ์ํ info logging๋ง ์ํํ๊ณ 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.
ํด๋น ๋ฐฉ์์ผ๋ก ์์ ํ์ต๋๋ค.
} | ||
logger.info("failed to loadValue. error: {}, key: {}", e.getMessage(), arcusKey); | ||
return null; | ||
logger.info("failed to put value got from valueLoader. error: {}, key: {}", e.getMessage(), arcusKey); |
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.
์ฌ๊ธฐ์ wantToGetException ์ฒ๋ฆฌ๋ฅผ ์ ์ธํ ์ด์ ๊ฐ ๋ฌด์์ธ๊ฐ์?
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.
public void put(final Object key, final Object value)
๋ด๋ถ์์ ํธ์ถ๋๋ putValue์ ๋ํด์๋ ์ ์ฌํญ ์ ์ฉํ์ง ์์๋ ๋๋์? (ํ์ฌ ๊ธฐ์ค line 189)
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.
@jhpark816 @uhm0311
๋ค์ ์๊ฐํด๋ณด๋ ์ด ๋ถ๋ถ์ ๊ผญ ๋ณ๊ฒฝํด์ผ ๋ ์ด์ ๊ฐ ์์ด๋ณด์
๋๋ค. get(Object key, Callable<T> valueLoader)
๋ฉ์๋๋ฅผ ํธ์ถํ์ฌ DB/์ธ๋ถ ์๋น์ค๋ก๋ถํฐ ๊ฐ์ ธ์จ ๋ฐ์ดํฐ๋ฅผ arcus์ putํ๋ ๊ณผ์ ์์ ์์ธ๊ฐ ๋ฐ์ํ๋ฉด ์ด ์ญ์ wantToGetException์ด ์ ์ฉ๋์ด์ผ ํ์ง ์๋์?
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.
์๋ 2 ์ฐ์ฐ์์ wantToGetException ์ฒ๋ฆฌ๋ฅผ ๋ค๋ฅด๊ฒ ๊ฐ์ ธํ ์ด์ ๊ฐ ์๋ค๋ ๊ฒ์ด๊ตฐ์.
- cache get ๊ณผ์ ์์ exception ๋ฐ์ํ๋ ๊ฒฝ์ฐ
- cache miss๋ก db ์กฐํํ์ฌ cache put ๊ณผ์ ์์ exception ๋ฐ์ํ๋ ๊ฒฝ์ฐ
OK. ๊ทธ๋ ๊ฒ ํฉ์๋ค.
๊ทธ๋ฌ๋ฉด, ํ์ฌ PR ์ฒ๋ผ wantToGetException = false์์ null ๋ฆฌํดํ๋ ๊ฒ๋ง ์ ๊ฑฐํ๋ฉด ๋๊ฒ ๋ค์.
5b69404
to
b1325c2
Compare
โฆf wantToGetException is false.
๐ Related Issue
โจ๏ธ What I did