[jira] [Commented] (KAFKA-1124) Sending to a new topic (with auto.create.topics.enable) returns ERROR

[Anuj Mehta (JIRA)]

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

Anuj Mehta commented on KAFKA-1124:
---

Hi Jason Rosenberg 

Just checked again. Yes there is an ERROR message in producer logs. Let me 
check if I can fix this

> Sending to a new topic (with auto.create.topics.enable) returns ERROR
> -
>
> Key: KAFKA-1124
> URL: https://issues.apache.org/jira/browse/KAFKA-1124
> Project: Kafka
>  Issue Type: Bug
>Affects Versions: 0.8
>Reporter: Jason Rosenberg
>
> I had thought this was reported issue, but can't seem to find a previous 
> report for it.
> If auto.create.topics.enable is true, a producer still gets an ERROR logged 
> on the first attempt to send a message to a new topic, e.g.:
> 2013-11-06 03:00:08,638 ERROR [Thread-1] async.DefaultEventHandler - Failed 
> to collate messages by topic, partition due to: Failed to fetch topic 
> metadata for topic: mynewtopic
> 2013-11-06 03:00:08,638  INFO [Thread-1] async.DefaultEventHandler - Back off 
> for 100 ms before retrying send. Remaining retries = 3
> This usually clears itself up immediately on retry (after 100 ms), as handled 
> by the the kafka.producer.async.DefaultEventHandler (with retries enabled).
> However, this is logged to the client as an ERROR, and looks scary, when in 
> fact it should have been a normal operation (since we have 
> auto.create.topics.enable=true).
> There should be a better interaction here between the producer client and the 
> server.
> Perhaps the server can create the topic in flight before returning the 
> metadata request.
> Or, if it needs to be asynchronous, it could return a code which indicates 
> something like: "The topic doesn't exist yet, it is being created, try again 
> shortly".and have the client automatically retry (even if retries not 
> enabled, since it's not an ERROR condition, really).
> The ERROR log level is a problem since apps often have alert systems set up 
> to notify when any ERROR happens, etc.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

[jira] [Commented] (KAFKA-1074) Reassign partitions should delete the old replicas from disk

[Jun Rao (JIRA)]

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

Jun Rao commented on KAFKA-1074:


Removing the old replica log from the disk itself is simple. What we need to 
make sure is that all potential outstanding operations on the deleted log are 
handled properly. In particular, we don't want any potential IOException due to 
the missing log causes the broker to halt.

1. All read operations are ok since we already handle unexpected exceptions in 
KafkaApi.

2. Writing to the log by the producer request, the replica fetcher or the log 
flusher: We need to make sure that after the log is deleted, no more 
writes/flushes will be attempted on the log. This can be achieved by:
2.1 For producer requests, the delete partition operation will synchronize on 
the leaderAndIsrUpdate lock.
2.2 For replica fetcher, this is already handled since the fetcher is removed 
before the log is deleted.
2.3 For log flusher, the flush and the delete will now synchronize on a delete 
lock.

> Reassign partitions should delete the old replicas from disk
> 
>
> Key: KAFKA-1074
> URL: https://issues.apache.org/jira/browse/KAFKA-1074
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.8
>Reporter: Jun Rao
> Fix For: 0.8.1
>
> Attachments: KAFKA-1074.patch
>
>
> Currently, after reassigning replicas to other brokers, the old replicas are 
> not removed from disk and have to be deleted manually.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

[jira] [Updated] (KAFKA-1074) Reassign partitions should delete the old replicas from disk

[Jun Rao (JIRA)]

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

Jun Rao updated KAFKA-1074:
---

Attachment: KAFKA-1074.patch

> Reassign partitions should delete the old replicas from disk
> 
>
> Key: KAFKA-1074
> URL: https://issues.apache.org/jira/browse/KAFKA-1074
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.8
>Reporter: Jun Rao
> Fix For: 0.8.1
>
> Attachments: KAFKA-1074.patch
>
>
> Currently, after reassigning replicas to other brokers, the old replicas are 
> not removed from disk and have to be deleted manually.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Review Request 15674: Reassign partitions should delete the old replicas from disk

[Jun Rao]

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

Review request for kafka.


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


Repository: kafka


Description
---

kafka-1074; fix 3


kafka-1074; fix 2


kafka-1074


Diffs
-

  core/src/main/scala/kafka/cluster/Partition.scala 
02ccc17c79b6d44c75f9bb6ca7cda8c51ae6f6fb 
  core/src/main/scala/kafka/log/Log.scala 
1883a53de112ad08449dc73a2ca08208c11a2537 
  core/src/main/scala/kafka/log/LogManager.scala 
81be88aa618ed5614703d45a0556b77c97290085 
  core/src/main/scala/kafka/log/LogSegment.scala 
0d6926ea105a99c9ff2cfc9ea6440f2f2d37bde8 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
161f58134f20f9335dbd2bee6ac3f71897cbef7c 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala 
c30069e837e54fb91bf1d5b75b133282a28dedf8 

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


Testing
---


Thanks,

Jun Rao

[jira] [Commented] (KAFKA-1074) Reassign partitions should delete the old replicas from disk

[Jun Rao (JIRA)]

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

Jun Rao commented on KAFKA-1074:


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

> Reassign partitions should delete the old replicas from disk
> 
>
> Key: KAFKA-1074
> URL: https://issues.apache.org/jira/browse/KAFKA-1074
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.8
>Reporter: Jun Rao
> Fix For: 0.8.1
>
> Attachments: KAFKA-1074.patch
>
>
> Currently, after reassigning replicas to other brokers, the old replicas are 
> not removed from disk and have to be deleted manually.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Re: Review Request 15665: Patch for KAFKA-1135

[Guozhang Wang]

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



core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala


Can we make the version number a global variable, so that when we upgrade 
in the future we only need to upgrade in once place?


- Guozhang Wang


On Nov. 19, 2013, 3:21 a.m., Swapnil Ghike wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15665/
> ---
> 
> (Updated Nov. 19, 2013, 3:21 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1135
> https://issues.apache.org/jira/browse/KAFKA-1135
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> iteration 2
> 
> 
> json.encode
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 8ff4bd5a5f6ea1a51df926c31155251bcc109238 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala 
> 26beb9698422ceb6cc682b86913b4f9d2d4f1307 
>   core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 
> 981d2bbecf2fa11f1d2c423535c7c30851d2d7bb 
>   core/src/main/scala/kafka/consumer/TopicCount.scala 
> a3eb53e8262115d1184cd1c7a2b47f21c22c077b 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> c0350cd05cf1b59866a1fedccbeb700b3e828d44 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 88792c2b2a360e928ab9cd00de151e5d5f94452d 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala 
> 33b73609b1178c56e692fb60e35aca04ad1af586 
>   core/src/main/scala/kafka/utils/Utils.scala 
> c9ca95f1937d0ef2e64c70e4d811a0d4f358d9db 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> 856d13605b0b4bf86010571eacbacc0fb0ba7950 
> 
> Diff: https://reviews.apache.org/r/15665/diff/
> 
> 
> Testing
> ---
> 
> Verified that zookeeper data looks like the structures defined in 
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+data+structures+in+Zookeeper
> 
> 
> Thanks,
> 
> Swapnil Ghike
> 
>

Re: Review Request 15674: Reassign partitions should delete the old replicas from disk

[Guozhang Wang]

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



core/src/main/scala/kafka/log/LogSegment.scala


Probably we could throw the exception if log.delete failed so we still have 
the index for trouble shooting?


Currently we have the the locking hierarchy as:

ReplicaManager : replicaStateChangeLock
|
Partition : leaderIsrUpdateLock
|
LogManager : logCreationOrDeletionLock
|
Log : this.lock
|
LogSegment : this.lock

I think probably we can reduce some of the lower level synchronization if they 
have already been covered by the higher level ones?

- Guozhang Wang


On Nov. 19, 2013, 4:28 p.m., Jun Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15674/
> ---
> 
> (Updated Nov. 19, 2013, 4:28 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1074
> https://issues.apache.org/jira/browse/KAFKA-1074
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> kafka-1074; fix 3
> 
> 
> kafka-1074; fix 2
> 
> 
> kafka-1074
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 02ccc17c79b6d44c75f9bb6ca7cda8c51ae6f6fb 
>   core/src/main/scala/kafka/log/Log.scala 
> 1883a53de112ad08449dc73a2ca08208c11a2537 
>   core/src/main/scala/kafka/log/LogManager.scala 
> 81be88aa618ed5614703d45a0556b77c97290085 
>   core/src/main/scala/kafka/log/LogSegment.scala 
> 0d6926ea105a99c9ff2cfc9ea6440f2f2d37bde8 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 161f58134f20f9335dbd2bee6ac3f71897cbef7c 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala 
> c30069e837e54fb91bf1d5b75b133282a28dedf8 
> 
> Diff: https://reviews.apache.org/r/15674/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Re: Review Request 15674: Reassign partitions should delete the old replicas from disk

[Jay Kreps]

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


What happens if I am doing a read or write concurrently with a delete?

Would it be simpler just to have the delete log work like the segment delete 
where rather than trying to lock we remove it from the segment list and then 
just enqueue a delete in 60 seconds. My concern is just that reasoning about 
the various locking strategies in the log is getting increasingly difficult.

- Jay Kreps


On Nov. 19, 2013, 4:28 p.m., Jun Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15674/
> ---
> 
> (Updated Nov. 19, 2013, 4:28 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1074
> https://issues.apache.org/jira/browse/KAFKA-1074
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> kafka-1074; fix 3
> 
> 
> kafka-1074; fix 2
> 
> 
> kafka-1074
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 02ccc17c79b6d44c75f9bb6ca7cda8c51ae6f6fb 
>   core/src/main/scala/kafka/log/Log.scala 
> 1883a53de112ad08449dc73a2ca08208c11a2537 
>   core/src/main/scala/kafka/log/LogManager.scala 
> 81be88aa618ed5614703d45a0556b77c97290085 
>   core/src/main/scala/kafka/log/LogSegment.scala 
> 0d6926ea105a99c9ff2cfc9ea6440f2f2d37bde8 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 161f58134f20f9335dbd2bee6ac3f71897cbef7c 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala 
> c30069e837e54fb91bf1d5b75b133282a28dedf8 
> 
> Diff: https://reviews.apache.org/r/15674/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Re: Review Request 15665: Patch for KAFKA-1135

[Neha Narkhede]

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

Ship it!


Ship It!

- Neha Narkhede


On Nov. 19, 2013, 3:21 a.m., Swapnil Ghike wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15665/
> ---
> 
> (Updated Nov. 19, 2013, 3:21 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1135
> https://issues.apache.org/jira/browse/KAFKA-1135
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> iteration 2
> 
> 
> json.encode
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 8ff4bd5a5f6ea1a51df926c31155251bcc109238 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala 
> 26beb9698422ceb6cc682b86913b4f9d2d4f1307 
>   core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 
> 981d2bbecf2fa11f1d2c423535c7c30851d2d7bb 
>   core/src/main/scala/kafka/consumer/TopicCount.scala 
> a3eb53e8262115d1184cd1c7a2b47f21c22c077b 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> c0350cd05cf1b59866a1fedccbeb700b3e828d44 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 88792c2b2a360e928ab9cd00de151e5d5f94452d 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala 
> 33b73609b1178c56e692fb60e35aca04ad1af586 
>   core/src/main/scala/kafka/utils/Utils.scala 
> c9ca95f1937d0ef2e64c70e4d811a0d4f358d9db 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> 856d13605b0b4bf86010571eacbacc0fb0ba7950 
> 
> Diff: https://reviews.apache.org/r/15665/diff/
> 
> 
> Testing
> ---
> 
> Verified that zookeeper data looks like the structures defined in 
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+data+structures+in+Zookeeper
> 
> 
> Thanks,
> 
> Swapnil Ghike
> 
>

Re: Review Request 15674: Reassign partitions should delete the old replicas from disk

[Jun Rao]

> On Nov. 19, 2013, 6:40 p.m., Jay Kreps wrote:
> > What happens if I am doing a read or write concurrently with a delete?
> > 
> > Would it be simpler just to have the delete log work like the segment 
> > delete where rather than trying to lock we remove it from the segment list 
> > and then just enqueue a delete in 60 seconds. My concern is just that 
> > reasoning about the various locking strategies in the log is getting 
> > increasingly difficult.

Yes, we could try deleting the log asynchronously. The issues there are:

1. The same partition could be moved back to this broker during the delayed 
window.
2. It's not clear if 60 secs (or any value) is good enough since the time that 
an ongoing scheduled flush takes is unbounded.

The following is how this patch handles outstanding reads/writes on the deleted 
data.

1. All read operations are ok since we already handle unexpected exceptions in 
KafkaApi. The caller will get an error.
2. Currently, if we hit an IOException while writing to the log by the producer 
request, the replica fetcher or the log flusher, we halt the broker. We need to 
make sure that the deletion of a log doesn't cause the halt. This is achieved 
by preventing those operations on the log once it's deleted.
2.1 For producer requests, the delete partition operation will synchronize on 
the leaderAndIsrUpdate lock.
2.2 For replica fetcher, this is already handled since the fetcher is removed 
before the log is deleted.
2.3 For log flusher, the flush and the delete will now synchronize on a delete 
lock.

I agree that this approach uses more locks, which potentially makes the code 
harder to understand. However, my feeling is that this is probably a less hacky 
approach than the async delete one.


- Jun


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


On Nov. 19, 2013, 4:28 p.m., Jun Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15674/
> ---
> 
> (Updated Nov. 19, 2013, 4:28 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1074
> https://issues.apache.org/jira/browse/KAFKA-1074
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> kafka-1074; fix 3
> 
> 
> kafka-1074; fix 2
> 
> 
> kafka-1074
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 02ccc17c79b6d44c75f9bb6ca7cda8c51ae6f6fb 
>   core/src/main/scala/kafka/log/Log.scala 
> 1883a53de112ad08449dc73a2ca08208c11a2537 
>   core/src/main/scala/kafka/log/LogManager.scala 
> 81be88aa618ed5614703d45a0556b77c97290085 
>   core/src/main/scala/kafka/log/LogSegment.scala 
> 0d6926ea105a99c9ff2cfc9ea6440f2f2d37bde8 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 161f58134f20f9335dbd2bee6ac3f71897cbef7c 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala 
> c30069e837e54fb91bf1d5b75b133282a28dedf8 
> 
> Diff: https://reviews.apache.org/r/15674/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Re: Review Request 15665: Patch for KAFKA-1135

[Swapnil Ghike]

> On Nov. 19, 2013, 5:21 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala, line 52
> > 
> >
> > Can we make the version number a global variable, so that when we 
> > upgrade in the future we only need to upgrade in once place?

My understanding was that the code may evolve to deal with situations wherein 
we have some zookeeper paths that are on version n, and others are on version 
n' and some more are on version n''. To make this explicit, I wonder if it 
makes sense to let each zookeeper path have its own version value and not put 
any global value that everyone else refers to.

But I could be wrong, comments?


- Swapnil


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


On Nov. 19, 2013, 3:21 a.m., Swapnil Ghike wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15665/
> ---
> 
> (Updated Nov. 19, 2013, 3:21 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1135
> https://issues.apache.org/jira/browse/KAFKA-1135
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> iteration 2
> 
> 
> json.encode
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 8ff4bd5a5f6ea1a51df926c31155251bcc109238 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala 
> 26beb9698422ceb6cc682b86913b4f9d2d4f1307 
>   core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 
> 981d2bbecf2fa11f1d2c423535c7c30851d2d7bb 
>   core/src/main/scala/kafka/consumer/TopicCount.scala 
> a3eb53e8262115d1184cd1c7a2b47f21c22c077b 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> c0350cd05cf1b59866a1fedccbeb700b3e828d44 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 88792c2b2a360e928ab9cd00de151e5d5f94452d 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala 
> 33b73609b1178c56e692fb60e35aca04ad1af586 
>   core/src/main/scala/kafka/utils/Utils.scala 
> c9ca95f1937d0ef2e64c70e4d811a0d4f358d9db 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> 856d13605b0b4bf86010571eacbacc0fb0ba7950 
> 
> Diff: https://reviews.apache.org/r/15665/diff/
> 
> 
> Testing
> ---
> 
> Verified that zookeeper data looks like the structures defined in 
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+data+structures+in+Zookeeper
> 
> 
> Thanks,
> 
> Swapnil Ghike
> 
>

Re: Review Request 15659: Patch for KAFKA-1103

[Joel Koshy]

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


Looks good to me except for minor comments. I'll leave open for now in case 
anyone else wants to review.


core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala


So the config is no longer required.



core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala


Typo in message; however, I think we can just drop the entire else block.


- Joel Koshy


On Nov. 18, 2013, 11:30 p.m., Guozhang Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15659/
> ---
> 
> (Updated Nov. 18, 2013, 11:30 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1103
> https://issues.apache.org/jira/browse/KAFKA-1103
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1103.v1
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/consumer/TopicFilter.scala 
> cf3853b223095e1fe0921175c407a906828b8113 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> c0350cd05cf1b59866a1fedccbeb700b3e828d44 
>   core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala 
> a67c193df9f7cbfc52f75dc1b71dc017de1b5fe2 
>   core/src/test/scala/unit/kafka/consumer/TopicFilterTest.scala 
> 40a2bf7a9277eb5f94bc07b40d7726d81860cefc 
> 
> Diff: https://reviews.apache.org/r/15659/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Re: Review Request 15659: Patch for KAFKA-1103

[Guozhang Wang]

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



core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala


will do



core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala


will fix the typo, but I think it is still worth leaving it here in case 
zkClient is null (theoretically it will never be).


- Guozhang Wang


On Nov. 18, 2013, 11:30 p.m., Guozhang Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15659/
> ---
> 
> (Updated Nov. 18, 2013, 11:30 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1103
> https://issues.apache.org/jira/browse/KAFKA-1103
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1103.v1
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/consumer/TopicFilter.scala 
> cf3853b223095e1fe0921175c407a906828b8113 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> c0350cd05cf1b59866a1fedccbeb700b3e828d44 
>   core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala 
> a67c193df9f7cbfc52f75dc1b71dc017de1b5fe2 
>   core/src/test/scala/unit/kafka/consumer/TopicFilterTest.scala 
> 40a2bf7a9277eb5f94bc07b40d7726d81860cefc 
> 
> Diff: https://reviews.apache.org/r/15659/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Re: Review Request 15659: Patch for KAFKA-1103

[Joel Koshy]

> On Nov. 19, 2013, 9:48 p.m., Joel Koshy wrote:
> > Looks good to me except for minor comments. I'll leave open for now in case 
> > anyone else wants to review.

Also, could you run the mirror-maker system test suite to make sure there are 
no obvious issues?


- Joel


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


On Nov. 18, 2013, 11:30 p.m., Guozhang Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15659/
> ---
> 
> (Updated Nov. 18, 2013, 11:30 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1103
> https://issues.apache.org/jira/browse/KAFKA-1103
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1103.v1
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/consumer/TopicFilter.scala 
> cf3853b223095e1fe0921175c407a906828b8113 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> c0350cd05cf1b59866a1fedccbeb700b3e828d44 
>   core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala 
> a67c193df9f7cbfc52f75dc1b71dc017de1b5fe2 
>   core/src/test/scala/unit/kafka/consumer/TopicFilterTest.scala 
> 40a2bf7a9277eb5f94bc07b40d7726d81860cefc 
> 
> Diff: https://reviews.apache.org/r/15659/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Re: Review Request 15665: Patch for KAFKA-1135

[Guozhang Wang]

> On Nov. 19, 2013, 5:21 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala, line 52
> > 
> >
> > Can we make the version number a global variable, so that when we 
> > upgrade in the future we only need to upgrade in once place?
> 
> Swapnil Ghike wrote:
> My understanding was that the code may evolve to deal with situations 
> wherein we have some zookeeper paths that are on version n, and others are on 
> version n' and some more are on version n''. To make this explicit, I wonder 
> if it makes sense to let each zookeeper path have its own version value and 
> not put any global value that everyone else refers to.
> 
> But I could be wrong, comments?

Probably I did not make it clear, I was suggesting we put a currentVersion in 
the kafka code, and use this variable for all writes in ZK, for reads from ZK, 
we may need a swtich/case for different versions. How does that sound?


- Guozhang


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


On Nov. 19, 2013, 3:21 a.m., Swapnil Ghike wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15665/
> ---
> 
> (Updated Nov. 19, 2013, 3:21 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1135
> https://issues.apache.org/jira/browse/KAFKA-1135
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> iteration 2
> 
> 
> json.encode
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 8ff4bd5a5f6ea1a51df926c31155251bcc109238 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala 
> 26beb9698422ceb6cc682b86913b4f9d2d4f1307 
>   core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 
> 981d2bbecf2fa11f1d2c423535c7c30851d2d7bb 
>   core/src/main/scala/kafka/consumer/TopicCount.scala 
> a3eb53e8262115d1184cd1c7a2b47f21c22c077b 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> c0350cd05cf1b59866a1fedccbeb700b3e828d44 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 88792c2b2a360e928ab9cd00de151e5d5f94452d 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala 
> 33b73609b1178c56e692fb60e35aca04ad1af586 
>   core/src/main/scala/kafka/utils/Utils.scala 
> c9ca95f1937d0ef2e64c70e4d811a0d4f358d9db 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> 856d13605b0b4bf86010571eacbacc0fb0ba7950 
> 
> Diff: https://reviews.apache.org/r/15665/diff/
> 
> 
> Testing
> ---
> 
> Verified that zookeeper data looks like the structures defined in 
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+data+structures+in+Zookeeper
> 
> 
> Thanks,
> 
> Swapnil Ghike
> 
>

Re: Review Request 15665: Patch for KAFKA-1135

[Swapnil Ghike]

> On Nov. 19, 2013, 5:21 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala, line 52
> > 
> >
> > Can we make the version number a global variable, so that when we 
> > upgrade in the future we only need to upgrade in once place?
> 
> Swapnil Ghike wrote:
> My understanding was that the code may evolve to deal with situations 
> wherein we have some zookeeper paths that are on version n, and others are on 
> version n' and some more are on version n''. To make this explicit, I wonder 
> if it makes sense to let each zookeeper path have its own version value and 
> not put any global value that everyone else refers to.
> 
> But I could be wrong, comments?
> 
> Guozhang Wang wrote:
> Probably I did not make it clear, I was suggesting we put a 
> currentVersion in the kafka code, and use this variable for all writes in ZK, 
> for reads from ZK, we may need a swtich/case for different versions. How does 
> that sound?

I see. Let's say we change the zookeeper data version for one path and update 
the currentVersion. This will update the version of all other data in zookeeper 
even though the format for other data did not really change. I would like to 
avoid this, but what do you think?


- Swapnil


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


On Nov. 19, 2013, 3:21 a.m., Swapnil Ghike wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15665/
> ---
> 
> (Updated Nov. 19, 2013, 3:21 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1135
> https://issues.apache.org/jira/browse/KAFKA-1135
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> iteration 2
> 
> 
> json.encode
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 8ff4bd5a5f6ea1a51df926c31155251bcc109238 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala 
> 26beb9698422ceb6cc682b86913b4f9d2d4f1307 
>   core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 
> 981d2bbecf2fa11f1d2c423535c7c30851d2d7bb 
>   core/src/main/scala/kafka/consumer/TopicCount.scala 
> a3eb53e8262115d1184cd1c7a2b47f21c22c077b 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> c0350cd05cf1b59866a1fedccbeb700b3e828d44 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 88792c2b2a360e928ab9cd00de151e5d5f94452d 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala 
> 33b73609b1178c56e692fb60e35aca04ad1af586 
>   core/src/main/scala/kafka/utils/Utils.scala 
> c9ca95f1937d0ef2e64c70e4d811a0d4f358d9db 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> 856d13605b0b4bf86010571eacbacc0fb0ba7950 
> 
> Diff: https://reviews.apache.org/r/15665/diff/
> 
> 
> Testing
> ---
> 
> Verified that zookeeper data looks like the structures defined in 
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+data+structures+in+Zookeeper
> 
> 
> Thanks,
> 
> Swapnil Ghike
> 
>

Review Request 15711: Patch for KAFKA-930

[Sriram Subramanian]

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

Review request for kafka.


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


Repository: kafka


Description
---

some more changes


fix merge conflicts


Add auto leader rebalance support


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into trunk


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into trunk

Conflicts:
core/src/main/scala/kafka/admin/AdminUtils.scala
core/src/main/scala/kafka/admin/TopicCommand.scala

change comments


commit the remaining changes


Move AddPartitions into TopicCommand


Diffs
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
88792c2b2a360e928ab9cd00de151e5d5f94452d 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
b324344d0a383398db8bfe2cbeec2c1378fe13c9 

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


Testing
---


Thanks,

Sriram Subramanian

[jira] [Updated] (KAFKA-930) Integrate preferred replica election logic into kafka

[Sriram Subramanian (JIRA)]

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

Sriram Subramanian updated KAFKA-930:
-

Attachment: KAFKA-930.patch

> Integrate preferred replica election logic into kafka
> -
>
> Key: KAFKA-930
> URL: https://issues.apache.org/jira/browse/KAFKA-930
> Project: Kafka
>  Issue Type: Bug
>Reporter: Sriram Subramanian
>Assignee: Sriram Subramanian
> Fix For: 0.9
>
> Attachments: KAFKA-930.patch
>
>
> It seems useful to integrate the preferred replica election logic into kafka 
> controller. A simple way to implement this would be to have a background 
> thread that periodically finds the topic partitions that are not assigned to 
> the preferred broker and initiate the move. We could come up with some 
> heuristics to initiate the move only if the imbalance over a specific 
> threshold in order to avoid rebalancing too aggressively. Making the software 
> do this reduces operational cost.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

[jira] [Commented] (KAFKA-930) Integrate preferred replica election logic into kafka

[Sriram Subramanian (JIRA)]

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

Sriram Subramanian commented on KAFKA-930:
--

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

> Integrate preferred replica election logic into kafka
> -
>
> Key: KAFKA-930
> URL: https://issues.apache.org/jira/browse/KAFKA-930
> Project: Kafka
>  Issue Type: Bug
>Reporter: Sriram Subramanian
>Assignee: Sriram Subramanian
> Fix For: 0.9
>
> Attachments: KAFKA-930.patch
>
>
> It seems useful to integrate the preferred replica election logic into kafka 
> controller. A simple way to implement this would be to have a background 
> thread that periodically finds the topic partitions that are not assigned to 
> the preferred broker and initiate the move. We could come up with some 
> heuristics to initiate the move only if the imbalance over a specific 
> threshold in order to avoid rebalancing too aggressively. Making the software 
> do this reduces operational cost.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Re: Review Request 15711: Patch for KAFKA-930

[Sriram Subramanian]

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

(Updated Nov. 20, 2013, 1:37 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

some more changes


fix merge conflicts


Add auto leader rebalance support


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into trunk


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into trunk

Conflicts:
core/src/main/scala/kafka/admin/AdminUtils.scala
core/src/main/scala/kafka/admin/TopicCommand.scala

change comments


commit the remaining changes


Move AddPartitions into TopicCommand


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
88792c2b2a360e928ab9cd00de151e5d5f94452d 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
b324344d0a383398db8bfe2cbeec2c1378fe13c9 

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


Testing
---


Thanks,

Sriram Subramanian

[jira] [Commented] (KAFKA-930) Integrate preferred replica election logic into kafka

[Sriram Subramanian (JIRA)]

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

Sriram Subramanian commented on KAFKA-930:
--

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

> Integrate preferred replica election logic into kafka
> -
>
> Key: KAFKA-930
> URL: https://issues.apache.org/jira/browse/KAFKA-930
> Project: Kafka
>  Issue Type: Bug
>Reporter: Sriram Subramanian
>Assignee: Sriram Subramanian
> Fix For: 0.9
>
> Attachments: KAFKA-930.patch, KAFKA-930_2013-11-19_17:37:29.patch
>
>
> It seems useful to integrate the preferred replica election logic into kafka 
> controller. A simple way to implement this would be to have a background 
> thread that periodically finds the topic partitions that are not assigned to 
> the preferred broker and initiate the move. We could come up with some 
> heuristics to initiate the move only if the imbalance over a specific 
> threshold in order to avoid rebalancing too aggressively. Making the software 
> do this reduces operational cost.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

[jira] [Updated] (KAFKA-930) Integrate preferred replica election logic into kafka

[Sriram Subramanian (JIRA)]

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

Sriram Subramanian updated KAFKA-930:
-

Attachment: KAFKA-930_2013-11-19_17:37:29.patch

> Integrate preferred replica election logic into kafka
> -
>
> Key: KAFKA-930
> URL: https://issues.apache.org/jira/browse/KAFKA-930
> Project: Kafka
>  Issue Type: Bug
>Reporter: Sriram Subramanian
>Assignee: Sriram Subramanian
> Fix For: 0.9
>
> Attachments: KAFKA-930.patch, KAFKA-930_2013-11-19_17:37:29.patch
>
>
> It seems useful to integrate the preferred replica election logic into kafka 
> controller. A simple way to implement this would be to have a background 
> thread that periodically finds the topic partitions that are not assigned to 
> the preferred broker and initiate the move. We could come up with some 
> heuristics to initiate the move only if the imbalance over a specific 
> threshold in order to avoid rebalancing too aggressively. Making the software 
> do this reduces operational cost.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Re: Review Request 15711: Patch for KAFKA-930

[Sriram Subramanian]

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

(Updated Nov. 20, 2013, 1:38 a.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

commit missing code


some more changes


fix merge conflicts


Add auto leader rebalance support


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into trunk


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into trunk

Conflicts:
core/src/main/scala/kafka/admin/AdminUtils.scala
core/src/main/scala/kafka/admin/TopicCommand.scala

change comments


commit the remaining changes


Move AddPartitions into TopicCommand


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
88792c2b2a360e928ab9cd00de151e5d5f94452d 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
b324344d0a383398db8bfe2cbeec2c1378fe13c9 

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


Testing
---


Thanks,

Sriram Subramanian

[jira] [Updated] (KAFKA-930) Integrate preferred replica election logic into kafka

[Sriram Subramanian (JIRA)]

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

Sriram Subramanian updated KAFKA-930:
-

Attachment: KAFKA-930_2013-11-19_17:38:49.patch

> Integrate preferred replica election logic into kafka
> -
>
> Key: KAFKA-930
> URL: https://issues.apache.org/jira/browse/KAFKA-930
> Project: Kafka
>  Issue Type: Bug
>Reporter: Sriram Subramanian
>Assignee: Sriram Subramanian
> Fix For: 0.9
>
> Attachments: KAFKA-930.patch, KAFKA-930_2013-11-19_17:37:29.patch, 
> KAFKA-930_2013-11-19_17:38:49.patch
>
>
> It seems useful to integrate the preferred replica election logic into kafka 
> controller. A simple way to implement this would be to have a background 
> thread that periodically finds the topic partitions that are not assigned to 
> the preferred broker and initiate the move. We could come up with some 
> heuristics to initiate the move only if the imbalance over a specific 
> threshold in order to avoid rebalancing too aggressively. Making the software 
> do this reduces operational cost.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

[jira] [Commented] (KAFKA-930) Integrate preferred replica election logic into kafka

[Sriram Subramanian (JIRA)]

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

Sriram Subramanian commented on KAFKA-930:
--

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

> Integrate preferred replica election logic into kafka
> -
>
> Key: KAFKA-930
> URL: https://issues.apache.org/jira/browse/KAFKA-930
> Project: Kafka
>  Issue Type: Bug
>Reporter: Sriram Subramanian
>Assignee: Sriram Subramanian
> Fix For: 0.9
>
> Attachments: KAFKA-930.patch, KAFKA-930_2013-11-19_17:37:29.patch, 
> KAFKA-930_2013-11-19_17:38:49.patch
>
>
> It seems useful to integrate the preferred replica election logic into kafka 
> controller. A simple way to implement this would be to have a background 
> thread that periodically finds the topic partitions that are not assigned to 
> the preferred broker and initiate the move. We could come up with some 
> heuristics to initiate the move only if the imbalance over a specific 
> threshold in order to avoid rebalancing too aggressively. Making the software 
> do this reduces operational cost.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Re: Review Request 15665: Patch for KAFKA-1135

[Guozhang Wang]

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

Ship it!


- Guozhang Wang


On Nov. 19, 2013, 3:21 a.m., Swapnil Ghike wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15665/
> ---
> 
> (Updated Nov. 19, 2013, 3:21 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1135
> https://issues.apache.org/jira/browse/KAFKA-1135
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> iteration 2
> 
> 
> json.encode
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 8ff4bd5a5f6ea1a51df926c31155251bcc109238 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala 
> 26beb9698422ceb6cc682b86913b4f9d2d4f1307 
>   core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 
> 981d2bbecf2fa11f1d2c423535c7c30851d2d7bb 
>   core/src/main/scala/kafka/consumer/TopicCount.scala 
> a3eb53e8262115d1184cd1c7a2b47f21c22c077b 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> c0350cd05cf1b59866a1fedccbeb700b3e828d44 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 88792c2b2a360e928ab9cd00de151e5d5f94452d 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala 
> 33b73609b1178c56e692fb60e35aca04ad1af586 
>   core/src/main/scala/kafka/utils/Utils.scala 
> c9ca95f1937d0ef2e64c70e4d811a0d4f358d9db 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> 856d13605b0b4bf86010571eacbacc0fb0ba7950 
> 
> Diff: https://reviews.apache.org/r/15665/diff/
> 
> 
> Testing
> ---
> 
> Verified that zookeeper data looks like the structures defined in 
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+data+structures+in+Zookeeper
> 
> 
> Thanks,
> 
> Swapnil Ghike
> 
>

Re: Review Request 15665: Patch for KAFKA-1135

[Guozhang Wang]

> On Nov. 19, 2013, 5:21 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala, line 52
> > 
> >
> > Can we make the version number a global variable, so that when we 
> > upgrade in the future we only need to upgrade in once place?
> 
> Swapnil Ghike wrote:
> My understanding was that the code may evolve to deal with situations 
> wherein we have some zookeeper paths that are on version n, and others are on 
> version n' and some more are on version n''. To make this explicit, I wonder 
> if it makes sense to let each zookeeper path have its own version value and 
> not put any global value that everyone else refers to.
> 
> But I could be wrong, comments?
> 
> Guozhang Wang wrote:
> Probably I did not make it clear, I was suggesting we put a 
> currentVersion in the kafka code, and use this variable for all writes in ZK, 
> for reads from ZK, we may need a swtich/case for different versions. How does 
> that sound?
> 
> Swapnil Ghike wrote:
> I see. Let's say we change the zookeeper data version for one path and 
> update the currentVersion. This will update the version of all other data in 
> zookeeper even though the format for other data did not really change. I 
> would like to avoid this, but what do you think?

Yeah, that makes sense.


- Guozhang


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


On Nov. 19, 2013, 3:21 a.m., Swapnil Ghike wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15665/
> ---
> 
> (Updated Nov. 19, 2013, 3:21 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1135
> https://issues.apache.org/jira/browse/KAFKA-1135
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> iteration 2
> 
> 
> json.encode
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 
> 8ff4bd5a5f6ea1a51df926c31155251bcc109238 
>   core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala 
> 26beb9698422ceb6cc682b86913b4f9d2d4f1307 
>   core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 
> 981d2bbecf2fa11f1d2c423535c7c30851d2d7bb 
>   core/src/main/scala/kafka/consumer/TopicCount.scala 
> a3eb53e8262115d1184cd1c7a2b47f21c22c077b 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> c0350cd05cf1b59866a1fedccbeb700b3e828d44 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 88792c2b2a360e928ab9cd00de151e5d5f94452d 
>   core/src/main/scala/kafka/server/ZookeeperLeaderElector.scala 
> 33b73609b1178c56e692fb60e35aca04ad1af586 
>   core/src/main/scala/kafka/utils/Utils.scala 
> c9ca95f1937d0ef2e64c70e4d811a0d4f358d9db 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> 856d13605b0b4bf86010571eacbacc0fb0ba7950 
> 
> Diff: https://reviews.apache.org/r/15665/diff/
> 
> 
> Testing
> ---
> 
> Verified that zookeeper data looks like the structures defined in 
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+data+structures+in+Zookeeper
> 
> 
> Thanks,
> 
> Swapnil Ghike
> 
>