-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
HDFS-16859. RBF: Move LOG.debug into if condition acquirePermit #5181
Conversation
💔 -1 overall
This message was automatically generated. |
@@ -63,10 +63,14 @@ public void init(Configuration conf) { | |||
@Override | |||
public boolean acquirePermit(String nsId) { | |||
try { | |||
LOG.debug("Taking lock for nameservice {}", nsId); | |||
if (LOG.isDebugEnabled()) { | |||
LOG.debug("Taking lock for nameservice {}", nsId); |
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.
We have gone through this in the past.
LOG.debug with {} should already be equivalent to adding the safeguard.
Do you have some instrumentation showing that LOG.isDebugEnabled()
is cheaper than just calling LOG.debug()
?
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.
i.m sorry, @goiri , here does not need if condition statement exactlly. i will close this pr.
Description of PR
The invoke frequency of method AbstractRouterRpcFairnessPolicyController#acquirePermit is high. before getting the permit of a nameservice, there is always a LOG.debug statement.
It is better to move the statement into if condition statement.
How was this patch tested?
no need to test.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?