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.

Sign up for GitHub

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

Jump to bottom

ZOOKEEPER-4515: ZK Cli quit command always logs error #1856

Closed
wants to merge 2 commits into from
Closed

ZOOKEEPER-4515: ZK Cli quit command always logs error #1856

wants to merge 2 commits into from

Conversation

arshadmohammad
Copy link
Contributor

  1. For connection closing state scenario, changed the log level to debug
  2. When JVM exiting with code 0, then logging info instead of error

@arshadmohammad
ZOOKEEPER-4515: ZK Cli quit command always logs error
31e124a 
1. For connection closing state scenario, changed the log level to debug
2. When JVM exiting with code 0, then logging info instead of error
tisonkun
tisonkun reviewed Apr 8, 2022
View reviewed changes
Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @arshadmohammad! Generally looks good. Minor comments inline.

zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
Comment on lines +1287 to +1291
if (LOG.isDebugEnabled()) {
LOG.debug(
"An exception was thrown while closing send thread for session 0x{}.",
Long.toHexString(getSessionId()), e);
}
Copy link
Member

Choose a reason for hiding this comment

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

IIUC LOG.debug already contains a call to LOG.isDebugEnabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not a core code flow. So for me it is ok to go either way.
I have put debug enable check to void unnecessary toHexString call.
shall we keep as it is or shall it change it?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for your explanation. Then we can keep as is.

zookeeper-server/src/main/java/org/apache/zookeeper/util/ServiceUtils.java Show resolved Hide resolved
tisonkun
tisonkun approved these changes Apr 9, 2022
View reviewed changes
Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

LGTM.

cc @eolivelli @maoling

eolivelli
eolivelli approved these changes Apr 9, 2022
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Very good

@eolivelli
Copy link
Contributor

Please chery pick to all active branches

@asfgit asfgit closed this in e5f84f4 Apr 9, 2022
asfgit pushed a commit that referenced this pull request Apr 9, 2022
@arshadmohammad
ZOOKEEPER-4515: ZK Cli quit command always logs error
e35536d 
1. For connection closing state scenario, changed the log level to debug
2. When JVM exiting with code 0, then logging info instead of error

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: tison <wander4096@gmail.com>, Enrico Olivelli <eolivelli@apache.org>

Closes #1856 from arshadmohammad/ZOOKEEPER-4515-cli and squashes the following commits:

e7e248b [Mohammad Arshad] Logging error only when exit code is non zero
31e124a [Mohammad Arshad] ZOOKEEPER-4515: ZK Cli quit command always logs error 1. For connection closing state scenario, changed the log level to debug 2. When JVM exiting with code 0, then logging info instead of error

(cherry picked from commit e5f84f4)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
asfgit pushed a commit that referenced this pull request Apr 9, 2022
@arshadmohammad
ZOOKEEPER-4515: ZK Cli quit command always logs error
b633128 
1. For connection closing state scenario, changed the log level to debug
2. When JVM exiting with code 0, then logging info instead of error

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: tison <wander4096@gmail.com>, Enrico Olivelli <eolivelli@apache.org>

Closes #1856 from arshadmohammad/ZOOKEEPER-4515-cli and squashes the following commits:

e7e248b [Mohammad Arshad] Logging error only when exit code is non zero
31e124a [Mohammad Arshad] ZOOKEEPER-4515: ZK Cli quit command always logs error 1. For connection closing state scenario, changed the log level to debug 2. When JVM exiting with code 0, then logging info instead of error

(cherry picked from commit e5f84f4)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
asfgit pushed a commit that referenced this pull request Apr 9, 2022
@arshadmohammad
ZOOKEEPER-4515: ZK Cli quit command always logs error
430c027 
1. For connection closing state scenario, changed the log level to debug
2. When JVM exiting with code 0, then logging info instead of error

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: tison <wander4096@gmail.com>, Enrico Olivelli <eolivelli@apache.org>

Closes #1856 from arshadmohammad/ZOOKEEPER-4515-cli and squashes the following commits:

e7e248b [Mohammad Arshad] Logging error only when exit code is non zero
31e124a [Mohammad Arshad] ZOOKEEPER-4515: ZK Cli quit command always logs error 1. For connection closing state scenario, changed the log level to debug 2. When JVM exiting with code 0, then logging info instead of error

(cherry picked from commit e5f84f4)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
@anuragmadnawat1 anuragmadnawat1 mentioned this pull request Nov 1, 2022
Merged
anuragmadnawat1 pushed a commit to anuragmadnawat1/zookeeper that referenced this pull request Nov 1, 2022
@arshadmohammad @anurag-harness
ZOOKEEPER-4515: ZK Cli quit command always logs error
c7fa017 
1. For connection closing state scenario, changed the log level to debug
2. When JVM exiting with code 0, then logging info instead of error

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: tison <wander4096@gmail.com>, Enrico Olivelli <eolivelli@apache.org>

Closes apache#1856 from arshadmohammad/ZOOKEEPER-4515-cli and squashes the following commits:

e7e248b [Mohammad Arshad] Logging error only when exit code is non zero
31e124a [Mohammad Arshad] ZOOKEEPER-4515: ZK Cli quit command always logs error 1. For connection closing state scenario, changed the log level to debug 2. When JVM exiting with code 0, then logging info instead of error
anuragmadnawat1 added a commit to anuragmadnawat1/zookeeper that referenced this pull request Nov 1, 2022
@anuragmadnawat1 @arshadmohammad
ZOOKEEPER-4515: ZK Cli quit command always logs error (#55)
06febdb 
1. For connection closing state scenario, changed the log level to debug
2. When JVM exiting with code 0, then logging info instead of error

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: tison <wander4096@gmail.com>, Enrico Olivelli <eolivelli@apache.org>

Closes apache#1856 from arshadmohammad/ZOOKEEPER-4515-cli and squashes the following commits:

e7e248b [Mohammad Arshad] Logging error only when exit code is non zero
31e124a [Mohammad Arshad] ZOOKEEPER-4515: ZK Cli quit command always logs error 1. For connection closing state scenario, changed the log level to debug 2. When JVM exiting with code 0, then logging info instead of error

Co-authored-by: Mohammad Arshad <arshad@apache.org>
@anurag-harness anurag-harness mentioned this pull request Jan 13, 2023
Merged
anurag-harness pushed a commit to anurag-harness/zookeeper that referenced this pull request Jan 13, 2023
@arshadmohammad @anurag-harness
ZOOKEEPER-4515: ZK Cli quit command always logs error
11dae71 
1. For connection closing state scenario, changed the log level to debug
2. When JVM exiting with code 0, then logging info instead of error

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: tison <wander4096@gmail.com>, Enrico Olivelli <eolivelli@apache.org>

Closes apache#1856 from arshadmohammad/ZOOKEEPER-4515-cli and squashes the following commits:

e7e248b [Mohammad Arshad] Logging error only when exit code is non zero
31e124a [Mohammad Arshad] ZOOKEEPER-4515: ZK Cli quit command always logs error 1. For connection closing state scenario, changed the log level to debug 2. When JVM exiting with code 0, then logging info instead of error
anurag-harness added a commit to anurag-harness/zookeeper that referenced this pull request Jan 13, 2023
@anurag-harness @arshadmohammad
ZOOKEEPER-4515: ZK Cli quit command always logs error (#22)
568d3f4 
1. For connection closing state scenario, changed the log level to debug
2. When JVM exiting with code 0, then logging info instead of error

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: tison <wander4096@gmail.com>, Enrico Olivelli <eolivelli@apache.org>

Closes apache#1856 from arshadmohammad/ZOOKEEPER-4515-cli and squashes the following commits:

e7e248b [Mohammad Arshad] Logging error only when exit code is non zero
31e124a [Mohammad Arshad] ZOOKEEPER-4515: ZK Cli quit command always logs error 1. For connection closing state scenario, changed the log level to debug 2. When JVM exiting with code 0, then logging info instead of error

Co-authored-by: Mohammad Arshad <arshad@apache.org>
desaikomal pushed a commit to linkedin/zookeeper that referenced this pull request Jun 17, 2023
@arshadmohammad @desaikomal
ZOOKEEPER-4515: ZK Cli quit command always logs error
d9b24dd 
1. For connection closing state scenario, changed the log level to debug
2. When JVM exiting with code 0, then logging info instead of error

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: tison <wander4096@gmail.com>, Enrico Olivelli <eolivelli@apache.org>

Closes apache#1856 from arshadmohammad/ZOOKEEPER-4515-cli and squashes the following commits:

e7e248b [Mohammad Arshad] Logging error only when exit code is non zero
31e124a [Mohammad Arshad] ZOOKEEPER-4515: ZK Cli quit command always logs error 1. For connection closing state scenario, changed the log level to debug 2. When JVM exiting with code 0, then logging info instead of error

(cherry picked from commit e5f84f4)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
desaikomal pushed a commit to linkedin/zookeeper that referenced this pull request Jun 27, 2023
@arshadmohammad @desaikomal
ZOOKEEPER-4515: ZK Cli quit command always logs error
68ef1e9 
1. For connection closing state scenario, changed the log level to debug
2. When JVM exiting with code 0, then logging info instead of error

Author: Mohammad Arshad <arshad@apache.org>

Reviewers: tison <wander4096@gmail.com>, Enrico Olivelli <eolivelli@apache.org>

Closes apache#1856 from arshadmohammad/ZOOKEEPER-4515-cli and squashes the following commits:

e7e248b [Mohammad Arshad] Logging error only when exit code is non zero
31e124a [Mohammad Arshad] ZOOKEEPER-4515: ZK Cli quit command always logs error 1. For connection closing state scenario, changed the log level to debug 2. When JVM exiting with code 0, then logging info instead of error

(cherry picked from commit e5f84f4)
Signed-off-by: Mohammad Arshad <arshad@apache.org>
@Govind85
Copy link

Govind85 commented Aug 4, 2023

@arshadmohammad Could you please apply the same fix to 3.8.0 version?
It is showing same warning in 3.8.0 version and it is writing lot of logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eolivelli eolivelli eolivelli approved these changes

@tisonkun tisonkun tisonkun approved these changes

@symat symat Awaiting requested review from symat

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arshadmohammad @eolivelli @Govind85 @tisonkun