Re: Review Request 33088: add heartbeat to coordinator

2015-05-14 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/#review83851 --- Ship it! Ship It! - Guozhang Wang On May 14, 2015, 7:38 a.m., On

Re: Review Request 33088: add heartbeat to coordinator

2015-05-14 Thread Onur Karaman
> On May 7, 2015, 10:07 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/coordinator/CoordinatorMetadata.scala, line 53 > > > > > > Add @nonthreadsafe for this function. > > Onur Karaman wrote: > It would

Re: Review Request 33088: add heartbeat to coordinator

2015-05-14 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated May 14, 2015, 7:38 a.m.) Review request for kafka. Bugs: KAFKA-1334

Re: Review Request 33088: add heartbeat to coordinator

2015-05-08 Thread Onur Karaman
> On April 28, 2015, 12:13 a.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala, line 261 > > > > > > Function name "scheduleHeartbeatExpiration" is a bit misleading, it is >

Re: Review Request 33088: add heartbeat to coordinator

2015-05-08 Thread Onur Karaman
> On May 7, 2015, 10:07 p.m., Guozhang Wang wrote: > > I think we can check this in if you agree with the following minor comments > > (I can do them while committing), and then move on to the rebalance logic / > > leftover works. > > Guozhang Wang wrote: > Also could you rebase since curr

Re: Review Request 33088: add heartbeat to coordinator

2015-05-08 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated May 8, 2015, 5:55 p.m.) Review request for kafka. Bugs: KAFKA-1334

Re: Review Request 33088: add heartbeat to coordinator

2015-05-07 Thread Onur Karaman
> On May 7, 2015, 10:07 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/coordinator/CoordinatorMetadata.scala, line 53 > > > > > > Add @nonthreadsafe for this function. It would be awkward to have the class c

Re: Review Request 33088: add heartbeat to coordinator

2015-05-07 Thread Onur Karaman
> On May 7, 2015, 10:07 p.m., Guozhang Wang wrote: > > I think we can check this in if you agree with the following minor comments > > (I can do them while committing), and then move on to the rebalance logic / > > leftover works. > > Guozhang Wang wrote: > Also could you rebase since curr

Re: Review Request 33088: add heartbeat to coordinator

2015-05-07 Thread Guozhang Wang
> On May 7, 2015, 10:07 p.m., Guozhang Wang wrote: > > I think we can check this in if you agree with the following minor comments > > (I can do them while committing), and then move on to the rebalance logic / > > leftover works. Also could you rebase since currently patch does not apply clea

Re: Review Request 33088: add heartbeat to coordinator

2015-05-07 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/#review82921 --- Ship it! I think we can check this in if you agree with the followi

Re: Review Request 33088: add heartbeat to coordinator

2015-05-05 Thread Onur Karaman
> On April 28, 2015, 12:13 a.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala, lines > > 281-355 > > > > > > If we add the CoordinatorMetadata class, these functions can all

Re: Review Request 33088: add heartbeat to coordinator

2015-05-05 Thread Onur Karaman
> On April 28, 2015, 12:13 a.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/common/protocol/Errors.java, lines > > 73-76 > > > > > > Maybe we can merge them into one error code, > > UNKNOWN_OR_INC

Re: Review Request 33088: add heartbeat to coordinator

2015-05-05 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated May 5, 2015, 5:50 p.m.) Review request for kafka. Bugs: KAFKA-1334

Re: Review Request 33088: add heartbeat to coordinator

2015-04-27 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/#review81743 --- clients/src/main/java/org/apache/kafka/common/protocol/Errors.java

Re: Review Request 33088: add heartbeat to coordinator

2015-04-25 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated April 26, 2015, 12:57 a.m.) Review request for kafka. Bugs: KAFKA-13

Re: Review Request 33088: add heartbeat to coordinator

2015-04-25 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated April 25, 2015, 10:21 p.m.) Review request for kafka. Bugs: KAFKA-13

Re: Review Request 33088: add heartbeat to coordinator

2015-04-24 Thread Onur Karaman
> On April 22, 2015, 2:33 a.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala, line 32 > > > > > > Move kafka imports above scala / external lib imports. Sorry I made the rb a

Re: Review Request 33088: add heartbeat to coordinator

2015-04-24 Thread Onur Karaman
> On April 22, 2015, 2:33 a.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala, line 123 > > > > > > How about handleConsumerJoinGroup? I agree with one of Jay's earlier comme

Re: Review Request 33088: add heartbeat to coordinator

2015-04-24 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated April 25, 2015, 5:46 a.m.) Review request for kafka. Bugs: KAFKA-133

Re: Review Request 33088: add heartbeat to coordinator

2015-04-22 Thread Onur Karaman
> On April 22, 2015, 2:33 a.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/api/RequestKeys.scala, lines 55-58 > > > > > > Can we just add these two request into keyToNameAndDeserializerMap? I undid this change

Re: Review Request 33088: add heartbeat to coordinator

2015-04-21 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/#review79762 --- core/src/main/scala/kafka/api/RequestKeys.scala

Re: Review Request 33088: add heartbeat to coordinator

2015-04-19 Thread Onur Karaman
> On April 18, 2015, 11:14 p.m., Jay Kreps wrote: > > core/src/main/scala/kafka/coordinator/DelayedHeartbeat.scala, line 37 > > > > > > Is it possible to get rid of this kind of ad hoc synchronization? You > > seem to

Re: Review Request 33088: add heartbeat to coordinator

2015-04-18 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/#review80619 --- core/src/main/scala/kafka/coordinator/DelayedHeartbeat.scala

Re: Review Request 33088: add heartbeat to coordinator

2015-04-18 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated April 18, 2015, 7:16 p.m.) Review request for kafka. Bugs: KAFKA-133

Re: Review Request 33088: add heartbeat to coordinator

2015-04-18 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated April 18, 2015, 5:16 p.m.) Review request for kafka. Bugs: KAFKA-133

Re: Review Request 33088: add heartbeat to coordinator

2015-04-13 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated April 13, 2015, 6:58 p.m.) Review request for kafka. Bugs: KAFKA-133

Re: Review Request 33088: add heartbeat to coordinator

2015-04-13 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated April 13, 2015, 6:55 p.m.) Review request for kafka. Bugs: KAFKA-133

Re: Review Request 33088: add heartbeat to coordinator

2015-04-13 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated April 13, 2015, 5:11 p.m.) Review request for kafka. Bugs: KAFKA-133

Re: Review Request 33088: add heartbeat to coordinator

2015-04-11 Thread Onur Karaman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33088/ --- (Updated April 12, 2015, 5:47 a.m.) Review request for kafka. Bugs: KAFKA-133