You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "mingyen066 (via GitHub)" <gi...@apache.org> on 2024/07/24 16:30:02 UTC
[PR] KAFKA-17185: Declare all Loggers static to avoid creating multiple logger instances [kafka]
mingyen066 opened a new pull request, #16680:
URL: https://github.com/apache/kafka/pull/16680
As discussed in https://github.com/apache/kafka/pull/16657#discussion_r1686938593
Replace all `private final Logger` to `private static final Logger`
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17185: Declare Loggers as static to prevent multiple logger instances [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #16680:
URL: https://github.com/apache/kafka/pull/16680
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17185: Declare Loggers as static to prevent multiple logger instances [kafka]
Posted by "mingyen066 (via GitHub)" <gi...@apache.org>.
mingyen066 commented on code in PR #16680:
URL: https://github.com/apache/kafka/pull/16680#discussion_r1691215301
##########
clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java:
##########
@@ -42,7 +42,7 @@ final class ClusterConnectionStates {
static final int CONNECTION_SETUP_TIMEOUT_EXP_BASE = 2;
static final double CONNECTION_SETUP_TIMEOUT_JITTER = 0.2;
private final Map<String, NodeConnectionState> nodeState;
- private final Logger log;
+ private static final Logger log;
Review Comment:
Thanks for finding the bug. This is the case that I shouldn't change the logger to static.
I have completed the code updates with different approach and also written the methodology and criteria used to PR message. PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17185: Declare Loggers as static to prevent multiple logger instances [kafka]
Posted by "mingyen066 (via GitHub)" <gi...@apache.org>.
mingyen066 commented on code in PR #16680:
URL: https://github.com/apache/kafka/pull/16680#discussion_r1691215301
##########
clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java:
##########
@@ -42,7 +42,7 @@ final class ClusterConnectionStates {
static final int CONNECTION_SETUP_TIMEOUT_EXP_BASE = 2;
static final double CONNECTION_SETUP_TIMEOUT_JITTER = 0.2;
private final Map<String, NodeConnectionState> nodeState;
- private final Logger log;
+ private static final Logger log;
Review Comment:
Thanks for finding the bug. This is the case that I shouldn't change the logger to static.
I have re-evaluated and used a different approach. I have completed the code updates and written a new PR message that includes the methodology and criteria used. PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17185: Declare Loggers as static to prevent multiple logger instances [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16680:
URL: https://github.com/apache/kafka/pull/16680#issuecomment-2255900989
@mingyen066 Could you please merge trunk to run CI again? there are thread leak. I don't think there are related to your PR, but it is always good to check it again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17185: Declare Loggers as static to prevent multiple logger instances [kafka]
Posted by "mingyen066 (via GitHub)" <gi...@apache.org>.
mingyen066 commented on PR #16680:
URL: https://github.com/apache/kafka/pull/16680#issuecomment-2256046375
@chia7712 Sure, I merged trunk branch and the CI is running again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17185: Declare all Loggers as static to prevent multiple logger instances [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #16680:
URL: https://github.com/apache/kafka/pull/16680#discussion_r1690134497
##########
clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java:
##########
@@ -42,7 +42,7 @@ final class ClusterConnectionStates {
static final int CONNECTION_SETUP_TIMEOUT_EXP_BASE = 2;
static final double CONNECTION_SETUP_TIMEOUT_JITTER = 0.2;
private final Map<String, NodeConnectionState> nodeState;
- private final Logger log;
+ private static final Logger log;
Review Comment:
Noted that this static variable should NOT be initialized in constructor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org