Hello Jun, Thanks for the comments!! Some responses below:
*> 100. Security: **We could also include AppId in produce request..* On brokers only PIDs are maintained and they are unaware of the AppIds, so I think it would be costly to prevent writes on the AppId level. On the other hand, having security based on AppId for transactional requests trying to write to the transaction logs seems sufficient to me, since producers always need to talk to the transaction coordinator first in order to send data to partition leaders. *> 101. A tricky case can be that the leader broker is on the new message format, but the follower broker is still on the old message format...* The upgrade path has been updated in the wiki page <https://cwiki.apache.org/confluence/display/KAFKA/KIP-98+-+Exactly+Once+Delivery+and+Transactional+Messaging#KIP-98-ExactlyOnceDeliveryandTransactionalMessaging-Compatibility,Deprecation,andMigrationPlan>. Note that we will only let clients to start using the idempotent / transactional features after the whole cluster has completed upgrading (both inter-broker protocol and message format). But to reduce temporary performance degradation we can consider letting clients to upgrade without using the new features so that they will send / consume data following the new message format, as indicated in step 3. *> 102. When there is a correlated hard failure (e.g., power outage), it's possible that an existing commit/abort marker is lost in all replicas...* As Apurva mentioned, we can provide an admin tool to let operators to fix such issues when correlated hard failure happens. Another potential solution is to let brokers to fsync on transaction boundaries (i.e. when the markers are being written), so that the likelihood of such hard failures causing markers to be completely lost can be reduced. *> 105. When the transaction coordinator changes (due to leadership changes), it's possible for both the old and the new coordinator sending requests to a broker at the same time (for a short period of time)...* This is a good question. We have updated the design doc to add a coordinator epoch in the WriteTxnMarkerRequest as well as added it in the transaction message's value payload and the PID snapshot file (see here <https://docs.google.com/document/d/11Jqy_GjUGtdXJK94XGsEIK7CP1SnQGdp2eF0wSw9ra8/edit#bookmark=id.ptdscx8pzota> for details). The coordinator epoch corresponds to the transaction log's leader epoch. *> 107. Could you include the default values for the newly introduced configs?* Have updated the design doc with the default values of newly added configs, see here <https://docs.google.com/document/d/11Jqy_GjUGtdXJK94XGsEIK7CP1SnQGdp2eF0wSw9ra8/edit#heading=h.78p7cgjqcbnx>, here <https://docs.google.com/document/d/11Jqy_GjUGtdXJK94XGsEIK7CP1SnQGdp2eF0wSw9ra8/edit#heading=h.iixbdsg65d7k> and here <https://docs.google.com/document/d/11Jqy_GjUGtdXJK94XGsEIK7CP1SnQGdp2eF0wSw9ra8/edit#heading=h.wvdrakld4019> . *> 117. UpdateTxnRequest: Could you explain the format of Marker?* Note that we have renamed UpdateTxnRequest to WriteTxnMarkerRequest to be more specific. We have update the doc <https://docs.google.com/document/d/11Jqy_GjUGtdXJK94XGsEIK7CP1SnQGdp2eF0wSw9ra8/edit#heading=h.jtpvkrldhb7> to include its current possible values. *> 118. TxnOffsetCommitRequest: How is retention time determined? Do we need a new config in producer or just default it to -1 as the consumer?* -1 will be used as the consumer. Corresponding section <https://docs.google.com/document/d/11Jqy_GjUGtdXJK94XGsEIK7CP1SnQGdp2eF0wSw9ra8/edit#heading=h.5695qbm2hne> is updated. *> 121. The ordering is important with idempotent producer, which means that max.in.flight.requests.per.connection should be set to 1. Do we want to enforce this?* I think it is actually not necessary, since the brokers will "strictly" check the sequence number that must be current sequence + 1, so as long as the first request fails, the rest will doom to fail as well. *> 122. Currently, since we don't know the number of messages in a compressed set, to finish the iteration, we rely on catching EOF in the decompressor, which adds a bit overhead in the consumer.* The logic is not to only relying on catching EOF, but also depending on the offsetDelta to determine the "higher bound" of the number of messages. So only if log compaction is triggered and the last message(s) are compacted, then we need to rely on catching EOFs, whose cost would be much less than KAFKA-4293. *> 123. I am wondering if the coordinator needs to add a "BEGIN transaction message" on a BeginTxnRequest. **Could we just wait until an AddPartitionsToTxnRequest?* It is possible, though no likely, that a client sends an AddOffsetsToTxnRequest right after a BeginTxnRequest, in this case we need to make sure that there is already an on-going transaction. Guozhang On Wed, Jan 25, 2017 at 6:00 PM, Apurva Mehta <apu...@confluent.io> wrote: > On Tue, Jan 17, 2017 at 6:17 PM, Apurva Mehta <apu...@confluent.io> wrote: > > > Hi Jun, > > > > Some answers in line. > > > > > > 109. Could you describe when Producer.send() will receive an Unrecognized > > > > MessageException? > > > > > > This exception will be thrown if the producer sends a sequence number > > which is greater than the sequence number expected by the broker (ie. > more > > than 1 greater than the previously sent sequence number). This can happen > > in two cases: > > > > a) If there is a bug in the producer where sequence numbers are > > incremented more than once per message. So the producer itself will send > > messages with gaps in sequence numbers. > > b) The broker somehow lost a previous message. In a cluster configured > for > > durability (ie. no unclean leader elections, replication factor of 3, > > min.isr of 2, acks=all, etc.), this should not happened. > > > > So realistically, this exception will only be thrown in clusters > > configured for high availability where brokers could lose messages. > > > > Becket raised the question if we should throw this exception at all in > > case b: it indicates a problem with a previously sent message and hence > the > > semantics are counter intuitive. We are still discussing this point, and > > suggestions are most welcome! > > > > > I updated the KIP wiki to clarify when this exception will be raised. > > First of all, I renamed this to OutOfOrderSequenceException. Based on Jay's > suggestion, this is a more precise name that is easier to understand. > > Secondly, I updated the proposed API so that the send call will never raise > this exception directly. Instead this exception will be returned in the > future or passed with the callback, if any. Further, since this is a fatal > exception, any _future_ invocations of send() or other data generating > methods in the producer will raise an IllegalStateException. I think this > makes the semantics clearer and addresses the feedback on this part of the > API update. > > Thanks, > Apurva > -- -- Guozhang
