Thanks for the votes Jun, Jason, Ron, Lucas and Guozhang. Thanks for the feedback Ron and Jun.
Agree with your comments Ron. I have updated those configurations to metadata.snapshot.min.changed_records.ratio and metadata.snapshot.min.new_records.size. I thought of using "clenable" to keep it consistent with the configuration of compaction policy. Snapshots are different enough that that consistency is not needed. Jun, I missed the incorrect mention of OFFSET_OUT_OF_RANGE. I have replaced it with POSITION_OUT_OF_RANGE. Changes to the KIP are here: https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158864763&selectedPageVersions=39&selectedPageVersions=37 I believe that we have enough votes to accept this KIP. I'll close the voting on Monday. On Mon, Oct 5, 2020 at 2:04 PM Jun Rao <j...@confluent.io> wrote: > > 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 > > > > > > > > > > -- -Jose
