You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Dong Lin <li...@gmail.com> on 2014/07/16 23:29:40 UTC
Review Request 23568: Patch for KAFKA-1523
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1523
https://issues.apache.org/jira/browse/KAFKA-1523
Repository: kafka
Description
-------
KAFKA-1523 transaction manager module
Diffs
-----
core/src/main/scala/kafka/admin/TopicCommand.scala 8d5c2e7088fc6e8bf69e775ea7f5893b94580fdf
core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
core/src/main/scala/kafka/controller/ControllerChannelManager.scala 8763968fbff697e4c5c98ab1274627c192a4d26a
core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
core/src/main/scala/kafka/server/KafkaApis.scala 0b668f230c8556fdf08654ce522a11847d0bf39b
core/src/main/scala/kafka/server/KafkaConfig.scala ef75b67b67676ae5b8931902cbc8c0c2cc72c0d3
core/src/main/scala/kafka/server/KafkaServer.scala c22e51e0412843ec993721ad3230824c0aadd2ba
core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
Diff: https://reviews.apache.org/r/23568/diff/
Testing
-------
Thanks,
Dong Lin
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, line 157
> > <https://reviews.apache.org/r/23568/diff/4/?file=635099#file635099line157>
> >
> > Looking at this method in the other patch - this only gives the head - what about the other partitions?
Originally, for each parition in txPartitions, a transactionRequest is created and sent to respective broker, with parition listed as the head of txPartitions. And the broker only needs to append request to the head of txPartitions.
Now I have batched the transactionRequest sent to the same broker, and the broker will append transactionRequest to all the partitions in txPartitions that it leads.
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, line 224
> > <https://reviews.apache.org/r/23568/diff/4/?file=635099#file635099line224>
> >
> > Rather than do this one partition at a time we should group them by broker.
Sure. Now I have batched the transactionRequest sent to the same broker
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, line 229
> > <https://reviews.apache.org/r/23568/diff/4/?file=635099#file635099line229>
> >
> > I think it is fine to use a channel manager similar to the controller channel manager but that is no longer specific to the controller. i.e., we should probably move it out to become a more generic re-usable "ChannelManager" module.
> >
> > In fact, given the critical nature of controller to broker communication we should probably dedicate a separate channel manager entirely to transactions so that it doesn't interfere with the controller-broker communication.
I re-write the code to use a separate channel manager used solely by transaction manager. For the sake of not changing existing code and leave code refactor to future work, I have duplicated BrokerChangeListener() in TransactionManager.scala.
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, line 274
> > <https://reviews.apache.org/r/23568/diff/4/?file=635099#file635099line274>
> >
> > Same comments here apply as the above (wrt duplicate code)
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 74
> > <https://reviews.apache.org/r/23568/diff/4/?file=635102#file635102line74>
> >
> > How about we come up some other name for this - or even just TransactionalHW but that is a bit too wordy. Just want to avoid confusion with the replica HW.
Sure. I have used checkpointTransactionHW. Is this better?
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 84
> > <https://reviews.apache.org/r/23568/diff/4/?file=635102#file635102line84>
> >
> > One issue with this approach is that every commit/abort will cause a linear scan of this queue - we can discuss some alternative ways to maintain the set of pending transactions and associated txcontrol offsets.
I think this approach will have O(1) average cost for every commit/abort. I can explain it in person.
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 115
> > <https://reviews.apache.org/r/23568/diff/4/?file=635102#file635102line115>
> >
> > As discussed elsewhere, txid should not be the key.
I see. I will use Array.empty[Byte] as message key.
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/message/Message.scala, line 32
> > <https://reviews.apache.org/r/23568/diff/4/?file=635098#file635098line32>
> >
> > Should also store the txcontrol in the message header.
Yes you are right. Originally we expect transaction manager and consumer to read txcontrol from payload. We should put txcontrol in the header instead.
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, line 151
> > <https://reviews.apache.org/r/23568/diff/4/?file=635099#file635099line151>
> >
> > Initially, I was thinking we could just append to local log (since we definitely want to avoid duplicating code) until we have the API for durable append (to a replicated log). That is part of refactoring KafkaApis and is actually blocked on KAFKA-1333 so unless you need this for working through all the failure cases I would suggest just doing a local append for now.
Yes. I recall our discussion to just append to local log. The purpose of using DelayedProduce here is to work through failure cases.
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/KafkaConfig.scala, line 304
> > <https://reviews.apache.org/r/23568/diff/4/?file=635100#file635100line304>
> >
> > Should remove the comment on "atomic" commits - that was only for the consumer offsets topic.
OK!
> On July 21, 2014, 6:56 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 39
> > <https://reviews.apache.org/r/23568/diff/4/?file=635102#file635102line39>
> >
> > We generally avoid using tuples and use case classes instead - since that is a lot clearer.
I see. Will use case classes in the future.
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review48261
-----------------------------------------------------------
On July 22, 2014, 11:45 p.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated July 22, 2014, 11:45 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager module
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 8d5c2e7088fc6e8bf69e775ea7f5893b94580fdf
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala 8763968fbff697e4c5c98ab1274627c192a4d26a
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala 0b668f230c8556fdf08654ce522a11847d0bf39b
> core/src/main/scala/kafka/server/KafkaConfig.scala ef75b67b67676ae5b8931902cbc8c0c2cc72c0d3
> core/src/main/scala/kafka/server/KafkaServer.scala c22e51e0412843ec993721ad3230824c0aadd2ba
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review48261
-----------------------------------------------------------
core/src/main/scala/kafka/admin/TopicCommand.scala
<https://reviews.apache.org/r/23568/#comment84637>
Will need to modify this error message.
core/src/main/scala/kafka/controller/ControllerChannelManager.scala
<https://reviews.apache.org/r/23568/#comment84639>
See comment below in KafkaApis
core/src/main/scala/kafka/message/Message.scala
<https://reviews.apache.org/r/23568/#comment84645>
Should also store the txcontrol in the message header.
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/23568/#comment84646>
Initially, I was thinking we could just append to local log (since we definitely want to avoid duplicating code) until we have the API for durable append (to a replicated log). That is part of refactoring KafkaApis and is actually blocked on KAFKA-1333 so unless you need this for working through all the failure cases I would suggest just doing a local append for now.
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/23568/#comment84647>
Looking at this method in the other patch - this only gives the head - what about the other partitions?
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/23568/#comment84648>
Rather than do this one partition at a time we should group them by broker.
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/23568/#comment84649>
I think it is fine to use a channel manager similar to the controller channel manager but that is no longer specific to the controller. i.e., we should probably move it out to become a more generic re-usable "ChannelManager" module.
In fact, given the critical nature of controller to broker communication we should probably dedicate a separate channel manager entirely to transactions so that it doesn't interfere with the controller-broker communication.
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/23568/#comment84650>
Same comments here apply as the above (wrt duplicate code)
core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/23568/#comment84654>
Should remove the comment on "atomic" commits - that was only for the consumer offsets topic.
core/src/main/scala/kafka/server/TransactionManager.scala
<https://reviews.apache.org/r/23568/#comment84658>
We generally avoid using tuples and use case classes instead - since that is a lot clearer.
core/src/main/scala/kafka/server/TransactionManager.scala
<https://reviews.apache.org/r/23568/#comment84655>
Incorrect comment.
core/src/main/scala/kafka/server/TransactionManager.scala
<https://reviews.apache.org/r/23568/#comment84667>
How about we come up some other name for this - or even just TransactionalHW but that is a bit too wordy. Just want to avoid confusion with the replica HW.
core/src/main/scala/kafka/server/TransactionManager.scala
<https://reviews.apache.org/r/23568/#comment84661>
One issue with this approach is that every commit/abort will cause a linear scan of this queue - we can discuss some alternative ways to maintain the set of pending transactions and associated txcontrol offsets.
core/src/main/scala/kafka/server/TransactionManager.scala
<https://reviews.apache.org/r/23568/#comment84662>
As discussed elsewhere, txid should not be the key.
- Joel Koshy
On July 18, 2014, 3:12 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated July 18, 2014, 3:12 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 transaction manager module (version 2)
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 8d5c2e7088fc6e8bf69e775ea7f5893b94580fdf
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala 8763968fbff697e4c5c98ab1274627c192a4d26a
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala 0b668f230c8556fdf08654ce522a11847d0bf39b
> core/src/main/scala/kafka/server/KafkaConfig.scala ef75b67b67676ae5b8931902cbc8c0c2cc72c0d3
> core/src/main/scala/kafka/server/KafkaServer.scala c22e51e0412843ec993721ad3230824c0aadd2ba
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
> On Aug. 6, 2014, 5:32 a.m., Timothy Chen wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 300
> > <https://reviews.apache.org/r/23568/diff/6/?file=653418#file653418line300>
> >
> > I'm just trying to think if there is a chance to have overlapping tx ids especially around failover. I'm not familiar with the tx changes so just wondering if the range size can cause issues. Seems like you're incrementing the zk version on each id fetch and allow a range of the batch size of them. I was thinking if I sent 999 messages and then failed over, and the new broker that took over has a small batch size now, even though the version has incremented the new range now overlaps then right? It might already been handled but just want to throw it out there.
Sure. Thanks for taking time to help think about it.
Here I assume the batch size should be the same across producers regardless of when they start. If a producer sends 999 messages, say with txid in range 1000 to 1999, and fails over, the next producer will get the txid in range 2000 to 2999. The range is guaranteed to be disjoint from ranges used by previous/existing producers. Does this make sense?
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review49696
-----------------------------------------------------------
On Aug. 6, 2014, 4:25 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:25 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager and its failover handling.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
> core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
> core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
> core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
> core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
> core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
> On Aug. 6, 2014, 5:32 a.m., Timothy Chen wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 300
> > <https://reviews.apache.org/r/23568/diff/6/?file=653418#file653418line300>
> >
> > I'm just trying to think if there is a chance to have overlapping tx ids especially around failover. I'm not familiar with the tx changes so just wondering if the range size can cause issues. Seems like you're incrementing the zk version on each id fetch and allow a range of the batch size of them. I was thinking if I sent 999 messages and then failed over, and the new broker that took over has a small batch size now, even though the version has incremented the new range now overlaps then right? It might already been handled but just want to throw it out there.
>
> Dong Lin wrote:
> Sure. Thanks for taking time to help think about it.
>
> Here I assume the batch size should be the same across producers regardless of when they start. If a producer sends 999 messages, say with txid in range 1000 to 1999, and fails over, the next producer will get the txid in range 2000 to 2999. The range is guaranteed to be disjoint from ranges used by previous/existing producers. Does this make sense?
Correction: I actually mean broker where I say producer in my response.
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review49696
-----------------------------------------------------------
On Aug. 6, 2014, 4:25 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:25 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager and its failover handling.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
> core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
> core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
> core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
> core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
> core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 6, 2014, 5:32 a.m., Timothy Chen wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 300
> > <https://reviews.apache.org/r/23568/diff/6/?file=653418#file653418line300>
> >
> > I'm just trying to think if there is a chance to have overlapping tx ids especially around failover. I'm not familiar with the tx changes so just wondering if the range size can cause issues. Seems like you're incrementing the zk version on each id fetch and allow a range of the batch size of them. I was thinking if I sent 999 messages and then failed over, and the new broker that took over has a small batch size now, even though the version has incremented the new range now overlaps then right? It might already been handled but just want to throw it out there.
>
> Dong Lin wrote:
> Sure. Thanks for taking time to help think about it.
>
> Here I assume the batch size should be the same across producers regardless of when they start. If a producer sends 999 messages, say with txid in range 1000 to 1999, and fails over, the next producer will get the txid in range 2000 to 2999. The range is guaranteed to be disjoint from ranges used by previous/existing producers. Does this make sense?
>
> Dong Lin wrote:
> Correction: I actually mean broker where I say producer in my response.
>
> Timothy Chen wrote:
> The next producer will do so by looking at the default batch size with the version, which is what I was thinking there really isn't anything guarding aganist the range being changed from the code.
> The safest thing to do is perhaps write the range into zk node data and when the current range is smaller have a big warning.
> But minimally we will want to leave a comment in the code on the batch size to have this warning so this isn't something to experiment with as it can mess up existing transactions.
>
> Dong Lin wrote:
> It is the broker who looks at the default batch size with the version, and allocate unique txId to producer upon request. In other words, this code on the server side and we don't need to worry about Kafka user changing this code. Am I right?
>
> Timothy Chen wrote:
> You're right we don't need to worry about it for the user, but we need to worry about it when we change the default and do a rolling upgrade.
>
> Dong Lin wrote:
> Oh, I see your point. Sure, I agree that we should add a comment in the TransactionManger code so that any future developer should be aware of the effect of changing DefaultTransactionsIdBatchSize. But since this is just one of the many other configuration, I am not sure if it is worth saving this default value in zk node.
>
> Does that address your concern? Thanks!
Yes I don't think we need to do write it out, just a comment should be good IMO.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review49696
-----------------------------------------------------------
On Aug. 6, 2014, 4:25 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:25 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager and its failover handling.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
> core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
> core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
> core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
> core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
> core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
> On Aug. 6, 2014, 5:32 a.m., Timothy Chen wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 300
> > <https://reviews.apache.org/r/23568/diff/6/?file=653418#file653418line300>
> >
> > I'm just trying to think if there is a chance to have overlapping tx ids especially around failover. I'm not familiar with the tx changes so just wondering if the range size can cause issues. Seems like you're incrementing the zk version on each id fetch and allow a range of the batch size of them. I was thinking if I sent 999 messages and then failed over, and the new broker that took over has a small batch size now, even though the version has incremented the new range now overlaps then right? It might already been handled but just want to throw it out there.
>
> Dong Lin wrote:
> Sure. Thanks for taking time to help think about it.
>
> Here I assume the batch size should be the same across producers regardless of when they start. If a producer sends 999 messages, say with txid in range 1000 to 1999, and fails over, the next producer will get the txid in range 2000 to 2999. The range is guaranteed to be disjoint from ranges used by previous/existing producers. Does this make sense?
>
> Dong Lin wrote:
> Correction: I actually mean broker where I say producer in my response.
>
> Timothy Chen wrote:
> The next producer will do so by looking at the default batch size with the version, which is what I was thinking there really isn't anything guarding aganist the range being changed from the code.
> The safest thing to do is perhaps write the range into zk node data and when the current range is smaller have a big warning.
> But minimally we will want to leave a comment in the code on the batch size to have this warning so this isn't something to experiment with as it can mess up existing transactions.
>
> Dong Lin wrote:
> It is the broker who looks at the default batch size with the version, and allocate unique txId to producer upon request. In other words, this code on the server side and we don't need to worry about Kafka user changing this code. Am I right?
>
> Timothy Chen wrote:
> You're right we don't need to worry about it for the user, but we need to worry about it when we change the default and do a rolling upgrade.
Oh, I see your point. Sure, I agree that we should add a comment in the TransactionManger code so that any future developer should be aware of the effect of changing DefaultTransactionsIdBatchSize. But since this is just one of the many other configuration, I am not sure if it is worth saving this default value in zk node.
Does that address your concern? Thanks!
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review49696
-----------------------------------------------------------
On Aug. 6, 2014, 4:25 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:25 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager and its failover handling.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
> core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
> core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
> core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
> core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
> core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
> On Aug. 6, 2014, 5:32 a.m., Timothy Chen wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 300
> > <https://reviews.apache.org/r/23568/diff/6/?file=653418#file653418line300>
> >
> > I'm just trying to think if there is a chance to have overlapping tx ids especially around failover. I'm not familiar with the tx changes so just wondering if the range size can cause issues. Seems like you're incrementing the zk version on each id fetch and allow a range of the batch size of them. I was thinking if I sent 999 messages and then failed over, and the new broker that took over has a small batch size now, even though the version has incremented the new range now overlaps then right? It might already been handled but just want to throw it out there.
>
> Dong Lin wrote:
> Sure. Thanks for taking time to help think about it.
>
> Here I assume the batch size should be the same across producers regardless of when they start. If a producer sends 999 messages, say with txid in range 1000 to 1999, and fails over, the next producer will get the txid in range 2000 to 2999. The range is guaranteed to be disjoint from ranges used by previous/existing producers. Does this make sense?
>
> Dong Lin wrote:
> Correction: I actually mean broker where I say producer in my response.
>
> Timothy Chen wrote:
> The next producer will do so by looking at the default batch size with the version, which is what I was thinking there really isn't anything guarding aganist the range being changed from the code.
> The safest thing to do is perhaps write the range into zk node data and when the current range is smaller have a big warning.
> But minimally we will want to leave a comment in the code on the batch size to have this warning so this isn't something to experiment with as it can mess up existing transactions.
It is the broker who looks at the default batch size with the version, and allocate unique txId to producer upon request. In other words, this code on the server side and we don't need to worry about Kafka user changing this code. Am I right?
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review49696
-----------------------------------------------------------
On Aug. 6, 2014, 4:25 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:25 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager and its failover handling.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
> core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
> core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
> core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
> core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
> core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 6, 2014, 5:32 a.m., Timothy Chen wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 300
> > <https://reviews.apache.org/r/23568/diff/6/?file=653418#file653418line300>
> >
> > I'm just trying to think if there is a chance to have overlapping tx ids especially around failover. I'm not familiar with the tx changes so just wondering if the range size can cause issues. Seems like you're incrementing the zk version on each id fetch and allow a range of the batch size of them. I was thinking if I sent 999 messages and then failed over, and the new broker that took over has a small batch size now, even though the version has incremented the new range now overlaps then right? It might already been handled but just want to throw it out there.
>
> Dong Lin wrote:
> Sure. Thanks for taking time to help think about it.
>
> Here I assume the batch size should be the same across producers regardless of when they start. If a producer sends 999 messages, say with txid in range 1000 to 1999, and fails over, the next producer will get the txid in range 2000 to 2999. The range is guaranteed to be disjoint from ranges used by previous/existing producers. Does this make sense?
>
> Dong Lin wrote:
> Correction: I actually mean broker where I say producer in my response.
The next producer will do so by looking at the default batch size with the version, which is what I was thinking there really isn't anything guarding aganist the range being changed from the code.
The safest thing to do is perhaps write the range into zk node data and when the current range is smaller have a big warning.
But minimally we will want to leave a comment in the code on the batch size to have this warning so this isn't something to experiment with as it can mess up existing transactions.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review49696
-----------------------------------------------------------
On Aug. 6, 2014, 4:25 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:25 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager and its failover handling.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
> core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
> core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
> core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
> core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
> core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 6, 2014, 5:32 a.m., Timothy Chen wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 300
> > <https://reviews.apache.org/r/23568/diff/6/?file=653418#file653418line300>
> >
> > I'm just trying to think if there is a chance to have overlapping tx ids especially around failover. I'm not familiar with the tx changes so just wondering if the range size can cause issues. Seems like you're incrementing the zk version on each id fetch and allow a range of the batch size of them. I was thinking if I sent 999 messages and then failed over, and the new broker that took over has a small batch size now, even though the version has incremented the new range now overlaps then right? It might already been handled but just want to throw it out there.
>
> Dong Lin wrote:
> Sure. Thanks for taking time to help think about it.
>
> Here I assume the batch size should be the same across producers regardless of when they start. If a producer sends 999 messages, say with txid in range 1000 to 1999, and fails over, the next producer will get the txid in range 2000 to 2999. The range is guaranteed to be disjoint from ranges used by previous/existing producers. Does this make sense?
>
> Dong Lin wrote:
> Correction: I actually mean broker where I say producer in my response.
>
> Timothy Chen wrote:
> The next producer will do so by looking at the default batch size with the version, which is what I was thinking there really isn't anything guarding aganist the range being changed from the code.
> The safest thing to do is perhaps write the range into zk node data and when the current range is smaller have a big warning.
> But minimally we will want to leave a comment in the code on the batch size to have this warning so this isn't something to experiment with as it can mess up existing transactions.
>
> Dong Lin wrote:
> It is the broker who looks at the default batch size with the version, and allocate unique txId to producer upon request. In other words, this code on the server side and we don't need to worry about Kafka user changing this code. Am I right?
You're right we don't need to worry about it for the user, but we need to worry about it when we change the default and do a rolling upgrade.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review49696
-----------------------------------------------------------
On Aug. 6, 2014, 4:25 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:25 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager and its failover handling.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
> core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
> core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
> core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
> core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
> core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review49696
-----------------------------------------------------------
core/src/main/scala/kafka/server/TransactionManager.scala
<https://reviews.apache.org/r/23568/#comment86986>
I'm just trying to think if there is a chance to have overlapping tx ids especially around failover. I'm not familiar with the tx changes so just wondering if the range size can cause issues. Seems like you're incrementing the zk version on each id fetch and allow a range of the batch size of them. I was thinking if I sent 999 messages and then failed over, and the new broker that took over has a small batch size now, even though the version has incremented the new range now overlaps then right? It might already been handled but just want to throw it out there.
- Timothy Chen
On Aug. 6, 2014, 4:25 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:25 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager and its failover handling.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
> core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
> core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
> core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
> core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
> core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
> On Aug. 6, 2014, 4:39 a.m., Timothy Chen wrote:
> > core/src/main/scala/kafka/server/TransactionManager.scala, line 300
> > <https://reviews.apache.org/r/23568/diff/6/?file=653418#file653418line300>
> >
> > If the batch size changes to a smaller number does the tx Id range overlap then?
Why? The tx Id range should not overlap regardless of the actual value of batch size.
> On Aug. 6, 2014, 4:39 a.m., Timothy Chen wrote:
> > core/src/main/scala/kafka/admin/TopicCommand.scala, line 109
> > <https://reviews.apache.org/r/23568/diff/6/?file=653408#file653408line109>
> >
> > Why not use the InternalTopics array here?
Thanks! This is a good point, and I will use InternalTopics in the updated patch.
> On Aug. 6, 2014, 4:39 a.m., Timothy Chen wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, line 255
> > <https://reviews.apache.org/r/23568/diff/6/?file=653413#file653413line255>
> >
> > that is quite a mouthful, should it be called just producerTxRequestToCoordinator?
I agree, it is quite mouthful. When naming the functions producerRequestFromTxRequestToBroker and producerRequestFromTxRequestToBroker, I just want to be consistent with the name producerRequestFromOffsetCommit.
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review49692
-----------------------------------------------------------
On Aug. 6, 2014, 4:25 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:25 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager and its failover handling.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
> core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
> core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
> core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
> core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
> core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review49692
-----------------------------------------------------------
core/src/main/scala/kafka/admin/TopicCommand.scala
<https://reviews.apache.org/r/23568/#comment86976>
Why not use the InternalTopics array here?
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/23568/#comment86978>
that is quite a mouthful, should it be called just producerTxRequestToCoordinator?
core/src/main/scala/kafka/server/TransactionManager.scala
<https://reviews.apache.org/r/23568/#comment86979>
If the batch size changes to a smaller number does the tx Id range overlap then?
- Timothy Chen
On Aug. 6, 2014, 4:25 a.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2014, 4:25 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 Transaction manager and its failover handling.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
> core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
> core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
> core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
> core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
> core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/
-----------------------------------------------------------
(Updated Aug. 9, 2014, 4:36 a.m.)
Review request for kafka.
Bugs: KAFKA-1523
https://issues.apache.org/jira/browse/KAFKA-1523
Repository: kafka
Description (updated)
-------
KAFKA-1523; Transaction manager and its failover handling
Diffs (updated)
-----
core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala b1c4ce9f66aa86c68f2e6988f67546fa63ed1f9b
Diff: https://reviews.apache.org/r/23568/diff/
Testing
-------
Thanks,
Dong Lin
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/
-----------------------------------------------------------
(Updated Aug. 6, 2014, 4:25 a.m.)
Review request for kafka.
Bugs: KAFKA-1523
https://issues.apache.org/jira/browse/KAFKA-1523
Repository: kafka
Description (updated)
-------
KAFKA-1523 Transaction manager and its failover handling.
Diffs (updated)
-----
core/src/main/scala/kafka/admin/TopicCommand.scala 003a09c6160618bc94858ebc0d806b2aa4158e0a
core/src/main/scala/kafka/cluster/Partition.scala 134aef9c88068443d4d465189f376dd78605b4f8
core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
core/src/main/scala/kafka/controller/ControllerChannelManager.scala ecbfa0f328ba6a652a758ab20cacef324a8b2fb8
core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
core/src/main/scala/kafka/server/KafkaApis.scala fd5f12ee31e78cdcda8e24a0ab3e1740b3928884
core/src/main/scala/kafka/server/KafkaConfig.scala 1a45f8716ccc0398cf9395d91d66199d16882aae
core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d
core/src/main/scala/kafka/server/ReplicaManager.scala 897783cb756de548a8b634876f729b63ffe9925e
core/src/main/scala/kafka/server/RequestPurgatory.scala 3d0ff1e2dbd6a5c3587cffa283db70415af83a7b
core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
Diff: https://reviews.apache.org/r/23568/diff/
Testing
-------
Thanks,
Dong Lin
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/
-----------------------------------------------------------
(Updated July 22, 2014, 11:45 p.m.)
Review request for kafka.
Bugs: KAFKA-1523
https://issues.apache.org/jira/browse/KAFKA-1523
Repository: kafka
Description (updated)
-------
KAFKA-1523 Transaction manager module
Diffs (updated)
-----
core/src/main/scala/kafka/admin/TopicCommand.scala 8d5c2e7088fc6e8bf69e775ea7f5893b94580fdf
core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
core/src/main/scala/kafka/controller/ControllerChannelManager.scala 8763968fbff697e4c5c98ab1274627c192a4d26a
core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
core/src/main/scala/kafka/server/KafkaApis.scala 0b668f230c8556fdf08654ce522a11847d0bf39b
core/src/main/scala/kafka/server/KafkaConfig.scala ef75b67b67676ae5b8931902cbc8c0c2cc72c0d3
core/src/main/scala/kafka/server/KafkaServer.scala c22e51e0412843ec993721ad3230824c0aadd2ba
core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
Diff: https://reviews.apache.org/r/23568/diff/
Testing
-------
Thanks,
Dong Lin
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/
-----------------------------------------------------------
(Updated July 18, 2014, 3:12 a.m.)
Review request for kafka.
Bugs: KAFKA-1523
https://issues.apache.org/jira/browse/KAFKA-1523
Repository: kafka
Description
-------
KAFKA-1523 transaction manager module (version 2)
Diffs (updated)
-----
core/src/main/scala/kafka/admin/TopicCommand.scala 8d5c2e7088fc6e8bf69e775ea7f5893b94580fdf
core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
core/src/main/scala/kafka/controller/ControllerChannelManager.scala 8763968fbff697e4c5c98ab1274627c192a4d26a
core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
core/src/main/scala/kafka/server/KafkaApis.scala 0b668f230c8556fdf08654ce522a11847d0bf39b
core/src/main/scala/kafka/server/KafkaConfig.scala ef75b67b67676ae5b8931902cbc8c0c2cc72c0d3
core/src/main/scala/kafka/server/KafkaServer.scala c22e51e0412843ec993721ad3230824c0aadd2ba
core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
Diff: https://reviews.apache.org/r/23568/diff/
Testing
-------
Thanks,
Dong Lin
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/
-----------------------------------------------------------
(Updated July 18, 2014, 3:01 a.m.)
Review request for kafka.
Bugs: KAFKA-1523
https://issues.apache.org/jira/browse/KAFKA-1523
Repository: kafka
Description
-------
KAFKA-1523 transaction manager module (version 2)
Diffs (updated)
-----
core/src/main/scala/kafka/admin/TopicCommand.scala 8d5c2e7088fc6e8bf69e775ea7f5893b94580fdf
core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
core/src/main/scala/kafka/controller/ControllerChannelManager.scala 8763968fbff697e4c5c98ab1274627c192a4d26a
core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
core/src/main/scala/kafka/server/KafkaApis.scala 0b668f230c8556fdf08654ce522a11847d0bf39b
core/src/main/scala/kafka/server/KafkaConfig.scala ef75b67b67676ae5b8931902cbc8c0c2cc72c0d3
core/src/main/scala/kafka/server/KafkaServer.scala c22e51e0412843ec993721ad3230824c0aadd2ba
core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
Diff: https://reviews.apache.org/r/23568/diff/
Testing
-------
Thanks,
Dong Lin
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/
-----------------------------------------------------------
(Updated July 18, 2014, 2:26 a.m.)
Review request for kafka.
Bugs: KAFKA-1523
https://issues.apache.org/jira/browse/KAFKA-1523
Repository: kafka
Description (updated)
-------
KAFKA-1523 transaction manager module (version 2)
Diffs (updated)
-----
core/src/main/scala/kafka/admin/TopicCommand.scala 8d5c2e7088fc6e8bf69e775ea7f5893b94580fdf
core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
core/src/main/scala/kafka/controller/ControllerChannelManager.scala 8763968fbff697e4c5c98ab1274627c192a4d26a
core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
core/src/main/scala/kafka/server/KafkaApis.scala 0b668f230c8556fdf08654ce522a11847d0bf39b
core/src/main/scala/kafka/server/KafkaConfig.scala ef75b67b67676ae5b8931902cbc8c0c2cc72c0d3
core/src/main/scala/kafka/server/KafkaServer.scala c22e51e0412843ec993721ad3230824c0aadd2ba
core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
Diff: https://reviews.apache.org/r/23568/diff/
Testing
-------
Thanks,
Dong Lin
Re: Review Request 23568: Patch for KAFKA-1523
Posted by Dong Lin <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23568/#review48011
-----------------------------------------------------------
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/23568/#comment84260>
Need to specify "txId = txRequest.requestInfo.txId" in Message construction.
- Dong Lin
On July 16, 2014, 9:29 p.m., Dong Lin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23568/
> -----------------------------------------------------------
>
> (Updated July 16, 2014, 9:29 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1523
> https://issues.apache.org/jira/browse/KAFKA-1523
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1523 transaction manager module
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala 8d5c2e7088fc6e8bf69e775ea7f5893b94580fdf
> core/src/main/scala/kafka/common/Topic.scala ad759786d1c22f67c47808c0b8f227eb2b1a9aa8
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala 8763968fbff697e4c5c98ab1274627c192a4d26a
> core/src/main/scala/kafka/message/Message.scala d2a7293c7be4022af30884330924791340acc5c1
> core/src/main/scala/kafka/server/KafkaApis.scala 0b668f230c8556fdf08654ce522a11847d0bf39b
> core/src/main/scala/kafka/server/KafkaConfig.scala ef75b67b67676ae5b8931902cbc8c0c2cc72c0d3
> core/src/main/scala/kafka/server/KafkaServer.scala c22e51e0412843ec993721ad3230824c0aadd2ba
> core/src/main/scala/kafka/server/TransactionManager.scala PRE-CREATION
> core/src/main/scala/kafka/utils/ZkUtils.scala dcdc1ce2b02c996294e19cf480736106aaf29511
>
> Diff: https://reviews.apache.org/r/23568/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dong Lin
>
>