> 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
Excuse me.. But where is the typo? > 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. 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? > On June 26, 2014, 12:51 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/TransactionMetadataRequest.scala, line 65 > > <https://reviews.apache.org/r/22905/diff/1/?file=615399#file615399line65> > > > > same here I forgot the change this comment after copying it from ConsumerMetadataRequest.scala. I will update this comment to "return TransactionCoordinatorNotAvailable for all uncaught errors" > 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. I thought we will delay adding the feature for single-writer requirement. Should I add a field "genID" to the TransactionRequest now? > 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 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? > On June 26, 2014, 12:51 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/TransactionRequest.scala, line 89 > > <https://reviews.apache.org/r/22905/diff/1/?file=615401#file615401line89> > > > > Should probably append this only if details is true. Yes I agree. Will add this to the code. Thanks! - Dong ----------------------------------------------------------- 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 > >
