You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jose Garcia Sancio <js...@confluent.io> on 2020/10/02 22:18:38 UTC
[VOTE] KIP-630: Kafka Raft Snapshot
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
Re: [VOTE] KIP-630: Kafka Raft Snapshot
Posted by José Armando García Sancio <js...@confluent.io.INVALID>.
During the development of KIP-630 we made some minor changes to the
KIP to better match the implementation details. Here is a summary of
the changes we made to the KIP:
1. Added control records at the begin and end of the snapshots. The
control records are versioned. The snapshot header record includes the
append time of the last record from the log included in the snapshot.
This is useful when determining when to delete a snapshot.
2. The configuration property metadata.snapshot.min.new_records.size
was renamed to metadata.log.max.record.bytes.between.snapshots.
3. The FetchSnapshotRequest schema was changed to include the cluster
id and the current leader epoch. This is used by the leader of the
metadata log to validate that the request matches the current cluster
id and the current leader epoch.
4. The FetchSnapshotResponse schema was changed to include the snapshot id.
KIP-630: https://cwiki.apache.org/confluence/x/exV4CQ
Thanks!
-Jose
Re: [VOTE] KIP-630: Kafka Raft Snapshot
Posted by Jose Garcia Sancio <js...@confluent.io>.
Thanks everyone for the votes. KIP-630 has been accepted.
Binding: Guozhang, Jason and Jun
Non-binding: Ron and Lucas
On Fri, Oct 9, 2020 at 4:33 PM Jose Garcia Sancio <js...@confluent.io> wrote:
>
> 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 <ju...@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 <rn...@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 <wa...@gmail.com>
> > > wrote:
> > > > >
> > > > > > Thanks Jose! +1 from me.
> > > > > >
> > > > > > On Fri, Oct 2, 2020 at 3:18 PM Jose Garcia Sancio <
> > > > jsancio@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
--
-Jose
Re: [VOTE] KIP-630: Kafka Raft Snapshot
Posted by Jose Garcia Sancio <js...@confluent.io>.
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 <ju...@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 <rn...@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 <wa...@gmail.com>
> > wrote:
> > > >
> > > > > Thanks Jose! +1 from me.
> > > > >
> > > > > On Fri, Oct 2, 2020 at 3:18 PM Jose Garcia Sancio <
> > > jsancio@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
Re: [VOTE] KIP-630: Kafka Raft Snapshot
Posted by Jun Rao <ju...@confluent.io>.
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 <rn...@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 <wa...@gmail.com>
> wrote:
> > >
> > > > Thanks Jose! +1 from me.
> > > >
> > > > On Fri, Oct 2, 2020 at 3:18 PM Jose Garcia Sancio <
> > jsancio@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
> > > >
> >
>
Re: [VOTE] KIP-630: Kafka Raft Snapshot
Posted by Jason Gustafson <ja...@confluent.io>.
+1 Thanks for the KIP!
-Jason
On Mon, Oct 5, 2020 at 9:03 AM Ron Dagostino <rn...@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 <wa...@gmail.com> wrote:
> >
> > > Thanks Jose! +1 from me.
> > >
> > > On Fri, Oct 2, 2020 at 3:18 PM Jose Garcia Sancio <
> jsancio@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
> > >
>
Re: [VOTE] KIP-630: Kafka Raft Snapshot
Posted by Ron Dagostino <rn...@gmail.com>.
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 <wa...@gmail.com> wrote:
>
> > Thanks Jose! +1 from me.
> >
> > On Fri, Oct 2, 2020 at 3:18 PM Jose Garcia Sancio <js...@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
> >
Re: [VOTE] KIP-630: Kafka Raft Snapshot
Posted by Lucas Bradstreet <lu...@confluent.io>.
Thanks for the KIP! Non-binding +1
On Fri, Oct 2, 2020 at 3:30 PM Guozhang Wang <wa...@gmail.com> wrote:
> Thanks Jose! +1 from me.
>
> On Fri, Oct 2, 2020 at 3:18 PM Jose Garcia Sancio <js...@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
>
Re: [VOTE] KIP-630: Kafka Raft Snapshot
Posted by Guozhang Wang <wa...@gmail.com>.
Thanks Jose! +1 from me.
On Fri, Oct 2, 2020 at 3:18 PM Jose Garcia Sancio <js...@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