Thanks for the explanation, Guozhang. That makes sense. On Sun, Feb 19, 2017 at 7:28 PM, Guozhang Wang <wangg...@gmail.com> wrote:
> Thanks Becket. > > Actually sequence is associated with a message, not a message set. For > example if a message set sent by producer contains 100 messages, and the > first message's sequence is 5, then the last message's sequence number > would be 104, and the next message set's first sequence is expected to be > 105. > > > Guozhang > > > On Sun, Feb 19, 2017 at 4:48 PM, Becket Qin <becket....@gmail.com> wrote: > > > +1. Thanks for the great work on the KIP! > > > > I have only one minor question, in the wiki (and the doc) the new message > > set format has a "FirstSequence" field, should it just be "Sequence" if > the > > sequence is always associated with a message set? > > > > On Fri, Feb 17, 2017 at 3:28 AM, Michael Pearce <michael.pea...@ig.com> > > wrote: > > > > > +0 > > > > > > I think need some unified agreement on the VarInts. > > > > > > Would this also change in all other area’s of the protocol, e.g. value > > and > > > key length in message protocol, to keep this uniform across all > protocols > > > going forwards? > > > > > > > > > > > > On 17/02/2017, 00:23, "Apurva Mehta" <apu...@confluent.io> wrote: > > > > > > Hi Jun, > > > > > > Thanks for the reply. Comments inline. > > > > > > On Thu, Feb 16, 2017 at 2:29 PM, Jun Rao <j...@confluent.io> wrote: > > > > > > > Hi, Apurva, > > > > > > > > Thanks for the reply. A couple of comment below. > > > > > > > > On Wed, Feb 15, 2017 at 9:45 PM, Apurva Mehta < > apu...@confluent.io > > > > > > wrote: > > > > > > > > > Hi Jun, > > > > > > > > > > Answers inline: > > > > > > > > > > 210. Pid snapshots: Is the number of pid snapshot configurable > or > > > > hardcoded > > > > > > with 2? When do we decide to roll a new snapshot? Based on > > time, > > > byte, > > > > or > > > > > > offset? Is that configurable too? > > > > > > > > > > > > > > > > > > > > When a replica becomes a follower, we do a bit log truncation. > > > Having an > > > > older snapshot allows us to recover the PID->sequence mapping > much > > > quicker > > > > than rescanning the whole log. > > > > > > > > > This is a good point. I have updated the doc with a more detailed > > > proposal. > > > Essentially, snapshots will be created on a periodic basis. A > > > reasonable > > > period would be every 30 or 60 seconds. We will keep at most 2 > copies > > > of > > > the snapshot file. With this setup, we would have to replay at most > > 60 > > > or > > > 120 seconds of the log in the event of log truncation during leader > > > failover. > > > > > > If we need to make any of this configurable, we can expose a config > > in > > > the > > > future. It would be easier to add a config we need than remove one > > with > > > marginal utility. > > > > > > > > > > > > > > > > > > > > > > 211. I am wondering if we should store ExpirationTime in the > > > producer > > > > > > transactionalId mapping message as we do in the producer > > > transaction > > > > > status > > > > > > message. If a producer only calls initTransactions(), but > never > > > > publishes > > > > > > any data, we still want to be able to expire and remove the > > > producer > > > > > > transactionalId mapping message. > > > > > > > > > > > > > > > > > Actually, the document was inaccurate. The transactionalId will > > be > > > > expired > > > > > only if there is no active transaction, and the age of the last > > > > transaction > > > > > with that transactionalId is older than the transactioanlId > > > expiration > > > > > time. With these semantics, storing the expiration time in the > > > > > transactionalId mapping message won't be useful, since the > > > expiration > > > > time > > > > > is a moving target based on transaction activity. > > > > > > > > > > I have updated the doc with a clarification. > > > > > > > > > > > > > > > > > > > Currently, the producer transactionalId mapping message doesn't > > carry > > > > ExpirationTime, but the producer transaction status message does. > > > It would > > > > be useful if they are consistent. > > > > > > > > > > > You are right. The document has been updated to remove the > > > ExpirationTime > > > from the transaction status messages as well. Any utility for this > > > field > > > can be achieved by using the timestamp of the message itself along > > with > > > another expiration time (like transactionalId expiration time, > > > transaction > > > expiration time, etc.). > > > > > > Thanks, > > > Apurva > > > > > > > > > The information contained in this email is strictly confidential and > for > > > the use of the addressee only, unless otherwise indicated. If you are > not > > > the intended recipient, please do not read, copy, use or disclose to > > others > > > this message or any attachment. Please also notify the sender by > replying > > > to this email or by telephone (+44(020 7896 0011) and then delete the > > email > > > and any copies of it. Opinions, conclusion (etc) that do not relate to > > the > > > official business of this company shall be understood as neither given > > nor > > > endorsed by it. IG is a trading name of IG Markets Limited (a company > > > registered in England and Wales, company number 04008957) and IG Index > > > Limited (a company registered in England and Wales, company number > > > 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, > > > London EC4R 2YA. Both IG Markets Limited (register number 195355) and > IG > > > Index Limited (register number 114059) are authorised and regulated by > > the > > > Financial Conduct Authority. > > > > > > > > > -- > -- Guozhang >