-
Notifications
You must be signed in to change notification settings - Fork 13.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
KAFKA-3702: Change log level of SSL close_notify failure #5397
KAFKA-3702: Change log level of SSL close_notify failure #5397
Conversation
@ijuma Can you review, please? This is not a fix for KAFKA-3702 which was raised to shutdown SSL connections gracefully. Since we need to work with different SSL clients and proxies, I think we should reduce the level of this log anyway since no one understands what it means and it is not really useful anyway. Thank you. |
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.
LGTM. I agree the log message does not provide much value in practice.
Seems fine to me. But we are keeping the JIRA open, I guess? |
@ijuma Yes, just updating the log for now and leaving the JIRA open. |
@hachikuji @ijuma Thanks for the review, merging to trunk and 2.0 |
SslTransportLayer currently closes the SSL engine and logs a warning if close_notify message canot be sent because the remote end closed its connection. This tends to fill up broker logs, especially when using clients which close connections immediately. Since this log entry is not very useful anyway, it would be better to log at debug level. Reviewers: Jason Gustafson <[email protected]>, Ismael Juma <[email protected]>
SslTransportLayer currently closes the SSL engine and logs a warning if close_notify message canot be sent because the remote end closed its connection. This tends to fill up broker logs, especially when using clients which close connections immediately. Since this log entry is not very useful anyway, it would be better to log at debug level. Reviewers: Jason Gustafson <[email protected]>, Ismael Juma <[email protected]>
* KAFKA-3702: Change log level of SSL close_notify failure (apache#5397) SslTransportLayer currently closes the SSL engine and logs a warning if close_notify message canot be sent because the remote end closed its connection. This tends to fill up broker logs, especially when using clients which close connections immediately. Since this log entry is not very useful anyway, it would be better to log at debug level. Reviewers: Jason Gustafson <[email protected]>, Ismael Juma <[email protected]> * KAFKA-7454: Use lazy allocation for SslTransportLayer buffers and null them on close (apache#5713) Lazy allocation helps when there are a large number of connections that have been accepted, but where no data has been received from the clients. Each buffer is often around 16k (max TLS record size). Nulling the buffers should not make a difference in the current implementation since we release the reference to the channel and transport layer after we close them, but it's a good practice to release medium/large buffers after `close` is called. Reviewers: Jun Rao <[email protected]>, Ismael Juma <[email protected]> * KAFKA-7453: Expire registered channels not selected within idle timeout (apache#5712) Reviewers: Jun Rao <[email protected]>. Ismael Juma <[email protected]> * KAFKA-7304: Limit connection accept rate to prioritize other channels * Log close of idle connections at `info` * Checkstyle * Fix processor retry logic to advance to the next processor on retry * Add test for connection rate limit * Wrap socketChannel close in try block
SslTransportLayer currently closes the SSL engine and logs a warning if
close_notify
message canot be sent because the remote end closed its connection. This tends to fill up broker logs, especially when using clients which close connections immediately. Since this log entry is not very useful anyway, it would be better to log at debug level.Committer Checklist (excluded from commit message)