> On July 22, 2015, 3 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java,
> >  line 411
> > <https://reviews.apache.org/r/33620/diff/12/?file=1017027#file1017027line411>
> >
> >     Is renegotiation actually supported? It seems that renegotiation can 
> > happen in the middle of a regular send/receive.
> 
> Sriharsha Chintalapani wrote:
>     Yes it can happen during the middle of regular send/receive. But this 
> need to be initiated by server which we don't have any criteria i.e we are 
> checking any certificate expiration to trigger renegotiation. As we discussed 
> we are going to skip this for the initial version.

I enabled the test for renegotiation.


> On July 22, 2015, 3 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java,
> >  lines 394-397
> > <https://reviews.apache.org/r/33620/diff/12/?file=1017027#file1017027line394>
> >
> >     We talked about that before. It's possible that we read all bytes for 
> > multiple recieves into the appReadBuffer. Selector only reads off one 
> > receive at a time. If there are no more bytes from incoming network, this 
> > key may not be selected to read off the next receive in the appReadBuffer. 
> > How do we resolve that issue?
> 
> Sriharsha Chintalapani wrote:
>     yes. This is an issue which I am trying to address by adding the 
> stagedReceives on Selector side which is to read as many availbale reads as 
> possible in a single poll and push these into stagedReceives queue and pop 
> one out during that poll add to completedReceives. In other polls if the 
> selectionKey is not selected and its not mute and if there is a stagedReceive 
> waiting we will poll that and add to completedReceives. But this logic is not 
> working on serverside as I get selector.send IllegalStateException. I am 
> still looking into it will update the patch once I've a solution. Please do 
> let me know if the above description sounds right to you.

Added statgedReceives on selector side to read as many reads as we can in one 
poll and push into deque and poll one if the channel is not selected & not 
muted. Please take a look at the selector code.


> On July 22, 2015, 3 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java,
> >  lines 361-374
> > <https://reviews.apache.org/r/33620/diff/12/?file=1017027#file1017027line361>
> >
> >     Hmm, if status is ok and handshakeStatus is NEED_UNWRAP, could we get 
> > into infinite loop here?
> 
> Sriharsha Chintalapani wrote:
>     Sorry. I thought I mvoed the socketChannel read inside the loop. I missed 
> it.
> 
> Jun Rao wrote:
>     Hmm, then we have to keep checking if the socket is readable?

Changed this to read once from socketChannel. The loop exits either we read all 
contents from netReadBuffer or cont is false.


> On July 22, 2015, 3 p.m., Jun Rao wrote:
> > checkstyle/import-control.xml, line 56
> > <https://reviews.apache.org/r/33620/diff/12/?file=1017007#file1017007line56>
> >
> >     Is this needed? It doesn't seem that network needs to use any protocol.

Yes for tests under client EchoServer.java I am using SecurityProtocol for both 
PlainText and SSL.


- Sriharsha


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33620/#review92314
-----------------------------------------------------------


On July 20, 2015, 7 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> -----------------------------------------------------------
> 
> (Updated July 20, 2015, 7 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1690
>     https://issues.apache.org/jira/browse/KAFKA-1690
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. SSLFactory tests.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Added 
> PrincipalBuilder.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Addressing 
> reviews.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Addressing 
> reviews.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Addressing 
> reviews.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Fixed minor 
> issues with the patch.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Fixed minor 
> issues with the patch.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> Merge remote-tracking branch 'refs/remotes/origin/trunk' into KAFKA-1690-V1
> 
> 
> KAFKA-1690. Broker side ssl changes.
> 
> 
> KAFKA-1684. SSL for socketServer.
> 
> 
> KAFKA-1690. Added SSLProducerSendTest and fixes to get right port for SSL.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> KAFKA-1690. Post merge fixes.
> 
> 
> KAFKA-1690. Added SSLProducerSendTest.
> 
> 
> KAFKA-1690. Minor fixes based on patch review comments.
> 
> 
> Merge commit
> 
> 
> KAFKA-1690. Added SSL Consumer Test.
> 
> 
> KAFKA-1690. SSL Support.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> KAFKA-1690. Addressing reviews. Removed interestOps from SSLTransportLayer.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Diffs
> -----
> 
>   build.gradle fb9084307ae41bbb62e32720ccd7b94f26e910c8 
>   checkstyle/import-control.xml 19e0659ef9385433d9f94dee43cd70a52b18c9e5 
>   clients/src/main/java/org/apache/kafka/clients/ClientUtils.java 
> 0d68bf1e1e90fe9d5d4397ddf817b9a9af8d9f7a 
>   clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java 
> 2c421f42ed3fc5d61cf9c87a7eaa7bb23e26f63b 
>   clients/src/main/java/org/apache/kafka/clients/NetworkClient.java 
> 48fe7961e2215372d8033ece4af739ea06c6457b 
>   clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java 
> 70377ae2fa46deb381139d28590ce6d4115e1adc 
>   clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 
> bea3d737c51be77d5b5293cdd944d33b905422ba 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java 
> aa264202f2724907924985a5ecbe74afc4c6c04b 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 
> bae528d31516679bed88ee61b408f209f185a8cc 
>   clients/src/main/java/org/apache/kafka/common/config/SSLConfigs.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/Authenticator.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/ByteBufferSend.java 
> df0e6d5105ca97b7e1cb4d334ffb7b443506bd0b 
>   clients/src/main/java/org/apache/kafka/common/network/ChannelBuilder.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/DefaultAuthenticator.java
>  PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/NetworkReceive.java 
> 3ca0098b8ec8cfdf81158465b2d40afc47eb6f80 
>   
> clients/src/main/java/org/apache/kafka/common/network/PlainTextChannelBuilder.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/PlainTextTransportLayer.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/SSLChannelBuilder.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/Selectable.java 
> 618a0fa53848ae6befea7eba39c2f3285b734494 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> aaf60c98c2c0f4513a8d65ee0db67953a529d598 
>   clients/src/main/java/org/apache/kafka/common/network/Send.java 
> 8f6daadf6b67c3414911cda77765512131e56fd3 
>   clients/src/main/java/org/apache/kafka/common/network/TransportLayer.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/protocol/SecurityProtocol.java 
> dab1a94dd29563688b6ecf4eeb0e180b06049d3f 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/DefaultPrincipalBuilder.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/PrincipalBuilder.java
>  PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/utils/Utils.java 
> af9993cf9b3991f1e61e1201c94e19bc1bf76a68 
>   clients/src/test/java/org/apache/kafka/clients/ClientUtilsTest.java 
> 13ce519f03d13db041e1f2dbcd6b59414d2775b8 
>   
> clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java
>  f3f8334f848be4cc043d5a573975609a3681fe7e 
>   clients/src/test/java/org/apache/kafka/common/network/EchoServer.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/common/network/SSLFactoryTest.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/common/network/SSLSelectorTest.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 
> 158f9829ff64a969008f699e40c51e918287859e 
>   clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 
> e7951d835472e5defe49be435f2c93685ba544d5 
>   clients/src/test/java/org/apache/kafka/test/MockSelector.java 
> 51eb9d142f566c94a87add68b8c4f78b56d6ec3e 
>   clients/src/test/java/org/apache/kafka/test/TestSSLUtils.java PRE-CREATION 
>   core/src/main/scala/kafka/network/SocketServer.scala 
> 91319fa010b140cca632e5fa8050509bd2295fc9 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 
> dbe170f87331f43e2dc30165080d2cb7dfe5fdbf 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> 18917bc4464b9403b16d85d20c3fd4c24893d1d3 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 
> 9ce4bd5ee130ce3cb252b2883a3fd3c9acd742a5 
>   core/src/test/scala/integration/kafka/api/SSLConsumerTest.scala 
> PRE-CREATION 
>   core/src/test/scala/integration/kafka/api/SSLProducerSendTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 
> 8b14bcfe7af601fe4b0fb0a7c0c544e87403062a 
>   core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala 
> e4bf2df48dd59a251b646b7f96d63ec4b924fc0b 
>   core/src/test/scala/unit/kafka/network/SocketServerTest.scala 
> 7dc2fad542ea553ee888543dd215eb41ea57d509 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 
> 04a02e08a54139ee1a298c5354731bae009efef3 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> eb169d8b33c27d598cc24e5a2e5f78b789fa38d3 
> 
> Diff: https://reviews.apache.org/r/33620/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>

Reply via email to