[jira] [Updated] (KAFKA-2326) KafkaProducer - invoke Thread.start() during construction

[Steve Tian (JIRA)]

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

Steve Tian updated KAFKA-2326:
--
   Priority: Minor  (was: Major)
Description: KafkaProducer invokes Thread.start() during construction.  
(was: The this reference of KafkaProducer escapes during construction as 
KafkaProducer invokes Thread.start() during construction.)
Summary: KafkaProducer - invoke Thread.start() during construction  
(was: KafkaProducer - the this reference escapes during construction)

My bad.  The *this* reference does not escape during construction. This issue 
could be closed.

> KafkaProducer - invoke Thread.start() during construction
> -
>
> Key: KAFKA-2326
> URL: https://issues.apache.org/jira/browse/KAFKA-2326
> Project: Kafka
>  Issue Type: Bug
>  Components: clients, producer 
>Affects Versions: 0.8.2.0, 0.8.2.1
>Reporter: Steve Tian
>Assignee: Jun Rao
>Priority: Minor
>
> KafkaProducer invokes Thread.start() during construction.



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

[jira] [Resolved] (KAFKA-2326) KafkaProducer - invoke Thread.start() during construction

[Steve Tian (JIRA)]

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

Steve Tian resolved KAFKA-2326.
---
Resolution: Invalid

> KafkaProducer - invoke Thread.start() during construction
> -
>
> Key: KAFKA-2326
> URL: https://issues.apache.org/jira/browse/KAFKA-2326
> Project: Kafka
>  Issue Type: Bug
>  Components: clients, producer 
>Affects Versions: 0.8.2.0, 0.8.2.1
>Reporter: Steve Tian
>Assignee: Jun Rao
>Priority: Minor
>
> KafkaProducer invokes Thread.start() during construction.



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

[jira] [Created] (KAFKA-2328) merge-kafka-pr.py script should restore previous branch after merge is cancelled

[Ismael Juma (JIRA)]

Ismael Juma created KAFKA-2328:
--

 Summary: merge-kafka-pr.py script should restore previous branch 
after merge is cancelled
 Key: KAFKA-2328
 URL: https://issues.apache.org/jira/browse/KAFKA-2328
 Project: Kafka
  Issue Type: Improvement
Reporter: Ismael Juma
Priority: Minor


[~gwenshap] asked:

"If I start a merge and cancel (say, by choosing 'n' when asked if I want to 
proceed), I'm left on a detached branch. Any chance the script can put me back 
in the original branch? or in trunk?"

Reference 
https://issues.apache.org/jira/browse/KAFKA-2187?focusedCommentId=14621243&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14621243



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

[jira] [Commented] (KAFKA-2328) merge-kafka-pr.py script should restore previous branch after merge is cancelled

[Ismael Juma (JIRA)]

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

Ismael Juma commented on KAFKA-2328:


Workaround is to use `git checkout -`.

> merge-kafka-pr.py script should restore previous branch after merge is 
> cancelled
> 
>
> Key: KAFKA-2328
> URL: https://issues.apache.org/jira/browse/KAFKA-2328
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Ismael Juma
>Priority: Minor
>
> [~gwenshap] asked:
> "If I start a merge and cancel (say, by choosing 'n' when asked if I want to 
> proceed), I'm left on a detached branch. Any chance the script can put me 
> back in the original branch? or in trunk?"
> Reference 
> https://issues.apache.org/jira/browse/KAFKA-2187?focusedCommentId=14621243&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14621243



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

Re: Review Request 35880: Patch for KAFKA-2295

[Manikumar Reddy O]

> On July 6, 2015, 3:50 p.m., Guozhang Wang wrote:
> > clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java,
> >  line 57
> > 
> >
> > Is this intentional?

yes. Now ClassNotFoundException will be thrown from Utils.newInstance() as 
KafkaException.


> On July 6, 2015, 3:50 p.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java, line 
> > 266
> > 
> >
> > Can we use Utils.newInstance here as well?

Here we only need Class Object.


> On July 6, 2015, 3:50 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/utils/CoreUtils.scala, line 221
> > 
> >
> > Can we use scala's Utils.createObject here?

Are you sugessting to use Utils.newInstance()? We dont have scala's 
Utils.scala. Am I missing something?


- Manikumar Reddy


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


On July 6, 2015, 6:05 a.m., Manikumar Reddy O wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35880/
> ---
> 
> (Updated July 6, 2015, 6:05 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2295
> https://issues.apache.org/jira/browse/KAFKA-2295
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Addessing Guozhang's comments
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 
> bae528d31516679bed88ee61b408f209f185a8cc 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 
> 4170bcc7def5b50d8aa20e8e84089c35b705b527 
>   clients/src/main/java/org/apache/kafka/common/utils/Utils.java 
> af9993cf9b3991f1e61e1201c94e19bc1bf76a68 
>   
> clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java 
> db1b0ee9113215b5ad7fda0f93915f3bdd34ac55 
>   core/src/main/scala/kafka/utils/CoreUtils.scala 
> 168a18d380c200ee566eccb6988dd1ae85ed5b09 
> 
> Diff: https://reviews.apache.org/r/35880/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

Re: Review Request 35867: Patch for KAFKA-1901

[Manikumar Reddy O]

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

(Updated July 10, 2015, 11:15 a.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

patch after rebase


Diffs (updated)
-

  build.gradle d86f1a8b25197d53f11e16c54a6854487e175649 
  clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 
7aa076084c894bb8f47b9df2c086475b06f47060 
  clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 
  clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 
6b9590c418aedd2727544c5dd23c017b4b72467a 
  clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java 
PRE-CREATION 
  clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 
07b1b60d3a9cb1a399a2fe95b87229f64f539f3b 
  clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 
544e120594de78c43581a980b1e4087b4fb98ccb 
  core/src/main/scala/kafka/common/AppInfo.scala 
d642ca555f83c41451d4fcaa5c01a1f86eff0a1c 
  core/src/main/scala/kafka/server/KafkaServer.scala 
18917bc4464b9403b16d85d20c3fd4c24893d1d3 

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


Testing
---


Thanks,

Manikumar Reddy O

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

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-1901:


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

> 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
>Assignee: Manikumar Reddy
> Attachments: KAFKA-1901.patch, KAFKA-1901_2015-06-26_13:16:29.patch, 
> KAFKA-1901_2015-07-10_16:42:53.patch
>
>
> 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] [Updated] (KAFKA-1901) Move Kafka version to be generated in code by build (instead of in manifest)

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy updated KAFKA-1901:
---
Attachment: KAFKA-1901_2015-07-10_16:42:53.patch

> 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
>Assignee: Manikumar Reddy
> Attachments: KAFKA-1901.patch, KAFKA-1901_2015-06-26_13:16:29.patch, 
> KAFKA-1901_2015-07-10_16:42:53.patch
>
>
> 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-1901) Move Kafka version to be generated in code by build (instead of in manifest)

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-1901:


[~jjkoshy] pinging for review

> 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
>Assignee: Manikumar Reddy
> Attachments: KAFKA-1901.patch, KAFKA-1901_2015-06-26_13:16:29.patch, 
> KAFKA-1901_2015-07-10_16:42:53.patch
>
>
> 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-1901) Move Kafka version to be generated in code by build (instead of in manifest)

[Joel Koshy (JIRA)]

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

Joel Koshy commented on KAFKA-1901:
---

Will review today.

> 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
>Assignee: Manikumar Reddy
> Attachments: KAFKA-1901.patch, KAFKA-1901_2015-06-26_13:16:29.patch, 
> KAFKA-1901_2015-07-10_16:42:53.patch
>
>
> 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)

Re: Review Request 34965: Patch for KAFKA-2241

[Joel Koshy]

> On July 9, 2015, 7:19 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/AbstractFetcherThread.scala, line 76
> > 
> >
> > You could get around the above by retaining this call to 
> > simpleConsumer.close (although it would be mostly redundant). However this 
> > is still not ideal, since it is a caveat that the user of the (public) 
> > forceClose API needs to be aware of.
> 
> Dong Lin wrote:
> I agree. I have updated the code and comments to hopefully avoid any 
> confusion to user.
> 
> Joel Koshy wrote:
> Would it work to just modify what you had before in `forceClose` to:
> ```
> disconnect();
> close();
> ```
> 
> Dong Lin wrote:
> I think that won't work. The event sequence you described will still 
> cause problem.
> 
> The following sequence of events may happen:
> 
> - the forceClose() as well as close() is executed by thread 1
> - thread 2 calls sendRequest(). blockingChannel.send(request) will throw 
> ClosedChannelException which triggers reconnect().
> 
> It is possible to make this work by changing the way sendRequest() 
> handles ClosedChannelException. But I find the API in the second patch is 
> better.
> 
> Which solution do you prefer?

True - that won't work. Another option may be to change `connect` to throw a 
`afkaException` if `isClosed` is true. Your latest patch may be better though 
since that avoids modification of the existing API (and only adds to it) - 
although I think naming it `interruptConsumer` may be better. The javadoc can 
clearly state that it actually disconnects the consumer due to the JVM bug (and 
link to the stackoverflow question).


- Joel


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


On July 9, 2015, 10:35 p.m., Dong Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34965/
> ---
> 
> (Updated July 9, 2015, 10:35 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2241
> https://issues.apache.org/jira/browse/KAFKA-2241
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2241; AbstractFetcherThread.shutdown() should not block on 
> ReadableByteChannel.read(buffer)
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 
> c16f7edd322709060e54c77eb505c44cbd77a4ec 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala 
> 83fc47417dd7fe4edf030217fa7fd69d99b170b0 
> 
> Diff: https://reviews.apache.org/r/34965/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dong Lin
> 
>

Re: Review Request 34805: Patch for KAFKA-2213

[Manikumar Reddy O]

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

(Updated July 10, 2015, 2:50 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

rebase


Diffs (updated)
-

  clients/src/main/java/org/apache/kafka/common/record/Compressor.java 
e570b29d5ffba5d3754c46670b708f7d511086f3 
  clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 
5f1b45c2970e4de53bd04595afe8486b5ec49e5d 
  core/src/main/scala/kafka/log/LogCleaner.scala 
b36ea0dd7f954c2a0eb3270535fc9a8df7488d59 
  core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 
381e9aa6fe611887cd144b4731f9ac351d12bbf1 

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


Testing
---


Thanks,

Manikumar Reddy O

[jira] [Updated] (KAFKA-2213) Log cleaner should write compacted messages using configured compression type

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy updated KAFKA-2213:
---
Attachment: KAFKA-2213_2015-07-10_20:18:06.patch

> Log cleaner should write compacted messages using configured compression type
> -
>
> Key: KAFKA-2213
> URL: https://issues.apache.org/jira/browse/KAFKA-2213
> Project: Kafka
>  Issue Type: Bug
>Reporter: Joel Koshy
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2213.patch, KAFKA-2213_2015-05-30_00:23:01.patch, 
> KAFKA-2213_2015-06-17_16:05:53.patch, KAFKA-2213_2015-07-10_20:18:06.patch
>
>
> In KAFKA-1374 the log cleaner was improved to handle compressed messages. 
> There were a couple of follow-ups from that:
> * We write compacted messages using the original compression type in the 
> compressed message-set. We should instead append all retained messages with 
> the configured broker compression type of the topic.
> * While compressing messages we should ideally do some batching before 
> compression.
> * Investigate the use of the client compressor. (See the discussion in the 
> RBs for KAFKA-1374)



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

[jira] [Commented] (KAFKA-2213) Log cleaner should write compacted messages using configured compression type

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-2213:


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

> Log cleaner should write compacted messages using configured compression type
> -
>
> Key: KAFKA-2213
> URL: https://issues.apache.org/jira/browse/KAFKA-2213
> Project: Kafka
>  Issue Type: Bug
>Reporter: Joel Koshy
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2213.patch, KAFKA-2213_2015-05-30_00:23:01.patch, 
> KAFKA-2213_2015-06-17_16:05:53.patch, KAFKA-2213_2015-07-10_20:18:06.patch
>
>
> In KAFKA-1374 the log cleaner was improved to handle compressed messages. 
> There were a couple of follow-ups from that:
> * We write compacted messages using the original compression type in the 
> compressed message-set. We should instead append all retained messages with 
> the configured broker compression type of the topic.
> * While compressing messages we should ideally do some batching before 
> compression.
> * Investigate the use of the client compressor. (See the discussion in the 
> RBs for KAFKA-1374)



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

Re: Review Request 34805: Patch for KAFKA-2213

[Manikumar Reddy O]

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


Joel, need your inputs for proceeding further on this.

- Manikumar Reddy O


On July 10, 2015, 2:50 p.m., Manikumar Reddy O wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34805/
> ---
> 
> (Updated July 10, 2015, 2:50 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2213
> https://issues.apache.org/jira/browse/KAFKA-2213
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> rebase
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/common/record/Compressor.java 
> e570b29d5ffba5d3754c46670b708f7d511086f3 
>   clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 
> 5f1b45c2970e4de53bd04595afe8486b5ec49e5d 
>   core/src/main/scala/kafka/log/LogCleaner.scala 
> b36ea0dd7f954c2a0eb3270535fc9a8df7488d59 
>   core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 
> 381e9aa6fe611887cd144b4731f9ac351d12bbf1 
> 
> Diff: https://reviews.apache.org/r/34805/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

Re: Review Request 35454: Patch for KAFKA-2159

[Manikumar Reddy O]

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

(Updated July 10, 2015, 3:47 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

rebase


Diffs (updated)
-

  core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala 
6c2df4c4f9d26d4091f480b64c063574837fd941 

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


Testing
---


Thanks,

Manikumar Reddy O

[jira] [Updated] (KAFKA-2159) offsets.topic.segment.bytes and offsets.topic.retention.minutes are ignored

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy updated KAFKA-2159:
---
Attachment: KAFKA-2159_2015-07-10_21:14:26.patch

> offsets.topic.segment.bytes and offsets.topic.retention.minutes are ignored
> ---
>
> Key: KAFKA-2159
> URL: https://issues.apache.org/jira/browse/KAFKA-2159
> Project: Kafka
>  Issue Type: Bug
>  Components: offset manager
>Affects Versions: 0.8.2.1
>Reporter: Rafał Boniecki
>Assignee: Manikumar Reddy
>  Labels: newbie
> Attachments: KAFKA-2159.patch, KAFKA-2159_2015-06-17_11:44:03.patch, 
> KAFKA-2159_2015-07-10_21:14:26.patch
>
>
> My broker configuration:
> {quote}offsets.topic.num.partitions=20
> offsets.topic.segment.bytes=10485760
> offsets.topic.retention.minutes=10080{quote}
> Describe of __consumer_offsets topic:
> {quote}Topic:__consumer_offsets   PartitionCount:20   
> ReplicationFactor:3 Configs:segment.bytes=104857600,cleanup.policy=compact
>   Topic: __consumer_offsets   Partition: 0Leader: 112 
> Replicas: 112,212,312   Isr: 212,312,112
>   Topic: __consumer_offsets   Partition: 1Leader: 212 
> Replicas: 212,312,412   Isr: 212,312,412
>   Topic: __consumer_offsets   Partition: 2Leader: 312 
> Replicas: 312,412,512   Isr: 312,412,512
>   Topic: __consumer_offsets   Partition: 3Leader: 412 
> Replicas: 412,512,112   Isr: 412,512,112
>   Topic: __consumer_offsets   Partition: 4Leader: 512 
> Replicas: 512,112,212   Isr: 512,212,112
>   Topic: __consumer_offsets   Partition: 5Leader: 112 
> Replicas: 112,312,412   Isr: 312,412,112
>   Topic: __consumer_offsets   Partition: 6Leader: 212 
> Replicas: 212,412,512   Isr: 212,412,512
>   Topic: __consumer_offsets   Partition: 7Leader: 312 
> Replicas: 312,512,112   Isr: 312,512,112
>   Topic: __consumer_offsets   Partition: 8Leader: 412 
> Replicas: 412,112,212   Isr: 412,212,112
>   Topic: __consumer_offsets   Partition: 9Leader: 512 
> Replicas: 512,212,312   Isr: 512,212,312
>   Topic: __consumer_offsets   Partition: 10   Leader: 112 
> Replicas: 112,412,512   Isr: 412,512,112
>   Topic: __consumer_offsets   Partition: 11   Leader: 212 
> Replicas: 212,512,112   Isr: 212,512,112
>   Topic: __consumer_offsets   Partition: 12   Leader: 312 
> Replicas: 312,112,212   Isr: 312,212,112
>   Topic: __consumer_offsets   Partition: 13   Leader: 412 
> Replicas: 412,212,312   Isr: 412,212,312
>   Topic: __consumer_offsets   Partition: 14   Leader: 512 
> Replicas: 512,312,412   Isr: 512,312,412
>   Topic: __consumer_offsets   Partition: 15   Leader: 112 
> Replicas: 112,512,212   Isr: 512,212,112
>   Topic: __consumer_offsets   Partition: 16   Leader: 212 
> Replicas: 212,112,312   Isr: 212,312,112
>   Topic: __consumer_offsets   Partition: 17   Leader: 312 
> Replicas: 312,212,412   Isr: 312,212,412
>   Topic: __consumer_offsets   Partition: 18   Leader: 412 
> Replicas: 412,312,512   Isr: 412,312,512
>   Topic: __consumer_offsets   Partition: 19   Leader: 512 
> Replicas: 512,412,112   Isr: 512,412,112{quote}
> OffsetManager logs:
> {quote}2015-04-29 17:58:43:403 CEST DEBUG 
> [kafka-scheduler-3][kafka.server.OffsetManager] Compacting offsets cache.
> 2015-04-29 17:58:43:403 CEST DEBUG 
> [kafka-scheduler-3][kafka.server.OffsetManager] Found 1 stale offsets (older 
> than 8640 ms).
> 2015-04-29 17:58:43:404 CEST TRACE 
> [kafka-scheduler-3][kafka.server.OffsetManager] Removing stale offset and 
> metadata for [drafts,tasks,1]: OffsetAndMetadata[824,consumer_id = drafts, 
> time = 1430322433,0]
> 2015-04-29 17:58:43:404 CEST TRACE 
> [kafka-scheduler-3][kafka.server.OffsetManager] Marked 1 offsets in 
> [__consumer_offsets,2] for deletion.
> 2015-04-29 17:58:43:404 CEST DEBUG 
> [kafka-scheduler-3][kafka.server.OffsetManager] Removed 1 stale offsets in 1 
> milliseconds.{quote}
> Parameters are ignored and default values are used instead.



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

[jira] [Commented] (KAFKA-2159) offsets.topic.segment.bytes and offsets.topic.retention.minutes are ignored

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-2159:


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

> offsets.topic.segment.bytes and offsets.topic.retention.minutes are ignored
> ---
>
> Key: KAFKA-2159
> URL: https://issues.apache.org/jira/browse/KAFKA-2159
> Project: Kafka
>  Issue Type: Bug
>  Components: offset manager
>Affects Versions: 0.8.2.1
>Reporter: Rafał Boniecki
>Assignee: Manikumar Reddy
>  Labels: newbie
> Attachments: KAFKA-2159.patch, KAFKA-2159_2015-06-17_11:44:03.patch, 
> KAFKA-2159_2015-07-10_21:14:26.patch
>
>
> My broker configuration:
> {quote}offsets.topic.num.partitions=20
> offsets.topic.segment.bytes=10485760
> offsets.topic.retention.minutes=10080{quote}
> Describe of __consumer_offsets topic:
> {quote}Topic:__consumer_offsets   PartitionCount:20   
> ReplicationFactor:3 Configs:segment.bytes=104857600,cleanup.policy=compact
>   Topic: __consumer_offsets   Partition: 0Leader: 112 
> Replicas: 112,212,312   Isr: 212,312,112
>   Topic: __consumer_offsets   Partition: 1Leader: 212 
> Replicas: 212,312,412   Isr: 212,312,412
>   Topic: __consumer_offsets   Partition: 2Leader: 312 
> Replicas: 312,412,512   Isr: 312,412,512
>   Topic: __consumer_offsets   Partition: 3Leader: 412 
> Replicas: 412,512,112   Isr: 412,512,112
>   Topic: __consumer_offsets   Partition: 4Leader: 512 
> Replicas: 512,112,212   Isr: 512,212,112
>   Topic: __consumer_offsets   Partition: 5Leader: 112 
> Replicas: 112,312,412   Isr: 312,412,112
>   Topic: __consumer_offsets   Partition: 6Leader: 212 
> Replicas: 212,412,512   Isr: 212,412,512
>   Topic: __consumer_offsets   Partition: 7Leader: 312 
> Replicas: 312,512,112   Isr: 312,512,112
>   Topic: __consumer_offsets   Partition: 8Leader: 412 
> Replicas: 412,112,212   Isr: 412,212,112
>   Topic: __consumer_offsets   Partition: 9Leader: 512 
> Replicas: 512,212,312   Isr: 512,212,312
>   Topic: __consumer_offsets   Partition: 10   Leader: 112 
> Replicas: 112,412,512   Isr: 412,512,112
>   Topic: __consumer_offsets   Partition: 11   Leader: 212 
> Replicas: 212,512,112   Isr: 212,512,112
>   Topic: __consumer_offsets   Partition: 12   Leader: 312 
> Replicas: 312,112,212   Isr: 312,212,112
>   Topic: __consumer_offsets   Partition: 13   Leader: 412 
> Replicas: 412,212,312   Isr: 412,212,312
>   Topic: __consumer_offsets   Partition: 14   Leader: 512 
> Replicas: 512,312,412   Isr: 512,312,412
>   Topic: __consumer_offsets   Partition: 15   Leader: 112 
> Replicas: 112,512,212   Isr: 512,212,112
>   Topic: __consumer_offsets   Partition: 16   Leader: 212 
> Replicas: 212,112,312   Isr: 212,312,112
>   Topic: __consumer_offsets   Partition: 17   Leader: 312 
> Replicas: 312,212,412   Isr: 312,212,412
>   Topic: __consumer_offsets   Partition: 18   Leader: 412 
> Replicas: 412,312,512   Isr: 412,312,512
>   Topic: __consumer_offsets   Partition: 19   Leader: 512 
> Replicas: 512,412,112   Isr: 512,412,112{quote}
> OffsetManager logs:
> {quote}2015-04-29 17:58:43:403 CEST DEBUG 
> [kafka-scheduler-3][kafka.server.OffsetManager] Compacting offsets cache.
> 2015-04-29 17:58:43:403 CEST DEBUG 
> [kafka-scheduler-3][kafka.server.OffsetManager] Found 1 stale offsets (older 
> than 8640 ms).
> 2015-04-29 17:58:43:404 CEST TRACE 
> [kafka-scheduler-3][kafka.server.OffsetManager] Removing stale offset and 
> metadata for [drafts,tasks,1]: OffsetAndMetadata[824,consumer_id = drafts, 
> time = 1430322433,0]
> 2015-04-29 17:58:43:404 CEST TRACE 
> [kafka-scheduler-3][kafka.server.OffsetManager] Marked 1 offsets in 
> [__consumer_offsets,2] for deletion.
> 2015-04-29 17:58:43:404 CEST DEBUG 
> [kafka-scheduler-3][kafka.server.OffsetManager] Removed 1 stale offsets in 1 
> milliseconds.{quote}
> Parameters are ignored and default values are used instead.



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

Re: Review Request 34641: Patch for KAFKA-2214

[Manikumar Reddy O]

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

(Updated July 10, 2015, 4:28 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

rebase


Diffs (updated)
-

  core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala 
ea345895a52977c25bff57e95e12b8662331d7fe 

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


Testing
---


Thanks,

Manikumar Reddy O

[jira] [Updated] (KAFKA-2214) kafka-reassign-partitions.sh --verify should return non-zero exit codes when reassignment is not completed yet

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy updated KAFKA-2214:
---
Attachment: KAFKA-2214_2015-07-10_21:56:04.patch

> kafka-reassign-partitions.sh --verify should return non-zero exit codes when 
> reassignment is not completed yet
> --
>
> Key: KAFKA-2214
> URL: https://issues.apache.org/jira/browse/KAFKA-2214
> Project: Kafka
>  Issue Type: Improvement
>  Components: admin
>Affects Versions: 0.8.1.1, 0.8.2.0
>Reporter: Michael Noll
>Assignee: Manikumar Reddy
>Priority: Minor
> Attachments: KAFKA-2214.patch, KAFKA-2214_2015-07-10_21:56:04.patch
>
>
> h4. Background
> The admin script {{kafka-reassign-partitions.sh}} should integrate better 
> with automation tools such as Ansible, which rely on scripts adhering to Unix 
> best practices such as appropriate exit codes on success/failure.
> h4. Current behavior (incorrect)
> When reassignments are still in progress {{kafka-reassign-partitions.sh}} 
> prints {{ERROR}} messages but returns an exit code of zero, which indicates 
> success.  This behavior makes it a bit cumbersome to integrate the script 
> into automation tools such as Ansible.
> {code}
> $ kafka-reassign-partitions.sh --zookeeper zookeeper1:2181 
> --reassignment-json-file partitions-to-move.json --verify
> Status of partition reassignment:
> ERROR: Assigned replicas (316,324,311) don't match the list of replicas for 
> reassignment (316,324) for partition [mytopic,2]
> Reassignment of partition [mytopic,0] completed successfully
> Reassignment of partition [myothertopic,1] completed successfully
> Reassignment of partition [myothertopic,3] completed successfully
> ...
> $ echo $?
> 0
> # But preferably the exit code in the presence of ERRORs should be, say, 1.
> {code}
> h3. How to improve
> I'd suggest that, using the above as the running example, if there are any 
> {{ERROR}} entries in the output (i.e. if there are any assignments remaining 
> that don't match the desired assignments), then the 
> {{kafka-reassign-partitions.sh}}  should return a non-zero exit code.
> h3. Notes
> In Kafka 0.8.2 the output is a bit different: The ERROR messages are now 
> phrased differently.
> Before:
> {code}
> ERROR: Assigned replicas (316,324,311) don't match the list of replicas for 
> reassignment (316,324) for partition [mytopic,2]
> {code}
> Now:
> {code}
> Reassignment of partition [mytopic,2] is still in progress
> {code}



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

[jira] [Commented] (KAFKA-2214) kafka-reassign-partitions.sh --verify should return non-zero exit codes when reassignment is not completed yet

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-2214:


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

> kafka-reassign-partitions.sh --verify should return non-zero exit codes when 
> reassignment is not completed yet
> --
>
> Key: KAFKA-2214
> URL: https://issues.apache.org/jira/browse/KAFKA-2214
> Project: Kafka
>  Issue Type: Improvement
>  Components: admin
>Affects Versions: 0.8.1.1, 0.8.2.0
>Reporter: Michael Noll
>Assignee: Manikumar Reddy
>Priority: Minor
> Attachments: KAFKA-2214.patch, KAFKA-2214_2015-07-10_21:56:04.patch
>
>
> h4. Background
> The admin script {{kafka-reassign-partitions.sh}} should integrate better 
> with automation tools such as Ansible, which rely on scripts adhering to Unix 
> best practices such as appropriate exit codes on success/failure.
> h4. Current behavior (incorrect)
> When reassignments are still in progress {{kafka-reassign-partitions.sh}} 
> prints {{ERROR}} messages but returns an exit code of zero, which indicates 
> success.  This behavior makes it a bit cumbersome to integrate the script 
> into automation tools such as Ansible.
> {code}
> $ kafka-reassign-partitions.sh --zookeeper zookeeper1:2181 
> --reassignment-json-file partitions-to-move.json --verify
> Status of partition reassignment:
> ERROR: Assigned replicas (316,324,311) don't match the list of replicas for 
> reassignment (316,324) for partition [mytopic,2]
> Reassignment of partition [mytopic,0] completed successfully
> Reassignment of partition [myothertopic,1] completed successfully
> Reassignment of partition [myothertopic,3] completed successfully
> ...
> $ echo $?
> 0
> # But preferably the exit code in the presence of ERRORs should be, say, 1.
> {code}
> h3. How to improve
> I'd suggest that, using the above as the running example, if there are any 
> {{ERROR}} entries in the output (i.e. if there are any assignments remaining 
> that don't match the desired assignments), then the 
> {{kafka-reassign-partitions.sh}}  should return a non-zero exit code.
> h3. Notes
> In Kafka 0.8.2 the output is a bit different: The ERROR messages are now 
> phrased differently.
> Before:
> {code}
> ERROR: Assigned replicas (316,324,311) don't match the list of replicas for 
> reassignment (316,324) for partition [mytopic,2]
> {code}
> Now:
> {code}
> Reassignment of partition [mytopic,2] is still in progress
> {code}



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

[jira] [Commented] (KAFKA-2214) kafka-reassign-partitions.sh --verify should return non-zero exit codes when reassignment is not completed yet

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-2214:


[~junrao] pinging for review

> kafka-reassign-partitions.sh --verify should return non-zero exit codes when 
> reassignment is not completed yet
> --
>
> Key: KAFKA-2214
> URL: https://issues.apache.org/jira/browse/KAFKA-2214
> Project: Kafka
>  Issue Type: Improvement
>  Components: admin
>Affects Versions: 0.8.1.1, 0.8.2.0
>Reporter: Michael Noll
>Assignee: Manikumar Reddy
>Priority: Minor
> Attachments: KAFKA-2214.patch, KAFKA-2214_2015-07-10_21:56:04.patch
>
>
> h4. Background
> The admin script {{kafka-reassign-partitions.sh}} should integrate better 
> with automation tools such as Ansible, which rely on scripts adhering to Unix 
> best practices such as appropriate exit codes on success/failure.
> h4. Current behavior (incorrect)
> When reassignments are still in progress {{kafka-reassign-partitions.sh}} 
> prints {{ERROR}} messages but returns an exit code of zero, which indicates 
> success.  This behavior makes it a bit cumbersome to integrate the script 
> into automation tools such as Ansible.
> {code}
> $ kafka-reassign-partitions.sh --zookeeper zookeeper1:2181 
> --reassignment-json-file partitions-to-move.json --verify
> Status of partition reassignment:
> ERROR: Assigned replicas (316,324,311) don't match the list of replicas for 
> reassignment (316,324) for partition [mytopic,2]
> Reassignment of partition [mytopic,0] completed successfully
> Reassignment of partition [myothertopic,1] completed successfully
> Reassignment of partition [myothertopic,3] completed successfully
> ...
> $ echo $?
> 0
> # But preferably the exit code in the presence of ERRORs should be, say, 1.
> {code}
> h3. How to improve
> I'd suggest that, using the above as the running example, if there are any 
> {{ERROR}} entries in the output (i.e. if there are any assignments remaining 
> that don't match the desired assignments), then the 
> {{kafka-reassign-partitions.sh}}  should return a non-zero exit code.
> h3. Notes
> In Kafka 0.8.2 the output is a bit different: The ERROR messages are now 
> phrased differently.
> Before:
> {code}
> ERROR: Assigned replicas (316,324,311) don't match the list of replicas for 
> reassignment (316,324) for partition [mytopic,2]
> {code}
> Now:
> {code}
> Reassignment of partition [mytopic,2] is still in progress
> {code}



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

Re: Review Request 34403: Patch for KAFKA-2198

[Manikumar Reddy O]

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

(Updated July 10, 2015, 4:34 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

rebase


Diffs (updated)
-

  core/src/main/scala/kafka/admin/TopicCommand.scala 
a2ecb9620d647bf8f957a1f00f52896438e804a7 

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


Testing
---


Thanks,

Manikumar Reddy O

[jira] [Updated] (KAFKA-2198) kafka-topics.sh exits with 0 status on failures

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy updated KAFKA-2198:
---
Attachment: KAFKA-2198_2015-07-10_22:02:02.patch

> kafka-topics.sh exits with 0 status on failures
> ---
>
> Key: KAFKA-2198
> URL: https://issues.apache.org/jira/browse/KAFKA-2198
> Project: Kafka
>  Issue Type: Bug
>  Components: admin
>Affects Versions: 0.8.2.1
>Reporter: Bob Halley
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2198.patch, KAFKA-2198_2015-05-19_18:27:01.patch, 
> KAFKA-2198_2015-05-19_18:41:25.patch, KAFKA-2198_2015-07-10_22:02:02.patch
>
>
> In the two failure cases below, kafka-topics.sh exits with status 0.  You 
> shouldn't need to parse output from the command to know if it failed or not.
> Case 1: Forgetting to add Kafka zookeeper chroot path to zookeeper spec
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicas=2 
> --zookeeper 10.0.0.1 && echo succeeded
> succeeded
> Case 2: Bad config option.  (Also, do we really need the java backtrace?  
> It's a lot of noise most of the time.)
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicasTYPO=2 
> --zookeeper 10.0.0.1/kafka && echo succeeded
> Error while executing topic command requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> java.lang.IllegalArgumentException: requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> at scala.Predef$.require(Predef.scala:233)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:183)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:182)
> at scala.collection.Iterator$class.foreach(Iterator.scala:727)
> at scala.collection.AbstractIterator.foreach(Iterator.scala:1157)
> at kafka.log.LogConfig$.validateNames(LogConfig.scala:182)
> at kafka.log.LogConfig$.validate(LogConfig.scala:190)
> at 
> kafka.admin.TopicCommand$.parseTopicConfigsToBeAdded(TopicCommand.scala:205)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:103)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:100)
> at 
> scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
> at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
> at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:100)
> at kafka.admin.TopicCommand$.main(TopicCommand.scala:57)
> at kafka.admin.TopicCommand.main(TopicCommand.scala)
> succeeded



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

[jira] [Commented] (KAFKA-2198) kafka-topics.sh exits with 0 status on failures

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-2198:


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

> kafka-topics.sh exits with 0 status on failures
> ---
>
> Key: KAFKA-2198
> URL: https://issues.apache.org/jira/browse/KAFKA-2198
> Project: Kafka
>  Issue Type: Bug
>  Components: admin
>Affects Versions: 0.8.2.1
>Reporter: Bob Halley
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2198.patch, KAFKA-2198_2015-05-19_18:27:01.patch, 
> KAFKA-2198_2015-05-19_18:41:25.patch, KAFKA-2198_2015-07-10_22:02:02.patch
>
>
> In the two failure cases below, kafka-topics.sh exits with status 0.  You 
> shouldn't need to parse output from the command to know if it failed or not.
> Case 1: Forgetting to add Kafka zookeeper chroot path to zookeeper spec
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicas=2 
> --zookeeper 10.0.0.1 && echo succeeded
> succeeded
> Case 2: Bad config option.  (Also, do we really need the java backtrace?  
> It's a lot of noise most of the time.)
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicasTYPO=2 
> --zookeeper 10.0.0.1/kafka && echo succeeded
> Error while executing topic command requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> java.lang.IllegalArgumentException: requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> at scala.Predef$.require(Predef.scala:233)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:183)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:182)
> at scala.collection.Iterator$class.foreach(Iterator.scala:727)
> at scala.collection.AbstractIterator.foreach(Iterator.scala:1157)
> at kafka.log.LogConfig$.validateNames(LogConfig.scala:182)
> at kafka.log.LogConfig$.validate(LogConfig.scala:190)
> at 
> kafka.admin.TopicCommand$.parseTopicConfigsToBeAdded(TopicCommand.scala:205)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:103)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:100)
> at 
> scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
> at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
> at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:100)
> at kafka.admin.TopicCommand$.main(TopicCommand.scala:57)
> at kafka.admin.TopicCommand.main(TopicCommand.scala)
> succeeded



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

[jira] [Commented] (KAFKA-2198) kafka-topics.sh exits with 0 status on failures

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-2198:


[~junrao] pinging for review

> kafka-topics.sh exits with 0 status on failures
> ---
>
> Key: KAFKA-2198
> URL: https://issues.apache.org/jira/browse/KAFKA-2198
> Project: Kafka
>  Issue Type: Bug
>  Components: admin
>Affects Versions: 0.8.2.1
>Reporter: Bob Halley
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2198.patch, KAFKA-2198_2015-05-19_18:27:01.patch, 
> KAFKA-2198_2015-05-19_18:41:25.patch, KAFKA-2198_2015-07-10_22:02:02.patch
>
>
> In the two failure cases below, kafka-topics.sh exits with status 0.  You 
> shouldn't need to parse output from the command to know if it failed or not.
> Case 1: Forgetting to add Kafka zookeeper chroot path to zookeeper spec
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicas=2 
> --zookeeper 10.0.0.1 && echo succeeded
> succeeded
> Case 2: Bad config option.  (Also, do we really need the java backtrace?  
> It's a lot of noise most of the time.)
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicasTYPO=2 
> --zookeeper 10.0.0.1/kafka && echo succeeded
> Error while executing topic command requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> java.lang.IllegalArgumentException: requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> at scala.Predef$.require(Predef.scala:233)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:183)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:182)
> at scala.collection.Iterator$class.foreach(Iterator.scala:727)
> at scala.collection.AbstractIterator.foreach(Iterator.scala:1157)
> at kafka.log.LogConfig$.validateNames(LogConfig.scala:182)
> at kafka.log.LogConfig$.validate(LogConfig.scala:190)
> at 
> kafka.admin.TopicCommand$.parseTopicConfigsToBeAdded(TopicCommand.scala:205)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:103)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:100)
> at 
> scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
> at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
> at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:100)
> at kafka.admin.TopicCommand$.main(TopicCommand.scala:57)
> at kafka.admin.TopicCommand.main(TopicCommand.scala)
> succeeded



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

Re: Review Request 34403: Patch for KAFKA-2198

[Gwen Shapira]

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


Thanks for the patch!


core/src/main/scala/kafka/admin/TopicCommand.scala (lines 72 - 73)


This is a bit unclean. I think its more idiomatic when the catch block 
includes the System.exit(1).

Also, I'm afraid that printing the entire stack trace is intimidating to 
non-developers who use the CLI. Perhaps the stack trace should go under 
log.error(...)?


- Gwen Shapira


On July 10, 2015, 4:34 p.m., Manikumar Reddy O wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34403/
> ---
> 
> (Updated July 10, 2015, 4:34 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2198
> https://issues.apache.org/jira/browse/KAFKA-2198
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> rebase
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/TopicCommand.scala 
> a2ecb9620d647bf8f957a1f00f52896438e804a7 
> 
> Diff: https://reviews.apache.org/r/34403/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

[jira] [Updated] (KAFKA-2198) kafka-topics.sh exits with 0 status on failures

[Gwen Shapira (JIRA)]

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

Gwen Shapira updated KAFKA-2198:

Reviewer: Gwen Shapira  (was: Neha Narkhede)

> kafka-topics.sh exits with 0 status on failures
> ---
>
> Key: KAFKA-2198
> URL: https://issues.apache.org/jira/browse/KAFKA-2198
> Project: Kafka
>  Issue Type: Bug
>  Components: admin
>Affects Versions: 0.8.2.1
>Reporter: Bob Halley
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2198.patch, KAFKA-2198_2015-05-19_18:27:01.patch, 
> KAFKA-2198_2015-05-19_18:41:25.patch, KAFKA-2198_2015-07-10_22:02:02.patch
>
>
> In the two failure cases below, kafka-topics.sh exits with status 0.  You 
> shouldn't need to parse output from the command to know if it failed or not.
> Case 1: Forgetting to add Kafka zookeeper chroot path to zookeeper spec
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicas=2 
> --zookeeper 10.0.0.1 && echo succeeded
> succeeded
> Case 2: Bad config option.  (Also, do we really need the java backtrace?  
> It's a lot of noise most of the time.)
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicasTYPO=2 
> --zookeeper 10.0.0.1/kafka && echo succeeded
> Error while executing topic command requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> java.lang.IllegalArgumentException: requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> at scala.Predef$.require(Predef.scala:233)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:183)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:182)
> at scala.collection.Iterator$class.foreach(Iterator.scala:727)
> at scala.collection.AbstractIterator.foreach(Iterator.scala:1157)
> at kafka.log.LogConfig$.validateNames(LogConfig.scala:182)
> at kafka.log.LogConfig$.validate(LogConfig.scala:190)
> at 
> kafka.admin.TopicCommand$.parseTopicConfigsToBeAdded(TopicCommand.scala:205)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:103)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:100)
> at 
> scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
> at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
> at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:100)
> at kafka.admin.TopicCommand$.main(TopicCommand.scala:57)
> at kafka.admin.TopicCommand.main(TopicCommand.scala)
> succeeded



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

[jira] [Commented] (KAFKA-2198) kafka-topics.sh exits with 0 status on failures

[Gwen Shapira (JIRA)]

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

Gwen Shapira commented on KAFKA-2198:
-

Hope its ok if I review this, [~omkreddy].

> kafka-topics.sh exits with 0 status on failures
> ---
>
> Key: KAFKA-2198
> URL: https://issues.apache.org/jira/browse/KAFKA-2198
> Project: Kafka
>  Issue Type: Bug
>  Components: admin
>Affects Versions: 0.8.2.1
>Reporter: Bob Halley
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2198.patch, KAFKA-2198_2015-05-19_18:27:01.patch, 
> KAFKA-2198_2015-05-19_18:41:25.patch, KAFKA-2198_2015-07-10_22:02:02.patch
>
>
> In the two failure cases below, kafka-topics.sh exits with status 0.  You 
> shouldn't need to parse output from the command to know if it failed or not.
> Case 1: Forgetting to add Kafka zookeeper chroot path to zookeeper spec
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicas=2 
> --zookeeper 10.0.0.1 && echo succeeded
> succeeded
> Case 2: Bad config option.  (Also, do we really need the java backtrace?  
> It's a lot of noise most of the time.)
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicasTYPO=2 
> --zookeeper 10.0.0.1/kafka && echo succeeded
> Error while executing topic command requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> java.lang.IllegalArgumentException: requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> at scala.Predef$.require(Predef.scala:233)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:183)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:182)
> at scala.collection.Iterator$class.foreach(Iterator.scala:727)
> at scala.collection.AbstractIterator.foreach(Iterator.scala:1157)
> at kafka.log.LogConfig$.validateNames(LogConfig.scala:182)
> at kafka.log.LogConfig$.validate(LogConfig.scala:190)
> at 
> kafka.admin.TopicCommand$.parseTopicConfigsToBeAdded(TopicCommand.scala:205)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:103)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:100)
> at 
> scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
> at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
> at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:100)
> at kafka.admin.TopicCommand$.main(TopicCommand.scala:57)
> at kafka.admin.TopicCommand.main(TopicCommand.scala)
> succeeded



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

[jira] [Commented] (KAFKA-2198) kafka-topics.sh exits with 0 status on failures

[Gwen Shapira (JIRA)]

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

Gwen Shapira commented on KAFKA-2198:
-

Also, [~omkreddy], since the complaint is about usability, can you post sample 
output of the cases shown above? so we can show the improvement?

> kafka-topics.sh exits with 0 status on failures
> ---
>
> Key: KAFKA-2198
> URL: https://issues.apache.org/jira/browse/KAFKA-2198
> Project: Kafka
>  Issue Type: Bug
>  Components: admin
>Affects Versions: 0.8.2.1
>Reporter: Bob Halley
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2198.patch, KAFKA-2198_2015-05-19_18:27:01.patch, 
> KAFKA-2198_2015-05-19_18:41:25.patch, KAFKA-2198_2015-07-10_22:02:02.patch
>
>
> In the two failure cases below, kafka-topics.sh exits with status 0.  You 
> shouldn't need to parse output from the command to know if it failed or not.
> Case 1: Forgetting to add Kafka zookeeper chroot path to zookeeper spec
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicas=2 
> --zookeeper 10.0.0.1 && echo succeeded
> succeeded
> Case 2: Bad config option.  (Also, do we really need the java backtrace?  
> It's a lot of noise most of the time.)
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicasTYPO=2 
> --zookeeper 10.0.0.1/kafka && echo succeeded
> Error while executing topic command requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> java.lang.IllegalArgumentException: requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> at scala.Predef$.require(Predef.scala:233)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:183)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:182)
> at scala.collection.Iterator$class.foreach(Iterator.scala:727)
> at scala.collection.AbstractIterator.foreach(Iterator.scala:1157)
> at kafka.log.LogConfig$.validateNames(LogConfig.scala:182)
> at kafka.log.LogConfig$.validate(LogConfig.scala:190)
> at 
> kafka.admin.TopicCommand$.parseTopicConfigsToBeAdded(TopicCommand.scala:205)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:103)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:100)
> at 
> scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
> at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
> at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:100)
> at kafka.admin.TopicCommand$.main(TopicCommand.scala:57)
> at kafka.admin.TopicCommand.main(TopicCommand.scala)
> succeeded



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

Re: Review Request 34641: Patch for KAFKA-2214

[Gwen Shapira]

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


Thanks for the patch. Lets streamline error handling a bit. Also, can you post 
sample output to the JIRA, so we can make sure the errors are use-friendly?


core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala (lines 60 - 61)


Can we just exit in the exception clause?



core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala (line 79)


why not re-throw? 
APIs that can both return error codes and throw exceptions are pretty 
confusing (you need to handle errors in two different ways).


- Gwen Shapira


On July 10, 2015, 4:28 p.m., Manikumar Reddy O wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34641/
> ---
> 
> (Updated July 10, 2015, 4:28 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2214
> https://issues.apache.org/jira/browse/KAFKA-2214
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> rebase
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala 
> ea345895a52977c25bff57e95e12b8662331d7fe 
> 
> Diff: https://reviews.apache.org/r/34641/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

[jira] [Updated] (KAFKA-2214) kafka-reassign-partitions.sh --verify should return non-zero exit codes when reassignment is not completed yet

[Gwen Shapira (JIRA)]

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

Gwen Shapira updated KAFKA-2214:

Reviewer: Gwen Shapira  (was: Neha Narkhede)

> kafka-reassign-partitions.sh --verify should return non-zero exit codes when 
> reassignment is not completed yet
> --
>
> Key: KAFKA-2214
> URL: https://issues.apache.org/jira/browse/KAFKA-2214
> Project: Kafka
>  Issue Type: Improvement
>  Components: admin
>Affects Versions: 0.8.1.1, 0.8.2.0
>Reporter: Michael Noll
>Assignee: Manikumar Reddy
>Priority: Minor
> Attachments: KAFKA-2214.patch, KAFKA-2214_2015-07-10_21:56:04.patch
>
>
> h4. Background
> The admin script {{kafka-reassign-partitions.sh}} should integrate better 
> with automation tools such as Ansible, which rely on scripts adhering to Unix 
> best practices such as appropriate exit codes on success/failure.
> h4. Current behavior (incorrect)
> When reassignments are still in progress {{kafka-reassign-partitions.sh}} 
> prints {{ERROR}} messages but returns an exit code of zero, which indicates 
> success.  This behavior makes it a bit cumbersome to integrate the script 
> into automation tools such as Ansible.
> {code}
> $ kafka-reassign-partitions.sh --zookeeper zookeeper1:2181 
> --reassignment-json-file partitions-to-move.json --verify
> Status of partition reassignment:
> ERROR: Assigned replicas (316,324,311) don't match the list of replicas for 
> reassignment (316,324) for partition [mytopic,2]
> Reassignment of partition [mytopic,0] completed successfully
> Reassignment of partition [myothertopic,1] completed successfully
> Reassignment of partition [myothertopic,3] completed successfully
> ...
> $ echo $?
> 0
> # But preferably the exit code in the presence of ERRORs should be, say, 1.
> {code}
> h3. How to improve
> I'd suggest that, using the above as the running example, if there are any 
> {{ERROR}} entries in the output (i.e. if there are any assignments remaining 
> that don't match the desired assignments), then the 
> {{kafka-reassign-partitions.sh}}  should return a non-zero exit code.
> h3. Notes
> In Kafka 0.8.2 the output is a bit different: The ERROR messages are now 
> phrased differently.
> Before:
> {code}
> ERROR: Assigned replicas (316,324,311) don't match the list of replicas for 
> reassignment (316,324) for partition [mytopic,2]
> {code}
> Now:
> {code}
> Reassignment of partition [mytopic,2] is still in progress
> {code}



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

Re: Review Request 34641: Patch for KAFKA-2214

[Gwen Shapira]

> On May 26, 2015, 7:08 a.m., Michael Noll wrote:
> > core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala, line 81
> > 
> >
> > Should we also consider reassignments that are in-progress as "errors"?
> > 
> > The reasoning is that you'd like to do the following in (say) Ansible: 
> > "Trigger reassignment of partitions, wait until all are completed, and only 
> > then continue with the next action."
> > 
> > That being said, there are other ways to achieve this in tools like 
> > Ansible.  For instance, you can trigger reassignents, then repeatedly call 
> > `--verify` in a loop and capture its STDOUT, looking for "is still in 
> > progress" and "failed".  However, this is arguably more error prone because 
> > the log messages can change between Kafka versions (and oftentimes such 
> > changes are not prominently advertised, so you only notice this once your 
> > deployment script breaks).
> 
> Manikumar Reddy O wrote:
> Yes, we can consider in-progress as "errors". Other option could be 
> returning a different error code. Let us wait for others suggestions/concerns.

I wouldn't consider in-progress as error, since in-progress is pretty much the 
expected behavior here (reassigning takes a while and the tool exits 
immediately).
As you mentioned, the ansible script should include triggering re-assignment 
and then polling periodically until its completed. (like you would for HDFS 
rebalance, Oozie job, MR job, etc)

Perhaps just commenting on --verify output that this is a public API and should 
never change would be ok?


- Gwen


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


On July 10, 2015, 4:28 p.m., Manikumar Reddy O wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34641/
> ---
> 
> (Updated July 10, 2015, 4:28 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2214
> https://issues.apache.org/jira/browse/KAFKA-2214
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> rebase
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala 
> ea345895a52977c25bff57e95e12b8662331d7fe 
> 
> Diff: https://reviews.apache.org/r/34641/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

Re: Review Request 34641: Patch for KAFKA-2214

[Gwen Shapira]

> On May 26, 2015, 7:08 a.m., Michael Noll wrote:
> > core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala, line 81
> > 
> >
> > Should we also consider reassignments that are in-progress as "errors"?
> > 
> > The reasoning is that you'd like to do the following in (say) Ansible: 
> > "Trigger reassignment of partitions, wait until all are completed, and only 
> > then continue with the next action."
> > 
> > That being said, there are other ways to achieve this in tools like 
> > Ansible.  For instance, you can trigger reassignents, then repeatedly call 
> > `--verify` in a loop and capture its STDOUT, looking for "is still in 
> > progress" and "failed".  However, this is arguably more error prone because 
> > the log messages can change between Kafka versions (and oftentimes such 
> > changes are not prominently advertised, so you only notice this once your 
> > deployment script breaks).
> 
> Manikumar Reddy O wrote:
> Yes, we can consider in-progress as "errors". Other option could be 
> returning a different error code. Let us wait for others suggestions/concerns.
> 
> Gwen Shapira wrote:
> I wouldn't consider in-progress as error, since in-progress is pretty 
> much the expected behavior here (reassigning takes a while and the tool exits 
> immediately).
> As you mentioned, the ansible script should include triggering 
> re-assignment and then polling periodically until its completed. (like you 
> would for HDFS rebalance, Oozie job, MR job, etc)
> 
> Perhaps just commenting on --verify output that this is a public API and 
> should never change would be ok?

Sorry, I got confused. You meant - triggering a new re-assignment while the old 
one is still in progress. I agree this should be an error.
(but I still think the ansible script should use --verify, otherwise you risk 
triggering a new re-assignment accidentally)


- Gwen


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


On July 10, 2015, 4:28 p.m., Manikumar Reddy O wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34641/
> ---
> 
> (Updated July 10, 2015, 4:28 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2214
> https://issues.apache.org/jira/browse/KAFKA-2214
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> rebase
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala 
> ea345895a52977c25bff57e95e12b8662331d7fe 
> 
> Diff: https://reviews.apache.org/r/34641/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

Re: Review Request 36345: Patch for KAFKA-2322

[Jiangjie Qin]

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

Ship it!


LGTM.

- Jiangjie Qin


On July 9, 2015, 9:05 a.m., Ismael Juma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36345/
> ---
> 
> (Updated July 9, 2015, 9:05 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2322
> https://issues.apache.org/jira/browse/KAFKA-2322
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> kafka-2322; Use multi-catch to reduce redundancy
> 
> 
> kafka-2322; Use try with resources instead of try/finally
> 
> It's more concise and handles the exception from `close`
> better.
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/clients/ClientUtils.java 
> 0d68bf1e1e90fe9d5d4397ddf817b9a9af8d9f7a 
>   clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java 
> 9ebda5eae5936a6b0897e74cfb231803c9d6a2da 
>   clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java 
> 15d00d4e484bb5d51a9ae6857ed6e024a2cc1820 
>   clients/src/main/java/org/apache/kafka/clients/Metadata.java 
> 0387f2602c93a62cd333f1b3c569ca6b66b5b779 
>   clients/src/main/java/org/apache/kafka/clients/NetworkClient.java 
> 48fe7961e2215372d8033ece4af739ea06c6457b 
>   clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java 
> daff34db5bf2144e9dc274b23dc56b88f4efafdc 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRecords.java 
> eb75d2e797e3aa3992e4cf74b12f51c8f1545e02 
>   clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 
> 7aa076084c894bb8f47b9df2c086475b06f47060 
>   clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 
> 46e26a665a22625d50888efa7b53472279f36e79 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java
>  c1c8172cd45f6715262f9a6f497a7b1797a834a3 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
>  695eaf63db9a5fa20dc2ca68957901462a96cd96 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFuture.java
>  13fc9af7392b4ade958daf3b0c9a165ddda351a6 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/SubscriptionState.java
>  683745304c671952ff566f23b5dd4cf3ab75377a 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 
>   clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java 
> 36e7ffa2a0a0b9bfaa41c22feb1be8ae476ab321 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java 
> aa264202f2724907924985a5ecbe74afc4c6c04b 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java
>  4cb1e50d6c4ed55241aeaef1d3af09def5274103 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
>  a152bd7697dca55609a9ec4cfe0a82c10595fbc3 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java
>  06182db1c3a5da85648199b4c0c98b80ea7c6c0c 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 
> 0baf16e55046a2f49f6431e01d52c323c95eddf0 
>   
> clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java 
> 13f4d5958052afcc8ad66eacbcae50b6fd149398 
>   clients/src/main/java/org/apache/kafka/common/Cluster.java 
> 60594a7dce90130911a626ea80cf80d815aeb46e 
>   clients/src/main/java/org/apache/kafka/common/MetricName.java 
> 04b4a09badd5157a426812b78d491248a4d38fba 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 
> bae528d31516679bed88ee61b408f209f185a8cc 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 
> 4170bcc7def5b50d8aa20e8e84089c35b705b527 
>   clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 
> 6b9590c418aedd2727544c5dd23c017b4b72467a 
>   clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 
> 5f6caf957e3bd3789e575236b00b3996cd7731c2 
>   clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 
> ca823fd4639523018311b814fde69b6177e73b97 
>   
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java 
> 78c93e88fa0b886b8a618e80dfd86ff53f753507 
>   
> clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java 
> b341b7daaa10204906d78b812fb05fd27bc69373 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> aaf60c98c2c0f4513a8d65ee0db67953a529d598 
>   clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
> 4c0ecc3badd99727b5bd9d430364e61c184e0923 
>   
> clients/src/main/java/org/apache/kafka/common/protocol/Secu

Re: Review Request 36345: Patch for KAFKA-2322

[Mayuresh Gharat]

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

Ship it!


Ship It!

- Mayuresh Gharat


On July 9, 2015, 9:05 a.m., Ismael Juma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36345/
> ---
> 
> (Updated July 9, 2015, 9:05 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2322
> https://issues.apache.org/jira/browse/KAFKA-2322
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> kafka-2322; Use multi-catch to reduce redundancy
> 
> 
> kafka-2322; Use try with resources instead of try/finally
> 
> It's more concise and handles the exception from `close`
> better.
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/clients/ClientUtils.java 
> 0d68bf1e1e90fe9d5d4397ddf817b9a9af8d9f7a 
>   clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java 
> 9ebda5eae5936a6b0897e74cfb231803c9d6a2da 
>   clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java 
> 15d00d4e484bb5d51a9ae6857ed6e024a2cc1820 
>   clients/src/main/java/org/apache/kafka/clients/Metadata.java 
> 0387f2602c93a62cd333f1b3c569ca6b66b5b779 
>   clients/src/main/java/org/apache/kafka/clients/NetworkClient.java 
> 48fe7961e2215372d8033ece4af739ea06c6457b 
>   clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java 
> daff34db5bf2144e9dc274b23dc56b88f4efafdc 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRecords.java 
> eb75d2e797e3aa3992e4cf74b12f51c8f1545e02 
>   clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 
> 7aa076084c894bb8f47b9df2c086475b06f47060 
>   clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 
> 46e26a665a22625d50888efa7b53472279f36e79 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java
>  c1c8172cd45f6715262f9a6f497a7b1797a834a3 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
>  695eaf63db9a5fa20dc2ca68957901462a96cd96 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFuture.java
>  13fc9af7392b4ade958daf3b0c9a165ddda351a6 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/SubscriptionState.java
>  683745304c671952ff566f23b5dd4cf3ab75377a 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 
>   clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java 
> 36e7ffa2a0a0b9bfaa41c22feb1be8ae476ab321 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java 
> aa264202f2724907924985a5ecbe74afc4c6c04b 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java
>  4cb1e50d6c4ed55241aeaef1d3af09def5274103 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
>  a152bd7697dca55609a9ec4cfe0a82c10595fbc3 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java
>  06182db1c3a5da85648199b4c0c98b80ea7c6c0c 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 
> 0baf16e55046a2f49f6431e01d52c323c95eddf0 
>   
> clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java 
> 13f4d5958052afcc8ad66eacbcae50b6fd149398 
>   clients/src/main/java/org/apache/kafka/common/Cluster.java 
> 60594a7dce90130911a626ea80cf80d815aeb46e 
>   clients/src/main/java/org/apache/kafka/common/MetricName.java 
> 04b4a09badd5157a426812b78d491248a4d38fba 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 
> bae528d31516679bed88ee61b408f209f185a8cc 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 
> 4170bcc7def5b50d8aa20e8e84089c35b705b527 
>   clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 
> 6b9590c418aedd2727544c5dd23c017b4b72467a 
>   clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 
> 5f6caf957e3bd3789e575236b00b3996cd7731c2 
>   clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 
> ca823fd4639523018311b814fde69b6177e73b97 
>   
> clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java 
> 78c93e88fa0b886b8a618e80dfd86ff53f753507 
>   
> clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java 
> b341b7daaa10204906d78b812fb05fd27bc69373 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> aaf60c98c2c0f4513a8d65ee0db67953a529d598 
>   clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
> 4c0ecc3badd99727b5bd9d430364e61c184e0923 
>   
> clients/src/main/java/org/apache/kafka/common/protoco

Re: Review Request 36333: Patch for KAFKA-2123

[Ewen Cheslack-Postava]

> On July 9, 2015, 1:42 a.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, 
> > line 775
> > 
> >
> > I think KAFKA-1894 is already fixed in this patch + KAFKA-2168?
> 
> Jason Gustafson wrote:
> I think this is still debatable. Is wakeup() sufficient to assuage our 
> guilt for allowing poll to block indefinitely in spite of the passed timeout? 
> Perhaps I'm the only one, but I'm still holding out hope that we'll be able 
> to enforce the timeout even if we are in the middle of a join group. The code 
> is actually not that far from being able to do so.
> 
> Guozhang Wang wrote:
> Yeah makes sense.

I'm still holding out hope as well :) There were one or two places in this 
patch where timeouts aren't passed into some methods where I think they could 
be passed in and respsected safely, but I didn't want to make this patch even 
bigger.


- Ewen


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


On July 8, 2015, 9:19 p.m., Jason Gustafson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36333/
> ---
> 
> (Updated July 8, 2015, 9:19 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2123
> https://issues.apache.org/jira/browse/KAFKA-2123
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2123; resolve problems from rebase
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java 
> fd98740bff175cc9d5bc02e365d88e011ef65d22 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerCommitCallback.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRecords.java 
> eb75d2e797e3aa3992e4cf74b12f51c8f1545e02 
>   clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 
> 7aa076084c894bb8f47b9df2c086475b06f47060 
>   clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 
> 46e26a665a22625d50888efa7b53472279f36e79 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClient.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java
>  c1c8172cd45f6715262f9a6f497a7b1797a834a3 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/DelayedTask.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/DelayedTaskQueue.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
>  695eaf63db9a5fa20dc2ca68957901462a96cd96 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Heartbeat.java
>  51eae1944d5c17cf838be57adf560bafe36fbfbd 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoAvailableBrokersException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFuture.java
>  13fc9af7392b4ade958daf3b0c9a165ddda351a6 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFutureAdapter.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFutureListener.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/SendFailedException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/StaleMetadataException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/SubscriptionState.java
>  683745304c671952ff566f23b5dd4cf3ab75377a 
>   
> clients/src/main/java/org/apache/kafka/common/errors/ConsumerCoordinatorNotAvailableException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/errors/DisconnectException.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/errors/IllegalGenerationException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/errors/NotCoordinatorForConsumerException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/errors/OffsetLoadInProgressException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/errors/UnknownConsumerIdException.java
>  PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
> 4c0ecc3badd99727b5bd9d430364e61c184e0923 
>   
> clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClientTest.java
>  PRE-CREATION 
>   
> clients/src/test/java/org/apache/kafka/clients/consumer/internals/CoordinatorTest.

Re: Review Request 36333: Patch for KAFKA-2123

[Gwen Shapira]

> On July 9, 2015, 1:42 a.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, 
> > line 775
> > 
> >
> > I think KAFKA-1894 is already fixed in this patch + KAFKA-2168?
> 
> Jason Gustafson wrote:
> I think this is still debatable. Is wakeup() sufficient to assuage our 
> guilt for allowing poll to block indefinitely in spite of the passed timeout? 
> Perhaps I'm the only one, but I'm still holding out hope that we'll be able 
> to enforce the timeout even if we are in the middle of a join group. The code 
> is actually not that far from being able to do so.
> 
> Guozhang Wang wrote:
> Yeah makes sense.
> 
> Ewen Cheslack-Postava wrote:
> I'm still holding out hope as well :) There were one or two places in 
> this patch where timeouts aren't passed into some methods where I think they 
> could be passed in and respsected safely, but I didn't want to make this 
> patch even bigger.

Same here :) Is there a JIRA to track this plan?


- Gwen


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


On July 8, 2015, 9:19 p.m., Jason Gustafson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36333/
> ---
> 
> (Updated July 8, 2015, 9:19 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2123
> https://issues.apache.org/jira/browse/KAFKA-2123
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2123; resolve problems from rebase
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java 
> fd98740bff175cc9d5bc02e365d88e011ef65d22 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerCommitCallback.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRecords.java 
> eb75d2e797e3aa3992e4cf74b12f51c8f1545e02 
>   clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 
> 7aa076084c894bb8f47b9df2c086475b06f47060 
>   clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 
> 46e26a665a22625d50888efa7b53472279f36e79 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClient.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java
>  c1c8172cd45f6715262f9a6f497a7b1797a834a3 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/DelayedTask.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/DelayedTaskQueue.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
>  695eaf63db9a5fa20dc2ca68957901462a96cd96 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Heartbeat.java
>  51eae1944d5c17cf838be57adf560bafe36fbfbd 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoAvailableBrokersException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFuture.java
>  13fc9af7392b4ade958daf3b0c9a165ddda351a6 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFutureAdapter.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFutureListener.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/SendFailedException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/StaleMetadataException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/SubscriptionState.java
>  683745304c671952ff566f23b5dd4cf3ab75377a 
>   
> clients/src/main/java/org/apache/kafka/common/errors/ConsumerCoordinatorNotAvailableException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/errors/DisconnectException.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/errors/IllegalGenerationException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/errors/NotCoordinatorForConsumerException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/errors/OffsetLoadInProgressException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/errors/UnknownConsumerIdException.java
>  PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
> 4c0ecc3badd99727b5bd9d430364e61c184e0923 
>   
> clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClientTest.java
>  PRE-CREA

[jira] [Commented] (KAFKA-2198) kafka-topics.sh exits with 0 status on failures

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-2198:


[~gwenshap] Thanks for the review.  Sample output is given below.

{code}
sh kafka-topics.sh --alter --topic UNKNOWN -config min.insync.replicas=2 
--zookeeper localhost && echo succeeded
Error while executing topic command : Topic UNKNOWN does not exist on ZK path 
localhost
[2015-07-10 22:59:01,808] ERROR java.lang.IllegalArgumentException: Topic 
UNKNOWN does not exist on ZK path localhost
at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:104)
at kafka.admin.TopicCommand$.main(TopicCommand.scala:56)
at kafka.admin.TopicCommand.main(TopicCommand.scala)
 (kafka.admin.TopicCommand$)
{code}
{code}
$ sh kafka-topics.sh --alter --topic EVENT --config min.insync.replicasTYPO=2 
--zookeeper localhost && echo succeeded
Error while executing topic command : requirement failed: Unknown configuration 
"min.insync.replicasTYPO".
[2015-07-10 23:02:04,273] ERROR java.lang.IllegalArgumentException: requirement 
failed: Unknown configuration "min.insync.replicasTYPO".
at scala.Predef$.require(Predef.scala:233)
at 
kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:185)
at 
kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:184)
at scala.collection.Iterator$class.foreach(Iterator.scala:727)
at scala.collection.AbstractIterator.foreach(Iterator.scala:1157)
at kafka.log.LogConfig$.validateNames(LogConfig.scala:184)
at kafka.log.LogConfig$.validate(LogConfig.scala:192)
at 
kafka.admin.TopicCommand$.parseTopicConfigsToBeAdded(TopicCommand.scala:217)
at 
kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:110)
at 
kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:107)
at 
scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:107)
at kafka.admin.TopicCommand$.main(TopicCommand.scala:56)
at kafka.admin.TopicCommand.main(TopicCommand.scala)
 (kafka.admin.TopicCommand$)
{code}

> kafka-topics.sh exits with 0 status on failures
> ---
>
> Key: KAFKA-2198
> URL: https://issues.apache.org/jira/browse/KAFKA-2198
> Project: Kafka
>  Issue Type: Bug
>  Components: admin
>Affects Versions: 0.8.2.1
>Reporter: Bob Halley
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2198.patch, KAFKA-2198_2015-05-19_18:27:01.patch, 
> KAFKA-2198_2015-05-19_18:41:25.patch, KAFKA-2198_2015-07-10_22:02:02.patch
>
>
> In the two failure cases below, kafka-topics.sh exits with status 0.  You 
> shouldn't need to parse output from the command to know if it failed or not.
> Case 1: Forgetting to add Kafka zookeeper chroot path to zookeeper spec
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicas=2 
> --zookeeper 10.0.0.1 && echo succeeded
> succeeded
> Case 2: Bad config option.  (Also, do we really need the java backtrace?  
> It's a lot of noise most of the time.)
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicasTYPO=2 
> --zookeeper 10.0.0.1/kafka && echo succeeded
> Error while executing topic command requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> java.lang.IllegalArgumentException: requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> at scala.Predef$.require(Predef.scala:233)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:183)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:182)
> at scala.collection.Iterator$class.foreach(Iterator.scala:727)
> at scala.collection.AbstractIterator.foreach(Iterator.scala:1157)
> at kafka.log.LogConfig$.validateNames(LogConfig.scala:182)
> at kafka.log.LogConfig$.validate(LogConfig.scala:190)
> at 
> kafka.admin.TopicCommand$.parseTopicConfigsToBeAdded(TopicCommand.scala:205)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:103)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:100)
> at 
> scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
> at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
> at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:100)
> at kafka.admin.TopicCommand$.main(TopicCommand.scala:57)
> at kafka.admin.TopicCommand.main(TopicCommand.scala)
> succeeded



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

Re: Review Request 34403: Patch for KAFKA-2198

[Manikumar Reddy O]

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

(Updated July 10, 2015, 5:44 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

Addressing Gwen's comments


Diffs (updated)
-

  core/src/main/scala/kafka/admin/TopicCommand.scala 
a2ecb9620d647bf8f957a1f00f52896438e804a7 

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


Testing
---


Thanks,

Manikumar Reddy O

[jira] [Updated] (KAFKA-2198) kafka-topics.sh exits with 0 status on failures

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy updated KAFKA-2198:
---
Attachment: KAFKA-2198_2015-07-10_23:11:23.patch

> kafka-topics.sh exits with 0 status on failures
> ---
>
> Key: KAFKA-2198
> URL: https://issues.apache.org/jira/browse/KAFKA-2198
> Project: Kafka
>  Issue Type: Bug
>  Components: admin
>Affects Versions: 0.8.2.1
>Reporter: Bob Halley
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2198.patch, KAFKA-2198_2015-05-19_18:27:01.patch, 
> KAFKA-2198_2015-05-19_18:41:25.patch, KAFKA-2198_2015-07-10_22:02:02.patch, 
> KAFKA-2198_2015-07-10_23:11:23.patch
>
>
> In the two failure cases below, kafka-topics.sh exits with status 0.  You 
> shouldn't need to parse output from the command to know if it failed or not.
> Case 1: Forgetting to add Kafka zookeeper chroot path to zookeeper spec
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicas=2 
> --zookeeper 10.0.0.1 && echo succeeded
> succeeded
> Case 2: Bad config option.  (Also, do we really need the java backtrace?  
> It's a lot of noise most of the time.)
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicasTYPO=2 
> --zookeeper 10.0.0.1/kafka && echo succeeded
> Error while executing topic command requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> java.lang.IllegalArgumentException: requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> at scala.Predef$.require(Predef.scala:233)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:183)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:182)
> at scala.collection.Iterator$class.foreach(Iterator.scala:727)
> at scala.collection.AbstractIterator.foreach(Iterator.scala:1157)
> at kafka.log.LogConfig$.validateNames(LogConfig.scala:182)
> at kafka.log.LogConfig$.validate(LogConfig.scala:190)
> at 
> kafka.admin.TopicCommand$.parseTopicConfigsToBeAdded(TopicCommand.scala:205)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:103)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:100)
> at 
> scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
> at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
> at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:100)
> at kafka.admin.TopicCommand$.main(TopicCommand.scala:57)
> at kafka.admin.TopicCommand.main(TopicCommand.scala)
> succeeded



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

[jira] [Commented] (KAFKA-2198) kafka-topics.sh exits with 0 status on failures

[Manikumar Reddy (JIRA)]

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

Manikumar Reddy commented on KAFKA-2198:


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

> kafka-topics.sh exits with 0 status on failures
> ---
>
> Key: KAFKA-2198
> URL: https://issues.apache.org/jira/browse/KAFKA-2198
> Project: Kafka
>  Issue Type: Bug
>  Components: admin
>Affects Versions: 0.8.2.1
>Reporter: Bob Halley
>Assignee: Manikumar Reddy
> Attachments: KAFKA-2198.patch, KAFKA-2198_2015-05-19_18:27:01.patch, 
> KAFKA-2198_2015-05-19_18:41:25.patch, KAFKA-2198_2015-07-10_22:02:02.patch, 
> KAFKA-2198_2015-07-10_23:11:23.patch
>
>
> In the two failure cases below, kafka-topics.sh exits with status 0.  You 
> shouldn't need to parse output from the command to know if it failed or not.
> Case 1: Forgetting to add Kafka zookeeper chroot path to zookeeper spec
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicas=2 
> --zookeeper 10.0.0.1 && echo succeeded
> succeeded
> Case 2: Bad config option.  (Also, do we really need the java backtrace?  
> It's a lot of noise most of the time.)
> $ kafka-topics.sh --alter --topic foo --config min.insync.replicasTYPO=2 
> --zookeeper 10.0.0.1/kafka && echo succeeded
> Error while executing topic command requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> java.lang.IllegalArgumentException: requirement failed: Unknown configuration 
> "min.insync.replicasTYPO".
> at scala.Predef$.require(Predef.scala:233)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:183)
> at kafka.log.LogConfig$$anonfun$validateNames$1.apply(LogConfig.scala:182)
> at scala.collection.Iterator$class.foreach(Iterator.scala:727)
> at scala.collection.AbstractIterator.foreach(Iterator.scala:1157)
> at kafka.log.LogConfig$.validateNames(LogConfig.scala:182)
> at kafka.log.LogConfig$.validate(LogConfig.scala:190)
> at 
> kafka.admin.TopicCommand$.parseTopicConfigsToBeAdded(TopicCommand.scala:205)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:103)
> at 
> kafka.admin.TopicCommand$$anonfun$alterTopic$1.apply(TopicCommand.scala:100)
> at 
> scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
> at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
> at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:100)
> at kafka.admin.TopicCommand$.main(TopicCommand.scala:57)
> at kafka.admin.TopicCommand.main(TopicCommand.scala)
> succeeded



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

Re: Review Request 34403: Patch for KAFKA-2198

[Manikumar Reddy O]

> On July 10, 2015, 4:46 p.m., Gwen Shapira wrote:
> > core/src/main/scala/kafka/admin/TopicCommand.scala, lines 72-73
> > 
> >
> > This is a bit unclean. I think its more idiomatic when the catch block 
> > includes the System.exit(1).
> > 
> > Also, I'm afraid that printing the entire stack trace is intimidating 
> > to non-developers who use the CLI. Perhaps the stack trace should go under 
> > log.error(...)?

Calling System.exit(1) in catch block results unexecuted finally block. 

http://stackoverflow.com/questions/1410951/how-does-javas-system-exit-work-with-try-catch-finally-blocks
http://stackoverflow.com/questions/15264076/regarding-excuting-finally-block-in-system-exit-case-also-by-adding-shutdownhook?lq=1

log.error() used for printing stackTrace.


- Manikumar Reddy


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


On July 10, 2015, 5:44 p.m., Manikumar Reddy O wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34403/
> ---
> 
> (Updated July 10, 2015, 5:44 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2198
> https://issues.apache.org/jira/browse/KAFKA-2198
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Addressing Gwen's comments
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/TopicCommand.scala 
> a2ecb9620d647bf8f957a1f00f52896438e804a7 
> 
> Diff: https://reviews.apache.org/r/34403/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

Re: Review Request 33378: Patch for KAFKA-2136

[Joel Koshy]

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


LGTM - just a few minor comments.


clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 
(line 101)


Would prefer seeing this in this style:
```
func() {
  statement;
}
```



clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java
 (line 68)


EPSILON?



core/src/main/scala/kafka/api/FetchResponse.scala (line 169)


A: B



core/src/main/scala/kafka/api/FetchResponse.scala (line 170)


A: B



core/src/main/scala/kafka/api/FetchResponse.scala (line 179)


Prefer placing throttleTimeSize on the next line.



core/src/main/scala/kafka/api/FetchResponse.scala (line 229)


The throttleTimeSize...
(However, see following comment)



core/src/main/scala/kafka/api/FetchResponse.scala (line 230)


Just wondering if we should move all this to `FetchResponse` class (not 
object).

i.e., add `headerSize` and `writeHeaderTo(buffer)` member methods since the 
throttle-time is really part of the header. Depending on the version, those two 
methods can then do the right thing. This code will then just become:
```
private val buffer = ByteBuffer.allocate(4 + fetchResponse.headerSize)
fetchResponse.writeHeaderTo(buffer)
```



core/src/main/scala/kafka/server/DelayedFetch.scala (line 135)


For these, I'm wondering if we should put in the actual delay and in 
KAFKA-2136 just add a config to enable/disable quotas altogether.



core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala (line 81)


delayTime: Int


- Joel Koshy


On July 1, 2015, 2:44 a.m., Aditya Auradkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33378/
> ---
> 
> (Updated July 1, 2015, 2:44 a.m.)
> 
> 
> Review request for kafka, Joel Koshy and Jun Rao.
> 
> 
> Bugs: KAFKA-2136
> https://issues.apache.org/jira/browse/KAFKA-2136
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Changes are
> - protocol changes to the fetch request and response to return the 
> throttle_time_ms to clients
> - New producer/consumer metrics to expose the avg and max delay time for a 
> client
> - Test cases.
> - Addressed Joel's comments
> 
> For now the patch will publish a zero delay and return a response
> 
> 
> Diffs
> -
> 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
>  56281ee15cc33dfc96ff64d5b1e596047c7132a4 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 
> 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd 
>   clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 
> 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 
>   clients/src/main/java/org/apache/kafka/common/requests/FetchRequest.java 
> 8686d83aa52e435c6adafbe9ff4bd1602281072a 
>   clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java 
> eb8951fba48c335095cc43fc3672de1c733e07ff 
>   clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java 
> fabeae3083a8ea55cdacbb9568f3847ccd85bab4 
>   clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 
> 37ec0b79beafcf5735c386b066eb319fb697eff5 
>   
> clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java
>  419541011d652becf0cda7a5e62ce813cddb1732 
>   
> clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java
>  8b1805d3d2bcb9fe2bacb37d870c3236aa9532c4 
>   
> clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java
>  e3cc1967e407b64cc734548c19e30de700b64ba8 
>   core/src/main/scala/kafka/api/FetchRequest.scala 
> 5b38f8554898e54800abd65a7415dd0ac41fd958 
>   core/src/main/scala/kafka/api/FetchResponse.scala 
> 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 
>   core/src/main/scala/kafka/api/ProducerRequest.scala 
> c866180d3680da03e48d374415f10220f6ca68c4 
>   core/src/main/scala/kafka/api/ProducerResponse.scala 
> 5d1fac4cb8943f5bfaa487f8e9d9d2856efbd330 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 
> c16f7edd322709060e54c77eb505c44cbd77a4ec 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala 
> 83fc47417dd7fe4edf030217fa7f

Re: Review Request 33378: Patch for KAFKA-2136

[Joel Koshy]

> On June 25, 2015, 10:55 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/AbstractFetcherThread.scala, line 40
> > 
> >
> > I think we should add throttle time metrics to the old producer and 
> > consumer as well. What do you think?
> 
> Aditya Auradkar wrote:
> I think that sounds reasonable.. I initially decided against it in my 
> patch because I thought of this as an incentive to upgrade. Any concerns if I 
> submit a subsequent RB for this immediately after this is committed?

I think it is definitely something that we will need (for users that are still 
on old clients). So can you either create a separate jira labeled as quotas or 
do that as part of this patch?


- Joel


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


On July 1, 2015, 2:44 a.m., Aditya Auradkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33378/
> ---
> 
> (Updated July 1, 2015, 2:44 a.m.)
> 
> 
> Review request for kafka, Joel Koshy and Jun Rao.
> 
> 
> Bugs: KAFKA-2136
> https://issues.apache.org/jira/browse/KAFKA-2136
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Changes are
> - protocol changes to the fetch request and response to return the 
> throttle_time_ms to clients
> - New producer/consumer metrics to expose the avg and max delay time for a 
> client
> - Test cases.
> - Addressed Joel's comments
> 
> For now the patch will publish a zero delay and return a response
> 
> 
> Diffs
> -
> 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
>  56281ee15cc33dfc96ff64d5b1e596047c7132a4 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 
> 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd 
>   clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 
> 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 
>   clients/src/main/java/org/apache/kafka/common/requests/FetchRequest.java 
> 8686d83aa52e435c6adafbe9ff4bd1602281072a 
>   clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java 
> eb8951fba48c335095cc43fc3672de1c733e07ff 
>   clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java 
> fabeae3083a8ea55cdacbb9568f3847ccd85bab4 
>   clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java 
> 37ec0b79beafcf5735c386b066eb319fb697eff5 
>   
> clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java
>  419541011d652becf0cda7a5e62ce813cddb1732 
>   
> clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java
>  8b1805d3d2bcb9fe2bacb37d870c3236aa9532c4 
>   
> clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java
>  e3cc1967e407b64cc734548c19e30de700b64ba8 
>   core/src/main/scala/kafka/api/FetchRequest.scala 
> 5b38f8554898e54800abd65a7415dd0ac41fd958 
>   core/src/main/scala/kafka/api/FetchResponse.scala 
> 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 
>   core/src/main/scala/kafka/api/ProducerRequest.scala 
> c866180d3680da03e48d374415f10220f6ca68c4 
>   core/src/main/scala/kafka/api/ProducerResponse.scala 
> 5d1fac4cb8943f5bfaa487f8e9d9d2856efbd330 
>   core/src/main/scala/kafka/consumer/SimpleConsumer.scala 
> c16f7edd322709060e54c77eb505c44cbd77a4ec 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala 
> 83fc47417dd7fe4edf030217fa7fd69d99b170b0 
>   core/src/main/scala/kafka/server/DelayedFetch.scala 
> de6cf5bdaa0e70394162febc63b50b55ca0a92db 
>   core/src/main/scala/kafka/server/DelayedProduce.scala 
> 05078b24ef28f2f4e099afa943e43f1d00359fda 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd 
>   core/src/main/scala/kafka/server/OffsetManager.scala 
> 5cca85cf727975f6d3acb2223fd186753ad761dc 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 
> b31b432a226ba79546dd22ef1d2acbb439c2e9a3 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 
>   core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 
> 5717165f2344823fabe8f7cfafae4bb8af2d949a 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
> 00d59337a99ac135e8689bd1ecd928f7b1423d79 
> 
> Diff: https://reviews.apache.org/r/33378/diff/
> 
> 
> Testing
> ---
> 
> New tests added
> 
> 
> Thanks,
> 
> Aditya Auradkar
> 
>

Re: Review Request 35880: Patch for KAFKA-2295

[Guozhang Wang]

> On July 6, 2015, 3:50 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/utils/CoreUtils.scala, line 221
> > 
> >
> > Can we use scala's Utils.createObject here?
> 
> Manikumar Reddy O wrote:
> Are you sugessting to use Utils.newInstance()? We dont have scala's 
> Utils.scala. Am I missing something?

My bad, this is not a issue.


- Guozhang


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


On July 6, 2015, 6:05 a.m., Manikumar Reddy O wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35880/
> ---
> 
> (Updated July 6, 2015, 6:05 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2295
> https://issues.apache.org/jira/browse/KAFKA-2295
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Addessing Guozhang's comments
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 
> bae528d31516679bed88ee61b408f209f185a8cc 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 
> 4170bcc7def5b50d8aa20e8e84089c35b705b527 
>   clients/src/main/java/org/apache/kafka/common/utils/Utils.java 
> af9993cf9b3991f1e61e1201c94e19bc1bf76a68 
>   
> clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java 
> db1b0ee9113215b5ad7fda0f93915f3bdd34ac55 
>   core/src/main/scala/kafka/utils/CoreUtils.scala 
> 168a18d380c200ee566eccb6988dd1ae85ed5b09 
> 
> Diff: https://reviews.apache.org/r/35880/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

Re: Review Request 35880: Patch for KAFKA-2295

[Guozhang Wang]

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

Ship it!


Ship It!

- Guozhang Wang


On July 6, 2015, 6:05 a.m., Manikumar Reddy O wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35880/
> ---
> 
> (Updated July 6, 2015, 6:05 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2295
> https://issues.apache.org/jira/browse/KAFKA-2295
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Addessing Guozhang's comments
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 
> bae528d31516679bed88ee61b408f209f185a8cc 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 
> 4170bcc7def5b50d8aa20e8e84089c35b705b527 
>   clients/src/main/java/org/apache/kafka/common/utils/Utils.java 
> af9993cf9b3991f1e61e1201c94e19bc1bf76a68 
>   
> clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java 
> db1b0ee9113215b5ad7fda0f93915f3bdd34ac55 
>   core/src/main/scala/kafka/utils/CoreUtils.scala 
> 168a18d380c200ee566eccb6988dd1ae85ed5b09 
> 
> Diff: https://reviews.apache.org/r/35880/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

[Discussion] Limitations on topic names

[Gwen Shapira]

Hi Kafka Fans,

If you have one topic named "kafka_lab_2" and the other named
"kafka.lab.2", the topic level metrics will be named kafka_lab_2 for
both, effectively making it impossible to monitor them properly.

The reason this happens is that using "." in topic names is pretty
common, especially as a way to group topics into data centers,
relevant apps, etc - basically a work-around to our current lack of
name spaces. However, most metric monitoring systems using "." to
annotate hierarchy, so to avoid issues around metric names, Kafka
replaces the "." in the name with an underscore.

This generates good metric names, but creates the problem with name collisions.

I'm wondering if it makes sense to simply limit the range of
characters permitted in a topic name and disallow "_"? Obviously
existing topics will need to remain as is, which is a bit awkward.

If anyone has better backward-compatible solutions to this, I'm all ears :)

Gwen

Re: How "low-level" management of threads is justified in Mirror Maker?

[Jiangjie Qin]

Hi Kostya,

I actually did not think about this too much when write the code.
My gut feeling is that the benefit of using a thread pool is that you have
jobs come and go and want to safe your own resource management overhead.
In Mirror maker, there are only fixed number of mirror maker thread and
all of them are long running.
In that case there might not be too much difference between using Thread
explicitly vs thread pool + runnable. Extending Thread might make some of
the logic a little bit more explicit. Is there anything specific that
makes you want to move to runnables?

Thanks,

Jiangjie (Becket) Qin

On 7/9/15, 4:17 AM, "Kostya Golikov"  wrote:

>I've skimmed through a source code of Mirror Maker and I see that it
>relies
>on raw threads
>e3da/core/src/main/scala/kafka/tools/MirrorMaker.scala#L285-L287>,
>as opposed to "Runnable + Thread pool" combination. While former approach
>is just as functional as later, I am quite curious why such decision was
>done. Anything specific that prevents move to runnables?

[jira] [Commented] (KAFKA-2295) Dynamically loaded classes (encoders, etc.) may not be found by Kafka Producer

[Guozhang Wang (JIRA)]

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

Guozhang Wang commented on KAFKA-2295:
--

[~tdas] The latest patch from [~omkreddy] looks good to me, do you want to 
apply it and see if it resolves your Spark usecase?

Guozhang

> Dynamically loaded classes (encoders, etc.) may not be found by Kafka 
> Producer 
> ---
>
> Key: KAFKA-2295
> URL: https://issues.apache.org/jira/browse/KAFKA-2295
> Project: Kafka
>  Issue Type: Bug
>  Components: producer 
>Reporter: Tathagata Das
>Assignee: Manikumar Reddy
> Fix For: 0.9.0
>
> Attachments: KAFKA-2295.patch, KAFKA-2295_2015-07-06_11:32:58.patch
>
>
> Kafka Producer (via CoreUtils.createObject) effectively uses Class.forName to 
> load encoder classes. Class.forName is by design finds classes only in the 
> defining classloader of the enclosing class (which is often the bootstrap 
> class loader). It does not use the current thread context class loader. This 
> can lead to problems in environments where classes are dynamically loaded and 
> therefore may not be present in the bootstrap classloader.
> This leads to ClassNotFound Exceptions in environments like Spark where 
> classes are loaded dynamically using custom classloaders. Issues like this 
> have reported. E.g. - 
> https://www.mail-archive.com/user@spark.apache.org/msg30951.html
> Other references regarding this issue with Class.forName 
> http://stackoverflow.com/questions/21749741/though-my-class-was-loaded-class-forname-throws-classnotfoundexception
> This is a problem we have faced repeatedly in Apache Spark and we solved it 
> by explicitly specifying the class loader to use. See 
> https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/util/Utils.scala#L178



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

[jira] [Commented] (KAFKA-2303) Fix for KAFKA-2235 LogCleaner offset map overflow causes another compaction failures

[Jiangjie Qin (JIRA)]

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

Jiangjie Qin commented on KAFKA-2303:
-

Would the following help solve the issue?
1. Make log.cleaner.min.cleanable.ratio to be smaller
2. Make log.cleaner.backoff.ms smaller
It is supposedly make each cleaning scan over less records but do the cleaning 
more frequently. That should help a little bit.

> Fix for KAFKA-2235 LogCleaner offset map overflow causes another compaction 
> failures
> 
>
> Key: KAFKA-2303
> URL: https://issues.apache.org/jira/browse/KAFKA-2303
> Project: Kafka
>  Issue Type: Bug
>  Components: core, log
>Affects Versions: 0.8.2.1
>Reporter: Alexander Demidko
>Assignee: Jay Kreps
> Fix For: 0.8.3
>
>
> We have rolled out the patch for KAFKA-2235 to our kafka cluster, and 
> recently instead of 
> {code}
> "kafka.log.LogCleaner - [kafka-log-cleaner-thread-0], Error due to
> java.lang.IllegalArgumentException: requirement failed: Attempt to add a new 
> entry to a full offset map." 
> {code}
> we started to see 
> {code}
> kafka.log.LogCleaner - [kafka-log-cleaner-thread-0], Error due to
> java.lang.IllegalArgumentException: requirement failed: 131390902 messages in 
> segment -cgstate-8/79840768.log but offset map can 
> fit only 80530612. You can increase log.cleaner.dedupe.buffer.size or 
> decrease log.cleaner.threads
> {code}
> So, we had to roll it back to avoid disk depletion although I'm not sure if 
> it needs to be rolled back in trunk. This patch applies more strict checks 
> than were in place before: even if there is only one unique key for a 
> segment, cleanup will fail if this segment is too big. 
> Does it make sense to eliminate a limit for the offset map slots count, for 
> example to use an offset map backed by a memory mapped file?
> The limit of 80530612 slots comes from memory / bytesPerEntry, where memory 
> is Int.MaxValue (we use only one cleaner thread) and bytesPerEntry is 8 + 
> digest hash size. Might be wrong, but it seems if the overall number of 
> unique keys per partition is more than 80M slots in an OffsetMap, compaction 
> will always fail and cleaner thread will die. 



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

Re: [Discussion] Limitations on topic names

[Brock Noland]

On Fri, Jul 10, 2015 at 11:34 AM, Gwen Shapira  wrote:
> Hi Kafka Fans,
>
> If you have one topic named "kafka_lab_2" and the other named
> "kafka.lab.2", the topic level metrics will be named kafka_lab_2 for
> both, effectively making it impossible to monitor them properly.
>
> The reason this happens is that using "." in topic names is pretty
> common, especially as a way to group topics into data centers,
> relevant apps, etc - basically a work-around to our current lack of
> name spaces. However, most metric monitoring systems using "." to
> annotate hierarchy, so to avoid issues around metric names, Kafka
> replaces the "." in the name with an underscore.
>
> This generates good metric names, but creates the problem with name 
> collisions.
>
> I'm wondering if it makes sense to simply limit the range of
> characters permitted in a topic name and disallow "_"? Obviously
> existing topics will need to remain as is, which is a bit awkward.

Interesting problem! Many if not most users I personally am aware of
use "_" as a separator in topic names. I am sure that many users would
be quite surprised by this limitation. With that said, I am sure
they'd transition accordingly.

>
> If anyone has better backward-compatible solutions to this, I'm all ears :)
>
> Gwen

Re: [DISCUSS] Using GitHub Pull Requests for contributions and code review

[Guozhang Wang]

Hi Ismael,


I have a couple of comments on the wiki pages / merge scripts:


1. In the wiki page it mentions "If the change is new, then it usually
needs a new JIRA. However, trivial changes, where "what should change" is
virtually the same as "how it should change" do not require a JIRA.
Example: "Fix typos in Foo scaladoc"."


So it sounds we are going to allow pull requests without a JIRA ticket, but
later we are also mentioning:


"The PR title should be of the form [KAFKA-].." which is a bit
conflicting with the previous statement. Could you clarify it? Today our
commits are mostly with JIRAs except some trivial ones that are only done
by committers, I can either extend this to non-committers or not, just that
we need to make it clear.

2. The git commit message is a little verbose to me, for example:


--

commit ee88dbb67f19b787e12ccef37982c9459b78c7b6

Author: Geoff Anderson 

Date:   Thu Jul 9 14:58:01 2015 -0700

   KAFKA-2327; broker doesn't start if config defines advertised.host but
not advertised.port



   Added unit tests as well. These fail without the fix, but pass with the
fix.



   Author: Geoff Anderson 



   Closes #73 from granders/KAFKA-2327 and squashes the following commits:



   52a2085 [Geoff Anderson] Cleaned up unecessary toString calls

   23b3340 [Geoff Anderson] Fixes KAFKA-2327

--


The "Author" field is redundant, and we could probably also omit the github
commits. What do you think?


Guozhang

Re: [Discussion] Limitations on topic names

[Todd Palino]

I had to go look this one up again to make sure -
https://issues.apache.org/jira/browse/KAFKA-495

The only valid character names for topics are alphanumeric, underscore, and
dash. A period is not supposed to be a valid character to use. If you're
seeing them, then one of two things have happened:

1) You have topic names that are grandfathered in from before that patch
2) The patch is not working properly and there is somewhere in the broker
that the standard is not being enforced.

-Todd


On Fri, Jul 10, 2015 at 12:13 PM, Brock Noland  wrote:

> On Fri, Jul 10, 2015 at 11:34 AM, Gwen Shapira 
> wrote:
> > Hi Kafka Fans,
> >
> > If you have one topic named "kafka_lab_2" and the other named
> > "kafka.lab.2", the topic level metrics will be named kafka_lab_2 for
> > both, effectively making it impossible to monitor them properly.
> >
> > The reason this happens is that using "." in topic names is pretty
> > common, especially as a way to group topics into data centers,
> > relevant apps, etc - basically a work-around to our current lack of
> > name spaces. However, most metric monitoring systems using "." to
> > annotate hierarchy, so to avoid issues around metric names, Kafka
> > replaces the "." in the name with an underscore.
> >
> > This generates good metric names, but creates the problem with name
> collisions.
> >
> > I'm wondering if it makes sense to simply limit the range of
> > characters permitted in a topic name and disallow "_"? Obviously
> > existing topics will need to remain as is, which is a bit awkward.
>
> Interesting problem! Many if not most users I personally am aware of
> use "_" as a separator in topic names. I am sure that many users would
> be quite surprised by this limitation. With that said, I am sure
> they'd transition accordingly.
>
> >
> > If anyone has better backward-compatible solutions to this, I'm all ears
> :)
> >
> > Gwen
>

Re: [Discussion] Limitations on topic names

[Grant Henke]

kafka.common.Topic shows that currently period is a valid character and I
have verified I can use kafka-topics.sh to create a new topic with a period.


AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK currently uses
Topic.validate before writing to Zookeeper.

Should period character support be removed? I was under the same impression
as Gwen, that a period was used by many as a way to "group" topics.

The code is pasted below since its small:

object Topic {
  val legalChars = "[a-zA-Z0-9\\._\\-]"
  private val maxNameLength = 255
  private val rgx = new Regex(legalChars + "+")

  val InternalTopics = Set(OffsetManager.OffsetsTopicName)

  def validate(topic: String) {
if (topic.length <= 0)
  throw new InvalidTopicException("topic name is illegal, can't be
empty")
else if (topic.equals(".") || topic.equals(".."))
  throw new InvalidTopicException("topic name cannot be \".\" or
\"..\"")
else if (topic.length > maxNameLength)
  throw new InvalidTopicException("topic name is illegal, can't be
longer than " + maxNameLength + " characters")

rgx.findFirstIn(topic) match {
  case Some(t) =>
if (!t.equals(topic))
  throw new InvalidTopicException("topic name " + topic + " is
illegal, contains a character other than ASCII alphanumerics, '.', '_' and
'-'")
  case None => throw new InvalidTopicException("topic name " + topic +
" is illegal,  contains a character other than ASCII alphanumerics, '.',
'_' and '-'")
}
  }
}

On Fri, Jul 10, 2015 at 2:50 PM, Todd Palino  wrote:

> I had to go look this one up again to make sure -
> https://issues.apache.org/jira/browse/KAFKA-495
>
> The only valid character names for topics are alphanumeric, underscore, and
> dash. A period is not supposed to be a valid character to use. If you're
> seeing them, then one of two things have happened:
>
> 1) You have topic names that are grandfathered in from before that patch
> 2) The patch is not working properly and there is somewhere in the broker
> that the standard is not being enforced.
>
> -Todd
>
>
> On Fri, Jul 10, 2015 at 12:13 PM, Brock Noland  wrote:
>
> > On Fri, Jul 10, 2015 at 11:34 AM, Gwen Shapira 
> > wrote:
> > > Hi Kafka Fans,
> > >
> > > If you have one topic named "kafka_lab_2" and the other named
> > > "kafka.lab.2", the topic level metrics will be named kafka_lab_2 for
> > > both, effectively making it impossible to monitor them properly.
> > >
> > > The reason this happens is that using "." in topic names is pretty
> > > common, especially as a way to group topics into data centers,
> > > relevant apps, etc - basically a work-around to our current lack of
> > > name spaces. However, most metric monitoring systems using "." to
> > > annotate hierarchy, so to avoid issues around metric names, Kafka
> > > replaces the "." in the name with an underscore.
> > >
> > > This generates good metric names, but creates the problem with name
> > collisions.
> > >
> > > I'm wondering if it makes sense to simply limit the range of
> > > characters permitted in a topic name and disallow "_"? Obviously
> > > existing topics will need to remain as is, which is a bit awkward.
> >
> > Interesting problem! Many if not most users I personally am aware of
> > use "_" as a separator in topic names. I am sure that many users would
> > be quite surprised by this limitation. With that said, I am sure
> > they'd transition accordingly.
> >
> > >
> > > If anyone has better backward-compatible solutions to this, I'm all
> ears
> > :)
> > >
> > > Gwen
> >
>



-- 
Grant Henke
Solutions Consultant | Cloudera
ghe...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke

Re: [Discussion] Limitations on topic names

[Todd Palino]

This was definitely changed at some point after KAFKA-495. The question is
when and why.

Here's the relevant code from that patch:

===
--- core/src/main/scala/kafka/utils/Topic.scala (revision 1390178)
+++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
@@ -21,24 +21,21 @@
 import util.matching.Regex

 object Topic {
+  val legalChars = "[a-zA-Z0-9_-]"



-Todd


On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke  wrote:

> kafka.common.Topic shows that currently period is a valid character and I
> have verified I can use kafka-topics.sh to create a new topic with a
> period.
>
>
> AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK currently uses
> Topic.validate before writing to Zookeeper.
>
> Should period character support be removed? I was under the same impression
> as Gwen, that a period was used by many as a way to "group" topics.
>
> The code is pasted below since its small:
>
> object Topic {
>   val legalChars = "[a-zA-Z0-9\\._\\-]"
>   private val maxNameLength = 255
>   private val rgx = new Regex(legalChars + "+")
>
>   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
>
>   def validate(topic: String) {
> if (topic.length <= 0)
>   throw new InvalidTopicException("topic name is illegal, can't be
> empty")
> else if (topic.equals(".") || topic.equals(".."))
>   throw new InvalidTopicException("topic name cannot be \".\" or
> \"..\"")
> else if (topic.length > maxNameLength)
>   throw new InvalidTopicException("topic name is illegal, can't be
> longer than " + maxNameLength + " characters")
>
> rgx.findFirstIn(topic) match {
>   case Some(t) =>
> if (!t.equals(topic))
>   throw new InvalidTopicException("topic name " + topic + " is
> illegal, contains a character other than ASCII alphanumerics, '.', '_' and
> '-'")
>   case None => throw new InvalidTopicException("topic name " + topic +
> " is illegal,  contains a character other than ASCII alphanumerics, '.',
> '_' and '-'")
> }
>   }
> }
>
> On Fri, Jul 10, 2015 at 2:50 PM, Todd Palino  wrote:
>
> > I had to go look this one up again to make sure -
> > https://issues.apache.org/jira/browse/KAFKA-495
> >
> > The only valid character names for topics are alphanumeric, underscore,
> and
> > dash. A period is not supposed to be a valid character to use. If you're
> > seeing them, then one of two things have happened:
> >
> > 1) You have topic names that are grandfathered in from before that patch
> > 2) The patch is not working properly and there is somewhere in the broker
> > that the standard is not being enforced.
> >
> > -Todd
> >
> >
> > On Fri, Jul 10, 2015 at 12:13 PM, Brock Noland  wrote:
> >
> > > On Fri, Jul 10, 2015 at 11:34 AM, Gwen Shapira 
> > > wrote:
> > > > Hi Kafka Fans,
> > > >
> > > > If you have one topic named "kafka_lab_2" and the other named
> > > > "kafka.lab.2", the topic level metrics will be named kafka_lab_2 for
> > > > both, effectively making it impossible to monitor them properly.
> > > >
> > > > The reason this happens is that using "." in topic names is pretty
> > > > common, especially as a way to group topics into data centers,
> > > > relevant apps, etc - basically a work-around to our current lack of
> > > > name spaces. However, most metric monitoring systems using "." to
> > > > annotate hierarchy, so to avoid issues around metric names, Kafka
> > > > replaces the "." in the name with an underscore.
> > > >
> > > > This generates good metric names, but creates the problem with name
> > > collisions.
> > > >
> > > > I'm wondering if it makes sense to simply limit the range of
> > > > characters permitted in a topic name and disallow "_"? Obviously
> > > > existing topics will need to remain as is, which is a bit awkward.
> > >
> > > Interesting problem! Many if not most users I personally am aware of
> > > use "_" as a separator in topic names. I am sure that many users would
> > > be quite surprised by this limitation. With that said, I am sure
> > > they'd transition accordingly.
> > >
> > > >
> > > > If anyone has better backward-compatible solutions to this, I'm all
> > ears
> > > :)
> > > >
> > > > Gwen
> > >
> >
>
>
>
> --
> Grant Henke
> Solutions Consultant | Cloudera
> ghe...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
>

Re: [Discussion] Limitations on topic names

[Grant Henke]

Found it was added here: https://issues.apache.org/jira/browse/KAFKA-697

On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino  wrote:

> This was definitely changed at some point after KAFKA-495. The question is
> when and why.
>
> Here's the relevant code from that patch:
>
> ===
> --- core/src/main/scala/kafka/utils/Topic.scala (revision 1390178)
> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
> @@ -21,24 +21,21 @@
>  import util.matching.Regex
>
>  object Topic {
> +  val legalChars = "[a-zA-Z0-9_-]"
>
>
>
> -Todd
>
>
> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke  wrote:
>
> > kafka.common.Topic shows that currently period is a valid character and I
> > have verified I can use kafka-topics.sh to create a new topic with a
> > period.
> >
> >
> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK currently uses
> > Topic.validate before writing to Zookeeper.
> >
> > Should period character support be removed? I was under the same
> impression
> > as Gwen, that a period was used by many as a way to "group" topics.
> >
> > The code is pasted below since its small:
> >
> > object Topic {
> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
> >   private val maxNameLength = 255
> >   private val rgx = new Regex(legalChars + "+")
> >
> >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
> >
> >   def validate(topic: String) {
> > if (topic.length <= 0)
> >   throw new InvalidTopicException("topic name is illegal, can't be
> > empty")
> > else if (topic.equals(".") || topic.equals(".."))
> >   throw new InvalidTopicException("topic name cannot be \".\" or
> > \"..\"")
> > else if (topic.length > maxNameLength)
> >   throw new InvalidTopicException("topic name is illegal, can't be
> > longer than " + maxNameLength + " characters")
> >
> > rgx.findFirstIn(topic) match {
> >   case Some(t) =>
> > if (!t.equals(topic))
> >   throw new InvalidTopicException("topic name " + topic + " is
> > illegal, contains a character other than ASCII alphanumerics, '.', '_'
> and
> > '-'")
> >   case None => throw new InvalidTopicException("topic name " + topic
> +
> > " is illegal,  contains a character other than ASCII alphanumerics, '.',
> > '_' and '-'")
> > }
> >   }
> > }
> >
> > On Fri, Jul 10, 2015 at 2:50 PM, Todd Palino  wrote:
> >
> > > I had to go look this one up again to make sure -
> > > https://issues.apache.org/jira/browse/KAFKA-495
> > >
> > > The only valid character names for topics are alphanumeric, underscore,
> > and
> > > dash. A period is not supposed to be a valid character to use. If
> you're
> > > seeing them, then one of two things have happened:
> > >
> > > 1) You have topic names that are grandfathered in from before that
> patch
> > > 2) The patch is not working properly and there is somewhere in the
> broker
> > > that the standard is not being enforced.
> > >
> > > -Todd
> > >
> > >
> > > On Fri, Jul 10, 2015 at 12:13 PM, Brock Noland 
> wrote:
> > >
> > > > On Fri, Jul 10, 2015 at 11:34 AM, Gwen Shapira <
> gshap...@cloudera.com>
> > > > wrote:
> > > > > Hi Kafka Fans,
> > > > >
> > > > > If you have one topic named "kafka_lab_2" and the other named
> > > > > "kafka.lab.2", the topic level metrics will be named kafka_lab_2
> for
> > > > > both, effectively making it impossible to monitor them properly.
> > > > >
> > > > > The reason this happens is that using "." in topic names is pretty
> > > > > common, especially as a way to group topics into data centers,
> > > > > relevant apps, etc - basically a work-around to our current lack of
> > > > > name spaces. However, most metric monitoring systems using "." to
> > > > > annotate hierarchy, so to avoid issues around metric names, Kafka
> > > > > replaces the "." in the name with an underscore.
> > > > >
> > > > > This generates good metric names, but creates the problem with name
> > > > collisions.
> > > > >
> > > > > I'm wondering if it makes sense to simply limit the range of
> > > > > characters permitted in a topic name and disallow "_"? Obviously
> > > > > existing topics will need to remain as is, which is a bit awkward.
> > > >
> > > > Interesting problem! Many if not most users I personally am aware of
> > > > use "_" as a separator in topic names. I am sure that many users
> would
> > > > be quite surprised by this limitation. With that said, I am sure
> > > > they'd transition accordingly.
> > > >
> > > > >
> > > > > If anyone has better backward-compatible solutions to this, I'm all
> > > ears
> > > > :)
> > > > >
> > > > > Gwen
> > > >
> > >
> >
> >
> >
> > --
> > Grant Henke
> > Solutions Consultant | Cloudera
> > ghe...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
> >
>



-- 
Grant Henke
Solutions Consultant | Cloudera
ghe...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke

Re: [Discussion] Limitations on topic names

[Todd Palino]

Thanks, Grant. That seems like a bad solution to the problem that John ran
into in that ticket. It's entirely reasonable to have separate validators
for separate things, but it seems like the choice was made to try and mash
it all into a single validator. And it appears that despite the commentary
in the ticket at the time, Gwen's identified a very good reason to be
restrictive about topic naming.

-Todd



On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke  wrote:

> Found it was added here: https://issues.apache.org/jira/browse/KAFKA-697
>
> On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino  wrote:
>
> > This was definitely changed at some point after KAFKA-495. The question
> is
> > when and why.
> >
> > Here's the relevant code from that patch:
> >
> > ===
> > --- core/src/main/scala/kafka/utils/Topic.scala (revision 1390178)
> > +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
> > @@ -21,24 +21,21 @@
> >  import util.matching.Regex
> >
> >  object Topic {
> > +  val legalChars = "[a-zA-Z0-9_-]"
> >
> >
> >
> > -Todd
> >
> >
> > On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke 
> wrote:
> >
> > > kafka.common.Topic shows that currently period is a valid character
> and I
> > > have verified I can use kafka-topics.sh to create a new topic with a
> > > period.
> > >
> > >
> > > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK currently
> uses
> > > Topic.validate before writing to Zookeeper.
> > >
> > > Should period character support be removed? I was under the same
> > impression
> > > as Gwen, that a period was used by many as a way to "group" topics.
> > >
> > > The code is pasted below since its small:
> > >
> > > object Topic {
> > >   val legalChars = "[a-zA-Z0-9\\._\\-]"
> > >   private val maxNameLength = 255
> > >   private val rgx = new Regex(legalChars + "+")
> > >
> > >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
> > >
> > >   def validate(topic: String) {
> > > if (topic.length <= 0)
> > >   throw new InvalidTopicException("topic name is illegal, can't be
> > > empty")
> > > else if (topic.equals(".") || topic.equals(".."))
> > >   throw new InvalidTopicException("topic name cannot be \".\" or
> > > \"..\"")
> > > else if (topic.length > maxNameLength)
> > >   throw new InvalidTopicException("topic name is illegal, can't be
> > > longer than " + maxNameLength + " characters")
> > >
> > > rgx.findFirstIn(topic) match {
> > >   case Some(t) =>
> > > if (!t.equals(topic))
> > >   throw new InvalidTopicException("topic name " + topic + " is
> > > illegal, contains a character other than ASCII alphanumerics, '.', '_'
> > and
> > > '-'")
> > >   case None => throw new InvalidTopicException("topic name " +
> topic
> > +
> > > " is illegal,  contains a character other than ASCII alphanumerics,
> '.',
> > > '_' and '-'")
> > > }
> > >   }
> > > }
> > >
> > > On Fri, Jul 10, 2015 at 2:50 PM, Todd Palino 
> wrote:
> > >
> > > > I had to go look this one up again to make sure -
> > > > https://issues.apache.org/jira/browse/KAFKA-495
> > > >
> > > > The only valid character names for topics are alphanumeric,
> underscore,
> > > and
> > > > dash. A period is not supposed to be a valid character to use. If
> > you're
> > > > seeing them, then one of two things have happened:
> > > >
> > > > 1) You have topic names that are grandfathered in from before that
> > patch
> > > > 2) The patch is not working properly and there is somewhere in the
> > broker
> > > > that the standard is not being enforced.
> > > >
> > > > -Todd
> > > >
> > > >
> > > > On Fri, Jul 10, 2015 at 12:13 PM, Brock Noland 
> > wrote:
> > > >
> > > > > On Fri, Jul 10, 2015 at 11:34 AM, Gwen Shapira <
> > gshap...@cloudera.com>
> > > > > wrote:
> > > > > > Hi Kafka Fans,
> > > > > >
> > > > > > If you have one topic named "kafka_lab_2" and the other named
> > > > > > "kafka.lab.2", the topic level metrics will be named kafka_lab_2
> > for
> > > > > > both, effectively making it impossible to monitor them properly.
> > > > > >
> > > > > > The reason this happens is that using "." in topic names is
> pretty
> > > > > > common, especially as a way to group topics into data centers,
> > > > > > relevant apps, etc - basically a work-around to our current lack
> of
> > > > > > name spaces. However, most metric monitoring systems using "." to
> > > > > > annotate hierarchy, so to avoid issues around metric names, Kafka
> > > > > > replaces the "." in the name with an underscore.
> > > > > >
> > > > > > This generates good metric names, but creates the problem with
> name
> > > > > collisions.
> > > > > >
> > > > > > I'm wondering if it makes sense to simply limit the range of
> > > > > > characters permitted in a topic name and disallow "_"? Obviously
> > > > > > existing topics will need to remain as is, which is a bit
> awkward.
> > > > >
> > > > > Interesting problem! Many if not most users I personally

Re: [Discussion] Limitations on topic names

[Gwen Shapira]

Unintentional side effect from allowing IP addresses in consumer client IDs :)

So the question is, what do we do now?

1) disallow "."
2) disallow "_"
3) find a reversible way to encode "." and "_" that won't break existing metrics
4) all of the above?

btw. it looks like "." and ".." are currently valid. Topic names are
used for directories, right? this sounds like fun :)

I vote for option #1, although if someone has a good idea for #3 it
will be even better.

Gwen



On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke  wrote:
> Found it was added here: https://issues.apache.org/jira/browse/KAFKA-697
>
> On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino  wrote:
>
>> This was definitely changed at some point after KAFKA-495. The question is
>> when and why.
>>
>> Here's the relevant code from that patch:
>>
>> ===
>> --- core/src/main/scala/kafka/utils/Topic.scala (revision 1390178)
>> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
>> @@ -21,24 +21,21 @@
>>  import util.matching.Regex
>>
>>  object Topic {
>> +  val legalChars = "[a-zA-Z0-9_-]"
>>
>>
>>
>> -Todd
>>
>>
>> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke  wrote:
>>
>> > kafka.common.Topic shows that currently period is a valid character and I
>> > have verified I can use kafka-topics.sh to create a new topic with a
>> > period.
>> >
>> >
>> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK currently uses
>> > Topic.validate before writing to Zookeeper.
>> >
>> > Should period character support be removed? I was under the same
>> impression
>> > as Gwen, that a period was used by many as a way to "group" topics.
>> >
>> > The code is pasted below since its small:
>> >
>> > object Topic {
>> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
>> >   private val maxNameLength = 255
>> >   private val rgx = new Regex(legalChars + "+")
>> >
>> >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
>> >
>> >   def validate(topic: String) {
>> > if (topic.length <= 0)
>> >   throw new InvalidTopicException("topic name is illegal, can't be
>> > empty")
>> > else if (topic.equals(".") || topic.equals(".."))
>> >   throw new InvalidTopicException("topic name cannot be \".\" or
>> > \"..\"")
>> > else if (topic.length > maxNameLength)
>> >   throw new InvalidTopicException("topic name is illegal, can't be
>> > longer than " + maxNameLength + " characters")
>> >
>> > rgx.findFirstIn(topic) match {
>> >   case Some(t) =>
>> > if (!t.equals(topic))
>> >   throw new InvalidTopicException("topic name " + topic + " is
>> > illegal, contains a character other than ASCII alphanumerics, '.', '_'
>> and
>> > '-'")
>> >   case None => throw new InvalidTopicException("topic name " + topic
>> +
>> > " is illegal,  contains a character other than ASCII alphanumerics, '.',
>> > '_' and '-'")
>> > }
>> >   }
>> > }
>> >
>> > On Fri, Jul 10, 2015 at 2:50 PM, Todd Palino  wrote:
>> >
>> > > I had to go look this one up again to make sure -
>> > > https://issues.apache.org/jira/browse/KAFKA-495
>> > >
>> > > The only valid character names for topics are alphanumeric, underscore,
>> > and
>> > > dash. A period is not supposed to be a valid character to use. If
>> you're
>> > > seeing them, then one of two things have happened:
>> > >
>> > > 1) You have topic names that are grandfathered in from before that
>> patch
>> > > 2) The patch is not working properly and there is somewhere in the
>> broker
>> > > that the standard is not being enforced.
>> > >
>> > > -Todd
>> > >
>> > >
>> > > On Fri, Jul 10, 2015 at 12:13 PM, Brock Noland 
>> wrote:
>> > >
>> > > > On Fri, Jul 10, 2015 at 11:34 AM, Gwen Shapira <
>> gshap...@cloudera.com>
>> > > > wrote:
>> > > > > Hi Kafka Fans,
>> > > > >
>> > > > > If you have one topic named "kafka_lab_2" and the other named
>> > > > > "kafka.lab.2", the topic level metrics will be named kafka_lab_2
>> for
>> > > > > both, effectively making it impossible to monitor them properly.
>> > > > >
>> > > > > The reason this happens is that using "." in topic names is pretty
>> > > > > common, especially as a way to group topics into data centers,
>> > > > > relevant apps, etc - basically a work-around to our current lack of
>> > > > > name spaces. However, most metric monitoring systems using "." to
>> > > > > annotate hierarchy, so to avoid issues around metric names, Kafka
>> > > > > replaces the "." in the name with an underscore.
>> > > > >
>> > > > > This generates good metric names, but creates the problem with name
>> > > > collisions.
>> > > > >
>> > > > > I'm wondering if it makes sense to simply limit the range of
>> > > > > characters permitted in a topic name and disallow "_"? Obviously
>> > > > > existing topics will need to remain as is, which is a bit awkward.
>> > > >
>> > > > Interesting problem! Many if not most users I personally am aware of
>> > > > use "_" as a separator in topic names. I am sure that 

Re: [Discussion] Limitations on topic names

[Todd Palino]

My selfish point of view is that we do #1, as we use "_" extensively in
topic names here :) I also happen to think it's the right choice,
specifically because "." has more special meanings, as you noted.

-Todd


On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira  wrote:

> Unintentional side effect from allowing IP addresses in consumer client
> IDs :)
>
> So the question is, what do we do now?
>
> 1) disallow "."
> 2) disallow "_"
> 3) find a reversible way to encode "." and "_" that won't break existing
> metrics
> 4) all of the above?
>
> btw. it looks like "." and ".." are currently valid. Topic names are
> used for directories, right? this sounds like fun :)
>
> I vote for option #1, although if someone has a good idea for #3 it
> will be even better.
>
> Gwen
>
>
>
> On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke  wrote:
> > Found it was added here: https://issues.apache.org/jira/browse/KAFKA-697
> >
> > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino  wrote:
> >
> >> This was definitely changed at some point after KAFKA-495. The question
> is
> >> when and why.
> >>
> >> Here's the relevant code from that patch:
> >>
> >> ===
> >> --- core/src/main/scala/kafka/utils/Topic.scala (revision 1390178)
> >> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
> >> @@ -21,24 +21,21 @@
> >>  import util.matching.Regex
> >>
> >>  object Topic {
> >> +  val legalChars = "[a-zA-Z0-9_-]"
> >>
> >>
> >>
> >> -Todd
> >>
> >>
> >> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke 
> wrote:
> >>
> >> > kafka.common.Topic shows that currently period is a valid character
> and I
> >> > have verified I can use kafka-topics.sh to create a new topic with a
> >> > period.
> >> >
> >> >
> >> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK currently
> uses
> >> > Topic.validate before writing to Zookeeper.
> >> >
> >> > Should period character support be removed? I was under the same
> >> impression
> >> > as Gwen, that a period was used by many as a way to "group" topics.
> >> >
> >> > The code is pasted below since its small:
> >> >
> >> > object Topic {
> >> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
> >> >   private val maxNameLength = 255
> >> >   private val rgx = new Regex(legalChars + "+")
> >> >
> >> >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
> >> >
> >> >   def validate(topic: String) {
> >> > if (topic.length <= 0)
> >> >   throw new InvalidTopicException("topic name is illegal, can't be
> >> > empty")
> >> > else if (topic.equals(".") || topic.equals(".."))
> >> >   throw new InvalidTopicException("topic name cannot be \".\" or
> >> > \"..\"")
> >> > else if (topic.length > maxNameLength)
> >> >   throw new InvalidTopicException("topic name is illegal, can't be
> >> > longer than " + maxNameLength + " characters")
> >> >
> >> > rgx.findFirstIn(topic) match {
> >> >   case Some(t) =>
> >> > if (!t.equals(topic))
> >> >   throw new InvalidTopicException("topic name " + topic + " is
> >> > illegal, contains a character other than ASCII alphanumerics, '.', '_'
> >> and
> >> > '-'")
> >> >   case None => throw new InvalidTopicException("topic name " +
> topic
> >> +
> >> > " is illegal,  contains a character other than ASCII alphanumerics,
> '.',
> >> > '_' and '-'")
> >> > }
> >> >   }
> >> > }
> >> >
> >> > On Fri, Jul 10, 2015 at 2:50 PM, Todd Palino 
> wrote:
> >> >
> >> > > I had to go look this one up again to make sure -
> >> > > https://issues.apache.org/jira/browse/KAFKA-495
> >> > >
> >> > > The only valid character names for topics are alphanumeric,
> underscore,
> >> > and
> >> > > dash. A period is not supposed to be a valid character to use. If
> >> you're
> >> > > seeing them, then one of two things have happened:
> >> > >
> >> > > 1) You have topic names that are grandfathered in from before that
> >> patch
> >> > > 2) The patch is not working properly and there is somewhere in the
> >> broker
> >> > > that the standard is not being enforced.
> >> > >
> >> > > -Todd
> >> > >
> >> > >
> >> > > On Fri, Jul 10, 2015 at 12:13 PM, Brock Noland 
> >> wrote:
> >> > >
> >> > > > On Fri, Jul 10, 2015 at 11:34 AM, Gwen Shapira <
> >> gshap...@cloudera.com>
> >> > > > wrote:
> >> > > > > Hi Kafka Fans,
> >> > > > >
> >> > > > > If you have one topic named "kafka_lab_2" and the other named
> >> > > > > "kafka.lab.2", the topic level metrics will be named kafka_lab_2
> >> for
> >> > > > > both, effectively making it impossible to monitor them properly.
> >> > > > >
> >> > > > > The reason this happens is that using "." in topic names is
> pretty
> >> > > > > common, especially as a way to group topics into data centers,
> >> > > > > relevant apps, etc - basically a work-around to our current
> lack of
> >> > > > > name spaces. However, most metric monitoring systems using "."
> to
> >> > > > > annotate hierarchy, so to avoid issues around metric names,
> Kafka
> >> > > >

Re: [Discussion] Limitations on topic names

[Grant Henke]

I vote for #1 too.

A special reason Kafka may use '.' in the future is for hierarchical or
namespaced topics.

On Fri, Jul 10, 2015 at 3:32 PM, Todd Palino  wrote:

> My selfish point of view is that we do #1, as we use "_" extensively in
> topic names here :) I also happen to think it's the right choice,
> specifically because "." has more special meanings, as you noted.
>
> -Todd
>
>
> On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira 
> wrote:
>
> > Unintentional side effect from allowing IP addresses in consumer client
> > IDs :)
> >
> > So the question is, what do we do now?
> >
> > 1) disallow "."
> > 2) disallow "_"
> > 3) find a reversible way to encode "." and "_" that won't break existing
> > metrics
> > 4) all of the above?
> >
> > btw. it looks like "." and ".." are currently valid. Topic names are
> > used for directories, right? this sounds like fun :)
> >
> > I vote for option #1, although if someone has a good idea for #3 it
> > will be even better.
> >
> > Gwen
> >
> >
> >
> > On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke 
> wrote:
> > > Found it was added here:
> https://issues.apache.org/jira/browse/KAFKA-697
> > >
> > > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino 
> wrote:
> > >
> > >> This was definitely changed at some point after KAFKA-495. The
> question
> > is
> > >> when and why.
> > >>
> > >> Here's the relevant code from that patch:
> > >>
> > >> ===
> > >> --- core/src/main/scala/kafka/utils/Topic.scala (revision 1390178)
> > >> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
> > >> @@ -21,24 +21,21 @@
> > >>  import util.matching.Regex
> > >>
> > >>  object Topic {
> > >> +  val legalChars = "[a-zA-Z0-9_-]"
> > >>
> > >>
> > >>
> > >> -Todd
> > >>
> > >>
> > >> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke 
> > wrote:
> > >>
> > >> > kafka.common.Topic shows that currently period is a valid character
> > and I
> > >> > have verified I can use kafka-topics.sh to create a new topic with a
> > >> > period.
> > >> >
> > >> >
> > >> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK currently
> > uses
> > >> > Topic.validate before writing to Zookeeper.
> > >> >
> > >> > Should period character support be removed? I was under the same
> > >> impression
> > >> > as Gwen, that a period was used by many as a way to "group" topics.
> > >> >
> > >> > The code is pasted below since its small:
> > >> >
> > >> > object Topic {
> > >> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
> > >> >   private val maxNameLength = 255
> > >> >   private val rgx = new Regex(legalChars + "+")
> > >> >
> > >> >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
> > >> >
> > >> >   def validate(topic: String) {
> > >> > if (topic.length <= 0)
> > >> >   throw new InvalidTopicException("topic name is illegal, can't
> be
> > >> > empty")
> > >> > else if (topic.equals(".") || topic.equals(".."))
> > >> >   throw new InvalidTopicException("topic name cannot be \".\" or
> > >> > \"..\"")
> > >> > else if (topic.length > maxNameLength)
> > >> >   throw new InvalidTopicException("topic name is illegal, can't
> be
> > >> > longer than " + maxNameLength + " characters")
> > >> >
> > >> > rgx.findFirstIn(topic) match {
> > >> >   case Some(t) =>
> > >> > if (!t.equals(topic))
> > >> >   throw new InvalidTopicException("topic name " + topic + "
> is
> > >> > illegal, contains a character other than ASCII alphanumerics, '.',
> '_'
> > >> and
> > >> > '-'")
> > >> >   case None => throw new InvalidTopicException("topic name " +
> > topic
> > >> +
> > >> > " is illegal,  contains a character other than ASCII alphanumerics,
> > '.',
> > >> > '_' and '-'")
> > >> > }
> > >> >   }
> > >> > }
> > >> >
> > >> > On Fri, Jul 10, 2015 at 2:50 PM, Todd Palino 
> > wrote:
> > >> >
> > >> > > I had to go look this one up again to make sure -
> > >> > > https://issues.apache.org/jira/browse/KAFKA-495
> > >> > >
> > >> > > The only valid character names for topics are alphanumeric,
> > underscore,
> > >> > and
> > >> > > dash. A period is not supposed to be a valid character to use. If
> > >> you're
> > >> > > seeing them, then one of two things have happened:
> > >> > >
> > >> > > 1) You have topic names that are grandfathered in from before that
> > >> patch
> > >> > > 2) The patch is not working properly and there is somewhere in the
> > >> broker
> > >> > > that the standard is not being enforced.
> > >> > >
> > >> > > -Todd
> > >> > >
> > >> > >
> > >> > > On Fri, Jul 10, 2015 at 12:13 PM, Brock Noland 
> > >> wrote:
> > >> > >
> > >> > > > On Fri, Jul 10, 2015 at 11:34 AM, Gwen Shapira <
> > >> gshap...@cloudera.com>
> > >> > > > wrote:
> > >> > > > > Hi Kafka Fans,
> > >> > > > >
> > >> > > > > If you have one topic named "kafka_lab_2" and the other named
> > >> > > > > "kafka.lab.2", the topic level metrics will be named
> kafka_lab_2
> > >> for
> > >> > > > > both, effectively makin

Re: [Discussion] Limitations on topic names

[Jay Kreps]

Unfortunately '.' is pretty common too. I agree that it is perverse, but
people seem to do it. Breaking all the topics with '.' in the name seems
like it could be worse than combining metrics for people who have a
'foo_bar' AND 'foo.bar' (and after all, having both is DEEPLY perverse,
no?).

Where is our Dean of Compatibility, Ewen, on this?

-Jay

On Fri, Jul 10, 2015 at 1:32 PM, Todd Palino  wrote:

> My selfish point of view is that we do #1, as we use "_" extensively in
> topic names here :) I also happen to think it's the right choice,
> specifically because "." has more special meanings, as you noted.
>
> -Todd
>
>
> On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira 
> wrote:
>
> > Unintentional side effect from allowing IP addresses in consumer client
> > IDs :)
> >
> > So the question is, what do we do now?
> >
> > 1) disallow "."
> > 2) disallow "_"
> > 3) find a reversible way to encode "." and "_" that won't break existing
> > metrics
> > 4) all of the above?
> >
> > btw. it looks like "." and ".." are currently valid. Topic names are
> > used for directories, right? this sounds like fun :)
> >
> > I vote for option #1, although if someone has a good idea for #3 it
> > will be even better.
> >
> > Gwen
> >
> >
> >
> > On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke 
> wrote:
> > > Found it was added here:
> https://issues.apache.org/jira/browse/KAFKA-697
> > >
> > > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino 
> wrote:
> > >
> > >> This was definitely changed at some point after KAFKA-495. The
> question
> > is
> > >> when and why.
> > >>
> > >> Here's the relevant code from that patch:
> > >>
> > >> ===
> > >> --- core/src/main/scala/kafka/utils/Topic.scala (revision 1390178)
> > >> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
> > >> @@ -21,24 +21,21 @@
> > >>  import util.matching.Regex
> > >>
> > >>  object Topic {
> > >> +  val legalChars = "[a-zA-Z0-9_-]"
> > >>
> > >>
> > >>
> > >> -Todd
> > >>
> > >>
> > >> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke 
> > wrote:
> > >>
> > >> > kafka.common.Topic shows that currently period is a valid character
> > and I
> > >> > have verified I can use kafka-topics.sh to create a new topic with a
> > >> > period.
> > >> >
> > >> >
> > >> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK currently
> > uses
> > >> > Topic.validate before writing to Zookeeper.
> > >> >
> > >> > Should period character support be removed? I was under the same
> > >> impression
> > >> > as Gwen, that a period was used by many as a way to "group" topics.
> > >> >
> > >> > The code is pasted below since its small:
> > >> >
> > >> > object Topic {
> > >> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
> > >> >   private val maxNameLength = 255
> > >> >   private val rgx = new Regex(legalChars + "+")
> > >> >
> > >> >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
> > >> >
> > >> >   def validate(topic: String) {
> > >> > if (topic.length <= 0)
> > >> >   throw new InvalidTopicException("topic name is illegal, can't
> be
> > >> > empty")
> > >> > else if (topic.equals(".") || topic.equals(".."))
> > >> >   throw new InvalidTopicException("topic name cannot be \".\" or
> > >> > \"..\"")
> > >> > else if (topic.length > maxNameLength)
> > >> >   throw new InvalidTopicException("topic name is illegal, can't
> be
> > >> > longer than " + maxNameLength + " characters")
> > >> >
> > >> > rgx.findFirstIn(topic) match {
> > >> >   case Some(t) =>
> > >> > if (!t.equals(topic))
> > >> >   throw new InvalidTopicException("topic name " + topic + "
> is
> > >> > illegal, contains a character other than ASCII alphanumerics, '.',
> '_'
> > >> and
> > >> > '-'")
> > >> >   case None => throw new InvalidTopicException("topic name " +
> > topic
> > >> +
> > >> > " is illegal,  contains a character other than ASCII alphanumerics,
> > '.',
> > >> > '_' and '-'")
> > >> > }
> > >> >   }
> > >> > }
> > >> >
> > >> > On Fri, Jul 10, 2015 at 2:50 PM, Todd Palino 
> > wrote:
> > >> >
> > >> > > I had to go look this one up again to make sure -
> > >> > > https://issues.apache.org/jira/browse/KAFKA-495
> > >> > >
> > >> > > The only valid character names for topics are alphanumeric,
> > underscore,
> > >> > and
> > >> > > dash. A period is not supposed to be a valid character to use. If
> > >> you're
> > >> > > seeing them, then one of two things have happened:
> > >> > >
> > >> > > 1) You have topic names that are grandfathered in from before that
> > >> patch
> > >> > > 2) The patch is not working properly and there is somewhere in the
> > >> broker
> > >> > > that the standard is not being enforced.
> > >> > >
> > >> > > -Todd
> > >> > >
> > >> > >
> > >> > > On Fri, Jul 10, 2015 at 12:13 PM, Brock Noland 
> > >> wrote:
> > >> > >
> > >> > > > On Fri, Jul 10, 2015 at 11:34 AM, Gwen Shapira <
> > >> gshap...@cloudera.com>
> > >> > > > wrote:
> > >> > > > >

Re: [Discussion] Limitations on topic names

[Gwen Shapira]

I don't think we should break existing topics. Just disallow new
topics going forward.

Agree that having both is horrible, but we should have a solution that
fails when you run "kafka_topics.sh --create", not when you configure
Ganglia.

Gwen

On Fri, Jul 10, 2015 at 1:53 PM, Jay Kreps  wrote:
> Unfortunately '.' is pretty common too. I agree that it is perverse, but
> people seem to do it. Breaking all the topics with '.' in the name seems
> like it could be worse than combining metrics for people who have a
> 'foo_bar' AND 'foo.bar' (and after all, having both is DEEPLY perverse,
> no?).
>
> Where is our Dean of Compatibility, Ewen, on this?
>
> -Jay
>
> On Fri, Jul 10, 2015 at 1:32 PM, Todd Palino  wrote:
>
>> My selfish point of view is that we do #1, as we use "_" extensively in
>> topic names here :) I also happen to think it's the right choice,
>> specifically because "." has more special meanings, as you noted.
>>
>> -Todd
>>
>>
>> On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira 
>> wrote:
>>
>> > Unintentional side effect from allowing IP addresses in consumer client
>> > IDs :)
>> >
>> > So the question is, what do we do now?
>> >
>> > 1) disallow "."
>> > 2) disallow "_"
>> > 3) find a reversible way to encode "." and "_" that won't break existing
>> > metrics
>> > 4) all of the above?
>> >
>> > btw. it looks like "." and ".." are currently valid. Topic names are
>> > used for directories, right? this sounds like fun :)
>> >
>> > I vote for option #1, although if someone has a good idea for #3 it
>> > will be even better.
>> >
>> > Gwen
>> >
>> >
>> >
>> > On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke 
>> wrote:
>> > > Found it was added here:
>> https://issues.apache.org/jira/browse/KAFKA-697
>> > >
>> > > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino 
>> wrote:
>> > >
>> > >> This was definitely changed at some point after KAFKA-495. The
>> question
>> > is
>> > >> when and why.
>> > >>
>> > >> Here's the relevant code from that patch:
>> > >>
>> > >> ===
>> > >> --- core/src/main/scala/kafka/utils/Topic.scala (revision 1390178)
>> > >> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
>> > >> @@ -21,24 +21,21 @@
>> > >>  import util.matching.Regex
>> > >>
>> > >>  object Topic {
>> > >> +  val legalChars = "[a-zA-Z0-9_-]"
>> > >>
>> > >>
>> > >>
>> > >> -Todd
>> > >>
>> > >>
>> > >> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke 
>> > wrote:
>> > >>
>> > >> > kafka.common.Topic shows that currently period is a valid character
>> > and I
>> > >> > have verified I can use kafka-topics.sh to create a new topic with a
>> > >> > period.
>> > >> >
>> > >> >
>> > >> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK currently
>> > uses
>> > >> > Topic.validate before writing to Zookeeper.
>> > >> >
>> > >> > Should period character support be removed? I was under the same
>> > >> impression
>> > >> > as Gwen, that a period was used by many as a way to "group" topics.
>> > >> >
>> > >> > The code is pasted below since its small:
>> > >> >
>> > >> > object Topic {
>> > >> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
>> > >> >   private val maxNameLength = 255
>> > >> >   private val rgx = new Regex(legalChars + "+")
>> > >> >
>> > >> >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
>> > >> >
>> > >> >   def validate(topic: String) {
>> > >> > if (topic.length <= 0)
>> > >> >   throw new InvalidTopicException("topic name is illegal, can't
>> be
>> > >> > empty")
>> > >> > else if (topic.equals(".") || topic.equals(".."))
>> > >> >   throw new InvalidTopicException("topic name cannot be \".\" or
>> > >> > \"..\"")
>> > >> > else if (topic.length > maxNameLength)
>> > >> >   throw new InvalidTopicException("topic name is illegal, can't
>> be
>> > >> > longer than " + maxNameLength + " characters")
>> > >> >
>> > >> > rgx.findFirstIn(topic) match {
>> > >> >   case Some(t) =>
>> > >> > if (!t.equals(topic))
>> > >> >   throw new InvalidTopicException("topic name " + topic + "
>> is
>> > >> > illegal, contains a character other than ASCII alphanumerics, '.',
>> '_'
>> > >> and
>> > >> > '-'")
>> > >> >   case None => throw new InvalidTopicException("topic name " +
>> > topic
>> > >> +
>> > >> > " is illegal,  contains a character other than ASCII alphanumerics,
>> > '.',
>> > >> > '_' and '-'")
>> > >> > }
>> > >> >   }
>> > >> > }
>> > >> >
>> > >> > On Fri, Jul 10, 2015 at 2:50 PM, Todd Palino 
>> > wrote:
>> > >> >
>> > >> > > I had to go look this one up again to make sure -
>> > >> > > https://issues.apache.org/jira/browse/KAFKA-495
>> > >> > >
>> > >> > > The only valid character names for topics are alphanumeric,
>> > underscore,
>> > >> > and
>> > >> > > dash. A period is not supposed to be a valid character to use. If
>> > >> you're
>> > >> > > seeing them, then one of two things have happened:
>> > >> > >
>> > >> > > 1) You have topic names that are gra

Re: [Discussion] Limitations on topic names

[Todd Palino]

Yes, agree here. While it can be a little confusing, I think it's better to
just disallow the character for all creation steps so you can't create more
"bad" topic names, but not try and enforce it for topics that already
exist. Anyone who is in that situation is already there with regards to
metrics, and so they are probably making sure they don't collide names that
only differ in the use of "_" and ".". However, we don't want a new user to
accidentally do it.

-Todd


On Fri, Jul 10, 2015 at 2:02 PM, Gwen Shapira  wrote:

> I don't think we should break existing topics. Just disallow new
> topics going forward.
>
> Agree that having both is horrible, but we should have a solution that
> fails when you run "kafka_topics.sh --create", not when you configure
> Ganglia.
>
> Gwen
>
> On Fri, Jul 10, 2015 at 1:53 PM, Jay Kreps  wrote:
> > Unfortunately '.' is pretty common too. I agree that it is perverse, but
> > people seem to do it. Breaking all the topics with '.' in the name seems
> > like it could be worse than combining metrics for people who have a
> > 'foo_bar' AND 'foo.bar' (and after all, having both is DEEPLY perverse,
> > no?).
> >
> > Where is our Dean of Compatibility, Ewen, on this?
> >
> > -Jay
> >
> > On Fri, Jul 10, 2015 at 1:32 PM, Todd Palino  wrote:
> >
> >> My selfish point of view is that we do #1, as we use "_" extensively in
> >> topic names here :) I also happen to think it's the right choice,
> >> specifically because "." has more special meanings, as you noted.
> >>
> >> -Todd
> >>
> >>
> >> On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira 
> >> wrote:
> >>
> >> > Unintentional side effect from allowing IP addresses in consumer
> client
> >> > IDs :)
> >> >
> >> > So the question is, what do we do now?
> >> >
> >> > 1) disallow "."
> >> > 2) disallow "_"
> >> > 3) find a reversible way to encode "." and "_" that won't break
> existing
> >> > metrics
> >> > 4) all of the above?
> >> >
> >> > btw. it looks like "." and ".." are currently valid. Topic names are
> >> > used for directories, right? this sounds like fun :)
> >> >
> >> > I vote for option #1, although if someone has a good idea for #3 it
> >> > will be even better.
> >> >
> >> > Gwen
> >> >
> >> >
> >> >
> >> > On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke 
> >> wrote:
> >> > > Found it was added here:
> >> https://issues.apache.org/jira/browse/KAFKA-697
> >> > >
> >> > > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino 
> >> wrote:
> >> > >
> >> > >> This was definitely changed at some point after KAFKA-495. The
> >> question
> >> > is
> >> > >> when and why.
> >> > >>
> >> > >> Here's the relevant code from that patch:
> >> > >>
> >> > >> ===
> >> > >> --- core/src/main/scala/kafka/utils/Topic.scala (revision 1390178)
> >> > >> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
> >> > >> @@ -21,24 +21,21 @@
> >> > >>  import util.matching.Regex
> >> > >>
> >> > >>  object Topic {
> >> > >> +  val legalChars = "[a-zA-Z0-9_-]"
> >> > >>
> >> > >>
> >> > >>
> >> > >> -Todd
> >> > >>
> >> > >>
> >> > >> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke 
> >> > wrote:
> >> > >>
> >> > >> > kafka.common.Topic shows that currently period is a valid
> character
> >> > and I
> >> > >> > have verified I can use kafka-topics.sh to create a new topic
> with a
> >> > >> > period.
> >> > >> >
> >> > >> >
> >> > >> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK
> currently
> >> > uses
> >> > >> > Topic.validate before writing to Zookeeper.
> >> > >> >
> >> > >> > Should period character support be removed? I was under the same
> >> > >> impression
> >> > >> > as Gwen, that a period was used by many as a way to "group"
> topics.
> >> > >> >
> >> > >> > The code is pasted below since its small:
> >> > >> >
> >> > >> > object Topic {
> >> > >> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
> >> > >> >   private val maxNameLength = 255
> >> > >> >   private val rgx = new Regex(legalChars + "+")
> >> > >> >
> >> > >> >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
> >> > >> >
> >> > >> >   def validate(topic: String) {
> >> > >> > if (topic.length <= 0)
> >> > >> >   throw new InvalidTopicException("topic name is illegal,
> can't
> >> be
> >> > >> > empty")
> >> > >> > else if (topic.equals(".") || topic.equals(".."))
> >> > >> >   throw new InvalidTopicException("topic name cannot be
> \".\" or
> >> > >> > \"..\"")
> >> > >> > else if (topic.length > maxNameLength)
> >> > >> >   throw new InvalidTopicException("topic name is illegal,
> can't
> >> be
> >> > >> > longer than " + maxNameLength + " characters")
> >> > >> >
> >> > >> > rgx.findFirstIn(topic) match {
> >> > >> >   case Some(t) =>
> >> > >> > if (!t.equals(topic))
> >> > >> >   throw new InvalidTopicException("topic name " + topic
> + "
> >> is
> >> > >> > illegal, contains a character other than ASCII alphanumerics,
> '.',
> >> '_'
> >> > >> and
> 

Re: [Discussion] Limitations on topic names

[Neha Narkhede]

"." seems natural for grouping topic names. +1 for 2) going forward only
without breaking previously created topics with "_" though that might
require us to patch the code somewhat awkwardly till we phase it out a
couple (purposely left vague to stay out of Ewen's wrath :-)) versions
later.

On Fri, Jul 10, 2015 at 2:02 PM, Gwen Shapira  wrote:

> I don't think we should break existing topics. Just disallow new
> topics going forward.
>
> Agree that having both is horrible, but we should have a solution that
> fails when you run "kafka_topics.sh --create", not when you configure
> Ganglia.
>
> Gwen
>
> On Fri, Jul 10, 2015 at 1:53 PM, Jay Kreps  wrote:
> > Unfortunately '.' is pretty common too. I agree that it is perverse, but
> > people seem to do it. Breaking all the topics with '.' in the name seems
> > like it could be worse than combining metrics for people who have a
> > 'foo_bar' AND 'foo.bar' (and after all, having both is DEEPLY perverse,
> > no?).
> >
> > Where is our Dean of Compatibility, Ewen, on this?
> >
> > -Jay
> >
> > On Fri, Jul 10, 2015 at 1:32 PM, Todd Palino  wrote:
> >
> >> My selfish point of view is that we do #1, as we use "_" extensively in
> >> topic names here :) I also happen to think it's the right choice,
> >> specifically because "." has more special meanings, as you noted.
> >>
> >> -Todd
> >>
> >>
> >> On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira 
> >> wrote:
> >>
> >> > Unintentional side effect from allowing IP addresses in consumer
> client
> >> > IDs :)
> >> >
> >> > So the question is, what do we do now?
> >> >
> >> > 1) disallow "."
> >> > 2) disallow "_"
> >> > 3) find a reversible way to encode "." and "_" that won't break
> existing
> >> > metrics
> >> > 4) all of the above?
> >> >
> >> > btw. it looks like "." and ".." are currently valid. Topic names are
> >> > used for directories, right? this sounds like fun :)
> >> >
> >> > I vote for option #1, although if someone has a good idea for #3 it
> >> > will be even better.
> >> >
> >> > Gwen
> >> >
> >> >
> >> >
> >> > On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke 
> >> wrote:
> >> > > Found it was added here:
> >> https://issues.apache.org/jira/browse/KAFKA-697
> >> > >
> >> > > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino 
> >> wrote:
> >> > >
> >> > >> This was definitely changed at some point after KAFKA-495. The
> >> question
> >> > is
> >> > >> when and why.
> >> > >>
> >> > >> Here's the relevant code from that patch:
> >> > >>
> >> > >> ===
> >> > >> --- core/src/main/scala/kafka/utils/Topic.scala (revision 1390178)
> >> > >> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
> >> > >> @@ -21,24 +21,21 @@
> >> > >>  import util.matching.Regex
> >> > >>
> >> > >>  object Topic {
> >> > >> +  val legalChars = "[a-zA-Z0-9_-]"
> >> > >>
> >> > >>
> >> > >>
> >> > >> -Todd
> >> > >>
> >> > >>
> >> > >> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke 
> >> > wrote:
> >> > >>
> >> > >> > kafka.common.Topic shows that currently period is a valid
> character
> >> > and I
> >> > >> > have verified I can use kafka-topics.sh to create a new topic
> with a
> >> > >> > period.
> >> > >> >
> >> > >> >
> >> > >> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK
> currently
> >> > uses
> >> > >> > Topic.validate before writing to Zookeeper.
> >> > >> >
> >> > >> > Should period character support be removed? I was under the same
> >> > >> impression
> >> > >> > as Gwen, that a period was used by many as a way to "group"
> topics.
> >> > >> >
> >> > >> > The code is pasted below since its small:
> >> > >> >
> >> > >> > object Topic {
> >> > >> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
> >> > >> >   private val maxNameLength = 255
> >> > >> >   private val rgx = new Regex(legalChars + "+")
> >> > >> >
> >> > >> >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
> >> > >> >
> >> > >> >   def validate(topic: String) {
> >> > >> > if (topic.length <= 0)
> >> > >> >   throw new InvalidTopicException("topic name is illegal,
> can't
> >> be
> >> > >> > empty")
> >> > >> > else if (topic.equals(".") || topic.equals(".."))
> >> > >> >   throw new InvalidTopicException("topic name cannot be
> \".\" or
> >> > >> > \"..\"")
> >> > >> > else if (topic.length > maxNameLength)
> >> > >> >   throw new InvalidTopicException("topic name is illegal,
> can't
> >> be
> >> > >> > longer than " + maxNameLength + " characters")
> >> > >> >
> >> > >> > rgx.findFirstIn(topic) match {
> >> > >> >   case Some(t) =>
> >> > >> > if (!t.equals(topic))
> >> > >> >   throw new InvalidTopicException("topic name " + topic
> + "
> >> is
> >> > >> > illegal, contains a character other than ASCII alphanumerics,
> '.',
> >> '_'
> >> > >> and
> >> > >> > '-'")
> >> > >> >   case None => throw new InvalidTopicException("topic name "
> +
> >> > topic
> >> > >> +
> >> > >> > " is illegal,  contains a character other than A

Re: [Discussion] Limitations on topic names

[Ashish Singh]

The problem with '.' seems only to be in case of metrics. Should kafka
replace '.' with some special character, not in [a-zA-Z0-9\\._\\-] or some
reserved seq of characters?

On Fri, Jul 10, 2015 at 2:08 PM, Neha Narkhede  wrote:

> "." seems natural for grouping topic names. +1 for 2) going forward only
> without breaking previously created topics with "_" though that might
> require us to patch the code somewhat awkwardly till we phase it out a
> couple (purposely left vague to stay out of Ewen's wrath :-)) versions
> later.
>
> On Fri, Jul 10, 2015 at 2:02 PM, Gwen Shapira 
> wrote:
>
> > I don't think we should break existing topics. Just disallow new
> > topics going forward.
> >
> > Agree that having both is horrible, but we should have a solution that
> > fails when you run "kafka_topics.sh --create", not when you configure
> > Ganglia.
> >
> > Gwen
> >
> > On Fri, Jul 10, 2015 at 1:53 PM, Jay Kreps  wrote:
> > > Unfortunately '.' is pretty common too. I agree that it is perverse,
> but
> > > people seem to do it. Breaking all the topics with '.' in the name
> seems
> > > like it could be worse than combining metrics for people who have a
> > > 'foo_bar' AND 'foo.bar' (and after all, having both is DEEPLY perverse,
> > > no?).
> > >
> > > Where is our Dean of Compatibility, Ewen, on this?
> > >
> > > -Jay
> > >
> > > On Fri, Jul 10, 2015 at 1:32 PM, Todd Palino 
> wrote:
> > >
> > >> My selfish point of view is that we do #1, as we use "_" extensively
> in
> > >> topic names here :) I also happen to think it's the right choice,
> > >> specifically because "." has more special meanings, as you noted.
> > >>
> > >> -Todd
> > >>
> > >>
> > >> On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira 
> > >> wrote:
> > >>
> > >> > Unintentional side effect from allowing IP addresses in consumer
> > client
> > >> > IDs :)
> > >> >
> > >> > So the question is, what do we do now?
> > >> >
> > >> > 1) disallow "."
> > >> > 2) disallow "_"
> > >> > 3) find a reversible way to encode "." and "_" that won't break
> > existing
> > >> > metrics
> > >> > 4) all of the above?
> > >> >
> > >> > btw. it looks like "." and ".." are currently valid. Topic names are
> > >> > used for directories, right? this sounds like fun :)
> > >> >
> > >> > I vote for option #1, although if someone has a good idea for #3 it
> > >> > will be even better.
> > >> >
> > >> > Gwen
> > >> >
> > >> >
> > >> >
> > >> > On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke 
> > >> wrote:
> > >> > > Found it was added here:
> > >> https://issues.apache.org/jira/browse/KAFKA-697
> > >> > >
> > >> > > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino 
> > >> wrote:
> > >> > >
> > >> > >> This was definitely changed at some point after KAFKA-495. The
> > >> question
> > >> > is
> > >> > >> when and why.
> > >> > >>
> > >> > >> Here's the relevant code from that patch:
> > >> > >>
> > >> > >>
> ===
> > >> > >> --- core/src/main/scala/kafka/utils/Topic.scala (revision
> 1390178)
> > >> > >> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
> > >> > >> @@ -21,24 +21,21 @@
> > >> > >>  import util.matching.Regex
> > >> > >>
> > >> > >>  object Topic {
> > >> > >> +  val legalChars = "[a-zA-Z0-9_-]"
> > >> > >>
> > >> > >>
> > >> > >>
> > >> > >> -Todd
> > >> > >>
> > >> > >>
> > >> > >> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke <
> ghe...@cloudera.com>
> > >> > wrote:
> > >> > >>
> > >> > >> > kafka.common.Topic shows that currently period is a valid
> > character
> > >> > and I
> > >> > >> > have verified I can use kafka-topics.sh to create a new topic
> > with a
> > >> > >> > period.
> > >> > >> >
> > >> > >> >
> > >> > >> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK
> > currently
> > >> > uses
> > >> > >> > Topic.validate before writing to Zookeeper.
> > >> > >> >
> > >> > >> > Should period character support be removed? I was under the
> same
> > >> > >> impression
> > >> > >> > as Gwen, that a period was used by many as a way to "group"
> > topics.
> > >> > >> >
> > >> > >> > The code is pasted below since its small:
> > >> > >> >
> > >> > >> > object Topic {
> > >> > >> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
> > >> > >> >   private val maxNameLength = 255
> > >> > >> >   private val rgx = new Regex(legalChars + "+")
> > >> > >> >
> > >> > >> >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
> > >> > >> >
> > >> > >> >   def validate(topic: String) {
> > >> > >> > if (topic.length <= 0)
> > >> > >> >   throw new InvalidTopicException("topic name is illegal,
> > can't
> > >> be
> > >> > >> > empty")
> > >> > >> > else if (topic.equals(".") || topic.equals(".."))
> > >> > >> >   throw new InvalidTopicException("topic name cannot be
> > \".\" or
> > >> > >> > \"..\"")
> > >> > >> > else if (topic.length > maxNameLength)
> > >> > >> >   throw new InvalidTopicException("topic name is illegal,
> > can't
> > >> be
> > >> > >> > longer than " + m

Re: [DISCUSS] Using GitHub Pull Requests for contributions and code review

[Ismael Juma]

Hi Guozhang,

Comments inline.

On Fri, Jul 10, 2015 at 8:47 PM, Guozhang Wang  wrote:
>
> I have a couple of comments on the wiki pages / merge scripts:
>

Thanks, it's important to discuss these things as the current version is
based on what the Spark project does and may not match what we want to do
exactly.

1. In the wiki page it mentions "If the change is new, then it usually
> needs a new JIRA. However, trivial changes, where "what should change" is
> virtually the same as "how it should change" do not require a JIRA.
> Example: "Fix typos in Foo scaladoc"."
>
> So it sounds we are going to allow pull requests without a JIRA ticket, but
> later we are also mentioning:
>
> "The PR title should be of the form [KAFKA-].." which is a bit
> conflicting with the previous statement. Could you clarify it? Today our
> commits are mostly with JIRAs except some trivial ones that are only done
> by committers, I can either extend this to non-committers or not, just that
> we need to make it clear.
>

I agree that it's not very clear and it needs to be fixed. What do we want
to do though? It's a trade-off between consistency (always have a JIRA
ticket) and avoiding redundancy (skip the JIRA where it doesn't add
anything). The former is the more conservative option and I am happy to
update the documentation if that's the preferred option.

2. The git commit message is a little verbose to me, for example:


> --
>
> commit ee88dbb67f19b787e12ccef37982c9459b78c7b6
>
> Author: Geoff Anderson 
>
> Date:   Thu Jul 9 14:58:01 2015 -0700
>
>KAFKA-2327; broker doesn't start if config defines advertised.host but
> not advertised.port
>
>
>
>Added unit tests as well. These fail without the fix, but pass with the
> fix.
>
>
>
>Author: Geoff Anderson 
>
>
>
>Closes #73 from granders/KAFKA-2327 and squashes the following commits:
>
>
>
>52a2085 [Geoff Anderson] Cleaned up unecessary toString calls
>
>23b3340 [Geoff Anderson] Fixes KAFKA-2327
>
> --
>
>
> The "Author" field is redundant, and we could probably also omit the github
> commits. What do you think?
>

Is it harmful to have detailed information? None of it is actually
redundant as far as I can see (but I could be missing something). There is
the squashed commit author, the pull request message, the pull request
author and the information about the individual commits. Even though Geoff
worked on this PR by himself, multiple people can collaborate on a feature
and then it's useful to credit correctly.

The fact that we are keeping all the information encourages a style of
development where a few small commits (each commit should must make sense
on its own and pass the tests) are used in the pull request, which helps a
lot during code review and when trying to understand changes after the
fact. This is in contrast with the style where there is a single commit per
feature even when the feature is quite large (we have a few of those
ongoing at the moment). I don't know if you noticed, but GitHub actually
links to the original commits, which is quite handy:

https://github.com/apache/kafka/commit/ee88dbb67f19b787e12ccef37982c9459b78c7b6

This approach is a bit of a workaround, but it's easier as the script
handles everything (credit to the Spark project once again, it's their
code). The ideal scenario would be to keep the individual commits by
merging the branch into trunk after a rebase (no fast-forward, so that a
merge commit is maintained). This has a number of nice characteristics
(history is linear, it's clear what commits were part of the merged branch,
easy to bisect, easy to revert, etc.), but it requires a bit more work,
more Git knowledge and ideally a PR builder that builds and tests every
commit in the branch (Typesafe wrote a tool that does this for the Scala
project, for what is worth). In other words, a step too far for us right
now. :)

What do you think?

Best,
Ismael

Re: [Discussion] Limitations on topic names

[Todd Palino]

I absolutely disagree with #2, Neha. That will break a lot of
infrastructure within LinkedIn. That said, removing "." might break other
people as well, but I think we should have a clearer idea of how much usage
there is on either side.

-Todd


On Fri, Jul 10, 2015 at 2:08 PM, Neha Narkhede  wrote:

> "." seems natural for grouping topic names. +1 for 2) going forward only
> without breaking previously created topics with "_" though that might
> require us to patch the code somewhat awkwardly till we phase it out a
> couple (purposely left vague to stay out of Ewen's wrath :-)) versions
> later.
>
> On Fri, Jul 10, 2015 at 2:02 PM, Gwen Shapira 
> wrote:
>
> > I don't think we should break existing topics. Just disallow new
> > topics going forward.
> >
> > Agree that having both is horrible, but we should have a solution that
> > fails when you run "kafka_topics.sh --create", not when you configure
> > Ganglia.
> >
> > Gwen
> >
> > On Fri, Jul 10, 2015 at 1:53 PM, Jay Kreps  wrote:
> > > Unfortunately '.' is pretty common too. I agree that it is perverse,
> but
> > > people seem to do it. Breaking all the topics with '.' in the name
> seems
> > > like it could be worse than combining metrics for people who have a
> > > 'foo_bar' AND 'foo.bar' (and after all, having both is DEEPLY perverse,
> > > no?).
> > >
> > > Where is our Dean of Compatibility, Ewen, on this?
> > >
> > > -Jay
> > >
> > > On Fri, Jul 10, 2015 at 1:32 PM, Todd Palino 
> wrote:
> > >
> > >> My selfish point of view is that we do #1, as we use "_" extensively
> in
> > >> topic names here :) I also happen to think it's the right choice,
> > >> specifically because "." has more special meanings, as you noted.
> > >>
> > >> -Todd
> > >>
> > >>
> > >> On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira 
> > >> wrote:
> > >>
> > >> > Unintentional side effect from allowing IP addresses in consumer
> > client
> > >> > IDs :)
> > >> >
> > >> > So the question is, what do we do now?
> > >> >
> > >> > 1) disallow "."
> > >> > 2) disallow "_"
> > >> > 3) find a reversible way to encode "." and "_" that won't break
> > existing
> > >> > metrics
> > >> > 4) all of the above?
> > >> >
> > >> > btw. it looks like "." and ".." are currently valid. Topic names are
> > >> > used for directories, right? this sounds like fun :)
> > >> >
> > >> > I vote for option #1, although if someone has a good idea for #3 it
> > >> > will be even better.
> > >> >
> > >> > Gwen
> > >> >
> > >> >
> > >> >
> > >> > On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke 
> > >> wrote:
> > >> > > Found it was added here:
> > >> https://issues.apache.org/jira/browse/KAFKA-697
> > >> > >
> > >> > > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino 
> > >> wrote:
> > >> > >
> > >> > >> This was definitely changed at some point after KAFKA-495. The
> > >> question
> > >> > is
> > >> > >> when and why.
> > >> > >>
> > >> > >> Here's the relevant code from that patch:
> > >> > >>
> > >> > >>
> ===
> > >> > >> --- core/src/main/scala/kafka/utils/Topic.scala (revision
> 1390178)
> > >> > >> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
> > >> > >> @@ -21,24 +21,21 @@
> > >> > >>  import util.matching.Regex
> > >> > >>
> > >> > >>  object Topic {
> > >> > >> +  val legalChars = "[a-zA-Z0-9_-]"
> > >> > >>
> > >> > >>
> > >> > >>
> > >> > >> -Todd
> > >> > >>
> > >> > >>
> > >> > >> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke <
> ghe...@cloudera.com>
> > >> > wrote:
> > >> > >>
> > >> > >> > kafka.common.Topic shows that currently period is a valid
> > character
> > >> > and I
> > >> > >> > have verified I can use kafka-topics.sh to create a new topic
> > with a
> > >> > >> > period.
> > >> > >> >
> > >> > >> >
> > >> > >> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK
> > currently
> > >> > uses
> > >> > >> > Topic.validate before writing to Zookeeper.
> > >> > >> >
> > >> > >> > Should period character support be removed? I was under the
> same
> > >> > >> impression
> > >> > >> > as Gwen, that a period was used by many as a way to "group"
> > topics.
> > >> > >> >
> > >> > >> > The code is pasted below since its small:
> > >> > >> >
> > >> > >> > object Topic {
> > >> > >> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
> > >> > >> >   private val maxNameLength = 255
> > >> > >> >   private val rgx = new Regex(legalChars + "+")
> > >> > >> >
> > >> > >> >   val InternalTopics = Set(OffsetManager.OffsetsTopicName)
> > >> > >> >
> > >> > >> >   def validate(topic: String) {
> > >> > >> > if (topic.length <= 0)
> > >> > >> >   throw new InvalidTopicException("topic name is illegal,
> > can't
> > >> be
> > >> > >> > empty")
> > >> > >> > else if (topic.equals(".") || topic.equals(".."))
> > >> > >> >   throw new InvalidTopicException("topic name cannot be
> > \".\" or
> > >> > >> > \"..\"")
> > >> > >> > else if (topic.length > maxNameLength)
> > >> > >> >   throw new InvalidTopicException("top

Re: Review Request 36333: Patch for KAFKA-2123

[Jason Gustafson]

> On July 9, 2015, 1:42 a.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClient.java,
> >  lines 88-93
> > 
> >
> > This is not introduced in the patch, but I am not sure if this is the 
> > right way to respect backoff time. For example, if the destination broker 
> > is down for a short period of time, poll(retryBackoffMs) will immediately 
> > return, and hence this function will busy triggering poll() and fluding the 
> > network with metadata requests right?
> > 
> > What we want in this case, is that the consumer should wait for 
> > retryBackoffMs before retry sending the next metadata request.
> 
> Jason Gustafson wrote:
> This code was lifted from KafkaConsumer, but I think you are right. I'll 
> fix it for the next patch.

Actually, I think this implementation may be reasonable. Retries and backoff 
are already handled in NetworkClient, so this wouldn't flood the network with 
additional requests. I think we could use poll(Long.MAX_VALUE) if we wanted 
though.


> On July 9, 2015, 1:42 a.m., Guozhang Wang wrote:
> > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, 
> > line 775
> > 
> >
> > I think KAFKA-1894 is already fixed in this patch + KAFKA-2168?
> 
> Jason Gustafson wrote:
> I think this is still debatable. Is wakeup() sufficient to assuage our 
> guilt for allowing poll to block indefinitely in spite of the passed timeout? 
> Perhaps I'm the only one, but I'm still holding out hope that we'll be able 
> to enforce the timeout even if we are in the middle of a join group. The code 
> is actually not that far from being able to do so.
> 
> Guozhang Wang wrote:
> Yeah makes sense.
> 
> Ewen Cheslack-Postava wrote:
> I'm still holding out hope as well :) There were one or two places in 
> this patch where timeouts aren't passed into some methods where I think they 
> could be passed in and respsected safely, but I didn't want to make this 
> patch even bigger.
> 
> Gwen Shapira wrote:
> Same here :) Is there a JIRA to track this plan?

My plan was to resolve this as part of KAFKA-1894, but perhaps a different 
ticket is needed?


- Jason


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


On July 8, 2015, 9:19 p.m., Jason Gustafson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36333/
> ---
> 
> (Updated July 8, 2015, 9:19 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2123
> https://issues.apache.org/jira/browse/KAFKA-2123
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2123; resolve problems from rebase
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java 
> fd98740bff175cc9d5bc02e365d88e011ef65d22 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerCommitCallback.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRecords.java 
> eb75d2e797e3aa3992e4cf74b12f51c8f1545e02 
>   clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 
> 7aa076084c894bb8f47b9df2c086475b06f47060 
>   clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 
> 46e26a665a22625d50888efa7b53472279f36e79 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClient.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java
>  c1c8172cd45f6715262f9a6f497a7b1797a834a3 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/DelayedTask.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/DelayedTaskQueue.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
>  695eaf63db9a5fa20dc2ca68957901462a96cd96 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Heartbeat.java
>  51eae1944d5c17cf838be57adf560bafe36fbfbd 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/NoAvailableBrokersException.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFuture.java
>  13fc9af7392b4ade958daf3b0c9a165ddda351a6 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFutureAdapter.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestFutureListener.java
>  PRE-CREATION 
>   
> cli

[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=14622967#comment-14622967
 ] 

Ashish K Singh commented on KAFKA-972:
--

[~junrao] could you take a look, thanks.

> 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, KAFKA-972.patch, 
> KAFKA-972_2015-06-30_18:42:13.patch, KAFKA-972_2015-07-01_01:36:56.patch, 
> KAFKA-972_2015-07-01_01:42:42.patch, KAFKA-972_2015-07-01_08:06:03.patch, 
> KAFKA-972_2015-07-06_23:07:34.patch, KAFKA-972_2015-07-07_10:42:41.patch, 
> KAFKA-972_2015-07-07_23:24:13.patch
>
>
> 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": [

Re: [DISCUSS] Using GitHub Pull Requests for contributions and code review

[Guozhang Wang]

Commends inlined.

On Fri, Jul 10, 2015 at 2:10 PM, Ismael Juma  wrote:

> Hi Guozhang,
>
> Comments inline.
>
> On Fri, Jul 10, 2015 at 8:47 PM, Guozhang Wang  wrote:
> >
> > I have a couple of comments on the wiki pages / merge scripts:
> >
>
> Thanks, it's important to discuss these things as the current version is
> based on what the Spark project does and may not match what we want to do
> exactly.
>
> 1. In the wiki page it mentions "If the change is new, then it usually
> > needs a new JIRA. However, trivial changes, where "what should change" is
> > virtually the same as "how it should change" do not require a JIRA.
> > Example: "Fix typos in Foo scaladoc"."
> >
> > So it sounds we are going to allow pull requests without a JIRA ticket,
> but
> > later we are also mentioning:
> >
> > "The PR title should be of the form [KAFKA-].." which is a bit
> > conflicting with the previous statement. Could you clarify it? Today our
> > commits are mostly with JIRAs except some trivial ones that are only done
> > by committers, I can either extend this to non-committers or not, just
> that
> > we need to make it clear.
> >
>
> I agree that it's not very clear and it needs to be fixed. What do we want
> to do though? It's a trade-off between consistency (always have a JIRA
> ticket) and avoiding redundancy (skip the JIRA where it doesn't add
> anything). The former is the more conservative option and I am happy to
> update the documentation if that's the preferred option.
>
>
Personally I think it is better to not enforcing a JIRA ticket for minor /
hotfix commits, for example, we can format the title with [MINOR] [HOTFIX]
etc as in Spark:

https://github.com/apache/spark/commits/master

But I think it we need some broader discussion for this, maybe we could
start another email thread regarding this?


> 2. The git commit message is a little verbose to me, for example:
>
>
> > --
> >
> > commit ee88dbb67f19b787e12ccef37982c9459b78c7b6
> >
> > Author: Geoff Anderson 
> >
> > Date:   Thu Jul 9 14:58:01 2015 -0700
> >
> >KAFKA-2327; broker doesn't start if config defines advertised.host but
> > not advertised.port
> >
> >
> >
> >Added unit tests as well. These fail without the fix, but pass with
> the
> > fix.
> >
> >
> >
> >Author: Geoff Anderson 
> >
> >
> >
> >Closes #73 from granders/KAFKA-2327 and squashes the following
> commits:
> >
> >
> >
> >52a2085 [Geoff Anderson] Cleaned up unecessary toString calls
> >
> >23b3340 [Geoff Anderson] Fixes KAFKA-2327
> >
> > --
> >
> >
> > The "Author" field is redundant, and we could probably also omit the
> github
> > commits. What do you think?
> >
>
> Is it harmful to have detailed information? None of it is actually
> redundant as far as I can see (but I could be missing something). There is
> the squashed commit author, the pull request message, the pull request
> author and the information about the individual commits. Even though Geoff
> worked on this PR by himself, multiple people can collaborate on a feature
> and then it's useful to credit correctly.
>
> The fact that we are keeping all the information encourages a style of
> development where a few small commits (each commit should must make sense
> on its own and pass the tests) are used in the pull request, which helps a
> lot during code review and when trying to understand changes after the
> fact. This is in contrast with the style where there is a single commit per
> feature even when the feature is quite large (we have a few of those
> ongoing at the moment). I don't know if you noticed, but GitHub actually
> links to the original commits, which is quite handy:
>
>
> https://github.com/apache/kafka/commit/ee88dbb67f19b787e12ccef37982c9459b78c7b6
>
> This approach is a bit of a workaround, but it's easier as the script
> handles everything (credit to the Spark project once again, it's their
> code). The ideal scenario would be to keep the individual commits by
> merging the branch into trunk after a rebase (no fast-forward, so that a
> merge commit is maintained). This has a number of nice characteristics
> (history is linear, it's clear what commits were part of the merged branch,
> easy to bisect, easy to revert, etc.), but it requires a bit more work,
> more Git knowledge and ideally a PR builder that builds and tests every
> commit in the branch (Typesafe wrote a tool that does this for the Scala
> project, for what is worth). In other words, a step too far for us right
> now. :)
>
> What do you think?
>
>
I think I'm sold on your comments, makes sense :)


> Best,
> Ismael
>



-- 
-- Guozhang

Re: [Discussion] Limitations on topic names

[Gwen Shapira]

I find dots more common in my customer base, so I will definitely feel
the pain of removing them.

However, "." are already used in metrics, file names, directories, etc
- so if we keep the dots, we need to keep code that translates them
and document the translation. Just banning "." seems more natural.
Also, as Grant mentioned, we'll probably have our own special usage
for "." down the line.

On Fri, Jul 10, 2015 at 2:12 PM, Todd Palino  wrote:
> I absolutely disagree with #2, Neha. That will break a lot of
> infrastructure within LinkedIn. That said, removing "." might break other
> people as well, but I think we should have a clearer idea of how much usage
> there is on either side.
>
> -Todd
>
>
> On Fri, Jul 10, 2015 at 2:08 PM, Neha Narkhede  wrote:
>
>> "." seems natural for grouping topic names. +1 for 2) going forward only
>> without breaking previously created topics with "_" though that might
>> require us to patch the code somewhat awkwardly till we phase it out a
>> couple (purposely left vague to stay out of Ewen's wrath :-)) versions
>> later.
>>
>> On Fri, Jul 10, 2015 at 2:02 PM, Gwen Shapira 
>> wrote:
>>
>> > I don't think we should break existing topics. Just disallow new
>> > topics going forward.
>> >
>> > Agree that having both is horrible, but we should have a solution that
>> > fails when you run "kafka_topics.sh --create", not when you configure
>> > Ganglia.
>> >
>> > Gwen
>> >
>> > On Fri, Jul 10, 2015 at 1:53 PM, Jay Kreps  wrote:
>> > > Unfortunately '.' is pretty common too. I agree that it is perverse,
>> but
>> > > people seem to do it. Breaking all the topics with '.' in the name
>> seems
>> > > like it could be worse than combining metrics for people who have a
>> > > 'foo_bar' AND 'foo.bar' (and after all, having both is DEEPLY perverse,
>> > > no?).
>> > >
>> > > Where is our Dean of Compatibility, Ewen, on this?
>> > >
>> > > -Jay
>> > >
>> > > On Fri, Jul 10, 2015 at 1:32 PM, Todd Palino 
>> wrote:
>> > >
>> > >> My selfish point of view is that we do #1, as we use "_" extensively
>> in
>> > >> topic names here :) I also happen to think it's the right choice,
>> > >> specifically because "." has more special meanings, as you noted.
>> > >>
>> > >> -Todd
>> > >>
>> > >>
>> > >> On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira 
>> > >> wrote:
>> > >>
>> > >> > Unintentional side effect from allowing IP addresses in consumer
>> > client
>> > >> > IDs :)
>> > >> >
>> > >> > So the question is, what do we do now?
>> > >> >
>> > >> > 1) disallow "."
>> > >> > 2) disallow "_"
>> > >> > 3) find a reversible way to encode "." and "_" that won't break
>> > existing
>> > >> > metrics
>> > >> > 4) all of the above?
>> > >> >
>> > >> > btw. it looks like "." and ".." are currently valid. Topic names are
>> > >> > used for directories, right? this sounds like fun :)
>> > >> >
>> > >> > I vote for option #1, although if someone has a good idea for #3 it
>> > >> > will be even better.
>> > >> >
>> > >> > Gwen
>> > >> >
>> > >> >
>> > >> >
>> > >> > On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke 
>> > >> wrote:
>> > >> > > Found it was added here:
>> > >> https://issues.apache.org/jira/browse/KAFKA-697
>> > >> > >
>> > >> > > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino 
>> > >> wrote:
>> > >> > >
>> > >> > >> This was definitely changed at some point after KAFKA-495. The
>> > >> question
>> > >> > is
>> > >> > >> when and why.
>> > >> > >>
>> > >> > >> Here's the relevant code from that patch:
>> > >> > >>
>> > >> > >>
>> ===
>> > >> > >> --- core/src/main/scala/kafka/utils/Topic.scala (revision
>> 1390178)
>> > >> > >> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
>> > >> > >> @@ -21,24 +21,21 @@
>> > >> > >>  import util.matching.Regex
>> > >> > >>
>> > >> > >>  object Topic {
>> > >> > >> +  val legalChars = "[a-zA-Z0-9_-]"
>> > >> > >>
>> > >> > >>
>> > >> > >>
>> > >> > >> -Todd
>> > >> > >>
>> > >> > >>
>> > >> > >> On Fri, Jul 10, 2015 at 1:02 PM, Grant Henke <
>> ghe...@cloudera.com>
>> > >> > wrote:
>> > >> > >>
>> > >> > >> > kafka.common.Topic shows that currently period is a valid
>> > character
>> > >> > and I
>> > >> > >> > have verified I can use kafka-topics.sh to create a new topic
>> > with a
>> > >> > >> > period.
>> > >> > >> >
>> > >> > >> >
>> > >> > >> > AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK
>> > currently
>> > >> > uses
>> > >> > >> > Topic.validate before writing to Zookeeper.
>> > >> > >> >
>> > >> > >> > Should period character support be removed? I was under the
>> same
>> > >> > >> impression
>> > >> > >> > as Gwen, that a period was used by many as a way to "group"
>> > topics.
>> > >> > >> >
>> > >> > >> > The code is pasted below since its small:
>> > >> > >> >
>> > >> > >> > object Topic {
>> > >> > >> >   val legalChars = "[a-zA-Z0-9\\._\\-]"
>> > >> > >> >   private val maxNameLength = 255
>> > >> > >> >   private val rgx = new Regex(legalChars + "+")
>> 

[jira] [Commented] (KAFKA-2032) ConsumerConfig doesn't validate partition.assignment.strategy values

[Parth Brahmbhatt (JIRA)]

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

Parth Brahmbhatt commented on KAFKA-2032:
-

Pinging for review from committers.

> ConsumerConfig doesn't validate partition.assignment.strategy values
> 
>
> Key: KAFKA-2032
> URL: https://issues.apache.org/jira/browse/KAFKA-2032
> Project: Kafka
>  Issue Type: Bug
>Affects Versions: 0.8.1.2
>Reporter: Jason Rosenberg
>Assignee: Parth Brahmbhatt
> Attachments: KAFKA-2032.patch, KAFKA-2032.patch, 
> KAFKA-2032_2015-03-19_11:42:07.patch, KAFKA-2032_2015-03-19_11:44:48.patch, 
> KAFKA-2032_2015-03-19_11:47:24.patch, KAFKA-2032_2015-03-19_12:19:45.patch
>
>
> In the ConsumerConfig class, there are validation checks to make sure that 
> string based configuration properties conform to allowed values.  However, 
> this validation appears to be missing for the partition.assignment.strategy.  
> E.g. there is validation for autooffset.reset and offsets.storage.



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

[jira] [Commented] (KAFKA-2162) Kafka Auditing functionality

[Parth Brahmbhatt (JIRA)]

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

Parth Brahmbhatt commented on KAFKA-2162:
-

There are a few options here, we could either make auditor a separate interface 
of its own or we can assume that audit functionality will be implemented by the 
authoizers as well. The default authorizer implementation just uses log.debug 
for auditing and we can optimize it so it does some batching. I like the second 
option. 

> Kafka Auditing functionality
> 
>
> Key: KAFKA-2162
> URL: https://issues.apache.org/jira/browse/KAFKA-2162
> Project: Kafka
>  Issue Type: Bug
>Reporter: Sriharsha Chintalapani
>Assignee: Parth Brahmbhatt
>
> During Kafka authorization  discussion thread . There was concerns raised 
> about not having Auditing. Auditing is important functionality but its not 
> part of authorizer. This jira will track adding audit functionality to kafka.



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

[jira] [Commented] (KAFKA-1695) Authenticate connection to Zookeeper

[Parth Brahmbhatt (JIRA)]

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

Parth Brahmbhatt commented on KAFKA-1695:
-

[~gwenshap] I have upgraded zkClient to 0.5 however for existing clusters 
trying to move to security the changes in 0.5 were not enough. I submitted a 
patch to zkClient to set and get Acls for existing paths and it has been 
committed to the trunk. 
https://github.com/sgroschupf/zkclient/commit/c5d1dd2373eab343d606a0797d58664c0ee4781d.
 

ZkClient has not yet released a new version with that change so we will 
probably have to wait for the next release but once that is done [~gwenshap] if 
you don't mind I would like to take over this jira. I have already implemented 
setting acls and authentication to zookeeper as part of authorizer work. 

> Authenticate connection to Zookeeper
> 
>
> Key: KAFKA-1695
> URL: https://issues.apache.org/jira/browse/KAFKA-1695
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Gwen Shapira
>
> We need to make it possible to secure the Zookeeper cluster Kafka is using. 
> This would make use of the normal authentication ZooKeeper provides. 
> ZooKeeper supports a variety of authentication mechanisms so we will need to 
> figure out what has to be passed in to the zookeeper client.
> The intention is that when the current round of client work is done it should 
> be possible to run without clients needing access to Zookeeper so all we need 
> here is to make it so that only the Kafka cluster is able to read and write 
> to the Kafka znodes  (we shouldn't need to set any kind of acl on a per-znode 
> basis).



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

Re: [Discussion] Limitations on topic names

[Ewen Cheslack-Postava]

I figure you'll probably see complaints no matter what change you make.
Gwen, given that you raised this, another important question might be how
many people you see using *both*. I'm guessing this question came up
because you actually saw a conflict? But I'd imagine (or at least hope)
that most organizations are mostly consistent about naming topics -- they
standardize on one or the other.

Since there's no "right" way to name them, I'd just leave it supporting
both and document the potential conflict in metrics. And if people use both
naming schemes, they probably deserve to suffer for their inconsistency :)

-Ewen

On Fri, Jul 10, 2015 at 3:28 PM, Gwen Shapira  wrote:

> I find dots more common in my customer base, so I will definitely feel
> the pain of removing them.
>
> However, "." are already used in metrics, file names, directories, etc
> - so if we keep the dots, we need to keep code that translates them
> and document the translation. Just banning "." seems more natural.
> Also, as Grant mentioned, we'll probably have our own special usage
> for "." down the line.
>
> On Fri, Jul 10, 2015 at 2:12 PM, Todd Palino  wrote:
> > I absolutely disagree with #2, Neha. That will break a lot of
> > infrastructure within LinkedIn. That said, removing "." might break other
> > people as well, but I think we should have a clearer idea of how much
> usage
> > there is on either side.
> >
> > -Todd
> >
> >
> > On Fri, Jul 10, 2015 at 2:08 PM, Neha Narkhede 
> wrote:
> >
> >> "." seems natural for grouping topic names. +1 for 2) going forward only
> >> without breaking previously created topics with "_" though that might
> >> require us to patch the code somewhat awkwardly till we phase it out a
> >> couple (purposely left vague to stay out of Ewen's wrath :-)) versions
> >> later.
> >>
> >> On Fri, Jul 10, 2015 at 2:02 PM, Gwen Shapira 
> >> wrote:
> >>
> >> > I don't think we should break existing topics. Just disallow new
> >> > topics going forward.
> >> >
> >> > Agree that having both is horrible, but we should have a solution that
> >> > fails when you run "kafka_topics.sh --create", not when you configure
> >> > Ganglia.
> >> >
> >> > Gwen
> >> >
> >> > On Fri, Jul 10, 2015 at 1:53 PM, Jay Kreps  wrote:
> >> > > Unfortunately '.' is pretty common too. I agree that it is perverse,
> >> but
> >> > > people seem to do it. Breaking all the topics with '.' in the name
> >> seems
> >> > > like it could be worse than combining metrics for people who have a
> >> > > 'foo_bar' AND 'foo.bar' (and after all, having both is DEEPLY
> perverse,
> >> > > no?).
> >> > >
> >> > > Where is our Dean of Compatibility, Ewen, on this?
> >> > >
> >> > > -Jay
> >> > >
> >> > > On Fri, Jul 10, 2015 at 1:32 PM, Todd Palino 
> >> wrote:
> >> > >
> >> > >> My selfish point of view is that we do #1, as we use "_"
> extensively
> >> in
> >> > >> topic names here :) I also happen to think it's the right choice,
> >> > >> specifically because "." has more special meanings, as you noted.
> >> > >>
> >> > >> -Todd
> >> > >>
> >> > >>
> >> > >> On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira <
> gshap...@cloudera.com>
> >> > >> wrote:
> >> > >>
> >> > >> > Unintentional side effect from allowing IP addresses in consumer
> >> > client
> >> > >> > IDs :)
> >> > >> >
> >> > >> > So the question is, what do we do now?
> >> > >> >
> >> > >> > 1) disallow "."
> >> > >> > 2) disallow "_"
> >> > >> > 3) find a reversible way to encode "." and "_" that won't break
> >> > existing
> >> > >> > metrics
> >> > >> > 4) all of the above?
> >> > >> >
> >> > >> > btw. it looks like "." and ".." are currently valid. Topic names
> are
> >> > >> > used for directories, right? this sounds like fun :)
> >> > >> >
> >> > >> > I vote for option #1, although if someone has a good idea for #3
> it
> >> > >> > will be even better.
> >> > >> >
> >> > >> > Gwen
> >> > >> >
> >> > >> >
> >> > >> >
> >> > >> > On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke <
> ghe...@cloudera.com>
> >> > >> wrote:
> >> > >> > > Found it was added here:
> >> > >> https://issues.apache.org/jira/browse/KAFKA-697
> >> > >> > >
> >> > >> > > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino <
> tpal...@gmail.com>
> >> > >> wrote:
> >> > >> > >
> >> > >> > >> This was definitely changed at some point after KAFKA-495. The
> >> > >> question
> >> > >> > is
> >> > >> > >> when and why.
> >> > >> > >>
> >> > >> > >> Here's the relevant code from that patch:
> >> > >> > >>
> >> > >> > >>
> >> ===
> >> > >> > >> --- core/src/main/scala/kafka/utils/Topic.scala (revision
> >> 1390178)
> >> > >> > >> +++ core/src/main/scala/kafka/utils/Topic.scala (working copy)
> >> > >> > >> @@ -21,24 +21,21 @@
> >> > >> > >>  import util.matching.Regex
> >> > >> > >>
> >> > >> > >>  object Topic {
> >> > >> > >> +  val legalChars = "[a-zA-Z0-9_-]"
> >> > >> > >>
> >> > >> > >>
> >> > >> > >>
> >> > >> > >> -Todd
> >> > >> > >>
> >> > >> > >>
> >> > >> > >> On 

Re: [Discussion] Limitations on topic names

[Gwen Shapira]

Yeah, I have an actual customer who ran into this. Unfortunately,
inconsistencies in the way things are named are pretty common - just
look at Kafka's many CLI options.

I don't think that supporting both and pointing at the docs with "I
told you so" when our metrics break is a good solution.

On Fri, Jul 10, 2015 at 4:33 PM, Ewen Cheslack-Postava
 wrote:
> I figure you'll probably see complaints no matter what change you make.
> Gwen, given that you raised this, another important question might be how
> many people you see using *both*. I'm guessing this question came up
> because you actually saw a conflict? But I'd imagine (or at least hope)
> that most organizations are mostly consistent about naming topics -- they
> standardize on one or the other.
>
> Since there's no "right" way to name them, I'd just leave it supporting
> both and document the potential conflict in metrics. And if people use both
> naming schemes, they probably deserve to suffer for their inconsistency :)
>
> -Ewen
>
> On Fri, Jul 10, 2015 at 3:28 PM, Gwen Shapira  wrote:
>
>> I find dots more common in my customer base, so I will definitely feel
>> the pain of removing them.
>>
>> However, "." are already used in metrics, file names, directories, etc
>> - so if we keep the dots, we need to keep code that translates them
>> and document the translation. Just banning "." seems more natural.
>> Also, as Grant mentioned, we'll probably have our own special usage
>> for "." down the line.
>>
>> On Fri, Jul 10, 2015 at 2:12 PM, Todd Palino  wrote:
>> > I absolutely disagree with #2, Neha. That will break a lot of
>> > infrastructure within LinkedIn. That said, removing "." might break other
>> > people as well, but I think we should have a clearer idea of how much
>> usage
>> > there is on either side.
>> >
>> > -Todd
>> >
>> >
>> > On Fri, Jul 10, 2015 at 2:08 PM, Neha Narkhede 
>> wrote:
>> >
>> >> "." seems natural for grouping topic names. +1 for 2) going forward only
>> >> without breaking previously created topics with "_" though that might
>> >> require us to patch the code somewhat awkwardly till we phase it out a
>> >> couple (purposely left vague to stay out of Ewen's wrath :-)) versions
>> >> later.
>> >>
>> >> On Fri, Jul 10, 2015 at 2:02 PM, Gwen Shapira 
>> >> wrote:
>> >>
>> >> > I don't think we should break existing topics. Just disallow new
>> >> > topics going forward.
>> >> >
>> >> > Agree that having both is horrible, but we should have a solution that
>> >> > fails when you run "kafka_topics.sh --create", not when you configure
>> >> > Ganglia.
>> >> >
>> >> > Gwen
>> >> >
>> >> > On Fri, Jul 10, 2015 at 1:53 PM, Jay Kreps  wrote:
>> >> > > Unfortunately '.' is pretty common too. I agree that it is perverse,
>> >> but
>> >> > > people seem to do it. Breaking all the topics with '.' in the name
>> >> seems
>> >> > > like it could be worse than combining metrics for people who have a
>> >> > > 'foo_bar' AND 'foo.bar' (and after all, having both is DEEPLY
>> perverse,
>> >> > > no?).
>> >> > >
>> >> > > Where is our Dean of Compatibility, Ewen, on this?
>> >> > >
>> >> > > -Jay
>> >> > >
>> >> > > On Fri, Jul 10, 2015 at 1:32 PM, Todd Palino 
>> >> wrote:
>> >> > >
>> >> > >> My selfish point of view is that we do #1, as we use "_"
>> extensively
>> >> in
>> >> > >> topic names here :) I also happen to think it's the right choice,
>> >> > >> specifically because "." has more special meanings, as you noted.
>> >> > >>
>> >> > >> -Todd
>> >> > >>
>> >> > >>
>> >> > >> On Fri, Jul 10, 2015 at 1:30 PM, Gwen Shapira <
>> gshap...@cloudera.com>
>> >> > >> wrote:
>> >> > >>
>> >> > >> > Unintentional side effect from allowing IP addresses in consumer
>> >> > client
>> >> > >> > IDs :)
>> >> > >> >
>> >> > >> > So the question is, what do we do now?
>> >> > >> >
>> >> > >> > 1) disallow "."
>> >> > >> > 2) disallow "_"
>> >> > >> > 3) find a reversible way to encode "." and "_" that won't break
>> >> > existing
>> >> > >> > metrics
>> >> > >> > 4) all of the above?
>> >> > >> >
>> >> > >> > btw. it looks like "." and ".." are currently valid. Topic names
>> are
>> >> > >> > used for directories, right? this sounds like fun :)
>> >> > >> >
>> >> > >> > I vote for option #1, although if someone has a good idea for #3
>> it
>> >> > >> > will be even better.
>> >> > >> >
>> >> > >> > Gwen
>> >> > >> >
>> >> > >> >
>> >> > >> >
>> >> > >> > On Fri, Jul 10, 2015 at 1:22 PM, Grant Henke <
>> ghe...@cloudera.com>
>> >> > >> wrote:
>> >> > >> > > Found it was added here:
>> >> > >> https://issues.apache.org/jira/browse/KAFKA-697
>> >> > >> > >
>> >> > >> > > On Fri, Jul 10, 2015 at 3:18 PM, Todd Palino <
>> tpal...@gmail.com>
>> >> > >> wrote:
>> >> > >> > >
>> >> > >> > >> This was definitely changed at some point after KAFKA-495. The
>> >> > >> question
>> >> > >> > is
>> >> > >> > >> when and why.
>> >> > >> > >>
>> >> > >> > >> Here's the relevant code from that patch:
>> >> > >> > >>
>> >> > >> > >>
>> >> 

[jira] [Commented] (KAFKA-1695) Authenticate connection to Zookeeper

[Gwen Shapira (JIRA)]

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

Gwen Shapira commented on KAFKA-1695:
-

Sure, go ahead.

> Authenticate connection to Zookeeper
> 
>
> Key: KAFKA-1695
> URL: https://issues.apache.org/jira/browse/KAFKA-1695
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Gwen Shapira
>
> We need to make it possible to secure the Zookeeper cluster Kafka is using. 
> This would make use of the normal authentication ZooKeeper provides. 
> ZooKeeper supports a variety of authentication mechanisms so we will need to 
> figure out what has to be passed in to the zookeeper client.
> The intention is that when the current round of client work is done it should 
> be possible to run without clients needing access to Zookeeper so all we need 
> here is to make it so that only the Kafka cluster is able to read and write 
> to the Kafka znodes  (we shouldn't need to set any kind of acl on a per-znode 
> basis).



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

[jira] [Assigned] (KAFKA-1695) Authenticate connection to Zookeeper

[Parth Brahmbhatt (JIRA)]

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

Parth Brahmbhatt reassigned KAFKA-1695:
---

Assignee: Parth Brahmbhatt  (was: Gwen Shapira)

> Authenticate connection to Zookeeper
> 
>
> Key: KAFKA-1695
> URL: https://issues.apache.org/jira/browse/KAFKA-1695
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Parth Brahmbhatt
>
> We need to make it possible to secure the Zookeeper cluster Kafka is using. 
> This would make use of the normal authentication ZooKeeper provides. 
> ZooKeeper supports a variety of authentication mechanisms so we will need to 
> figure out what has to be passed in to the zookeeper client.
> The intention is that when the current round of client work is done it should 
> be possible to run without clients needing access to Zookeeper so all we need 
> here is to make it so that only the Kafka cluster is able to read and write 
> to the Kafka znodes  (we shouldn't need to set any kind of acl on a per-znode 
> basis).



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

[jira] [Commented] (KAFKA-2182) zkClient dies if there is any exception while reconnecting

[Parth Brahmbhatt (JIRA)]

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

Parth Brahmbhatt commented on KAFKA-2182:
-

[~junrao] I think I took care of this as part of KAFKA-2169. We now just 
system.exit when this exception is caught at least on the borker side. Can we 
close this jira as Fixed? Or am I missing the intent of this jira?

> zkClient dies if there is any exception while reconnecting
> --
>
> Key: KAFKA-2182
> URL: https://issues.apache.org/jira/browse/KAFKA-2182
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.8.1
>Reporter: Igor Maravić
>Assignee: Parth Brahmbhatt
>Priority: Critical
>
> We, Spotify, have just been hit by a BUG that's related to ZkClient. It made 
> a whole Kafka cluster go down.
> Long story short, we've restarted TOR switch and all of our brokers from the 
> cluster lost all the connectivity with the zookeeper cluster, which was 
> living in another rack. Because of that, all the connections to Zookeeper got 
> expired.
> Everything would be fine if we haven't lost hostname to IP Address mapping 
> for some reason. Since we did lost that mapping, we got a 
> UnknownHostException when the broker tried to reconnect. This exception got 
> swallowed up, and we never got reconnected to Zookeeper, which in turn made 
> our cluster of brokers useless.
> If we had "handleSessionEstablishmentError" function, the whole exception 
> could be caught, we could just completely kill KafkaServer process and start 
> it cleanly, since this kind of exception is fatal for the KafkaCluster.
> {code}
> 2015-05-05T12:49:01.709+00:00 127.0.0.1 apache-kafka[main-EventThread] INFO  
> zookeeper.ZooKeeper  - Initiating client connection, 
> connectString=zookeeper1.spotify.net:2181,zookeeper2.spotify.net:2181,zookeeper3.spotify.net:2181/gabobroker-local
>  sessionTimeout=6000 watcher=org.I0Itec.zkclient.ZkClient@7303d690
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 apache-kafka[main-EventThread] ERROR 
> zookeeper.ClientCnxn  - Error while calling watcher
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 java.lang.RuntimeException: Exception 
> while restarting zk client
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 at 
> org.I0Itec.zkclient.ZkClient.processStateChanged(ZkClient.java:462)
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 at 
> org.I0Itec.zkclient.ZkClient.process(ZkClient.java:368)
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 at 
> org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:522)
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 at 
> org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:498)
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 Caused by: 
> org.I0Itec.zkclient.exception.ZkException: Unable to connect to 
> zookeeper1.spotify.net:2181,zookeeper2.spotify.net:2181,zookeeper3.spotify.net:2181/gabobroker-local
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 at 
> org.I0Itec.zkclient.ZkConnection.connect(ZkConnection.java:66)
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 at 
> org.I0Itec.zkclient.ZkClient.reconnect(ZkClient.java:939)
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 at 
> org.I0Itec.zkclient.ZkClient.processStateChanged(ZkClient.java:458)
> 2015-05-05T12:49:01.711+00:00 127.0.0.1 ... 3 more
> 2015-05-05T12:49:01.712+00:00 127.0.0.1 Caused by: 
> java.net.UnknownHostException: zookeeper1.spotify.net: Name or service not 
> known
> 2015-05-05T12:49:01.712+00:00 127.0.0.1 at 
> java.net.Inet6AddressImpl.lookupAllHostAddr(Native Method)
> 2015-05-05T12:49:01.712+00:00 127.0.0.1 at 
> java.net.InetAddress$1.lookupAllHostAddr(InetAddress.java:901)
> 2015-05-05T12:49:01.712+00:00 127.0.0.1 at 
> java.net.InetAddress.getAddressesFromNameService(InetAddress.java:1293)
> 2015-05-05T12:49:01.712+00:00 127.0.0.1 at 
> java.net.InetAddress.getAllByName0(InetAddress.java:1246)
> 2015-05-05T12:49:01.712+00:00 127.0.0.1 at 
> java.net.InetAddress.getAllByName(InetAddress.java:1162)
> 2015-05-05T12:49:01.712+00:00 127.0.0.1 at 
> java.net.InetAddress.getAllByName(InetAddress.java:1098)
> 2015-05-05T12:49:01.712+00:00 127.0.0.1 at 
> org.apache.zookeeper.client.StaticHostProvider.(StaticHostProvider.java:61)
> 2015-05-05T12:49:01.712+00:00 127.0.0.1 at 
> org.apache.zookeeper.ZooKeeper.(ZooKeeper.java:445)
> 2015-05-05T12:49:01.712+00:00 127.0.0.1 at 
> org.apache.zookeeper.ZooKeeper.(ZooKeeper.java:380)
> 2015-05-05T12:49:01.713+00:00 127.0.0.1 at 
> org.I0Itec.zkclient.ZkConnection.connect(ZkConnection.java:64)
> 2015-05-05T12:49:01.713+00:00 127.0.0.1 ... 5 more
> 2015-05-05T12:49:01.713+00:00 127.0.0.1 
> apache-kafka[ZkClient-EventThread-18-zookeeper1.spotify.net:2181,zookeeper2.spotify.net:2181,zookeeper3.spotify.net:2181/gabobroker-local]
>  ERROR zkclient.ZkEventThre

[jira] [Commented] (KAFKA-2145) An option to add topic owners.

[Parth Brahmbhatt (JIRA)]

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

Parth Brahmbhatt commented on KAFKA-2145:
-

[~neelesh77] sorry for the delay on this

1) The objective is to have a Topic Owner field associated with the topic. 
Shall this be associated with the topic at all times?
Not sure what do you mean by "Shall this be associated with the topic at all 
times". The intent is to add a topic.config called owner which can either be a 
a string or a list of string (comma separated string) which is filled with the 
value of user executing the "create topic". This user could then add other 
users as co-owners in the same list. 
2) TopicCommand should be updated to reflect the owner field.

> An option to add topic owners. 
> ---
>
> Key: KAFKA-2145
> URL: https://issues.apache.org/jira/browse/KAFKA-2145
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Parth Brahmbhatt
>Assignee: Neelesh Srinivas Salian
>
> We need to expose a way so users can identify users/groups that share 
> ownership of topic. We discussed adding this as part of 
> https://issues.apache.org/jira/browse/KAFKA-2035 and agreed that it will be 
> simpler to add owner as a logconfig. 
> The owner field can be used for auditing and also by authorization layer to 
> grant access without having to explicitly configure acls. 



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