> On June 26, 2014, 12:51 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/RequestKeys.scala, line 36 > > <https://reviews.apache.org/r/22905/diff/1/?file=615398#file615398line36> > > > > Just wondering if TransactionMetadata is misleading - since we are > > actually inquiring about the transaction coordinator. That > > TransactionCoordinatorMetadata is a bit unwieldy though. > > Dong Lin wrote: > When I named this variable, I followed the name of "consumerMetadataKey". > The consumerMetadataKey is named after the originator, rather than > "offsetManagerMetadataKey". > > If TransactionCoordinatorMetadata is too long, how about > TxCoordinatorMetadataKey?
Yes - that's a good name. We should eventually look at whether it is reasonable to merge this with the ConsumerMetadataRequest and then call it "CoordinatorMetadataRequest" but we can do that later. > On June 26, 2014, 12:51 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/TransactionMetadataRequest.scala, line 60 > > <https://reviews.apache.org/r/22905/diff/1/?file=615399#file615399line60> > > > > typo in comment > > Dong Lin wrote: > Excuse me.. But where is the typo? Oh - it's just that it is still referring to the consumer (since this was probably based on the ConsumerMetadataRequest) > On June 26, 2014, 12:51 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/TransactionRequest.scala, line 42 > > <https://reviews.apache.org/r/22905/diff/1/?file=615401#file615401line42> > > > > We will most likely need to incorporate a generation as well to handle > > the single-writer requirement. > > Dong Lin wrote: > I thought we will delay adding the feature for single-writer requirement. > Should I add a field "genID" to the TransactionRequest now? Either way should be fine. Maybe we can hold off and add it when we implement protection for the single-writer case. > On June 26, 2014, 12:51 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/TransactionRequest.scala, line 45 > > <https://reviews.apache.org/r/22905/diff/1/?file=615401#file615401line45> > > > > Thinking about this: wondering if we should remove this and have the > > broker always treat it with required acks -1 > > > > i.e., why would a producer want to have transactions and then set this > > to anything other than -1 > > Dong Lin wrote: > One possible scenario is, the producer only wants atomicity and strict > ordering among transactions, and does not care whether the whole transaction > data is lost. > > What do you think? I don't think you can guarantee atomicity with anything but acks=-1. Say, if acks = 1, if the producer commits, gets an ack but the control record is not replicated and the coordinator fails the transaction will be expired by the new coordinator. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22905/#review46576 ----------------------------------------------------------- On June 24, 2014, 2:02 a.m., Dong Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22905/ > ----------------------------------------------------------- > > (Updated June 24, 2014, 2:02 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1477 > https://issues.apache.org/jira/browse/KAFKA-1477 > > > Repository: kafka > > > Description > ------- > > Add request and response for transaction control and its metadata > > > Diffs > ----- > > core/src/main/scala/kafka/api/RequestKeys.scala > fbfc9d3aeaffed4ca85902125fcc1050086835db > core/src/main/scala/kafka/api/TransactionMetadataRequest.scala PRE-CREATION > core/src/main/scala/kafka/api/TransactionMetadataResponse.scala > PRE-CREATION > core/src/main/scala/kafka/api/TransactionRequest.scala PRE-CREATION > core/src/main/scala/kafka/api/TransactionResponse.scala PRE-CREATION > core/src/main/scala/kafka/common/ErrorMapping.scala > 5559d26ba2b96059f719754a351fa4598ca8a70b > > Diff: https://reviews.apache.org/r/22905/diff/ > > > Testing > ------- > > > Thanks, > > Dong Lin > >
