Skip to content

Conversation

@zzmao
Copy link

@zzmao zzmao commented Jun 21, 2019

Warp data as many as possible in SslTransportLayer#write(ByteBuffer)
The change comes from Ambry, whose TransportLayer code is same with Kafka.
linkedin/ambry#1105

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

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

Warp data as many as possible in SslTransportLayer#write(ByteBuffer)
The change comes from Ambry, whose TransportLayer code is same with Kafka.
linkedin/ambry#1105
@smccauliff
Copy link

Can you provide a more readable explanation of your change? What do you mean by "warp"?

@smccauliff
Copy link

Also if this is not a hot fix (i.e. we don't need it to fix something immediately) then why not just submit this to upstream Kafka and go though their process. We will cherry pick it or rebase on changes on top of this.

//handle ssl renegotiation
if (wrapResult.getHandshakeStatus() != HandshakeStatus.NOT_HANDSHAKING && wrapResult.getStatus() == Status.OK)
throw renegotiationException();
//handle ssl renegotiation

Choose a reason for hiding this comment

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

nit: whitespace between // and handle

@@ -630,37 +630,42 @@ public long read(ByteBuffer[] dsts, int offset, int length) throws IOException {
*/

Choose a reason for hiding this comment

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

In the commit message, warp or wrap?

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