Re: [Copycat] How will copycat serialize its metadata

[Gwen Shapira]

Yeah, I agree that if we have the ser/de we can do anything :)

I'd actually feel more comfortable if the users *have* to go through our
APIs to get to the metadata (which again, is kind of internal to Copycat).
If they start writing their own code that depends on this data, who knows
what we may accidentally break?

I'd prefer a more well-defined contract here.

The JSON shop should still feel fairly comfortable using REST APIs... most
of them are :)

On Fri, Aug 14, 2015 at 8:14 PM, Ewen Cheslack-Postava 
wrote:

> On Fri, Aug 14, 2015 at 6:35 PM, Gwen Shapira  wrote:
>
> > Yeah, I missed the option to match serialization of offsets to data,
> which
> > solves the configuration overhead.
> >
> > It still doesn't give us the ability to easily evolve the metadata
> messages
> > or to use them in monitoring tools.
> >
> > And I am still not clear of the benefits of using user-defined
> > serialization for the offsets.
> >
>
> If we can get at the serialization config (e.g. via the REST API), then we
> can decode the data regardless of the format. The main drawback is that
> then any tool that needs to decode them needs the serializer jars on its
> classpath. I think the benefit is that it lets users process that data in
> their preferred format if they want to build their own tools. For example
> in a shop that prefers JSON, this be the difference between them
> considering it easily accessible (they just read the topic and parse using
> their favorite JSON library) vs not being accessible (they won't pull in
> whatever serializer we use internally, or don't want to write a custom
> parser for our custom serialization format).
>
> -Ewen
>
>
> >
> > Gwen
> >
> > On Fri, Aug 14, 2015 at 1:29 AM, Ewen Cheslack-Postava <
> e...@confluent.io>
> > wrote:
> >
> > > I'm not sure the existing discussion is clear about how the format of
> > > offset data is decided. One possibility is that we choose one fixed
> > format
> > > and that is what we use internally to store offsets no matter what
> > > serializer you choose. This would be similar to how the __offsets topic
> > is
> > > currently handled (with a custom serialization format). In other words,
> > we
> > > use format X to store offsets. If you serialize your data with Y or Z,
> we
> > > don't care, we still use format X. The other option (which is used in
> the
> > > current PR-99 patch) would still make offset serialization pluggable,
> but
> > > there wouldn't be a separate option for it. Offset serialization would
> > use
> > > the same format as the data serialization. If you use X for data, we
> use
> > X
> > > for offsets; you use Y for data, we use Y for offsets.
> > >
> > > @neha wrt providing access through a REST API, I guess you are
> suggesting
> > > that we can serialize that data to JSON for that API. I think it's
> > > important to point out that this is arbitrarily structured,
> > > connector-specific data. In many ways, it's not that different from the
> > > actual message data in that it is highly dependent on the connector and
> > > downstream consumers need to understand the connector and its data
> format
> > > to do anything meaningful with the data. Because of this, I'm not
> > convinced
> > > that serializing it in a format other than the one used for the data
> will
> > > be particularly useful.
> > >
> > >
> > > On Thu, Aug 13, 2015 at 11:22 PM, Neha Narkhede 
> > wrote:
> > >
> > > > Copycat enables streaming data in and out of Kafka. Connector writers
> > > need
> > > > to define the serde of the data as it is different per system.
> Metadata
> > > > should be entirely hidden by the copycat framework and isn't
> something
> > > > users or connector implementors need to serialize differently as long
> > as
> > > we
> > > > provide tools/REST APIs to access the metadata where required.
> > Moreover,
> > > as
> > > > you suggest, evolution, maintenance and configs are much simpler if
> it
> > > > remains hidden.
> > > >
> > > > +1 on keeping just the serializers for data configurable.
> > > >
> > > > On Thu, Aug 13, 2015 at 9:59 PM, Gwen Shapira 
> > wrote:
> > > >
> > > > > Hi Team Kafka,
> > > > >
> > > > > As you know from KIP-26 and PR-99, when you will use Copycat to
> move
> > > data
> > > > > from an external system to Kafka, in addition to storing the data
> > > itself,
> > > > > Copycat will also need to store some metadata.
> > > > >
> > > > > This metadata is currently offsets on the source system (say, SCN#
> > from
> > > > > Oracle redo log), but I can imagine to store a bit more.
> > > > >
> > > > > When storing data, we obviously want pluggable serializers, so
> users
> > > will
> > > > > get the data in a format they like.
> > > > >
> > > > > But the metadata seems internal. i.e users shouldn't care about it
> > and
> > > if
> > > > > we want them to read or change anything, we want to provide them
> with
> > > > tools
> > > > > to do it.
> > > > >
> > > > > Moreover, by controlling the format we can do three important

Re: [DISCUSS] Client-side Assignment for New Consumer

[Jiangjie Qin]

Neha, Ewen and Jason,

Maybe I am over concerning and I agree that it does depend on the metadata
change frequency. As Neha said, a few tests will be helpful. We can see how
it goes.

What worries me is that in LinkedIn we are in the progress of running Kafka
as a service. That means user will have the ability to create/delete topics
and update their topic configurations programmatically or through UI using
LinkedIn internal cloud service platform. And some automated test will also
be consistently running to create topic, produce/consume some data, then
clean up the topics. This brings us some new challenges for mirror maker
because we need to make sure it actually copies data instead of spending
too much time on rebalance. As what we see now is that each rebalance will
probably take 30 seconds or more to finish even with some tricks and tuning.

Another related use case I want to bring up is that today when we want to
do a rolling upgrade of mirror maker (26 nodes), there will be two rounds
of rebalance to bounce each node, each rebalance takes about 30 seconds. So
bouncing 26 nodes takes roughly half an hour. Awkwardly, during the rolling
bounce, because mirror maker is keeping rebalancing, it actually does not
really copy any data! So the pipeline will literally stall for half an
hour! Since we are designing the new protocol, it will also be good it we
make sure this use case is addressed.

Thanks,

Jiangjie (Becket) Qin

On Sat, Aug 15, 2015 at 10:50 AM, Neha Narkhede  wrote:

> Becket,
>
> This is a clever approach for to ensure that only one thing communicates
> the metadata so even if it is stale, the entire group has the same view.
> However, the big assumption this makes is that the coordinator is that one
> process that has the ability to know the metadata for group members, which
> does not work for any non-consumer use case.
>
> I wonder if we may be complicating the design of 95% use cases for the
> remaining 5%. For instance, how many times do people create and remove
> topics or even add partitions? We operated LI clusters for a long time and
> this wasn't a frequent event that would need us to optimize this design
> for.
>
> Also, this is something we can easily validate by running a few tests on
> the patch and I suggest we wait for that.
>
> Thanks,
> Neha
>
> On Sat, Aug 15, 2015 at 9:14 AM, Jason Gustafson 
> wrote:
>
> > Hey Jiangjie,
> >
> > I was thinking about the same problem. When metadata is changing
> > frequently, the clients may not be able to ever find agreement on the
> > current state. The server doesn't have this problem, as you say, because
> it
> > can just take a snapshot and send that to the clients. Adding a dampening
> > setting to the client would help if churn is sporadic, but not if it is
> > steady. I think the metadata would have to be changing really frequently
> > for this to be a problem practically, but this is a case where the
> > server-side approach has an advantage.
> >
> > Including the metadata in the join group response would require making
> the
> > subscription available to the coordinator, right? We lose a little bit of
> > the generality of the protocol, but it might not be too bad since most
> use
> > cases for reusing this protocol have a similar need for metadata (and
> they
> > can always pass an empty subscription if they don't). We've gone back and
> > forth a few times on this, and I'm generally not opposed. It might help
> if
> > we try to quantify the impact of the metadata churn in practice. I think
> > the rate of change would have to be in the low seconds for this to be a
> > real problem. It does seem nice though that we have a way to manage even
> > this much churn when we do this.
> >
> > -Jason
> >
> >
> >
> > On Fri, Aug 14, 2015 at 9:03 PM, Jiangjie Qin  >
> > wrote:
> >
> > > Ewen,
> > >
> > > I agree that if there is a churn in metadata, the consumers need
> several
> > > rounds of rebalances to succeed. The difference I am thinking is that
> > with
> > > coordinator as single source of truth, we can let the consumer finish
> one
> > > round of rebalance, work for a while and start the next round of
> > rebalance.
> > > If we purely depend on the consumers to synchronize by themselves based
> > on
> > > different metadata sources, is it possible we have some groups
> spending a
> > > lot of time on rebalancing but not able to make too much progress in
> > > consuming?
> > >
> > > I'm thinking can we let consumers to fetch metadata only from their
> > > coordinator? So the rebalance can be done in the following way:
> > >
> > > 1. Consumers refresh their metadata periodically
> > > 2. If one of the consumer sees a change in metadata that triggers a
> > > rebalance, it sends JoinGroupRequest to coordinator.
> > > 3. Once the coordinator receives the first JoinGroupRequest of a group,
> > it
> > > takes a snapshot of current metadata and the group enters
> > prepare-rebalance
> > > state.
> > > 4. The metadata snapshot will be use

Re: [Copycat] How will copycat serialize its metadata

[Neha Narkhede]

Ewen,

I meant we use format X to store offsets, whether you serialize your data
with Y or Z and we don't expose it as something that can be configured. As
far as the serialization format goes, I was suggesting just going with
simple base64 encoded strings (maybe there is a reason you are saying this
doesn't work?) for simplicity though I can see how we can just use the same
one used for the data. Don't have a strong preference either way as long as
the tooling and REST APIs can expose the data effortlessly.

Thanks,
Neha

On Fri, Aug 14, 2015 at 1:29 AM, Ewen Cheslack-Postava 
wrote:

> I'm not sure the existing discussion is clear about how the format of
> offset data is decided. One possibility is that we choose one fixed format
> and that is what we use internally to store offsets no matter what
> serializer you choose. This would be similar to how the __offsets topic is
> currently handled (with a custom serialization format). In other words, we
> use format X to store offsets. If you serialize your data with Y or Z, we
> don't care, we still use format X. The other option (which is used in the
> current PR-99 patch) would still make offset serialization pluggable, but
> there wouldn't be a separate option for it. Offset serialization would use
> the same format as the data serialization. If you use X for data, we use X
> for offsets; you use Y for data, we use Y for offsets.
>
> @neha wrt providing access through a REST API, I guess you are suggesting
> that we can serialize that data to JSON for that API. I think it's
> important to point out that this is arbitrarily structured,
> connector-specific data. In many ways, it's not that different from the
> actual message data in that it is highly dependent on the connector and
> downstream consumers need to understand the connector and its data format
> to do anything meaningful with the data. Because of this, I'm not convinced
> that serializing it in a format other than the one used for the data will
> be particularly useful.
>
>
> On Thu, Aug 13, 2015 at 11:22 PM, Neha Narkhede  wrote:
>
> > Copycat enables streaming data in and out of Kafka. Connector writers
> need
> > to define the serde of the data as it is different per system. Metadata
> > should be entirely hidden by the copycat framework and isn't something
> > users or connector implementors need to serialize differently as long as
> we
> > provide tools/REST APIs to access the metadata where required. Moreover,
> as
> > you suggest, evolution, maintenance and configs are much simpler if it
> > remains hidden.
> >
> > +1 on keeping just the serializers for data configurable.
> >
> > On Thu, Aug 13, 2015 at 9:59 PM, Gwen Shapira  wrote:
> >
> > > Hi Team Kafka,
> > >
> > > As you know from KIP-26 and PR-99, when you will use Copycat to move
> data
> > > from an external system to Kafka, in addition to storing the data
> itself,
> > > Copycat will also need to store some metadata.
> > >
> > > This metadata is currently offsets on the source system (say, SCN# from
> > > Oracle redo log), but I can imagine to store a bit more.
> > >
> > > When storing data, we obviously want pluggable serializers, so users
> will
> > > get the data in a format they like.
> > >
> > > But the metadata seems internal. i.e users shouldn't care about it and
> if
> > > we want them to read or change anything, we want to provide them with
> > tools
> > > to do it.
> > >
> > > Moreover, by controlling the format we can do three important things:
> > > * Read the metadata for monitoring / audit purposes
> > > * Evolve / modify it. If users serialize it in their own format, and
> > > actually write clients to use this metadata, we don't know if its safe
> to
> > > evolve.
> > > * Keep configuration a bit simpler. This adds at least 4 new
> > configuration
> > > items...
> > >
> > > What do you guys think?
> > >
> > > Gwen
> > >
> >
> >
> >
> > --
> > Thanks,
> > Neha
> >
>
>
>
> --
> Thanks,
> Ewen
>



-- 
Thanks,
Neha

Re: Review Request 33620: Patch for KAFKA-1690

[Sriharsha Chintalapani]

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java,
> >  lines 262-269
> > 
> >
> > Could we transition from NEED_WRAP to NOT_HANDSHAKING directly? Or 
> > NOT_HANDSHAKING can only be transitioned from FINIHED state?

NOT_HANDSHAKING can only be transitioned from FINISHED state. This is an issue 
SSLEngine in some JDKs.


- Sriharsha


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


On July 25, 2015, 7:11 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated July 25, 2015, 7:11 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.
> 
> 
> KAFKA-1690. added staged receives to selector.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Diffs
> -
> 
>   build.gradle 0abec26fb2d7be62c8a673f9ec838e926e64b2d1 
>   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 
> 

Re: Review Request 33620: Patch for KAFKA-1690

[Sriharsha Chintalapani]

> On July 27, 2015, 1:32 p.m., Ismael Juma wrote:
> > clients/src/main/java/org/apache/kafka/common/network/DefaultAuthenticator.java,
> >  line 34
> > 
> >
> > Why not create the principal at this point instead of in the 
> > `principal()` method? Is it a costly operation?

at this point we don't haven't gone through the handshake to establish principal


- Sriharsha


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


On July 25, 2015, 7:11 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated July 25, 2015, 7:11 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.
> 
> 
> KAFKA-1690. added staged receives to selector.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Diffs
> -
> 
>   build.gradle 0abec26fb2d7be62c8a673f9ec838e926e64b2d1 
>   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 

Re: Review Request 33620: Patch for KAFKA-1690

[Sriharsha Chintalapani]

> On July 27, 2015, 1:32 p.m., Ismael Juma wrote:
> > clients/src/main/java/org/apache/kafka/common/config/SSLConfigs.java, line 
> > 29
> > 
> >
> > SSL is deprecated 
> > (https://ma.ttias.be/rfc-7568-ssl-3-0-is-now-officially-deprecated/), 
> > should we be supporting it at all? If we do support it, then we should at 
> > least include a warning.
> 
> Ismael Juma wrote:
> I noticed that SSLv3 is actually disabled in newer JDK 8 releases by 
> default.

? I know ssl 3.0 is deprecated. As you can see from SSLConfigs.java we enable 
DEFAULT_ENABLED_PROTOCOLS = "TLSv1.2,TLSv1.1,TLSv1" only TLS not SSL.


- Sriharsha


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


On July 25, 2015, 7:11 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated July 25, 2015, 7:11 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.
> 
> 
> KAFKA-1690. added staged receives to selector.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Diffs
> -
> 
>   build.gradle 0abec26fb2d7be62c8a673f9ec838e926e64b2d1 
>   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/De

Re: Review Request 33620: Patch for KAFKA-1690

[Sriharsha Chintalapani]

> On July 29, 2015, 1:15 a.m., Dong Lin wrote:
> > clients/src/test/java/org/apache/kafka/common/network/SSLFactoryTest.java, 
> > line 52
> > 
> >
> > I am not sure about this. It is probably a stupid question. Do you 
> > indeed mean to use "useClientCert=false" in TestSSLUtils.createSSLConfig() 
> > to test client mode?

Yes. Fixed.


- Sriharsha


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


On July 25, 2015, 7:11 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated July 25, 2015, 7:11 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.
> 
> 
> KAFKA-1690. added staged receives to selector.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Diffs
> -
> 
>   build.gradle 0abec26fb2d7be62c8a673f9ec838e926e64b2d1 
>   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

Re: Review Request 33620: Patch for KAFKA-1690

[Sriharsha Chintalapani]

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java,
> >  lines 430-433
> > 
> >
> > Is this needed? If we need to expand appReadBuffer, netReadBuffer's 
> > position won't be 0 and we can just loop back.

Sorry don't follow if the dst still has space to fill we are checking if 
appReadBuffer has any data to add to it.


- Sriharsha


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


On July 25, 2015, 7:11 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated July 25, 2015, 7:11 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.
> 
> 
> KAFKA-1690. added staged receives to selector.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Diffs
> -
> 
>   build.gradle 0abec26fb2d7be62c8a673f9ec838e926e64b2d1 
>   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 
> 3c

Re: Review Request 33620: Patch for KAFKA-1690

[Sriharsha Chintalapani]

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote:
> > core/src/test/scala/unit/kafka/network/SocketServerTest.scala, line 207
> > 
> >
> > Is this test needed given the tests we have on EchoServer?
> > 
> > Also, do we have a test where the broker listens to multiple ports? 
> > This can be added in a followup patch though.

All of SSL tests (producerSendTest, ConsumerTest) have brokers listening on 
both plaintext & ssl. I can add test to send data over plaintext as well to 
these tests let me know.


- Sriharsha


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


On July 25, 2015, 7:11 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated July 25, 2015, 7:11 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.
> 
> 
> KAFKA-1690. added staged receives to selector.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Diffs
> -
> 
>   build.gradle 0abec26fb2d7be62c8a673f9ec838e926e64b2d1 
>   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/Kafk

Re: Review Request 33620: Patch for KAFKA-1690

[Sriharsha Chintalapani]

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/Selector.java, line 
> > 535
> > 
> >
> > Not sure why we need to check hasSend. It's possible for a channel to 
> > have both sends and receives at the same time since the NetworkClient 
> > supports pipelining.

Yes on server side we don't check any of it and keep sending .


- Sriharsha


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


On July 25, 2015, 7:11 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated July 25, 2015, 7:11 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.
> 
> 
> KAFKA-1690. added staged receives to selector.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Diffs
> -
> 
>   build.gradle 0abec26fb2d7be62c8a673f9ec838e926e64b2d1 
>   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 
> 3ca0098b8ec8cfdf8

Re: Review Request 33620: Patch for KAFKA-1690

[Sriharsha Chintalapani]

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/Selector.java, lines 
> > 250-251
> > 
> >
> > It seems that we will need to further check whether those channels in 
> > stagedReceives are muted or not.  Timeout should only be 0 if there is at 
> > least one unmuted channel in stagedReceives.

This might require adding a flag or looping over stagedReceives which one you 
prefer.


- Sriharsha


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


On July 25, 2015, 7:11 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated July 25, 2015, 7:11 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.
> 
> 
> KAFKA-1690. added staged receives to selector.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Diffs
> -
> 
>   build.gradle 0abec26fb2d7be62c8a673f9ec838e926e64b2d1 
>   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/kafk

Re: Review Request 33620: Patch for KAFKA-1690

[Sriharsha Chintalapani]

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java,
> >  line 417
> > 
> >
> > It's still not very clear to me how renegotiation can be supported in 
> > the middle of sends/receives. Suppose that the server initiates a 
> > handshake. This may involve the server sending some handshake bytes to the 
> > client. After this point, the server expects to read handshake bytes from 
> > the client. However, the client may still be sending some regular bytes 
> > over the socket.

Client will also be middle of renegotiation right how can it send regular 
bytes??


- Sriharsha


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


On July 25, 2015, 7:11 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated July 25, 2015, 7:11 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.
> 
> 
> KAFKA-1690. added staged receives to selector.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Diffs
> -
> 
>   build.gradle 0abec26fb2d7be62c8a673f9ec838e926e64b2d1 
>   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/apach

Re: [DISCUSS] Client-side Assignment for New Consumer

[Neha Narkhede]

Becket,

This is a clever approach for to ensure that only one thing communicates
the metadata so even if it is stale, the entire group has the same view.
However, the big assumption this makes is that the coordinator is that one
process that has the ability to know the metadata for group members, which
does not work for any non-consumer use case.

I wonder if we may be complicating the design of 95% use cases for the
remaining 5%. For instance, how many times do people create and remove
topics or even add partitions? We operated LI clusters for a long time and
this wasn't a frequent event that would need us to optimize this design for.

Also, this is something we can easily validate by running a few tests on
the patch and I suggest we wait for that.

Thanks,
Neha

On Sat, Aug 15, 2015 at 9:14 AM, Jason Gustafson  wrote:

> Hey Jiangjie,
>
> I was thinking about the same problem. When metadata is changing
> frequently, the clients may not be able to ever find agreement on the
> current state. The server doesn't have this problem, as you say, because it
> can just take a snapshot and send that to the clients. Adding a dampening
> setting to the client would help if churn is sporadic, but not if it is
> steady. I think the metadata would have to be changing really frequently
> for this to be a problem practically, but this is a case where the
> server-side approach has an advantage.
>
> Including the metadata in the join group response would require making the
> subscription available to the coordinator, right? We lose a little bit of
> the generality of the protocol, but it might not be too bad since most use
> cases for reusing this protocol have a similar need for metadata (and they
> can always pass an empty subscription if they don't). We've gone back and
> forth a few times on this, and I'm generally not opposed. It might help if
> we try to quantify the impact of the metadata churn in practice. I think
> the rate of change would have to be in the low seconds for this to be a
> real problem. It does seem nice though that we have a way to manage even
> this much churn when we do this.
>
> -Jason
>
>
>
> On Fri, Aug 14, 2015 at 9:03 PM, Jiangjie Qin 
> wrote:
>
> > Ewen,
> >
> > I agree that if there is a churn in metadata, the consumers need several
> > rounds of rebalances to succeed. The difference I am thinking is that
> with
> > coordinator as single source of truth, we can let the consumer finish one
> > round of rebalance, work for a while and start the next round of
> rebalance.
> > If we purely depend on the consumers to synchronize by themselves based
> on
> > different metadata sources, is it possible we have some groups spending a
> > lot of time on rebalancing but not able to make too much progress in
> > consuming?
> >
> > I'm thinking can we let consumers to fetch metadata only from their
> > coordinator? So the rebalance can be done in the following way:
> >
> > 1. Consumers refresh their metadata periodically
> > 2. If one of the consumer sees a change in metadata that triggers a
> > rebalance, it sends JoinGroupRequest to coordinator.
> > 3. Once the coordinator receives the first JoinGroupRequest of a group,
> it
> > takes a snapshot of current metadata and the group enters
> prepare-rebalance
> > state.
> > 4. The metadata snapshot will be used for this round of rebalance. i.e.
> the
> > metadata snapshot will be sent to consumers in JoinGroupResponse.
> > 4.1 If the consumers are subscribing to explicit topic lists (not regex),
> > the JoinGroupResponse needs only contain the metadata of all topics the
> > group is interested.
> > 4.2 If the consumers are subscribing using regex, all the topic metadata
> > will be returned to the consumer.
> > 5. Consumers get JoinGroupResponse, refresh metadata using the metadata
> in
> > JoinGroupResponse, run algorithm to assign partitions and start consume.
> > 6. Go back to 1.
> >
> > The benefit here is that we can let rebalance finish in one round, and
> all
> > the rest of changes will be captured in next consumer metadata refresh -
> so
> > we get group commit. One concern might be letting consumer refresh
> metadata
> > from coordinator might cause issue for big consumer groups. Maybe that is
> > OK because metadata refresh is infrequent.
> >
> > This approach actually is very similar to what is proposed now: rebalance
> > is triggered by metadata refresh, consumer provides subscription list to
> > pass around. The only difference is that we don't need metadata hash
> > anymore because the metadata is guaranteed to be the same. Replacing
> > metadata hash with actual metadata will not have too much overhead for
> > small subscription groups. There will be some overhead for regex
> > subscriptions, but this can save the potential extra round of metadata
> > fetch and will only occur when consumer see metadata change, which is
> > infrequent.
> >
> > Any thoughts?
> >
> > Thanks,
> >
> > Jiangjie (Becket) Qin
> >
> >
> > On Fri, Aug 14, 2015 

Re: Review Request 33620: Patch for KAFKA-1690

[Sriharsha Chintalapani]

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java,
> >  lines 306-320
> > 
> >
> > It seems that the logic here can be simpler. In handshake(), we call 
> > flush at the beginning. So, it seems that when handshakeFinished(), it 
> > should always be the case that there are no remaining bytes in 
> > netWriteBuffer. So, in handshakeFinished(), it seems that we can just 
> > simply set handshakeComplete to true and turn off OP_WRITE. Also, not sure 
> > if we need to check handshakeResult.getHandshakeStatus().

Not necessarily, since we can fall to handshakeComplete from handshakeWrap. Any 
reason checking netWriteBuffer here is an issue?


- Sriharsha


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


On July 25, 2015, 7:11 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated July 25, 2015, 7:11 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.
> 
> 
> KAFKA-1690. added staged receives to selector.
> 
> 
> KAFKA-1690. Addressing reviews.
> 
> 
> Merge branch 'trunk' into KAFKA-1690-V1
> 
> 
> Diffs
> -
> 
>   build.gradle 0abec26fb2d7be62c8a673f9ec838e926e64b2d1 
>   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

Re: [DISCUSS] Client-side Assignment for New Consumer

[Ewen Cheslack-Postava]

Agreed that talking about actual numbers would be helpful here.

Only 2 things affect this metadata: list of topics and number of
partitions. I don't think either of those can change *that* frequently to
be relevant to our 3s default heartbeat and 30s default session timeout, or
even the 5 minute metadata age.

Are there actual use cases where there is a steady stream of topics that
are constantly being added/removed (and they must be deleted since the
brokers won't scale up to a large number of topics anyway), or where # of
partitions on topics are constantly being adjusted (which presumably only
works for topics you don't use keys in since this would otherwise break
semantic partitioning)?

-Ewen


On Sat, Aug 15, 2015 at 9:14 AM, Jason Gustafson  wrote:

> Hey Jiangjie,
>
> I was thinking about the same problem. When metadata is changing
> frequently, the clients may not be able to ever find agreement on the
> current state. The server doesn't have this problem, as you say, because it
> can just take a snapshot and send that to the clients. Adding a dampening
> setting to the client would help if churn is sporadic, but not if it is
> steady. I think the metadata would have to be changing really frequently
> for this to be a problem practically, but this is a case where the
> server-side approach has an advantage.
>
> Including the metadata in the join group response would require making the
> subscription available to the coordinator, right? We lose a little bit of
> the generality of the protocol, but it might not be too bad since most use
> cases for reusing this protocol have a similar need for metadata (and they
> can always pass an empty subscription if they don't). We've gone back and
> forth a few times on this, and I'm generally not opposed. It might help if
> we try to quantify the impact of the metadata churn in practice. I think
> the rate of change would have to be in the low seconds for this to be a
> real problem. It does seem nice though that we have a way to manage even
> this much churn when we do this.
>
> -Jason
>
>
>
> On Fri, Aug 14, 2015 at 9:03 PM, Jiangjie Qin 
> wrote:
>
> > Ewen,
> >
> > I agree that if there is a churn in metadata, the consumers need several
> > rounds of rebalances to succeed. The difference I am thinking is that
> with
> > coordinator as single source of truth, we can let the consumer finish one
> > round of rebalance, work for a while and start the next round of
> rebalance.
> > If we purely depend on the consumers to synchronize by themselves based
> on
> > different metadata sources, is it possible we have some groups spending a
> > lot of time on rebalancing but not able to make too much progress in
> > consuming?
> >
> > I'm thinking can we let consumers to fetch metadata only from their
> > coordinator? So the rebalance can be done in the following way:
> >
> > 1. Consumers refresh their metadata periodically
> > 2. If one of the consumer sees a change in metadata that triggers a
> > rebalance, it sends JoinGroupRequest to coordinator.
> > 3. Once the coordinator receives the first JoinGroupRequest of a group,
> it
> > takes a snapshot of current metadata and the group enters
> prepare-rebalance
> > state.
> > 4. The metadata snapshot will be used for this round of rebalance. i.e.
> the
> > metadata snapshot will be sent to consumers in JoinGroupResponse.
> > 4.1 If the consumers are subscribing to explicit topic lists (not regex),
> > the JoinGroupResponse needs only contain the metadata of all topics the
> > group is interested.
> > 4.2 If the consumers are subscribing using regex, all the topic metadata
> > will be returned to the consumer.
> > 5. Consumers get JoinGroupResponse, refresh metadata using the metadata
> in
> > JoinGroupResponse, run algorithm to assign partitions and start consume.
> > 6. Go back to 1.
> >
> > The benefit here is that we can let rebalance finish in one round, and
> all
> > the rest of changes will be captured in next consumer metadata refresh -
> so
> > we get group commit. One concern might be letting consumer refresh
> metadata
> > from coordinator might cause issue for big consumer groups. Maybe that is
> > OK because metadata refresh is infrequent.
> >
> > This approach actually is very similar to what is proposed now: rebalance
> > is triggered by metadata refresh, consumer provides subscription list to
> > pass around. The only difference is that we don't need metadata hash
> > anymore because the metadata is guaranteed to be the same. Replacing
> > metadata hash with actual metadata will not have too much overhead for
> > small subscription groups. There will be some overhead for regex
> > subscriptions, but this can save the potential extra round of metadata
> > fetch and will only occur when consumer see metadata change, which is
> > infrequent.
> >
> > Any thoughts?
> >
> > Thanks,
> >
> > Jiangjie (Becket) Qin
> >
> >
> > On Fri, Aug 14, 2015 at 12:57 PM, Ewen Cheslack-Postava <
> e...@confluent.io
> > >
> >

Re: [DISCUSS] Client-side Assignment for New Consumer

[Jason Gustafson]

Hey Jiangjie,

I was thinking about the same problem. When metadata is changing
frequently, the clients may not be able to ever find agreement on the
current state. The server doesn't have this problem, as you say, because it
can just take a snapshot and send that to the clients. Adding a dampening
setting to the client would help if churn is sporadic, but not if it is
steady. I think the metadata would have to be changing really frequently
for this to be a problem practically, but this is a case where the
server-side approach has an advantage.

Including the metadata in the join group response would require making the
subscription available to the coordinator, right? We lose a little bit of
the generality of the protocol, but it might not be too bad since most use
cases for reusing this protocol have a similar need for metadata (and they
can always pass an empty subscription if they don't). We've gone back and
forth a few times on this, and I'm generally not opposed. It might help if
we try to quantify the impact of the metadata churn in practice. I think
the rate of change would have to be in the low seconds for this to be a
real problem. It does seem nice though that we have a way to manage even
this much churn when we do this.

-Jason



On Fri, Aug 14, 2015 at 9:03 PM, Jiangjie Qin 
wrote:

> Ewen,
>
> I agree that if there is a churn in metadata, the consumers need several
> rounds of rebalances to succeed. The difference I am thinking is that with
> coordinator as single source of truth, we can let the consumer finish one
> round of rebalance, work for a while and start the next round of rebalance.
> If we purely depend on the consumers to synchronize by themselves based on
> different metadata sources, is it possible we have some groups spending a
> lot of time on rebalancing but not able to make too much progress in
> consuming?
>
> I'm thinking can we let consumers to fetch metadata only from their
> coordinator? So the rebalance can be done in the following way:
>
> 1. Consumers refresh their metadata periodically
> 2. If one of the consumer sees a change in metadata that triggers a
> rebalance, it sends JoinGroupRequest to coordinator.
> 3. Once the coordinator receives the first JoinGroupRequest of a group, it
> takes a snapshot of current metadata and the group enters prepare-rebalance
> state.
> 4. The metadata snapshot will be used for this round of rebalance. i.e. the
> metadata snapshot will be sent to consumers in JoinGroupResponse.
> 4.1 If the consumers are subscribing to explicit topic lists (not regex),
> the JoinGroupResponse needs only contain the metadata of all topics the
> group is interested.
> 4.2 If the consumers are subscribing using regex, all the topic metadata
> will be returned to the consumer.
> 5. Consumers get JoinGroupResponse, refresh metadata using the metadata in
> JoinGroupResponse, run algorithm to assign partitions and start consume.
> 6. Go back to 1.
>
> The benefit here is that we can let rebalance finish in one round, and all
> the rest of changes will be captured in next consumer metadata refresh - so
> we get group commit. One concern might be letting consumer refresh metadata
> from coordinator might cause issue for big consumer groups. Maybe that is
> OK because metadata refresh is infrequent.
>
> This approach actually is very similar to what is proposed now: rebalance
> is triggered by metadata refresh, consumer provides subscription list to
> pass around. The only difference is that we don't need metadata hash
> anymore because the metadata is guaranteed to be the same. Replacing
> metadata hash with actual metadata will not have too much overhead for
> small subscription groups. There will be some overhead for regex
> subscriptions, but this can save the potential extra round of metadata
> fetch and will only occur when consumer see metadata change, which is
> infrequent.
>
> Any thoughts?
>
> Thanks,
>
> Jiangjie (Becket) Qin
>
>
> On Fri, Aug 14, 2015 at 12:57 PM, Ewen Cheslack-Postava  >
> wrote:
>
> > On Fri, Aug 14, 2015 at 10:59 AM, Jiangjie Qin  >
> > wrote:
> >
> > > Neha and Ewen,
> > >
> > > About the metadata change frequency. I guess it really depends on how
> > > frequent the metadata change might occur. If we run Kafka as a
> service, I
> > > can see that happens from time to time. As I can imagine people will
> > create
> > > some topic, test and maybe delete the topic in some automated test. If
> > so,
> > > the proposed protocol might be a little bit vulnerable.
> > >
> > > More specifically the scenario I am thinking is:
> > > 1. Consumer 0 periodically refresh metadata and detected a metadata
> > change.
> > > So it sends a JoinGroupRequest with metadata_hash_0.
> > > 2. Consumer 1 was notified by controller to start a rebalance, so it
> > > refreshes its metadata and send a JoingGroupRequest with
> metadata_hash_1,
> > > which is different from metadata hash 0.
> > > 3. Rebalance failed and both consumer refresh there metadata ag

[jira] [Commented] (KAFKA-2367) Add Copycat runtime data API

[Martin Kleppmann (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14698296#comment-14698296
 ] 

Martin Kleppmann commented on KAFKA-2367:
-

Just looked at this in the context of hopefully porting [Bottled 
Water|https://github.com/confluentinc/bottledwater-pg] (PostgreSQL change data 
capture to Kafka) to Copycat.

Bottled Water inspects the schema of the source database, and automatically 
generates an Avro schema from it (each PostgreSQL table definition is mapped to 
an Avro record type; each DB column becomes a field in the Avro record; record 
names and field names are taken from their names in the database). This makes 
integration quite smooth: you don't have to configure any data mappings (let 
alone write translation code), you just get a sensible data model by default.

So that background biases me towards Avro, and I'm happy for the Copycat data 
model to simply be Avro (although I'm not so keen on how the Avro code is 
currently unceremoniously copied and pasted into the Copycat patch). Here are a 
few more comments on bits of the discussion so far:

- Serialisation formats that use explicit field tags (Thrift, Protocol Buffers, 
Cap'n Proto) are painful with dynamically generated schemas, because their 
contract is that field numbers are forever. Say the schema is dynamically 
generated from a database schema, and someone drops a column from the middle of 
a table in the source database. If you don't forever keep that dropped column's 
field number reserved, you will generate invalid data in future. Avro doesn't 
have this problem, because fields are just identified by name. (Avro would only 
run into trouble if you create a new column with the same name as a column that 
previously existed and was dropped. Seems unlikely in practice.)

- I understand the desire to support JSON and other serialisation formats, but 
I don't think that using Avro as internal data model precludes that. We can 
make it easy to convert Avro objects at run-time into other formats, and even 
include support for a few popular formats. Making a "neutral" run-time format 
seems to me like unnecessary [standards proliferation|https://xkcd.com/927/].

- I think the claim that Copycat only needs 1% of Avro is rather exaggerated. A 
quick glance suggests that serialization is actually only about 30% of the Avro 
core code, and 70% is data model and schema management. If you start from the 
assumption that Copycat needs schemas, then you very quickly end up with 
something that looks very like Avro.

- IMHO, the problem with LinkedIn failing to upgrade from Avro 1.4 says more 
about problems with LinkedIn's dependency management than it says about Avro 
itself. Also, the Avro dependency we're talking about is only in Copycat 
connectors, so it is very localised, whereas LinkedIn is using it in every 
single application that has a Kafka client (i.e. basically everything).

To sum up, I agree with [~gwenshap]'s position.

> Add Copycat runtime data API
> 
>
> Key: KAFKA-2367
> URL: https://issues.apache.org/jira/browse/KAFKA-2367
> Project: Kafka
>  Issue Type: Sub-task
>  Components: copycat
>Reporter: Ewen Cheslack-Postava
>Assignee: Ewen Cheslack-Postava
> Fix For: 0.8.3
>
>
> Design the API used for runtime data in Copycat. This API is used to 
> construct schemas and records that Copycat processes. This needs to be a 
> fairly general data model (think Avro, JSON, Protobufs, Thrift) in order to 
> support complex, varied data types that may be input from/output to many data 
> systems.
> This should issue should also address the serialization interfaces used 
> within Copycat, which translate the runtime data into serialized byte[] form. 
> It is important that these be considered together because the data format can 
> be used in multiple ways (records, partition IDs, partition offsets), so it 
> and the corresponding serializers must be sufficient for all these use cases.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (KAFKA-2436) log.retention.hours should be honored by LogManager

[Dong Lin (JIRA)]

Dong Lin created KAFKA-2436:
---

 Summary: log.retention.hours should be honored by LogManager
 Key: KAFKA-2436
 URL: https://issues.apache.org/jira/browse/KAFKA-2436
 Project: Kafka
  Issue Type: Bug
Reporter: Dong Lin
Assignee: Dong Lin


Currently log.retention.hours is used to calculate 
KafkaConfig.logRetentionTimeMillis. But it is not used in LogManager to decide 
when to delete a log. LogManager is only using the log.retention.ms in the 
broker configuration.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (KAFKA-2072) Add StopReplica request/response to o.a.k.common.requests and replace the usage in core module

[David Jacot (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2072?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

David Jacot updated KAFKA-2072:
---
Status: Patch Available  (was: Open)

> Add StopReplica request/response to o.a.k.common.requests and replace the 
> usage in core module
> --
>
> Key: KAFKA-2072
> URL: https://issues.apache.org/jira/browse/KAFKA-2072
> Project: Kafka
>  Issue Type: Sub-task
>Reporter: Gwen Shapira
>Assignee: David Jacot
> Fix For: 0.8.3
>
>
> Add StopReplica request/response to o.a.k.common.requests and replace the 
> usage in core module



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2072) Add StopReplica request/response to o.a.k.common.requests and replace the usage in core module

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2072?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14698177#comment-14698177
 ] 

ASF GitHub Bot commented on KAFKA-2072:
---

GitHub user dajac opened a pull request:

https://github.com/apache/kafka/pull/141

KAFKA-2072 [WIP]: Add StopReplica request/response to o.a.k.common.requests 
and replace the usage in core module 

Migration is done but this PR will need to be rebased on #110. I have 
copied some code (ef669a5) for now.

I'd appreciate feedback on it mainly around how I handle things in the 
ControllerChannelManager. I have introduced a new 'sendRequest' method for 
o.a.k.common.requests and kept the old one for compatibility reason. We'll be 
able to remove the old one in the future when migration of all requests and 
responses to o.a.k.common.requests is completed.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/dajac/kafka KAFKA-2072

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/141.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #141


commit 22f2466f985cde1787a41f13a0191a538fc3a23f
Author: David Jacot 
Date:   2015-08-13T15:36:49Z

Add o.a.k.c.r.StopReplicaRequest and o.a.k.c.r.StopReplicaResponse.

commit 3a31ba9dc3d912e91535cf3a4e373b8f56b347b4
Author: David Jacot 
Date:   2015-08-13T17:15:18Z

Replace k.a.StopReplicaRequest and k.a.StopReplicaResponse in KafkaApis by 
their org.apache.kafka.common.requests equivalents.

commit ef669a5ff5fc125624d5d1ec79b92940d43ca3bb
Author: David Jacot 
Date:   2015-08-14T18:42:37Z

Code cherry-picked from KAFKA-2071. It can be removed when KAFKA-2071 is 
merged.

commit cbaa987385d989fad8cc3f40d50a24c2ee25ae78
Author: David Jacot 
Date:   2015-08-14T18:46:21Z

Replace k.a.StopReplicaRequest and k.a.StopReplicaResponse in Controller by 
their org.apache.kafka.common.requests equivalents.

commit 48a05d81c94ca30fff96df8c82587e64db4260b0
Author: David Jacot 
Date:   2015-08-14T18:53:32Z

Remove k.a.StopReplicaRequest and k.a.StopReplicaResponse.




> Add StopReplica request/response to o.a.k.common.requests and replace the 
> usage in core module
> --
>
> Key: KAFKA-2072
> URL: https://issues.apache.org/jira/browse/KAFKA-2072
> Project: Kafka
>  Issue Type: Sub-task
>Reporter: Gwen Shapira
>Assignee: David Jacot
> Fix For: 0.8.3
>
>
> Add StopReplica request/response to o.a.k.common.requests and replace the 
> usage in core module



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: KAFKA-2072 [WIP]: Add StopReplica request/resp...

[dajac]

GitHub user dajac opened a pull request:

https://github.com/apache/kafka/pull/141

KAFKA-2072 [WIP]: Add StopReplica request/response to o.a.k.common.requests 
and replace the usage in core module 

Migration is done but this PR will need to be rebased on #110. I have 
copied some code (ef669a5) for now.

I'd appreciate feedback on it mainly around how I handle things in the 
ControllerChannelManager. I have introduced a new 'sendRequest' method for 
o.a.k.common.requests and kept the old one for compatibility reason. We'll be 
able to remove the old one in the future when migration of all requests and 
responses to o.a.k.common.requests is completed.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/dajac/kafka KAFKA-2072

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/141.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #141


commit 22f2466f985cde1787a41f13a0191a538fc3a23f
Author: David Jacot 
Date:   2015-08-13T15:36:49Z

Add o.a.k.c.r.StopReplicaRequest and o.a.k.c.r.StopReplicaResponse.

commit 3a31ba9dc3d912e91535cf3a4e373b8f56b347b4
Author: David Jacot 
Date:   2015-08-13T17:15:18Z

Replace k.a.StopReplicaRequest and k.a.StopReplicaResponse in KafkaApis by 
their org.apache.kafka.common.requests equivalents.

commit ef669a5ff5fc125624d5d1ec79b92940d43ca3bb
Author: David Jacot 
Date:   2015-08-14T18:42:37Z

Code cherry-picked from KAFKA-2071. It can be removed when KAFKA-2071 is 
merged.

commit cbaa987385d989fad8cc3f40d50a24c2ee25ae78
Author: David Jacot 
Date:   2015-08-14T18:46:21Z

Replace k.a.StopReplicaRequest and k.a.StopReplicaResponse in Controller by 
their org.apache.kafka.common.requests equivalents.

commit 48a05d81c94ca30fff96df8c82587e64db4260b0
Author: David Jacot 
Date:   2015-08-14T18:53:32Z

Remove k.a.StopReplicaRequest and k.a.StopReplicaResponse.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---