Hi, Jose, Thanks for the KIP. +1. A couple of minor comments below.
1. The new configuration names suggested by Ron sound reasonable. 2. It seems that OFFSET_OUT_OF_RANGE in the wiki needs to be changed to POSITION_OUT_OF_RANGE. Jun On Mon, Oct 5, 2020 at 9:46 AM Jason Gustafson <ja...@confluent.io> wrote: > +1 Thanks for the KIP! > > -Jason > > On Mon, Oct 5, 2020 at 9:03 AM Ron Dagostino <rndg...@gmail.com> wrote: > > > Thanks for the KIP, Jose. +1 (non-binding) from me. > > > > I do have one comment/confusion. > > > > Upon re-reading the latest version, I am confused about the name of > > the proposed "metadata.snapshot.min.records" config. Is this a size, > > or is it a count? I think it is about a size but want to be sure. I > > also wonder if it is about changes (updates/deletes) rather than just > > additions/accretions, or is it independent of that? > > > > I'm also unclear about the definition of the > > "metadata.snapshot.min.cleanable.ratio" config -- is that a ratio of a > > *number* of new records to the number of snapshot records? Or is it a > > *size* ratio? I think it is a ratio of numbers of records rather than > > a ratio of sizes. I think this one is also about changes > > (updates/deletes) rather than just additions/accretions. > > > > I'm wondering if we can be clearer with the names of these two configs > > to make their definitions more apparent. For example, assuming > > certain definitions as mentioned above: > > > > metadata.snapshot.min.new_records.size -- the minimum size of new > > records required before a snapshot can occur > > metadata.snapshot.min.change_records.ratio -- the minimum ratio of the > > number of change (i.e. not simply accretion) records to the number of > > records in the last snapshot (if any) that must be achieved before a > > snapshot can occur. > > > > For example, if there is no snapshot yet, then ".new_records.size" > > must be written before a snapshot is allowed. If there is a snapshot > > with N records, then before a snapshot is allowed both > > ".new_records.size" must be written and ".change_records.ratio" must > > be satisfied such that the number of changes (not accretions) divided > > by N meets the ratio. > > > > Ron > > > > > > > > > > > > On Fri, Oct 2, 2020 at 8:14 PM Lucas Bradstreet <lu...@confluent.io> > > wrote: > > > > > > Thanks for the KIP! Non-binding +1 > > > > > > On Fri, Oct 2, 2020 at 3:30 PM Guozhang Wang <wangg...@gmail.com> > wrote: > > > > > > > Thanks Jose! +1 from me. > > > > > > > > On Fri, Oct 2, 2020 at 3:18 PM Jose Garcia Sancio < > > jsan...@confluent.io> > > > > wrote: > > > > > > > > > Hi all, > > > > > > > > > > I would like to start a vote on KIP-630. > > > > > > > > > > KIP: https://cwiki.apache.org/confluence/x/exV4CQ > > > > > Discussion Thread: > > > > > > > > > > > > > > > > > https://lists.apache.org/thread.html/r9468d1f276385695a2d6d48f6dfbdc504c445fc5745aaa606d138fed%40%3Cdev.kafka.apache.org%3E > > > > > > > > > > Thank you > > > > > -- > > > > > -Jose > > > > > > > > > > > > > > > > > -- > > > > -- Guozhang > > > > > > >
