[jira] [Commented] (KAFKA-972) MetadataRequest returns stale list of brokers

[Ashish K Singh (JIRA)]

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

Ashish K Singh commented on KAFKA-972:
--

Hey Guys,

I spent some time reproducing the issue and finding the root cause. Turns out 
KAFKA-1367 is not the issue here. Below is the problem and my suggested 
solution.

Problem: Alive brokers list not being propagated to brokers by coordinator. 
When a broker is started, it writes to ZK brokers path. Coordinator watches 
that path and notices the new broker. On noticing a new broker, the coordinator 
sends the UpdateMetadataRequest to only the new broker that just started up. 
The other brokers in cluster never gets to know that there are new brokers in 
the cluster.

Effect of KAFKA-1367: After KAFKA-1367 goes in it correct alive brokers 
information will be propagated to all live brokers after ISR changes at any 
broker. However, if there are no topics/ partitions KAFKA-1367 will not help 
and this issue will still be there.

Solution: Instead of sending the UpdateMetadataRequest only to new broker, send 
it to all live brokers in the cluster.

[~junrao], [~nehanarkhede], [~granthenke], [~gwenshap], [~charmalloc], 
[~jjkoshy] please provide your thoughts. I have a patch ready which I will post 
if you guys think this is indeed the correct approach.

> MetadataRequest returns stale list of brokers
> -
>
> Key: KAFKA-972
> URL: https://issues.apache.org/jira/browse/KAFKA-972
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.8.0
>Reporter: Vinicius Carvalho
>Assignee: Ashish K Singh
> Attachments: BrokerMetadataTest.scala
>
>
> When we issue an metadatarequest towards the cluster, the list of brokers is 
> stale. I mean, even when a broker is down, it's returned back to the client. 
> The following are examples of two invocations one with both brokers online 
> and the second with a broker down:
> {
> "brokers": [
> {
> "nodeId": 0,
> "host": "10.139.245.106",
> "port": 9092,
> "byteLength": 24
> },
> {
> "nodeId": 1,
> "host": "localhost",
> "port": 9093,
> "byteLength": 19
> }
> ],
> "topicMetadata": [
> {
> "topicErrorCode": 0,
> "topicName": "foozbar",
> "partitions": [
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 0,
> "leader": 0,
> "byteLength": 26
> },
> {
> "replicas": [
> 1
> ],
> "isr": [
> 1
> ],
> "partitionErrorCode": 0,
> "partitionId": 1,
> "leader": 1,
> "byteLength": 26
> },
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 2,
> "leader": 0,
> "byteLength": 26
> },
> {
> "replicas": [
> 1
> ],
> "isr": [
> 1
> ],
> "partitionErrorCode": 0,
> "partitionId": 3,
> "leader": 1,
> "byteLength": 26
> },
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 4,
> "leader": 0,
> "byteLength": 26
> }
> ],
> "byteLength": 145
> }
> ],
> "responseSize": 200,
> "correlationId": -1000
> }
> {
> "brokers": [
> {
> "nodeId": 0,
> "host": "10.139.245.106",
> "port": 9092,
> "byteLength": 24
> },
> {
> "nodeId": 1,
> "host": "localhost",
> "port": 9093,
> "byteLength": 19
> }
> ],
> "topicMetadata": [
> {
>

[jira] [Comment Edited] (KAFKA-972) MetadataRequest returns stale list of brokers

[Ashish K Singh (JIRA)]

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

Ashish K Singh edited comment on KAFKA-972 at 6/25/15 3:10 AM:
---

Hey Guys,

I spent some time reproducing the issue and finding the root cause. Turns out 
KAFKA-1367 is not the issue here. Below is the problem and my suggested 
solution.

Problem:
Alive brokers list not being propagated to brokers by coordinator. When a 
broker is started, it writes to ZK brokers path. Coordinator watches that path 
and notices the new broker. On noticing a new broker, the coordinator sends the 
UpdateMetadataRequest to only the new broker that just started up. The other 
brokers in cluster never gets to know that there are new brokers in the cluster.

Effect of KAFKA-1367: After KAFKA-1367 goes in it correct alive brokers 
information will be propagated to all live brokers after ISR changes at any 
broker. However, if there are no topics/ partitions KAFKA-1367 will not help 
and this issue will still be there.

Solution:
Instead of sending the UpdateMetadataRequest only to new broker, send it to all 
live brokers in the cluster.

[~junrao], [~nehanarkhede], [~granthenke], [~gwenshap], [~charmalloc], 
[~jjkoshy] please provide your thoughts. I have a patch ready which I will post 
if you guys think this is indeed the correct approach.


was (Author: singhashish):
Hey Guys,

I spent some time reproducing the issue and finding the root cause. Turns out 
KAFKA-1367 is not the issue here. Below is the problem and my suggested 
solution.

Problem: Alive brokers list not being propagated to brokers by coordinator. 
When a broker is started, it writes to ZK brokers path. Coordinator watches 
that path and notices the new broker. On noticing a new broker, the coordinator 
sends the UpdateMetadataRequest to only the new broker that just started up. 
The other brokers in cluster never gets to know that there are new brokers in 
the cluster.

Effect of KAFKA-1367: After KAFKA-1367 goes in it correct alive brokers 
information will be propagated to all live brokers after ISR changes at any 
broker. However, if there are no topics/ partitions KAFKA-1367 will not help 
and this issue will still be there.

Solution: Instead of sending the UpdateMetadataRequest only to new broker, send 
it to all live brokers in the cluster.

[~junrao], [~nehanarkhede], [~granthenke], [~gwenshap], [~charmalloc], 
[~jjkoshy] please provide your thoughts. I have a patch ready which I will post 
if you guys think this is indeed the correct approach.

> MetadataRequest returns stale list of brokers
> -
>
> Key: KAFKA-972
> URL: https://issues.apache.org/jira/browse/KAFKA-972
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.8.0
>Reporter: Vinicius Carvalho
>Assignee: Ashish K Singh
> Attachments: BrokerMetadataTest.scala
>
>
> When we issue an metadatarequest towards the cluster, the list of brokers is 
> stale. I mean, even when a broker is down, it's returned back to the client. 
> The following are examples of two invocations one with both brokers online 
> and the second with a broker down:
> {
> "brokers": [
> {
> "nodeId": 0,
> "host": "10.139.245.106",
> "port": 9092,
> "byteLength": 24
> },
> {
> "nodeId": 1,
> "host": "localhost",
> "port": 9093,
> "byteLength": 19
> }
> ],
> "topicMetadata": [
> {
> "topicErrorCode": 0,
> "topicName": "foozbar",
> "partitions": [
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 0,
> "leader": 0,
> "byteLength": 26
> },
> {
> "replicas": [
> 1
> ],
> "isr": [
> 1
> ],
> "partitionErrorCode": 0,
> "partitionId": 1,
> "leader": 1,
> "byteLength": 26
> },
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 2,
> "leader": 0,
> "byteLength": 26
> },
> 

[jira] [Comment Edited] (KAFKA-972) MetadataRequest returns stale list of brokers

[Ashish K Singh (JIRA)]

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

Ashish K Singh edited comment on KAFKA-972 at 6/25/15 3:10 AM:
---

Hey Guys,

I spent some time reproducing the issue and finding the root cause. Turns out 
KAFKA-1367 is not the issue here. Below is the problem and my suggested 
solution.

Problem:
Alive brokers list not being propagated to brokers by coordinator. When a 
broker is started, it writes to ZK brokers path. Coordinator watches that path 
and notices the new broker. On noticing a new broker, the coordinator sends the 
UpdateMetadataRequest to only the new broker that just started up. The other 
brokers in cluster never gets to know that there are new brokers in the cluster.

Effect of KAFKA-1367: After KAFKA-1367 goes in it correct alive brokers 
information will be propagated to all live brokers after ISR changes at any 
broker. However, if there are no topics/ partitions KAFKA-1367 will not help 
and this issue will still be there.

Solution:
Instead of sending the UpdateMetadataRequest only to new broker, send it to all 
live brokers in the cluster.

[~junrao], [~nehanarkhede], [~granthenke], [~gwenshap], [~charmalloc], 
[~jjkoshy] please provide your thoughts. I have a patch ready which I will post 
if you guys think this is indeed the correct approach. I have verified that 
above approach fixes the issue.


was (Author: singhashish):
Hey Guys,

I spent some time reproducing the issue and finding the root cause. Turns out 
KAFKA-1367 is not the issue here. Below is the problem and my suggested 
solution.

Problem:
Alive brokers list not being propagated to brokers by coordinator. When a 
broker is started, it writes to ZK brokers path. Coordinator watches that path 
and notices the new broker. On noticing a new broker, the coordinator sends the 
UpdateMetadataRequest to only the new broker that just started up. The other 
brokers in cluster never gets to know that there are new brokers in the cluster.

Effect of KAFKA-1367: After KAFKA-1367 goes in it correct alive brokers 
information will be propagated to all live brokers after ISR changes at any 
broker. However, if there are no topics/ partitions KAFKA-1367 will not help 
and this issue will still be there.

Solution:
Instead of sending the UpdateMetadataRequest only to new broker, send it to all 
live brokers in the cluster.

[~junrao], [~nehanarkhede], [~granthenke], [~gwenshap], [~charmalloc], 
[~jjkoshy] please provide your thoughts. I have a patch ready which I will post 
if you guys think this is indeed the correct approach.

> MetadataRequest returns stale list of brokers
> -
>
> Key: KAFKA-972
> URL: https://issues.apache.org/jira/browse/KAFKA-972
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.8.0
>Reporter: Vinicius Carvalho
>Assignee: Ashish K Singh
> Attachments: BrokerMetadataTest.scala
>
>
> When we issue an metadatarequest towards the cluster, the list of brokers is 
> stale. I mean, even when a broker is down, it's returned back to the client. 
> The following are examples of two invocations one with both brokers online 
> and the second with a broker down:
> {
> "brokers": [
> {
> "nodeId": 0,
> "host": "10.139.245.106",
> "port": 9092,
> "byteLength": 24
> },
> {
> "nodeId": 1,
> "host": "localhost",
> "port": 9093,
> "byteLength": 19
> }
> ],
> "topicMetadata": [
> {
> "topicErrorCode": 0,
> "topicName": "foozbar",
> "partitions": [
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 0,
> "leader": 0,
> "byteLength": 26
> },
> {
> "replicas": [
> 1
> ],
> "isr": [
> 1
> ],
> "partitionErrorCode": 0,
> "partitionId": 1,
> "leader": 1,
> "byteLength": 26
> },
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 2,
> "leader": 0,
>

[jira] [Assigned] (KAFKA-972) MetadataRequest returns stale list of brokers

[Ashish K Singh (JIRA)]

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

Ashish K Singh reassigned KAFKA-972:


Assignee: Ashish K Singh

> MetadataRequest returns stale list of brokers
> -
>
> Key: KAFKA-972
> URL: https://issues.apache.org/jira/browse/KAFKA-972
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.8.0
>Reporter: Vinicius Carvalho
>Assignee: Ashish K Singh
> Attachments: BrokerMetadataTest.scala
>
>
> When we issue an metadatarequest towards the cluster, the list of brokers is 
> stale. I mean, even when a broker is down, it's returned back to the client. 
> The following are examples of two invocations one with both brokers online 
> and the second with a broker down:
> {
> "brokers": [
> {
> "nodeId": 0,
> "host": "10.139.245.106",
> "port": 9092,
> "byteLength": 24
> },
> {
> "nodeId": 1,
> "host": "localhost",
> "port": 9093,
> "byteLength": 19
> }
> ],
> "topicMetadata": [
> {
> "topicErrorCode": 0,
> "topicName": "foozbar",
> "partitions": [
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 0,
> "leader": 0,
> "byteLength": 26
> },
> {
> "replicas": [
> 1
> ],
> "isr": [
> 1
> ],
> "partitionErrorCode": 0,
> "partitionId": 1,
> "leader": 1,
> "byteLength": 26
> },
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 2,
> "leader": 0,
> "byteLength": 26
> },
> {
> "replicas": [
> 1
> ],
> "isr": [
> 1
> ],
> "partitionErrorCode": 0,
> "partitionId": 3,
> "leader": 1,
> "byteLength": 26
> },
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 4,
> "leader": 0,
> "byteLength": 26
> }
> ],
> "byteLength": 145
> }
> ],
> "responseSize": 200,
> "correlationId": -1000
> }
> {
> "brokers": [
> {
> "nodeId": 0,
> "host": "10.139.245.106",
> "port": 9092,
> "byteLength": 24
> },
> {
> "nodeId": 1,
> "host": "localhost",
> "port": 9093,
> "byteLength": 19
> }
> ],
> "topicMetadata": [
> {
> "topicErrorCode": 0,
> "topicName": "foozbar",
> "partitions": [
> {
> "replicas": [
> 0
> ],
> "isr": [],
> "partitionErrorCode": 5,
> "partitionId": 0,
> "leader": -1,
> "byteLength": 22
> },
> {
> "replicas": [
> 1
> ],
> "isr": [
> 1
> ],
> "partitionErrorCode": 0,
> "partitionId": 1,
> "leader": 1,
> "byteLength": 26
> },
> {
> "replicas": [
> 0
> ],
> "isr": [],
> "partitionErrorCode": 5,
> "partitionId": 2,
> "leader": -1,
> "byteLength": 22
> },
> {
> "replicas": [
>

Re: Review Request 35850: Patch for KAFKA-2301

[Ewen Cheslack-Postava]

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


Is this what was intended by "deprecate"? I'm not sure there's been serious 
discussion about this yet, but this doesn't really deprecate the command in a 
compatible way -- just changing to ConsumerGroupCommand requires changing 
command lines that previously would have worked (e.g., 
./bin/kafka-consumer-groups.sh --zookeeper localhost:2181 --group foo will now 
output "Command must include exactly one action: --list, --describe, 
--delete"). Also, I'm not sure removing the bat file entirely is ok -- 
shouldn't that have basically the same change as the sh file?

- Ewen Cheslack-Postava


On June 24, 2015, 10:10 p.m., Ashish Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35850/
> ---
> 
> (Updated June 24, 2015, 10:10 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2301
> https://issues.apache.org/jira/browse/KAFKA-2301
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2301: Deprecate ConsumerOffsetChecker
> 
> 
> Diffs
> -
> 
>   bin/kafka-consumer-offset-checker.sh 
> c275f7ef6975c3e1226c9b286d64ad55a67b0a8b 
>   bin/windows/kafka-consumer-offset-checker.bat 
> b6967c49609c3560786921d54e9df50d70c68713 
>   core/src/main/scala/kafka/tools/ConsumerOffsetChecker.scala 
> 3d52f62c88a509a655cf1df6232b738c25fa9b69 
>   system_test/broker_failure/config/log4j.properties 
> 23ece9bb5b3d521bb692870bc929865f68caabe0 
> 
> Diff: https://reviews.apache.org/r/35850/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

[jira] [Commented] (KAFKA-282) Currently iterated chunk is not cleared during consumer shutdown

[Matthieu Sprunck (JIRA)]

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

Matthieu Sprunck commented on KAFKA-282:


I still have this issue with the API version 0.8.1.1. During the connector 
shutdown and after offsets have been committed, the ConsumerIterator give me 
more messages.

What was the resolution to this issue ?

> Currently iterated chunk is not cleared during consumer shutdown
> 
>
> Key: KAFKA-282
> URL: https://issues.apache.org/jira/browse/KAFKA-282
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Reporter: Joel Koshy
>Assignee: Joel Koshy
>
> During consumer connector shutdown, fetch queues are cleared, but the 
> currently iterated chunk is not cleared.



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

Review Request 35850: Patch for KAFKA-2301

[Ashish Singh]

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

Review request for kafka.


Bugs: KAFKA-2301
https://issues.apache.org/jira/browse/KAFKA-2301


Repository: kafka


Description
---

KAFKA-2301: Deprecate ConsumerOffsetChecker


Diffs
-

  bin/kafka-consumer-offset-checker.sh c275f7ef6975c3e1226c9b286d64ad55a67b0a8b 
  bin/windows/kafka-consumer-offset-checker.bat 
b6967c49609c3560786921d54e9df50d70c68713 
  core/src/main/scala/kafka/tools/ConsumerOffsetChecker.scala 
3d52f62c88a509a655cf1df6232b738c25fa9b69 
  system_test/broker_failure/config/log4j.properties 
23ece9bb5b3d521bb692870bc929865f68caabe0 

Diff: https://reviews.apache.org/r/35850/diff/


Testing
---


Thanks,

Ashish Singh

[jira] [Updated] (KAFKA-2301) Deprecate ConsumerOffsetChecker

[Ashish K Singh (JIRA)]

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

Ashish K Singh updated KAFKA-2301:
--
Status: Patch Available  (was: Open)

> Deprecate ConsumerOffsetChecker
> ---
>
> Key: KAFKA-2301
> URL: https://issues.apache.org/jira/browse/KAFKA-2301
> Project: Kafka
>  Issue Type: Sub-task
>  Components: tools
>Reporter: Ashish K Singh
>Assignee: Ashish K Singh
> Fix For: 0.8.3
>
> Attachments: KAFKA-2301.patch
>
>




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

[jira] [Updated] (KAFKA-2301) Deprecate ConsumerOffsetChecker

[Ashish K Singh (JIRA)]

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

Ashish K Singh updated KAFKA-2301:
--
Attachment: KAFKA-2301.patch

> Deprecate ConsumerOffsetChecker
> ---
>
> Key: KAFKA-2301
> URL: https://issues.apache.org/jira/browse/KAFKA-2301
> Project: Kafka
>  Issue Type: Sub-task
>  Components: tools
>Reporter: Ashish K Singh
>Assignee: Ashish K Singh
> Fix For: 0.8.3
>
> Attachments: KAFKA-2301.patch
>
>




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

[jira] [Commented] (KAFKA-2301) Deprecate ConsumerOffsetChecker

[Ashish K Singh (JIRA)]

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

Ashish K Singh commented on KAFKA-2301:
---

Created reviewboard https://reviews.apache.org/r/35850/
 against branch trunk

> Deprecate ConsumerOffsetChecker
> ---
>
> Key: KAFKA-2301
> URL: https://issues.apache.org/jira/browse/KAFKA-2301
> Project: Kafka
>  Issue Type: Sub-task
>  Components: tools
>Reporter: Ashish K Singh
>Assignee: Ashish K Singh
> Fix For: 0.8.3
>
> Attachments: KAFKA-2301.patch
>
>




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

[jira] [Commented] (KAFKA-2203) Get gradle build to work with Java 8

[Jakob Homan (JIRA)]

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

Jakob Homan commented on KAFKA-2203:


A couple data points:
* Samza has a script that runs tests against all supported JDKs and Scala 
versions: https://github.com/apache/samza/blob/master/bin/check-all.sh  It's 
expected to be run before commits, both by the contributor and committer.  
However, Samza's tests aren't as long as a Kafka's.
* Samza tried Travis, but eventually disabled it (SAMZA-642).  The parallel, 
multiconfiguration testing matrix was awesome, but we were seeing so many false 
positive test failures and the fact that it ran as a post-commit test (rather 
than pre) meant the most committers ignored it.

> Get gradle build to work with Java 8
> 
>
> Key: KAFKA-2203
> URL: https://issues.apache.org/jira/browse/KAFKA-2203
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Affects Versions: 0.8.1.1
>Reporter: Gaju Bhat
>Priority: Minor
> Fix For: 0.8.1.2
>
> Attachments: 0001-Special-case-java-8-and-javadoc-handling.patch
>
>
> The gradle build halts because javadoc in java 8 is a lot stricter about 
> valid html.
> It might be worthwhile to special case java 8 as described 
> [here|http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html].



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

Re: Review Request 35791: Patch for KAFKA-2298

[Jason Gustafson]

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

Ship it!


Ship It!

- Jason Gustafson


On June 24, 2015, 8 p.m., Dong Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35791/
> ---
> 
> (Updated June 24, 2015, 8 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2298
> https://issues.apache.org/jira/browse/KAFKA-2298
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2298; Client Selector can drop connections on InvalidReceiveException 
> without notifying NetworkClient
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> 4aee214b24fd990be003adc36d675f015bf22fe6 
> 
> Diff: https://reviews.apache.org/r/35791/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dong Lin
> 
>

[jira] [Commented] (KAFKA-2298) Client Selector can drop connections on InvalidReceiveException without notifying NetworkClient

[Dong Lin (JIRA)]

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

Dong Lin commented on KAFKA-2298:
-

Updated reviewboard https://reviews.apache.org/r/35791/diff/
 against branch origin/trunk

> Client Selector can drop connections on InvalidReceiveException without 
> notifying NetworkClient
> ---
>
> Key: KAFKA-2298
> URL: https://issues.apache.org/jira/browse/KAFKA-2298
> Project: Kafka
>  Issue Type: Bug
>Reporter: Dong Lin
>Assignee: Dong Lin
>  Labels: quotas
> Attachments: KAFKA-2298.patch, KAFKA-2298_2015-06-23_18:47:54.patch, 
> KAFKA-2298_2015-06-24_13:00:39.patch
>
>
> I run into the problem described in KAFKA-2266 when testing quota. I was told 
> the bug was fixed in KAFKA-2266 after I figured out the problem.
> But the patch provided in KAFKA-2266 probably doesn't solve all related 
> problems. From reading the code there is still one edge case where the client 
> selector can close connection in poll() without notifying NetworkClient.



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

Re: Review Request 35791: Patch for KAFKA-2298

[Dong Lin]

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

(Updated June 24, 2015, 8 p.m.)


Review request for kafka.


Bugs: KAFKA-2298
https://issues.apache.org/jira/browse/KAFKA-2298


Repository: kafka


Description
---

KAFKA-2298; Client Selector can drop connections on InvalidReceiveException 
without notifying NetworkClient


Diffs (updated)
-

  clients/src/main/java/org/apache/kafka/common/network/Selector.java 
4aee214b24fd990be003adc36d675f015bf22fe6 

Diff: https://reviews.apache.org/r/35791/diff/


Testing
---


Thanks,

Dong Lin

[jira] [Updated] (KAFKA-2298) Client Selector can drop connections on InvalidReceiveException without notifying NetworkClient

[Dong Lin (JIRA)]

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

Dong Lin updated KAFKA-2298:

Attachment: KAFKA-2298_2015-06-24_13:00:39.patch

> Client Selector can drop connections on InvalidReceiveException without 
> notifying NetworkClient
> ---
>
> Key: KAFKA-2298
> URL: https://issues.apache.org/jira/browse/KAFKA-2298
> Project: Kafka
>  Issue Type: Bug
>Reporter: Dong Lin
>Assignee: Dong Lin
>  Labels: quotas
> Attachments: KAFKA-2298.patch, KAFKA-2298_2015-06-23_18:47:54.patch, 
> KAFKA-2298_2015-06-24_13:00:39.patch
>
>
> I run into the problem described in KAFKA-2266 when testing quota. I was told 
> the bug was fixed in KAFKA-2266 after I figured out the problem.
> But the patch provided in KAFKA-2266 probably doesn't solve all related 
> problems. From reading the code there is still one edge case where the client 
> selector can close connection in poll() without notifying NetworkClient.



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

Re: Review Request 35791: Patch for KAFKA-2298

[Dong Lin]

> On June 23, 2015, 5:59 p.m., Jason Gustafson wrote:
> > clients/src/main/java/org/apache/kafka/common/network/Selector.java, line 
> > 282
> > 
> >
> > Do you think we should just move the disconnected.add() into the close 
> > method?
> 
> Dong Lin wrote:
> I have thought about it as well. But probabaly no. Because in 
> Selector.send() we put failed destinationId is put in failedSends rather than 
> disconnected. The reason we use failedSends is because send() and poll() in 
> Selector will be called asynchronously by different threads.
> 
> Jason Gustafson wrote:
> Yeah, that makes sense. I wonder if we should be using a Set internally 
> instead of a List? Then we wouldn't need to worry about adding to 
> disconnected multiple times. Guess there might be some performance impact, 
> but we would have an easier time ensuring correctness.
> 
> Dong Lin wrote:
> Yeah that can be useful. I am not sure if currently some id may be added 
> to disconnected multiple times. Even it does, this should be not a problme in 
> NetworkClient.handleDisconnections(). I personally prefer to keep existing 
> code unless there is good reason (i.e. performance or functionality) for 
> change. Not sure what others think about this change.
> 
> Jason Gustafson wrote:
> Seems unnecessary now that I think about it. In fact, it wouldn't hurt to 
> do the disconnected.add() into close() as long as we also add it to 
> failedSends, but that might make the code tougher to follow. Anyway, I think 
> the patch is fine as it is. I was just wondering if there was an easy way to 
> prevent this issue from popping up again in the future.
> 
> Dong Lin wrote:
> Yeah I agree with you. I will add to disconnected in close() if I were to 
> write this class/function from scratch. But since this is a small patch to 
> existing module I will make the change that solves the problem with least 
> dispute to make peer review smooth.
> 
> Joel Koshy wrote:
> Hi Dong - I'm not sure about your comment on multiple threads calling 
> send/poll. NetworkClient and Selector are both documented as being 
> `@nonthreadsafe` so that is probably a non-issue. The producer's sender for 
> e.g., does send/poll in the same (sender) thread .
> 
> Dong Lin wrote:
> Hi Joel, Jason,
> 
> Yeah you are right. Thanks for your correction. I have moved 
> disconnected.add() into close() in the updated patch. Please have a look.
> 
> Jason Gustafson wrote:
> I think the only issue now is that the disconnected list can be updated 
> in a failed send, which technically violates the API. I doubt this would 
> cause any problems given current usage, but it's probably better to stay 
> consistent with the API. You could get around this perhaps by deferring the 
> close on a failed send until poll (which would introduce a nice invariant 
> that close is only called in poll). There may be other workarounds as well, 
> if you want to have a look. Otherwise, the initial patch might be the better 
> way to go.
> 
> Dong Lin wrote:
> Jason, could you explain why it technically violates the API? I think 
> when send fails, the connection is indeed closed and therefore it is OK to 
> update disconnected list.
> 
> Jason Gustafson wrote:
> The javadoc on the disconnected method says it returns the list of 
> disconnects that finished on the last poll. The implication is that this list 
> does not change except when poll() is called. Like I said, I don't think it 
> would create problems with current usage, but we should at least update the 
> javadoc if we want to allow modification of the disconnects during send() as 
> well. I think deferring close to poll would address the issue, but maybe 
> there are other implications. Since it seems we have to accept some ugliness 
> conforming to this interface either way, perhaps the initial patch might be 
> the most consistent way.

Oh I see. Thanks much for your explanation. I agree with you that first patch 
is better.


- Dong


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


On June 24, 2015, 1:48 a.m., Dong Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35791/
> ---
> 
> (Updated June 24, 2015, 1:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2298
> https://issues.apache.org/jira/browse/KAFKA-2298
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2298; Client Selector can drop connections on InvalidReceiveException 
> without notifying NetworkClient
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/comm

Re: [GitHub] kafka pull request: Kafka 2276

[Geoffrey Anderson]

Hi Gwen,

A couple other thoughts:

services/templates
These are jinja2 templates which can be populated with values as desired.

Although currently many individual configs in various templates have preset
values, we can easily make these more parametrizable (presumably with
default values).
There is no uniform config file strategy or convention. The templates will
be reused, but where actual values used to populate the templates come from
is totally flexible.
My experience with the existing tests suggests it's not that useful or
clear to mandate per-test config files for every service used, but there's
nothing here preventing some configs from living in files.

multi-version
When I prototyped some of this, it was fairly reasonable to have individual
services support multiple versions, and this makes it possible to have
clusters with *mixed* versions (i.e. some nodes on some versions and some
nodes on other versions).

kafkatest/tests
It's not totally clear whether we'll need to break this into
subdirectories. My inclination is to start with modules and break into
subdirectories if a particular module becomes complex/big enough. With
ducktape, running all tests in a directory is no harder than
running all tests in a file, so subdirectories don't change anything from a
user perspective.

Thanks,
Geoff



On Tue, Jun 23, 2015 at 7:56 PM, Gwen Shapira  wrote:

> I'm unclear on the directory structure and few high level things (and
> I can't figure out how to comment on more than a single line in
> github):
>
> tests/kafkatest - isn't it redundant? do we expect any non-kafka tests?
>
> services/templates - those are basically all configuration files to be
> used by the services, right? Do we expect a single set for the entire
> system? or different templates for different tests? I'm checking
> because "systemtests" had config files per test.
>
> any thoughts on how multi-version tests will work? will we have
> service per version?
>
> kafkatest/tests - do we expect every test to be a single script? or
> should we have subdirectories here, to start things in the right
> direction? Maybe subdirectories for planned groups of tests?
>
> kafka/vagrant - is this intentionally not under tests? Is this at all
> stand-alone? If so, maybe a separate Jira? This has a bunch of stuff
> that I'm not sure we want in the project at all... specific IPs,
> Vagrant install from debian packages, etc.
>
> Gwen
>
>
>
>
>
>
>
>
>
> On Tue, Jun 23, 2015 at 10:45 AM, Gwen Shapira 
> wrote:
> > Awesome, thanks :)
> >
> > On Tue, Jun 23, 2015 at 10:32 AM, Geoffrey Anderson 
> wrote:
> >> Hi Gwen,
> >>
> >> That is indeed the plan, and my understanding is that the merge script
> >> Ismael has been working on helps committers with this step.
> >>
> >> I'm trying out the Github flow roughly as outlined here:
> >>
> http://mail-archives.apache.org/mod_mbox/kafka-dev/201504.mbox/%3ccad5tkzab-hkey-zcr8x4wtxawybxpojx62k1vbv+ycknuxq...@mail.gmail.com%3E
> >>
> >> Ismael's script is here:
> https://issues.apache.org/jira/browse/KAFKA-2187
> >>
> >>
> >> Thanks,
> >>
> >> Geoff
> >>
> >> On Mon, Jun 22, 2015 at 9:59 PM, Gwen Shapira 
> wrote:
> >>
> >>> Thanks, I indeed missed the original :)
> >>>
> >>> Is the plan to squash the commits and merge a pull request with single
> >>> commit that matches the JIRA #?
> >>> This will be more in line with how commits were organized until now
> >>> and will make life much easier when cherry-picking.
> >>>
> >>> Gwen
> >>>
> >>> On Mon, Jun 22, 2015 at 1:58 PM, Geoffrey Anderson  >
> >>> wrote:
> >>> > Hi,
> >>> >
> >>> > I'm pinging the dev list regarding KAFKA-2276 (KIP-25 initial patch)
> >>> again
> >>> > since it sounds like at least one person I spoke with did not see the
> >>> > initial pull request.
> >>> >
> >>> > Pull request: https://github.com/apache/kafka/pull/70/
> >>> > JIRA: https://issues.apache.org/jira/browse/KAFKA-2276
> >>> >
> >>> > Thanks!
> >>> > Geoff
> >>> >
> >>> >
> >>> > On Tue, Jun 16, 2015 at 2:50 PM, granders 
> wrote:
> >>> >
> >>> >> GitHub user granders opened a pull request:
> >>> >>
> >>> >> https://github.com/apache/kafka/pull/70
> >>> >>
> >>> >> Kafka 2276
> >>> >>
> >>> >> Initial patch for KIP-25
> >>> >>
> >>> >> Note that to install ducktape, do *not* use pip to install
> ducktape.
> >>> >> Instead:
> >>> >>
> >>> >> ```
> >>> >> $ git clone g...@github.com:confluentinc/ducktape.git
> >>> >> $ cd ducktape
> >>> >> $ python setup.py install
> >>> >> ```
> >>> >>
> >>> >>
> >>> >> You can merge this pull request into a Git repository by running:
> >>> >>
> >>> >> $ git pull https://github.com/confluentinc/kafka KAFKA-2276
> >>> >>
> >>> >> Alternatively you can review and apply these changes as the patch
> at:
> >>> >>
> >>> >> https://github.com/apache/kafka/pull/70.patch
> >>> >>
> >>> >> To close this pull request, make a commit to your master/trunk
> branch
> >>> >> with (at least) the following in the commit message:

Re: Review Request 35791: Patch for KAFKA-2298

[Jason Gustafson]

> On June 23, 2015, 5:59 p.m., Jason Gustafson wrote:
> > clients/src/main/java/org/apache/kafka/common/network/Selector.java, line 
> > 282
> > 
> >
> > Do you think we should just move the disconnected.add() into the close 
> > method?
> 
> Dong Lin wrote:
> I have thought about it as well. But probabaly no. Because in 
> Selector.send() we put failed destinationId is put in failedSends rather than 
> disconnected. The reason we use failedSends is because send() and poll() in 
> Selector will be called asynchronously by different threads.
> 
> Jason Gustafson wrote:
> Yeah, that makes sense. I wonder if we should be using a Set internally 
> instead of a List? Then we wouldn't need to worry about adding to 
> disconnected multiple times. Guess there might be some performance impact, 
> but we would have an easier time ensuring correctness.
> 
> Dong Lin wrote:
> Yeah that can be useful. I am not sure if currently some id may be added 
> to disconnected multiple times. Even it does, this should be not a problme in 
> NetworkClient.handleDisconnections(). I personally prefer to keep existing 
> code unless there is good reason (i.e. performance or functionality) for 
> change. Not sure what others think about this change.
> 
> Jason Gustafson wrote:
> Seems unnecessary now that I think about it. In fact, it wouldn't hurt to 
> do the disconnected.add() into close() as long as we also add it to 
> failedSends, but that might make the code tougher to follow. Anyway, I think 
> the patch is fine as it is. I was just wondering if there was an easy way to 
> prevent this issue from popping up again in the future.
> 
> Dong Lin wrote:
> Yeah I agree with you. I will add to disconnected in close() if I were to 
> write this class/function from scratch. But since this is a small patch to 
> existing module I will make the change that solves the problem with least 
> dispute to make peer review smooth.
> 
> Joel Koshy wrote:
> Hi Dong - I'm not sure about your comment on multiple threads calling 
> send/poll. NetworkClient and Selector are both documented as being 
> `@nonthreadsafe` so that is probably a non-issue. The producer's sender for 
> e.g., does send/poll in the same (sender) thread .
> 
> Dong Lin wrote:
> Hi Joel, Jason,
> 
> Yeah you are right. Thanks for your correction. I have moved 
> disconnected.add() into close() in the updated patch. Please have a look.
> 
> Jason Gustafson wrote:
> I think the only issue now is that the disconnected list can be updated 
> in a failed send, which technically violates the API. I doubt this would 
> cause any problems given current usage, but it's probably better to stay 
> consistent with the API. You could get around this perhaps by deferring the 
> close on a failed send until poll (which would introduce a nice invariant 
> that close is only called in poll). There may be other workarounds as well, 
> if you want to have a look. Otherwise, the initial patch might be the better 
> way to go.
> 
> Dong Lin wrote:
> Jason, could you explain why it technically violates the API? I think 
> when send fails, the connection is indeed closed and therefore it is OK to 
> update disconnected list.

The javadoc on the disconnected method says it returns the list of disconnects 
that finished on the last poll. The implication is that this list does not 
change except when poll() is called. Like I said, I don't think it would create 
problems with current usage, but we should at least update the javadoc if we 
want to allow modification of the disconnects during send() as well. I think 
deferring close to poll would address the issue, but maybe there are other 
implications. Since it seems we have to accept some ugliness conforming to this 
interface either way, perhaps the initial patch might be the most consistent 
way.


- Jason


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


On June 24, 2015, 1:48 a.m., Dong Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35791/
> ---
> 
> (Updated June 24, 2015, 1:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2298
> https://issues.apache.org/jira/browse/KAFKA-2298
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2298; Client Selector can drop connections on InvalidReceiveException 
> without notifying NetworkClient
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> 4aee214b24fd990be003adc36d675f015bf22fe6 
> 
> Diff: https://reviews.apache.org/r/35791/diff/
> 
> 
> Testing
>

[jira] [Commented] (KAFKA-2300) Error in controller log when broker tries to rejoin cluster

[Kurt Harriger (JIRA)]

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

Kurt Harriger commented on KAFKA-2300:
--

Looking at the code it appears once the controller has this invalid state new 
brokers are unable to join the cluster and this state cannot be cleared until 
the current controller is restarted.  The only code path that is calls clear on 
the updateMetadataRequestMap follows a call to newBatch and thus is 
unreachable.  If newBatch fails it probably needs to clear the invalid state.  

The only way it seems to clear the new invalid state is to restart the current 
controller node.  Its a bit scary to restart an otherwise working node when you 
have another node sitting idle and currently unable to join the cluster.


> Error in controller log when broker tries to rejoin cluster
> ---
>
> Key: KAFKA-2300
> URL: https://issues.apache.org/jira/browse/KAFKA-2300
> Project: Kafka
>  Issue Type: Bug
>Affects Versions: 0.8.2.1
>Reporter: Johnny Brown
>
> Hello Kafka folks,
> We are having an issue where a broker attempts to join the cluster after 
> being restarted, but is never added to the ISR for its assigned partitions. 
> This is a three-node cluster, and the controller is broker 2.
> When broker 1 starts, we see the following message in broker 2's 
> controller.log.
> {{
> [2015-06-23 13:57:16,535] ERROR [BrokerChangeListener on Controller 2]: Error 
> while handling broker changes 
> (kafka.controller.ReplicaStateMachine$BrokerChangeListener)
> java.lang.IllegalStateException: Controller to broker state change requests 
> batch is not empty while creating a new one. Some UpdateMetadata state 
> changes Map(2 -> Map([prod-sver-end,1] -> 
> (LeaderAndIsrInfo:(Leader:-2,ISR:1,LeaderEpoch:0,ControllerEpoch:165),ReplicationFactor:1),AllReplicas:1)),
>  1 -> Map([prod-sver-end,1] -> 
> (LeaderAndIsrInfo:(Leader:-2,ISR:1,LeaderEpoch:0,ControllerEpoch:165),ReplicationFactor:1),AllReplicas:1)),
>  3 -> Map([prod-sver-end,1] -> 
> (LeaderAndIsrInfo:(Leader:-2,ISR:1,LeaderEpoch:0,ControllerEpoch:165),ReplicationFactor:1),AllReplicas:1)))
>  might be lost 
>   at 
> kafka.controller.ControllerBrokerRequestBatch.newBatch(ControllerChannelManager.scala:202)
>   at 
> kafka.controller.KafkaController.sendUpdateMetadataRequest(KafkaController.scala:974)
>   at 
> kafka.controller.KafkaController.onBrokerStartup(KafkaController.scala:399)
>   at 
> kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1$$anonfun$apply$mcV$sp$1.apply$mcV$sp(ReplicaStateMachine.scala:371)
>   at 
> kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1$$anonfun$apply$mcV$sp$1.apply(ReplicaStateMachine.scala:359)
>   at 
> kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1$$anonfun$apply$mcV$sp$1.apply(ReplicaStateMachine.scala:359)
>   at kafka.metrics.KafkaTimer.time(KafkaTimer.scala:33)
>   at 
> kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1.apply$mcV$sp(ReplicaStateMachine.scala:358)
>   at 
> kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1.apply(ReplicaStateMachine.scala:357)
>   at 
> kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1.apply(ReplicaStateMachine.scala:357)
>   at kafka.utils.Utils$.inLock(Utils.scala:535)
>   at 
> kafka.controller.ReplicaStateMachine$BrokerChangeListener.handleChildChange(ReplicaStateMachine.scala:356)
>   at org.I0Itec.zkclient.ZkClient$7.run(ZkClient.java:568)
>   at org.I0Itec.zkclient.ZkEventThread.run(ZkEventThread.java:71)
> }}
> {{prod-sver-end}} is a topic we previously deleted. It seems some remnant of 
> it persists in the controller's memory, causing an exception which interrupts 
> the state change triggered by the broker startup.
> Has anyone seen something like this? Any idea what's happening here? Any 
> information would be greatly appreciated.
> Thanks,
> Johnny



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

Re: Review Request 35791: Patch for KAFKA-2298

[Dong Lin]

> On June 23, 2015, 5:59 p.m., Jason Gustafson wrote:
> > clients/src/main/java/org/apache/kafka/common/network/Selector.java, line 
> > 282
> > 
> >
> > Do you think we should just move the disconnected.add() into the close 
> > method?
> 
> Dong Lin wrote:
> I have thought about it as well. But probabaly no. Because in 
> Selector.send() we put failed destinationId is put in failedSends rather than 
> disconnected. The reason we use failedSends is because send() and poll() in 
> Selector will be called asynchronously by different threads.
> 
> Jason Gustafson wrote:
> Yeah, that makes sense. I wonder if we should be using a Set internally 
> instead of a List? Then we wouldn't need to worry about adding to 
> disconnected multiple times. Guess there might be some performance impact, 
> but we would have an easier time ensuring correctness.
> 
> Dong Lin wrote:
> Yeah that can be useful. I am not sure if currently some id may be added 
> to disconnected multiple times. Even it does, this should be not a problme in 
> NetworkClient.handleDisconnections(). I personally prefer to keep existing 
> code unless there is good reason (i.e. performance or functionality) for 
> change. Not sure what others think about this change.
> 
> Jason Gustafson wrote:
> Seems unnecessary now that I think about it. In fact, it wouldn't hurt to 
> do the disconnected.add() into close() as long as we also add it to 
> failedSends, but that might make the code tougher to follow. Anyway, I think 
> the patch is fine as it is. I was just wondering if there was an easy way to 
> prevent this issue from popping up again in the future.
> 
> Dong Lin wrote:
> Yeah I agree with you. I will add to disconnected in close() if I were to 
> write this class/function from scratch. But since this is a small patch to 
> existing module I will make the change that solves the problem with least 
> dispute to make peer review smooth.
> 
> Joel Koshy wrote:
> Hi Dong - I'm not sure about your comment on multiple threads calling 
> send/poll. NetworkClient and Selector are both documented as being 
> `@nonthreadsafe` so that is probably a non-issue. The producer's sender for 
> e.g., does send/poll in the same (sender) thread .
> 
> Dong Lin wrote:
> Hi Joel, Jason,
> 
> Yeah you are right. Thanks for your correction. I have moved 
> disconnected.add() into close() in the updated patch. Please have a look.
> 
> Jason Gustafson wrote:
> I think the only issue now is that the disconnected list can be updated 
> in a failed send, which technically violates the API. I doubt this would 
> cause any problems given current usage, but it's probably better to stay 
> consistent with the API. You could get around this perhaps by deferring the 
> close on a failed send until poll (which would introduce a nice invariant 
> that close is only called in poll). There may be other workarounds as well, 
> if you want to have a look. Otherwise, the initial patch might be the better 
> way to go.

Jason, could you explain why it technically violates the API? I think when send 
fails, the connection is indeed closed and therefore it is OK to update 
disconnected list.


- Dong


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


On June 24, 2015, 1:48 a.m., Dong Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35791/
> ---
> 
> (Updated June 24, 2015, 1:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2298
> https://issues.apache.org/jira/browse/KAFKA-2298
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2298; Client Selector can drop connections on InvalidReceiveException 
> without notifying NetworkClient
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> 4aee214b24fd990be003adc36d675f015bf22fe6 
> 
> Diff: https://reviews.apache.org/r/35791/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dong Lin
> 
>

[jira] [Updated] (KAFKA-313) Add JSON/CSV output and looping options to ConsumerGroupCommand

[Ashish K Singh (JIRA)]

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

Ashish K Singh updated KAFKA-313:
-
Attachment: KAFKA-313_2015-06-24_11:14:24.patch

> Add JSON/CSV output and looping options to ConsumerGroupCommand
> ---
>
> Key: KAFKA-313
> URL: https://issues.apache.org/jira/browse/KAFKA-313
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Dave DeMaagd
>Assignee: Ashish K Singh
>Priority: Minor
>  Labels: newbie, patch
> Fix For: 0.8.3
>
> Attachments: KAFKA-313-2012032200.diff, KAFKA-313.1.patch, 
> KAFKA-313.patch, KAFKA-313_2015-02-23_18:11:32.patch, 
> KAFKA-313_2015-06-24_11:14:24.patch
>
>
> Adds:
> * '--loop N' - causes the program to loop forever, sleeping for up to N 
> seconds between loops (loop time minus collection time, unless that's less 
> than 0, at which point it will just run again immediately)
> * '--asjson' - display as a JSON string instead of the more human readable 
> output format.
> Neither of the above  depend on each other (you can loop in the human 
> readable output, or do a single shot execution with JSON output).  Existing 
> behavior/output maintained if neither of the above are used.  Diff Attached.
> Impacted files:
> core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala



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

[jira] [Commented] (KAFKA-313) Add JSON/CSV output and looping options to ConsumerGroupCommand

[Ashish K Singh (JIRA)]

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

Ashish K Singh commented on KAFKA-313:
--

Updated reviewboard https://reviews.apache.org/r/28096/
 against branch trunk

> Add JSON/CSV output and looping options to ConsumerGroupCommand
> ---
>
> Key: KAFKA-313
> URL: https://issues.apache.org/jira/browse/KAFKA-313
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Dave DeMaagd
>Assignee: Ashish K Singh
>Priority: Minor
>  Labels: newbie, patch
> Fix For: 0.8.3
>
> Attachments: KAFKA-313-2012032200.diff, KAFKA-313.1.patch, 
> KAFKA-313.patch, KAFKA-313_2015-02-23_18:11:32.patch, 
> KAFKA-313_2015-06-24_11:14:24.patch
>
>
> Adds:
> * '--loop N' - causes the program to loop forever, sleeping for up to N 
> seconds between loops (loop time minus collection time, unless that's less 
> than 0, at which point it will just run again immediately)
> * '--asjson' - display as a JSON string instead of the more human readable 
> output format.
> Neither of the above  depend on each other (you can loop in the human 
> readable output, or do a single shot execution with JSON output).  Existing 
> behavior/output maintained if neither of the above are used.  Diff Attached.
> Impacted files:
> core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala



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

Re: Review Request 28096: Patch for KAFKA-313

[Ashish Singh]

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

(Updated June 24, 2015, 6:14 p.m.)


Review request for kafka, Gwen Shapira, Jarek Cecho, and Joel Koshy.


Bugs: KAFKA-313
https://issues.apache.org/jira/browse/KAFKA-313


Repository: kafka


Description
---

KAFKA-313: Add JSON/CSV output and looping options to ConsumerGroupCommand


Diffs (updated)
-

  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala 
f23120ede5f9bf0cfaf795c65c9845f42d8784d0 

Diff: https://reviews.apache.org/r/28096/diff/


Testing
---

Ran ConsumerOffsetChecker with different combinations of --output.format and 
--loop options.


Thanks,

Ashish Singh

[jira] [Created] (KAFKA-2301) Deprecate ConsumerOffsetChecker

[Ashish K Singh (JIRA)]

Ashish K Singh created KAFKA-2301:
-

 Summary: Deprecate ConsumerOffsetChecker
 Key: KAFKA-2301
 URL: https://issues.apache.org/jira/browse/KAFKA-2301
 Project: Kafka
  Issue Type: Sub-task
  Components: tools
Reporter: Ashish K Singh
Assignee: Ashish K Singh






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

Re: [GitHub] kafka pull request: Kafka 2276

[Gwen Shapira]

> There are loads of other improvements that could me
> made too -- for example, I'd love seeing Docker support, which would make
> running tests way cheaper/simpler.

Haha, that was my first thought when I saw Vagrant in there - why not Docker? :)
I didn't realize we already had Vagrant integration in Kafka.

Gwen


On Wed, Jun 24, 2015 at 10:53 AM, Ewen Cheslack-Postava
 wrote:
> Github either lets you do inline comments on the diff ("Files Changed" tab)
> or on the entire commit ("Conversation"). One of the things review board
> does right that's a bit frustrating in Github his that you can't group a
> bunch of comments into a single review and post it all at once (and get
> only one email!). Both tools have their drawbacks...
>
> On your particular comments:
>
> tests/kafkatest - Sort of redundant, but useful for being able to publish
> on PyPI, which will make services reusable by other projects. I actually
> hate this even about normal Python libraries too, where you need to have a
> top-level directory in the repository containing the project name. On the
> other hand, it actually keeps the organization fairly clean since it leaves
> space for docs, tests, and other directories, although those are less
> relevant here. (By the way, there are other issues with reusability that we
> may need to address eventually, such as the assumed layout of code in the
> workers; we probably need to get an initial version in and try reusing it
> in another project before we can really figure out everything that needs
> fixing for that.)
>
> multi-version - Great question, although I'd hope not to hold up an initial
> version only for this. Personally I think the right way to do this is to
> make each of the services support different versions (and for upgrade-style
> tests, they actually need to be able to change versions in some cases, i.e.
> rolling upgrades). To support this, we'll need to update the current build
> process (pre-test) to include grabbing the appropriate zips/jars, and the
> services will need to know about the layout of the trunk build vs.
> pre-built binaries. Then I think the tests just use the services but also
> specify versions (rather than relying on what I assume would be the
> default, which is the trunk build). If we want to test compatibility across
> many versions, then we might want to have a single test for compatibility
> between two versions, but make it parameterizable (which is another can of
> worms)
>
> kafkatest/tests - Organizationally I think a single file will usually make
> sense, but you could also have an entire module if you needed enough code
> for the test that it made sense to spread it across multiple files. So far
> I think our experience has been that tests usually are pretty small because
> more of the code tends to end up in services or utilities. We can see if
> @granders agrees with that or not, he has now probably written more of
> these than I have.
>
> kafka/vagrant - This already existed as part of
> https://issues.apache.org/jira/browse/KAFKA-1173 These are updates that
> make it work well if you want to run the driver machine (the one running
> ducktape) on EC2, having it allocate more EC2 nodes to use as workers for
> the tests. The IP address you saw is EC2-specific, but the Vagrantfile also
> isolates those scripts to be used when using the vagrant-aws plugin. Other
> stuff like being Debian-specific can be generalized by extending the
> Vagrantfile and scripts, which I think would be great, but somebody needs
> to invest the time. There are loads of other improvements that could me
> made too -- for example, I'd love seeing Docker support, which would make
> running tests way cheaper/simpler.
>
>
> On Tue, Jun 23, 2015 at 7:56 PM, Gwen Shapira  wrote:
>
>> I'm unclear on the directory structure and few high level things (and
>> I can't figure out how to comment on more than a single line in
>> github):
>>
>> tests/kafkatest - isn't it redundant? do we expect any non-kafka tests?
>>
>> services/templates - those are basically all configuration files to be
>> used by the services, right? Do we expect a single set for the entire
>> system? or different templates for different tests? I'm checking
>> because "systemtests" had config files per test.
>>
>> any thoughts on how multi-version tests will work? will we have
>> service per version?
>>
>> kafkatest/tests - do we expect every test to be a single script? or
>> should we have subdirectories here, to start things in the right
>> direction? Maybe subdirectories for planned groups of tests?
>>
>> kafka/vagrant - is this intentionally not under tests? Is this at all
>> stand-alone? If so, maybe a separate Jira? This has a bunch of stuff
>> that I'm not sure we want in the project at all... specific IPs,
>> Vagrant install from debian packages, etc.
>>
>> Gwen
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> On Tue, Jun 23, 2015 at 10:45 AM, Gwen Shapira 
>> wrote:
>> > Awesome, thanks :)
>> >
>> > On Tue, Jun 23, 2015

Re: [GitHub] kafka pull request: Kafka 2276

[Ewen Cheslack-Postava]

Github either lets you do inline comments on the diff ("Files Changed" tab)
or on the entire commit ("Conversation"). One of the things review board
does right that's a bit frustrating in Github his that you can't group a
bunch of comments into a single review and post it all at once (and get
only one email!). Both tools have their drawbacks...

On your particular comments:

tests/kafkatest - Sort of redundant, but useful for being able to publish
on PyPI, which will make services reusable by other projects. I actually
hate this even about normal Python libraries too, where you need to have a
top-level directory in the repository containing the project name. On the
other hand, it actually keeps the organization fairly clean since it leaves
space for docs, tests, and other directories, although those are less
relevant here. (By the way, there are other issues with reusability that we
may need to address eventually, such as the assumed layout of code in the
workers; we probably need to get an initial version in and try reusing it
in another project before we can really figure out everything that needs
fixing for that.)

multi-version - Great question, although I'd hope not to hold up an initial
version only for this. Personally I think the right way to do this is to
make each of the services support different versions (and for upgrade-style
tests, they actually need to be able to change versions in some cases, i.e.
rolling upgrades). To support this, we'll need to update the current build
process (pre-test) to include grabbing the appropriate zips/jars, and the
services will need to know about the layout of the trunk build vs.
pre-built binaries. Then I think the tests just use the services but also
specify versions (rather than relying on what I assume would be the
default, which is the trunk build). If we want to test compatibility across
many versions, then we might want to have a single test for compatibility
between two versions, but make it parameterizable (which is another can of
worms)

kafkatest/tests - Organizationally I think a single file will usually make
sense, but you could also have an entire module if you needed enough code
for the test that it made sense to spread it across multiple files. So far
I think our experience has been that tests usually are pretty small because
more of the code tends to end up in services or utilities. We can see if
@granders agrees with that or not, he has now probably written more of
these than I have.

kafka/vagrant - This already existed as part of
https://issues.apache.org/jira/browse/KAFKA-1173 These are updates that
make it work well if you want to run the driver machine (the one running
ducktape) on EC2, having it allocate more EC2 nodes to use as workers for
the tests. The IP address you saw is EC2-specific, but the Vagrantfile also
isolates those scripts to be used when using the vagrant-aws plugin. Other
stuff like being Debian-specific can be generalized by extending the
Vagrantfile and scripts, which I think would be great, but somebody needs
to invest the time. There are loads of other improvements that could me
made too -- for example, I'd love seeing Docker support, which would make
running tests way cheaper/simpler.


On Tue, Jun 23, 2015 at 7:56 PM, Gwen Shapira  wrote:

> I'm unclear on the directory structure and few high level things (and
> I can't figure out how to comment on more than a single line in
> github):
>
> tests/kafkatest - isn't it redundant? do we expect any non-kafka tests?
>
> services/templates - those are basically all configuration files to be
> used by the services, right? Do we expect a single set for the entire
> system? or different templates for different tests? I'm checking
> because "systemtests" had config files per test.
>
> any thoughts on how multi-version tests will work? will we have
> service per version?
>
> kafkatest/tests - do we expect every test to be a single script? or
> should we have subdirectories here, to start things in the right
> direction? Maybe subdirectories for planned groups of tests?
>
> kafka/vagrant - is this intentionally not under tests? Is this at all
> stand-alone? If so, maybe a separate Jira? This has a bunch of stuff
> that I'm not sure we want in the project at all... specific IPs,
> Vagrant install from debian packages, etc.
>
> Gwen
>
>
>
>
>
>
>
>
>
> On Tue, Jun 23, 2015 at 10:45 AM, Gwen Shapira 
> wrote:
> > Awesome, thanks :)
> >
> > On Tue, Jun 23, 2015 at 10:32 AM, Geoffrey Anderson 
> wrote:
> >> Hi Gwen,
> >>
> >> That is indeed the plan, and my understanding is that the merge script
> >> Ismael has been working on helps committers with this step.
> >>
> >> I'm trying out the Github flow roughly as outlined here:
> >>
> http://mail-archives.apache.org/mod_mbox/kafka-dev/201504.mbox/%3ccad5tkzab-hkey-zcr8x4wtxawybxpojx62k1vbv+ycknuxq...@mail.gmail.com%3E
> >>
> >> Ismael's script is here:
> https://issues.apache.org/jira/browse/KAFKA-2187
> >>
> >>
> >> Thanks,
> >>
>

Re: Review Request 35791: Patch for KAFKA-2298

[Jason Gustafson]

> On June 23, 2015, 5:59 p.m., Jason Gustafson wrote:
> > clients/src/main/java/org/apache/kafka/common/network/Selector.java, line 
> > 282
> > 
> >
> > Do you think we should just move the disconnected.add() into the close 
> > method?
> 
> Dong Lin wrote:
> I have thought about it as well. But probabaly no. Because in 
> Selector.send() we put failed destinationId is put in failedSends rather than 
> disconnected. The reason we use failedSends is because send() and poll() in 
> Selector will be called asynchronously by different threads.
> 
> Jason Gustafson wrote:
> Yeah, that makes sense. I wonder if we should be using a Set internally 
> instead of a List? Then we wouldn't need to worry about adding to 
> disconnected multiple times. Guess there might be some performance impact, 
> but we would have an easier time ensuring correctness.
> 
> Dong Lin wrote:
> Yeah that can be useful. I am not sure if currently some id may be added 
> to disconnected multiple times. Even it does, this should be not a problme in 
> NetworkClient.handleDisconnections(). I personally prefer to keep existing 
> code unless there is good reason (i.e. performance or functionality) for 
> change. Not sure what others think about this change.
> 
> Jason Gustafson wrote:
> Seems unnecessary now that I think about it. In fact, it wouldn't hurt to 
> do the disconnected.add() into close() as long as we also add it to 
> failedSends, but that might make the code tougher to follow. Anyway, I think 
> the patch is fine as it is. I was just wondering if there was an easy way to 
> prevent this issue from popping up again in the future.
> 
> Dong Lin wrote:
> Yeah I agree with you. I will add to disconnected in close() if I were to 
> write this class/function from scratch. But since this is a small patch to 
> existing module I will make the change that solves the problem with least 
> dispute to make peer review smooth.
> 
> Joel Koshy wrote:
> Hi Dong - I'm not sure about your comment on multiple threads calling 
> send/poll. NetworkClient and Selector are both documented as being 
> `@nonthreadsafe` so that is probably a non-issue. The producer's sender for 
> e.g., does send/poll in the same (sender) thread .
> 
> Dong Lin wrote:
> Hi Joel, Jason,
> 
> Yeah you are right. Thanks for your correction. I have moved 
> disconnected.add() into close() in the updated patch. Please have a look.

I think the only issue now is that the disconnected list can be updated in a 
failed send, which technically violates the API. I doubt this would cause any 
problems given current usage, but it's probably better to stay consistent with 
the API. You could get around this perhaps by deferring the close on a failed 
send until poll (which would introduce a nice invariant that close is only 
called in poll). There may be other workarounds as well, if you want to have a 
look. Otherwise, the initial patch might be the better way to go.


- Jason


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


On June 24, 2015, 1:48 a.m., Dong Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35791/
> ---
> 
> (Updated June 24, 2015, 1:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2298
> https://issues.apache.org/jira/browse/KAFKA-2298
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2298; Client Selector can drop connections on InvalidReceiveException 
> without notifying NetworkClient
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> 4aee214b24fd990be003adc36d675f015bf22fe6 
> 
> Diff: https://reviews.apache.org/r/35791/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dong Lin
> 
>

[jira] [Commented] (KAFKA-2203) Get gradle build to work with Java 8

[Gwen Shapira (JIRA)]

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

Gwen Shapira commented on KAFKA-2203:
-

Having pre-commit comments on a JIRA will be significantly better than what we 
have at the moment.

I agree that there are additional concerns:
* Speed / parallelism (I think Hive and Sqoop solved it by running tests 
themselves in parallel)
* We need a good way to test with multiple jdk versions - possibly with 
multiple Jenkins jobs
* Integration of Travis with JIRA (IMO thats the biggest concern at the moment)

However, none of those seem unsolvable, and even a partial solution will be 
very useful. I really miss the pre-commit comments I get with other projects.  
It just seems to me that we can get cool improvements with very little effort. 
KAFKA-1856 is waiting for someone to create a jenkins job and set up the 
precommit trigger.

> Get gradle build to work with Java 8
> 
>
> Key: KAFKA-2203
> URL: https://issues.apache.org/jira/browse/KAFKA-2203
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Affects Versions: 0.8.1.1
>Reporter: Gaju Bhat
>Priority: Minor
> Fix For: 0.8.1.2
>
> Attachments: 0001-Special-case-java-8-and-javadoc-handling.patch
>
>
> The gradle build halts because javadoc in java 8 is a lot stricter about 
> valid html.
> It might be worthwhile to special case java 8 as described 
> [here|http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html].



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

Re: [DISCUSS] KIP-26 - Add Copycat, a connector framework for data import/export

[Jay Kreps]

Hey Sriram,

Good question here was the thinking:

1. I think the argument we are making is that a stream processing framework
is the right way to do complex transformations. We can bake in some
mechanism for simple, single-row transforms in copycat, but for anything
more complex copycat is really the equivalent of the "spout" for the stream
processors "bolt" (to steal storm's slightly odd terminology). I think we
would encourage use of simple transforms like project, filter, or coerce
done in a declarative way to avoid the whole code lifecycle problem (i.e.
if you write a custom transformer and need to continuously iterate on it
how does it get deployed, packaged, etc). So our target use cases for
transform in copycat would be things like removing passwords, or coercing
types to a target database schema, etc.

2. For what it is worth we did a pretty deep dive with Chris and some of
the Samza folks on just using Samza as a stream processor + ingestor and
using it's pluggable system apis as the mechanism for writing connectors.
That was our original strawman plan. Our take was that this was not ideal
for a couple reasons:
a. Samza system apis are pretty complex
b. In some sense what you want for ingestion is a simple, dumb tool like
scp or rsync. The stream processing systems are necessarily more complex
and most of what they do isn't helpful for ingestion.
c. Philosophically we favor a ELT style of data population where you
publish data as-is and then transform it to a derived/improved format as a
second topic. The rationale here is to separate use-case specific
munging/mangling from what actually occurred. Using a stream processor
tends to encourage transformation as part of ingestion.
d. We thought that in practice there is a big difference between targeting
1000 streams and dozens of sources and targeting a couple streams and a
single source. Stream processors are really in the later category--a given
job is only going to use a few inputs. The ingestion stuff needs to be done
in a more factory style where you manage ingestion against lots of dbs,
etc. These two uses tend to pull in slightly different directions.
e. We thought that ultimately you would get a bigger connector ecosystem if
it wasn't tied to a particular stream processing system--this way you can
do ingestion with a "non-denominational" tool and use whatever stream
processor you like (or none).

-Jay

On Tue, Jun 23, 2015 at 6:18 PM, Sriram Subramanian <
srsubraman...@linkedin.com.invalid> wrote:

> I am still not convinced why a stream processing framework closely tied to
> Kafka will not help with this (since we are also referring to some basic
> transformations). The devil is in the details of the design and I would be
> able to better comment on it after that. I would love to see a detailed
> design doc on the internals!
>
> On 6/23/15 2:59 PM, "Ewen Cheslack-Postava"  wrote:
>
> >There was some discussion on the KIP call today. I'll give my summary of
> >what I heard here to make sure this thread has the complete context for
> >ongoing discussion.
> >
> >* Where the project should live, and if in Kafka, where should connectors
> >live? If some are in Kafka and some not, how many and which ones? - There
> >was little disagreement on the tradeoffs (coding and packaging together
> >can
> >make things easier for end users especially for a few very popular
> >connectors, maintaining internally can lead to messier code base with more
> >dependencies that's harder to work with, etc). Seems to be more focus on
> >location of connectors than framework right now; we'll probably only make
> >progress on this issue with some concrete proposals.
> >* Organizational issues within Kafka - subproject? - Jay mentioned desire
> >for consistency, which can be a problem even across subprojects.
> >* Will streaming data be supported? - Yes, "Streaming and batch" section
> >of
> >design goals should cover this; this is a very important use case.
> >* Additional transformations in copycat - Updated wiki to leave this a bit
> >more open. Original motivation for leaving it out was to keep the scope of
> >this KIP and the Copycat framework very clear since there is a danger in
> >overgeneralizing and ending up with a stream processing framework;
> >however,
> >it's clear there are some very useful, very common examples like scrubbing
> >data during import.
> >* Schemas and how the data model works - this requires a more in depth
> >answer when we get to a complete proposal, but the prototype we've been
> >playing with internally uses something that can work with data roughly
> >like
> >Avro or JSON, and supports schemas. The goal is for this data model to
> >only
> >be used at runtime and for the serialization that is used for storing data
> >in Kafka to be pluggable. Each type of serialization plugin might handle
> >things like schemas in different ways. The reason we are proposing the
> >inclusion of schemas is that it lets you cleanly carry important info
> 

[jira] [Commented] (KAFKA-2203) Get gradle build to work with Java 8

[Jeff Maxwell (JIRA)]

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

Jeff Maxwell commented on KAFKA-2203:
-

FWIW Spring's solution appears to be dedicated CI builds: 
https://build.spring.io/browse/BOOT-JOJ8.


> Get gradle build to work with Java 8
> 
>
> Key: KAFKA-2203
> URL: https://issues.apache.org/jira/browse/KAFKA-2203
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Affects Versions: 0.8.1.1
>Reporter: Gaju Bhat
>Priority: Minor
> Fix For: 0.8.1.2
>
> Attachments: 0001-Special-case-java-8-and-javadoc-handling.patch
>
>
> The gradle build halts because javadoc in java 8 is a lot stricter about 
> valid html.
> It might be worthwhile to special case java 8 as described 
> [here|http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html].



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

[jira] [Updated] (KAFKA-2132) Move Log4J appender to a separate module

[Ashish K Singh (JIRA)]

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

Ashish K Singh updated KAFKA-2132:
--
Attachment: KAFKA-2132_2015-06-24_10:25:43.patch

> Move Log4J appender to a separate module
> 
>
> Key: KAFKA-2132
> URL: https://issues.apache.org/jira/browse/KAFKA-2132
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Gwen Shapira
>Assignee: Ashish K Singh
> Attachments: KAFKA-2132.patch, KAFKA-2132_2015-04-27_19:59:46.patch, 
> KAFKA-2132_2015-04-30_12:22:02.patch, KAFKA-2132_2015-04-30_15:53:17.patch, 
> KAFKA-2132_2015-06-13_21:18:59.patch, KAFKA-2132_2015-06-24_10:19:56.patch, 
> KAFKA-2132_2015-06-24_10:25:43.patch
>
>
> Log4j appender is just a producer.
> Since we have a new producer in the clients module, no need to keep Log4J 
> appender in "core" and force people to package all of Kafka with their apps.
> Lets move the Log4jAppender to clients module.



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

[jira] [Commented] (KAFKA-2132) Move Log4J appender to a separate module

[Ashish K Singh (JIRA)]

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

Ashish K Singh commented on KAFKA-2132:
---

Updated reviewboard https://reviews.apache.org/r/33614/
 against branch trunk

> Move Log4J appender to a separate module
> 
>
> Key: KAFKA-2132
> URL: https://issues.apache.org/jira/browse/KAFKA-2132
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Gwen Shapira
>Assignee: Ashish K Singh
> Attachments: KAFKA-2132.patch, KAFKA-2132_2015-04-27_19:59:46.patch, 
> KAFKA-2132_2015-04-30_12:22:02.patch, KAFKA-2132_2015-04-30_15:53:17.patch, 
> KAFKA-2132_2015-06-13_21:18:59.patch, KAFKA-2132_2015-06-24_10:19:56.patch, 
> KAFKA-2132_2015-06-24_10:25:43.patch
>
>
> Log4j appender is just a producer.
> Since we have a new producer in the clients module, no need to keep Log4J 
> appender in "core" and force people to package all of Kafka with their apps.
> Lets move the Log4jAppender to clients module.



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

Re: Review Request 33614: Patch for KAFKA-2132

[Ashish Singh]

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

(Updated June 24, 2015, 5:25 p.m.)


Review request for kafka.


Bugs: KAFKA-2132
https://issues.apache.org/jira/browse/KAFKA-2132


Repository: kafka


Description
---

KAFKA-2132: Move Log4J appender to clients module


Diffs (updated)
-

  build.gradle 30d1cf2f1ff9ed3f86a060da8099bb0774b4cf91 
  checkstyle/import-control.xml f2e6cec267e67ce8e261341e373718e14a8e8e03 
  core/src/main/scala/kafka/producer/KafkaLog4jAppender.scala 
5d36a019e3dbfb93737a9cd23404dcd1c5d836d1 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 
41366a14590d318fced0e83d6921d8035fa882da 
  
log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java
 PRE-CREATION 
  
log4j-appender/src/test/java/org/apache/kafka/log4jappender/KafkaLog4jAppenderTest.java
 PRE-CREATION 
  
log4j-appender/src/test/java/org/apache/kafka/log4jappender/MockKafkaLog4jAppender.java
 PRE-CREATION 
  settings.gradle 83f764e6a4a15a5fdba232dce74a369870f26b45 

Diff: https://reviews.apache.org/r/33614/diff/


Testing
---


Thanks,

Ashish Singh

Re: Review Request 33614: Patch for KAFKA-2132

[Ashish Singh]

> On June 16, 2015, 1:31 a.m., Jun Rao wrote:
> > Thanks for the patch. There seems to be a compilation error.
> > 
> > :log4j-appender:compileTestJava
> > /Users/junrao/intellij/kafka/log4j-appender/src/test/java/org/apache/kafka/log4jappender/MockKafkaLog4jAppender.java:27:
> >  cannot find symbol
> > symbol  : constructor MockProducer()
> > location: class org.apache.kafka.clients.producer.MockProducer
> > private MockProducer mockProducer = new MockProducer();
> > ^

Jun, thanks for the review. I had to rebase. It should be fine now.


> On June 16, 2015, 1:31 a.m., Jun Rao wrote:
> > checkstyle/import-control.xml, line 98
> > 
> >
> > Shouldn't the subpackage be log4jappender?

Yes, I am surprised that even log4j worked :)


- Ashish


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


On June 24, 2015, 5:20 p.m., Ashish Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33614/
> ---
> 
> (Updated June 24, 2015, 5:20 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2132
> https://issues.apache.org/jira/browse/KAFKA-2132
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2132: Move Log4J appender to clients module
> 
> 
> Diffs
> -
> 
>   build.gradle 30d1cf2f1ff9ed3f86a060da8099bb0774b4cf91 
>   checkstyle/import-control.xml f2e6cec267e67ce8e261341e373718e14a8e8e03 
>   core/src/main/scala/kafka/producer/KafkaLog4jAppender.scala 
> 5d36a019e3dbfb93737a9cd23404dcd1c5d836d1 
>   core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 
> 41366a14590d318fced0e83d6921d8035fa882da 
>   
> log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java
>  PRE-CREATION 
>   
> log4j-appender/src/test/java/org/apache/kafka/log4jappender/KafkaLog4jAppenderTest.java
>  PRE-CREATION 
>   
> log4j-appender/src/test/java/org/apache/kafka/log4jappender/MockKafkaLog4jAppender.java
>  PRE-CREATION 
>   settings.gradle 83f764e6a4a15a5fdba232dce74a369870f26b45 
> 
> Diff: https://reviews.apache.org/r/33614/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

[jira] [Commented] (KAFKA-2203) Get gradle build to work with Java 8

[Ewen Cheslack-Postava (JIRA)]

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

Ewen Cheslack-Postava commented on KAFKA-2203:
--

[~gwenshap] Agreed that precommit is the ideal solution. Not sure that the 
setup in KAFKA-1856 works for this though -- can we get all the configurations 
we'd want, i.e. does a single jenkins job have access to all the jdk versions 
we'd need here? 

Also, if the entire test matrix was run, at current rates it'd take something 
like 20min * 2 JDK versions (min and max versions, at a minimum) * 4 scala 
versions = 2.66 hrs minimum for feedback since I don't think we can parallelize 
those unless we spread across multiple jenkins jobs? Or maybe it just requires 
a more complex Jenkins job to parameterize? Long turn around time is bad for 
submitters too...

For the precommit stuff, if we're switching to Github PRs, we could also 
possibly use Travis, which ASF supports in some fashion since they pay for 
capacity instead of relying on the "fair use" servers. Not sure how that 
integrates with JIRA though.


> Get gradle build to work with Java 8
> 
>
> Key: KAFKA-2203
> URL: https://issues.apache.org/jira/browse/KAFKA-2203
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Affects Versions: 0.8.1.1
>Reporter: Gaju Bhat
>Priority: Minor
> Fix For: 0.8.1.2
>
> Attachments: 0001-Special-case-java-8-and-javadoc-handling.patch
>
>
> The gradle build halts because javadoc in java 8 is a lot stricter about 
> valid html.
> It might be worthwhile to special case java 8 as described 
> [here|http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html].



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

[jira] [Updated] (KAFKA-2132) Move Log4J appender to a separate module

[Ashish K Singh (JIRA)]

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

Ashish K Singh updated KAFKA-2132:
--
Attachment: KAFKA-2132_2015-06-24_10:19:56.patch

> Move Log4J appender to a separate module
> 
>
> Key: KAFKA-2132
> URL: https://issues.apache.org/jira/browse/KAFKA-2132
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Gwen Shapira
>Assignee: Ashish K Singh
> Attachments: KAFKA-2132.patch, KAFKA-2132_2015-04-27_19:59:46.patch, 
> KAFKA-2132_2015-04-30_12:22:02.patch, KAFKA-2132_2015-04-30_15:53:17.patch, 
> KAFKA-2132_2015-06-13_21:18:59.patch, KAFKA-2132_2015-06-24_10:19:56.patch
>
>
> Log4j appender is just a producer.
> Since we have a new producer in the clients module, no need to keep Log4J 
> appender in "core" and force people to package all of Kafka with their apps.
> Lets move the Log4jAppender to clients module.



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

[jira] [Commented] (KAFKA-2132) Move Log4J appender to a separate module

[Ashish K Singh (JIRA)]

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

Ashish K Singh commented on KAFKA-2132:
---

Updated reviewboard https://reviews.apache.org/r/33614/
 against branch trunk

> Move Log4J appender to a separate module
> 
>
> Key: KAFKA-2132
> URL: https://issues.apache.org/jira/browse/KAFKA-2132
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Gwen Shapira
>Assignee: Ashish K Singh
> Attachments: KAFKA-2132.patch, KAFKA-2132_2015-04-27_19:59:46.patch, 
> KAFKA-2132_2015-04-30_12:22:02.patch, KAFKA-2132_2015-04-30_15:53:17.patch, 
> KAFKA-2132_2015-06-13_21:18:59.patch, KAFKA-2132_2015-06-24_10:19:56.patch
>
>
> Log4j appender is just a producer.
> Since we have a new producer in the clients module, no need to keep Log4J 
> appender in "core" and force people to package all of Kafka with their apps.
> Lets move the Log4jAppender to clients module.



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

Re: Review Request 33614: Patch for KAFKA-2132

[Ashish Singh]

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

(Updated June 24, 2015, 5:20 p.m.)


Review request for kafka.


Bugs: KAFKA-2132
https://issues.apache.org/jira/browse/KAFKA-2132


Repository: kafka


Description
---

KAFKA-2132: Move Log4J appender to clients module


Diffs (updated)
-

  build.gradle 30d1cf2f1ff9ed3f86a060da8099bb0774b4cf91 
  checkstyle/import-control.xml f2e6cec267e67ce8e261341e373718e14a8e8e03 
  core/src/main/scala/kafka/producer/KafkaLog4jAppender.scala 
5d36a019e3dbfb93737a9cd23404dcd1c5d836d1 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 
41366a14590d318fced0e83d6921d8035fa882da 
  
log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java
 PRE-CREATION 
  
log4j-appender/src/test/java/org/apache/kafka/log4jappender/KafkaLog4jAppenderTest.java
 PRE-CREATION 
  
log4j-appender/src/test/java/org/apache/kafka/log4jappender/MockKafkaLog4jAppender.java
 PRE-CREATION 
  settings.gradle 83f764e6a4a15a5fdba232dce74a369870f26b45 

Diff: https://reviews.apache.org/r/33614/diff/


Testing
---


Thanks,

Ashish Singh

[jira] [Commented] (KAFKA-2199) Make signing artifacts optional, setting maven repository possible from command line

[Ewen Cheslack-Postava (JIRA)]

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

Ewen Cheslack-Postava commented on KAFKA-2199:
--

[~omkreddy] Sorry about that, ./gradlew build isn't one that I run normally. 
Submitted a new patch that just removes the licenseTest. We shouldn't really 
commit this until KAFKA-2248 is committed, but 2248 is more general since it 
doesn't just apply to source files, so I think it's the preferable solution.

> Make signing artifacts optional, setting maven repository possible from 
> command line
> 
>
> Key: KAFKA-2199
> URL: https://issues.apache.org/jira/browse/KAFKA-2199
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Reporter: Ewen Cheslack-Postava
>Assignee: Ewen Cheslack-Postava
>Priority: Minor
> Attachments: KAFKA-2199.patch, KAFKA-2199.patch, 
> KAFKA-2199_2015-05-29_11:00:44.patch
>
>
> Currently it's annoying to work with snapshot builds if you want to install 
> them rather than just build & test. There are a couple of problems. First, if 
> you try to install (any of the upload* tasks), then you are required to sign 
> the artifacts with a GPG key. Second, the way the variables are defined in 
> gradle.properties seems to make it impossible to override them from the 
> command line. You're required to edit your ~/.gradle/gradle.properties file 
> (which is shared by all gradle projects).



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

[jira] [Commented] (KAFKA-2199) Make signing artifacts optional, setting maven repository possible from command line

[Ewen Cheslack-Postava (JIRA)]

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

Ewen Cheslack-Postava commented on KAFKA-2199:
--

Created reviewboard https://reviews.apache.org/r/35832/diff/
 against branch origin/trunk

> Make signing artifacts optional, setting maven repository possible from 
> command line
> 
>
> Key: KAFKA-2199
> URL: https://issues.apache.org/jira/browse/KAFKA-2199
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Reporter: Ewen Cheslack-Postava
>Assignee: Ewen Cheslack-Postava
>Priority: Minor
> Attachments: KAFKA-2199.patch, KAFKA-2199.patch, 
> KAFKA-2199_2015-05-29_11:00:44.patch
>
>
> Currently it's annoying to work with snapshot builds if you want to install 
> them rather than just build & test. There are a couple of problems. First, if 
> you try to install (any of the upload* tasks), then you are required to sign 
> the artifacts with a GPG key. Second, the way the variables are defined in 
> gradle.properties seems to make it impossible to override them from the 
> command line. You're required to edit your ~/.gradle/gradle.properties file 
> (which is shared by all gradle projects).



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

[jira] [Updated] (KAFKA-2199) Make signing artifacts optional, setting maven repository possible from command line

[Ewen Cheslack-Postava (JIRA)]

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

Ewen Cheslack-Postava updated KAFKA-2199:
-
Attachment: KAFKA-2199.patch

> Make signing artifacts optional, setting maven repository possible from 
> command line
> 
>
> Key: KAFKA-2199
> URL: https://issues.apache.org/jira/browse/KAFKA-2199
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Reporter: Ewen Cheslack-Postava
>Assignee: Ewen Cheslack-Postava
>Priority: Minor
> Attachments: KAFKA-2199.patch, KAFKA-2199.patch, 
> KAFKA-2199_2015-05-29_11:00:44.patch
>
>
> Currently it's annoying to work with snapshot builds if you want to install 
> them rather than just build & test. There are a couple of problems. First, if 
> you try to install (any of the upload* tasks), then you are required to sign 
> the artifacts with a GPG key. Second, the way the variables are defined in 
> gradle.properties seems to make it impossible to override them from the 
> command line. You're required to edit your ~/.gradle/gradle.properties file 
> (which is shared by all gradle projects).



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

Review Request 35832: Patch for KAFKA-2199

[Ewen Cheslack-Postava]

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

Review request for kafka.


Bugs: KAFKA-2199
https://issues.apache.org/jira/browse/KAFKA-2199


Repository: kafka


Description
---

KAFKA-2199: Follow-up: disable licenseTest override that was using removed 
function in favor of relying on KAFKA-2248's Rat support which is more general


Diffs
-

  build.gradle 30d1cf2f1ff9ed3f86a060da8099bb0774b4cf91 

Diff: https://reviews.apache.org/r/35832/diff/


Testing
---


Thanks,

Ewen Cheslack-Postava

[jira] [Created] (KAFKA-2300) Error in controller log when broker tries to rejoin cluster

[Johnny Brown (JIRA)]

Johnny Brown created KAFKA-2300:
---

 Summary: Error in controller log when broker tries to rejoin 
cluster
 Key: KAFKA-2300
 URL: https://issues.apache.org/jira/browse/KAFKA-2300
 Project: Kafka
  Issue Type: Bug
Affects Versions: 0.8.2.1
Reporter: Johnny Brown


Hello Kafka folks,

We are having an issue where a broker attempts to join the cluster after being 
restarted, but is never added to the ISR for its assigned partitions. This is a 
three-node cluster, and the controller is broker 2.

When broker 1 starts, we see the following message in broker 2's controller.log.

{{
[2015-06-23 13:57:16,535] ERROR [BrokerChangeListener on Controller 2]: Error 
while handling broker changes 
(kafka.controller.ReplicaStateMachine$BrokerChangeListener)
java.lang.IllegalStateException: Controller to broker state change requests 
batch is not empty while creating a new one. Some UpdateMetadata state changes 
Map(2 -> Map([prod-sver-end,1] -> 
(LeaderAndIsrInfo:(Leader:-2,ISR:1,LeaderEpoch:0,ControllerEpoch:165),ReplicationFactor:1),AllReplicas:1)),
 1 -> Map([prod-sver-end,1] -> 
(LeaderAndIsrInfo:(Leader:-2,ISR:1,LeaderEpoch:0,ControllerEpoch:165),ReplicationFactor:1),AllReplicas:1)),
 3 -> Map([prod-sver-end,1] -> 
(LeaderAndIsrInfo:(Leader:-2,ISR:1,LeaderEpoch:0,ControllerEpoch:165),ReplicationFactor:1),AllReplicas:1)))
 might be lost 
  at 
kafka.controller.ControllerBrokerRequestBatch.newBatch(ControllerChannelManager.scala:202)
  at 
kafka.controller.KafkaController.sendUpdateMetadataRequest(KafkaController.scala:974)
  at kafka.controller.KafkaController.onBrokerStartup(KafkaController.scala:399)
  at 
kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1$$anonfun$apply$mcV$sp$1.apply$mcV$sp(ReplicaStateMachine.scala:371)
  at 
kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1$$anonfun$apply$mcV$sp$1.apply(ReplicaStateMachine.scala:359)
  at 
kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1$$anonfun$apply$mcV$sp$1.apply(ReplicaStateMachine.scala:359)
  at kafka.metrics.KafkaTimer.time(KafkaTimer.scala:33)
  at 
kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1.apply$mcV$sp(ReplicaStateMachine.scala:358)
  at 
kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1.apply(ReplicaStateMachine.scala:357)
  at 
kafka.controller.ReplicaStateMachine$BrokerChangeListener$$anonfun$handleChildChange$1.apply(ReplicaStateMachine.scala:357)
  at kafka.utils.Utils$.inLock(Utils.scala:535)
  at 
kafka.controller.ReplicaStateMachine$BrokerChangeListener.handleChildChange(ReplicaStateMachine.scala:356)
  at org.I0Itec.zkclient.ZkClient$7.run(ZkClient.java:568)
  at org.I0Itec.zkclient.ZkEventThread.run(ZkEventThread.java:71)
}}

{{prod-sver-end}} is a topic we previously deleted. It seems some remnant of it 
persists in the controller's memory, causing an exception which interrupts the 
state change triggered by the broker startup.

Has anyone seen something like this? Any idea what's happening here? Any 
information would be greatly appreciated.

Thanks,
Johnny



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

Re: Review Request 33620: Patch for KAFKA-1690

[Jiangjie Qin]

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


Thanks for the patch, Sriharsha. A few comments below.


clients/src/main/java/org/apache/kafka/common/network/SSLChannelBuilder.java 
(lines 52 - 53)


I read some java doc but still not quite understand how are those two 
parameteres going to be used. Can you elaborate the usage of that a little bit?



clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java (line 40)


Nit pick, keyStore and trustStore?



clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java (line 111)


We already have a default algorithm set in config, do we still need to set 
default here?



clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java (lines 
135 - 138)


Should we mention in the doc that if NeedClientAuth is set, it will be 
used, otherwise WantClientAuth is used. 
So if user knows what they get when set both to be true.



clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java (line 138)


setWantClientAuth?



clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java (lines 
140 - 143)


Any reason why we only set the endpoint identification algorithm on client 
side? Also, it looks the SSLParameters actually contains:
enabledCipherSuites, enabledProtocols, need clientAuth, wantClientAuth, can 
we just use one SSLParameter instead of calling different methods?



clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java (line 156)


Can we put these checks in checkKeyStoreConfigs()?



clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java (line 167)


ditto above.



clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java 
(lines 75 - 78)


Can they be replaced by clear() method?



clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java 
(line 101)


It seems we just removed the OP_CONNECT operation.



clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java 
(lines 127 - 139)


Not sure if it matters, but JSSE reference guide suggested to have a loop 
checking handshake state when shutdown.


- Jiangjie Qin


On June 23, 2015, 8:18 p.m., Sriharsha Chintalapani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> ---
> 
> (Updated June 23, 2015, 8:18 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.
> 
> 
> Diffs
> -
> 
>   build.gradle 30d1cf2f1ff9ed3f86a060da8099bb0774b4cf91 
>   checkstyl

[jira] [Commented] (KAFKA-2199) Make signing artifacts optional, setting maven repository possible from command line

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-2199:


 ./gradlew build is failing with below error.  isVerificationRequired() gradle 
task is removed in this patch.

{code}
FAILURE: Build failed with an exception.

* Where:
Build file '/safe/KAFKA/kafkaTrunk/build.gradle' line: 55

* What went wrong:
Could not evaluate onlyIf predicate for task ':clients:licenseTest'.
> Could not find method isVerificationRequired() for arguments [project 
> ':clients'] on root project 'kafkaTrunk'.
{code}

> Make signing artifacts optional, setting maven repository possible from 
> command line
> 
>
> Key: KAFKA-2199
> URL: https://issues.apache.org/jira/browse/KAFKA-2199
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Reporter: Ewen Cheslack-Postava
>Assignee: Ewen Cheslack-Postava
>Priority: Minor
> Attachments: KAFKA-2199.patch, KAFKA-2199_2015-05-29_11:00:44.patch
>
>
> Currently it's annoying to work with snapshot builds if you want to install 
> them rather than just build & test. There are a couple of problems. First, if 
> you try to install (any of the upload* tasks), then you are required to sign 
> the artifacts with a GPG key. Second, the way the variables are defined in 
> gradle.properties seems to make it impossible to override them from the 
> command line. You're required to edit your ~/.gradle/gradle.properties file 
> (which is shared by all gradle projects).



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

[jira] [Commented] (KAFKA-2203) Get gradle build to work with Java 8

[Gwen Shapira (JIRA)]

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

Gwen Shapira commented on KAFKA-2203:
-

This should be done pre-commit and nearly every Apache project does it.

Just finish this:
https://issues.apache.org/jira/browse/KAFKA-1856

Every time we attach a patch (or we can modify this to work with pull requests) 
- Apache Jenkins will run all the tests / builds we need and post results on 
the JIRA. Our review cycle is long enough that Apache infra latency shouldn't 
be a major concern, and committers will be able to know when a patch passes.

> Get gradle build to work with Java 8
> 
>
> Key: KAFKA-2203
> URL: https://issues.apache.org/jira/browse/KAFKA-2203
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Affects Versions: 0.8.1.1
>Reporter: Gaju Bhat
>Priority: Minor
> Fix For: 0.8.1.2
>
> Attachments: 0001-Special-case-java-8-and-javadoc-handling.patch
>
>
> The gradle build halts because javadoc in java 8 is a lot stricter about 
> valid html.
> It might be worthwhile to special case java 8 as described 
> [here|http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html].



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

[jira] [Commented] (KAFKA-2203) Get gradle build to work with Java 8

[Ewen Cheslack-Postava (JIRA)]

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

Ewen Cheslack-Postava commented on KAFKA-2203:
--

The drawback is that I don't think we have good enough automated testing right 
now to catch issues across all the JDK versions and we still support JDK6+. 
We're likely to keep breaking things -- people who compile with < JDK8 will 
miss doc errors and people with > JDK6 can easily introduce non-JDK6 compatible 
code. The latter is somewhat alleviated by the fact that I think many 
committers check the build with JDK6.

Probably a better solution than this patch would be to get the testing 
infrastructure to properly test the entire matrix (JDK versions, scala 
versions, etc.) so we would catch issues like this quickly, but that's probably 
a bigger project and I think is currently hampered by the fact that the Apache 
infrastructure has high enough latency that the feedback is too disconnected 
from commits to be useful anyway.

> Get gradle build to work with Java 8
> 
>
> Key: KAFKA-2203
> URL: https://issues.apache.org/jira/browse/KAFKA-2203
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Affects Versions: 0.8.1.1
>Reporter: Gaju Bhat
>Priority: Minor
> Fix For: 0.8.1.2
>
> Attachments: 0001-Special-case-java-8-and-javadoc-handling.patch
>
>
> The gradle build halts because javadoc in java 8 is a lot stricter about 
> valid html.
> It might be worthwhile to special case java 8 as described 
> [here|http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html].



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

[jira] [Updated] (KAFKA-972) MetadataRequest returns stale list of brokers

[Grant Henke (JIRA)]

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

Grant Henke updated KAFKA-972:
--
Attachment: BrokerMetadataTest.scala

Sample Failing Tests:
- testBrokerMetadataOnClusterWithNoTopics
- testBrokerMetadataOnBrokerShutdown
- testBrokerMetadataOnBrokerAddition

> MetadataRequest returns stale list of brokers
> -
>
> Key: KAFKA-972
> URL: https://issues.apache.org/jira/browse/KAFKA-972
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.8.0
>Reporter: Vinicius Carvalho
> Attachments: BrokerMetadataTest.scala
>
>
> When we issue an metadatarequest towards the cluster, the list of brokers is 
> stale. I mean, even when a broker is down, it's returned back to the client. 
> The following are examples of two invocations one with both brokers online 
> and the second with a broker down:
> {
> "brokers": [
> {
> "nodeId": 0,
> "host": "10.139.245.106",
> "port": 9092,
> "byteLength": 24
> },
> {
> "nodeId": 1,
> "host": "localhost",
> "port": 9093,
> "byteLength": 19
> }
> ],
> "topicMetadata": [
> {
> "topicErrorCode": 0,
> "topicName": "foozbar",
> "partitions": [
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 0,
> "leader": 0,
> "byteLength": 26
> },
> {
> "replicas": [
> 1
> ],
> "isr": [
> 1
> ],
> "partitionErrorCode": 0,
> "partitionId": 1,
> "leader": 1,
> "byteLength": 26
> },
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 2,
> "leader": 0,
> "byteLength": 26
> },
> {
> "replicas": [
> 1
> ],
> "isr": [
> 1
> ],
> "partitionErrorCode": 0,
> "partitionId": 3,
> "leader": 1,
> "byteLength": 26
> },
> {
> "replicas": [
> 0
> ],
> "isr": [
> 0
> ],
> "partitionErrorCode": 0,
> "partitionId": 4,
> "leader": 0,
> "byteLength": 26
> }
> ],
> "byteLength": 145
> }
> ],
> "responseSize": 200,
> "correlationId": -1000
> }
> {
> "brokers": [
> {
> "nodeId": 0,
> "host": "10.139.245.106",
> "port": 9092,
> "byteLength": 24
> },
> {
> "nodeId": 1,
> "host": "localhost",
> "port": 9093,
> "byteLength": 19
> }
> ],
> "topicMetadata": [
> {
> "topicErrorCode": 0,
> "topicName": "foozbar",
> "partitions": [
> {
> "replicas": [
> 0
> ],
> "isr": [],
> "partitionErrorCode": 5,
> "partitionId": 0,
> "leader": -1,
> "byteLength": 22
> },
> {
> "replicas": [
> 1
> ],
> "isr": [
> 1
> ],
> "partitionErrorCode": 0,
> "partitionId": 1,
> "leader": 1,
> "byteLength": 26
> },
> {
> "replicas": [
> 0
> ],
> "isr": [],
> "partitionErrorCode": 5,
> "partitionId": 2,
> "leader": -1,
> "byteLength": 22
> 

[jira] [Commented] (KAFKA-2203) Get gradle build to work with Java 8

[Jeff Maxwell (JIRA)]

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

Jeff Maxwell commented on KAFKA-2203:
-

The KAFKA-2294 patch allows java 8 builds to compile error (not warning) free 
for later versions of scala (>=2.10.4) without disabling doclint.
Should this issue be closed or should the coding guide 
(http://kafka.apache.org/coding-guide.html) be updated with regard to javadoc 
comments?

> Get gradle build to work with Java 8
> 
>
> Key: KAFKA-2203
> URL: https://issues.apache.org/jira/browse/KAFKA-2203
> Project: Kafka
>  Issue Type: Bug
>  Components: build
>Affects Versions: 0.8.1.1
>Reporter: Gaju Bhat
>Priority: Minor
> Fix For: 0.8.1.2
>
> Attachments: 0001-Special-case-java-8-and-javadoc-handling.patch
>
>
> The gradle build halts because javadoc in java 8 is a lot stricter about 
> valid html.
> It might be worthwhile to special case java 8 as described 
> [here|http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html].



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

[jira] [Commented] (KAFKA-1901) Move Kafka version to be generated in code by build (instead of in manifest)

[Jason Rosenberg (JIRA)]

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

Jason Rosenberg commented on KAFKA-1901:


[~omkreddy] Yes, that sounds find.  Preferably it should not be at the 
top-level directory, so as not to be confusing when bundled as part of a shaded 
jar!

> Move Kafka version to be generated in code by build (instead of in manifest)
> 
>
> Key: KAFKA-1901
> URL: https://issues.apache.org/jira/browse/KAFKA-1901
> Project: Kafka
>  Issue Type: Bug
>Affects Versions: 0.8.2.0
>Reporter: Jason Rosenberg
>
> With 0.8.2 (rc2), I've started seeing this warning in the logs of apps 
> deployed to our staging (both server and client):
> {code}
> 2015-01-23 00:55:25,273  WARN [async-message-sender-0] common.AppInfo$ - 
> Can't read Kafka version from MANIFEST.MF. Possible cause: 
> java.lang.NullPointerException
> {code}
> The issues is that in our deployment, apps are deployed with single 'shaded' 
> jars (e.g. using the maven shade plugin).  This means the MANIFEST.MF file 
> won't have a kafka version.  Instead, suggest the kafka build generate the 
> proper version in code, as part of the build.



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

[jira] [Commented] (KAFKA-2299) kafka-patch-review tool does not correctly capture testing done

[Ashish K Singh (JIRA)]

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

Ashish K Singh commented on KAFKA-2299:
---

[~nehanarkhede] do you mind taking a look at this trivial patch?

> kafka-patch-review tool does not correctly capture testing done
> ---
>
> Key: KAFKA-2299
> URL: https://issues.apache.org/jira/browse/KAFKA-2299
> Project: Kafka
>  Issue Type: Bug
>Reporter: Ashish K Singh
>Assignee: Ashish K Singh
> Attachments: KAFKA-2299.patch
>
>
> kafka-patch-review tool does not correctly capture testing done when 
> specified with -t or --testing-done.



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

[jira] [Commented] (KAFKA-2299) kafka-patch-review tool does not correctly capture testing done

[Ashish K Singh (JIRA)]

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

Ashish K Singh commented on KAFKA-2299:
---

Created reviewboard https://reviews.apache.org/r/35822/
 against branch trunk

> kafka-patch-review tool does not correctly capture testing done
> ---
>
> Key: KAFKA-2299
> URL: https://issues.apache.org/jira/browse/KAFKA-2299
> Project: Kafka
>  Issue Type: Bug
>Reporter: Ashish K Singh
>Assignee: Ashish K Singh
> Attachments: KAFKA-2299.patch
>
>
> kafka-patch-review tool does not correctly capture testing done when 
> specified with -t or --testing-done.



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

Review Request 35822: Patch for KAFKA-2299

[Ashish Singh]

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

Review request for kafka.


Bugs: KAFKA-2299
https://issues.apache.org/jira/browse/KAFKA-2299


Repository: kafka


Description
---

KAFKA-2299: kafka-patch-review tool does not correctly capture testing done


Diffs
-

  kafka-patch-review.py b5a2e950110d44d20d21e3d72be1783ceb49627d 

Diff: https://reviews.apache.org/r/35822/diff/


Testing
---


Thanks,

Ashish Singh

[jira] [Updated] (KAFKA-2299) kafka-patch-review tool does not correctly capture testing done

[Ashish K Singh (JIRA)]

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

Ashish K Singh updated KAFKA-2299:
--
Status: Patch Available  (was: Open)

> kafka-patch-review tool does not correctly capture testing done
> ---
>
> Key: KAFKA-2299
> URL: https://issues.apache.org/jira/browse/KAFKA-2299
> Project: Kafka
>  Issue Type: Bug
>Reporter: Ashish K Singh
>Assignee: Ashish K Singh
> Attachments: KAFKA-2299.patch
>
>
> kafka-patch-review tool does not correctly capture testing done when 
> specified with -t or --testing-done.



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

[jira] [Updated] (KAFKA-2299) kafka-patch-review tool does not correctly capture testing done

[Ashish K Singh (JIRA)]

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

Ashish K Singh updated KAFKA-2299:
--
Attachment: KAFKA-2299.patch

> kafka-patch-review tool does not correctly capture testing done
> ---
>
> Key: KAFKA-2299
> URL: https://issues.apache.org/jira/browse/KAFKA-2299
> Project: Kafka
>  Issue Type: Bug
>Reporter: Ashish K Singh
>Assignee: Ashish K Singh
> Attachments: KAFKA-2299.patch
>
>
> kafka-patch-review tool does not correctly capture testing done when 
> specified with -t or --testing-done.



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

[jira] [Created] (KAFKA-2299) kafka-patch-review tool does not correctly capture testing done

[Ashish K Singh (JIRA)]

Ashish K Singh created KAFKA-2299:
-

 Summary: kafka-patch-review tool does not correctly capture 
testing done
 Key: KAFKA-2299
 URL: https://issues.apache.org/jira/browse/KAFKA-2299
 Project: Kafka
  Issue Type: Bug
Reporter: Ashish K Singh
Assignee: Ashish K Singh


kafka-patch-review tool does not correctly capture testing done when specified 
with -t or --testing-done.



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