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.

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

Merged
merged 1 commit into from
Jul 20, 2018

Conversation

rajinisivaram
Copy link
Contributor

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)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@rajinisivaram rajinisivaram requested a review from ijuma July 19, 2018 20:40
@rajinisivaram
Copy link
Contributor Author

@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.

Copy link
Contributor

@hachikuji hachikuji left a 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.

@ijuma
Copy link
Contributor

ijuma commented Jul 20, 2018

Seems fine to me. But we are keeping the JIRA open, I guess?

@rajinisivaram
Copy link
Contributor Author

@ijuma Yes, just updating the log for now and leaving the JIRA open.

@rajinisivaram
Copy link
Contributor Author

@hachikuji @ijuma Thanks for the review, merging to trunk and 2.0

@rajinisivaram rajinisivaram merged commit 980b725 into apache:trunk Jul 20, 2018
rajinisivaram added a commit that referenced this pull request Jul 20, 2018
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]>
colinhicks pushed a commit to confluentinc/kafka that referenced this pull request May 24, 2019
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]>
colinhicks added a commit to confluentinc/kafka that referenced this pull request May 24, 2019
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants