You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Sushant Mahajan <su...@gmail.com> on 2026/07/09 17:24:51 UTC
Re: [DISCUSS] KIP-1349 : Configurable snapshot frequency for share groups
Hi,
Went over the new byte based approach.
Few questions:
sm01 - currently share coord module does not depend on the group
coordinator. Could you detail out how the new group level config will be
injected in the share coordinator?
sm02 - since we have default defined as 20MB, how to differentiate between
older update count set vs byte limit set? The kip mentions if set byte
level will be honored. How to determine set vs default?
sm03 - could you drop a line about backward compatibility in case of
cluster roll?
sm04 - should we also add any new metrics?
sm05 - do measurements need to take compression into account? Does it
affect the 20 MB limit?
Regards,
Sushant Mahajan
On Sun, 14 Jun 2026, 18:50 Muralidhar Basani via dev, <de...@kafka.apache.org>
wrote:
> Hi,
> Apologies for the delayed reply.
>
> Sushant, thanks for the reply. Just to flag the KIP has switched to
> bytes-based control (pls see below), which supersedes most of the
> records-based tuning discussion.
>
> Chia, thank you for the suggestions. Both the points are now in the KIP:
> - chia_00: broker floor stays at 0 (no compatibility break)
> - chia_01: bytes-based control adopted, matching
> metadata.log.max.record.bytes.between.snapshots, default 20 MB.
>
> Records-based config will be marked as deprecated.
>
> Updated KIP:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Bytes-based+configurable+snapshot+frequency+for+share+groups
>
> Please take a look again.
>
> Thanks,
> Murali
>
> On Thu, Jun 11, 2026 at 5:20 AM Chia-Ping Tsai <ch...@apache.org>
> wrote:
>
> > hi
> >
> > chia_00: Changing the lower bound of the existing config is a
> > compatibility issue. If we want to avoid heavy snapshots, we can set the
> > lower bound of the new group-level configuration to 200 by default. Also,
> > the broker-level constraint should be changed to 0-1000, which won’t
> bring
> > compatibility issues
> >
> > chia_01: Have you considered using bytes instead of records as the
> > controlled unit?
> >
> >
> > On 2026/05/22 19:30:38 Muralidhar Basani via dev wrote:
> > > Hi all,
> > >
> > > I would like to start a discussion on KIP-1349, which allows
> configurable
> > > snapshot frequency of share groups.
> > >
> > > KIP :
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Configurable+snapshot+frequency+for+share+groups
> > >
> > > Thanks,
> > > Murali
> > >
> >
>
Re: [DISCUSS] KIP-1349 : Configurable snapshot frequency for share groups
Posted by Muralidhar Basani via dev <de...@kafka.apache.org>.
Hi all,
If there are no further concerns, I'd like to start a vote soon.
Please let me know if any questions.
Thanks,
Murali
On Sat, Jul 25, 2026 at 8:10 PM Muralidhar Basani <
muralidhar.basani@aiven.io> wrote:
> Thanks Andrew,
> I have updated kip - added the performance info to the motivation,
> renamed the configs to the max.record.bytes.between.snapshots form, and
> changed the per-group config importance level to LOW as you suggested.
>
> Pls let me know if you have any further feedback.
>
> Thanks,
> Murali
>
>
> On Fri, Jul 24, 2026 at 4:24 PM Andrew Schofield <sc...@apache.org>
> wrote:
>
>> Hi Murali,
>> Thanks for your response.
>>
>> AS1: Thanks for the performance information. Adding this into the KIP is
>> definitely helpful in the motivation.
>>
>> AS2: Thanks. Sounds good to me.
>>
>> AS3: HIGH and MEDIUM is fine with me, but my suggestion for LOW was
>> because I think it will be rare for people to adjust the per-group config.
>> It's a small point and happy for you to choose.
>>
>> Thanks,
>> Andrew
>>
>> On 2026/07/16 21:12:58 Muralidhar Basani via dev wrote:
>> > Thanks Sushant & Andrew.
>> >
>> > AS1
>> > On performance testing so far:
>> >
>> > I ran a test on a local 4.3 cluster to see how big the snapshot related
>> > records actually are.
>> >
>> > +++++++++++
>> >
>> config_value,record_count,snapshot_bytes,update_bytes,snapshot_count,update_count,snapshot_byte_ratio,avg_update_bytes,avg_snapshot_bytes,updates_per_20mb,cliff_ratio_vs_N,broker_restart_load_secs
>> > 50,200000,1284,51561,8,337,0.024,153.0,160.5,137069,2741.4,3
>> > 100,200000,592,52206,4,339,0.011,154.0,148.0,136179,1361.8,3
>> > 200,200000,592,52360,4,340,0.011,154.0,148.0,136179,680.9,4
>> > 300,200000,592,52514,4,341,0.011,154.0,148.0,136179,453.9,4
>> > 500,200000,592,52206,4,339,0.011,154.0,148.0,136179,272.4,4
>> > +++++++++++
>> >
>> > (Note: update_count is low because acks were in-order. pls see the
>> caveat
>> > below.)
>> >
>> > The main finding: each "update" record is tiny, about 154 bytes on
>> average.
>> > This stayed the same no matter what I changed, because these records
>> only
>> > store small (bookkeeping) data (which messages were delivered/acked),
>> not
>> > the message contents.
>> >
>> > Currently the old setting takes a snapshot every N update records (max
>> > 500). The new setting takes a snapshot every 20 MB. Since each update is
>> > only ~154 bytes, 20 MB works out to about 136,000 updates between
>> snapshots.
>> >
>> > So when we eventually remove the old config, a group that used to
>> snapshot
>> > every 500 updates would instead snapshot every ~136,000, roughly ~270×
>> less
>> > often at the old maximum of 500, and even more dramatic at lower
>> settings
>> > (up to ~2700× at N=50). Fewer snapshots means recovery after a restart
>> > takes longer and more log is kept on disk before cleanup, because there
>> are
>> > more update records to replay before reaching a snapshot. It does not
>> > affect throughput.
>> >
>> > In my test, the consumer acknowledged messages in order, so 200,000
>> > messages collapsed into only ~340 update records. That was enough to
>> > measure the size of an update, but it did not trigger the snapshot
>> limits,
>> > so restart time stayed flat (~3–4s) in every run. To properly measure
>> the
>> > recovery-time difference, I'll use a consumer that acknowledges out of
>> > order, which produces many more (and larger) update records and actually
>> > exercises the snapshot limits, then compare restart time and on-disk log
>> > size at the old vs. new settings.
>> >
>> > Agree, the new code does a little extra work per update (one more
>> counter,
>> > plus measuring each record's size). I'll benchmark that against trunk
>> > during impl, before merge. I will add a performance investigation note
>> to
>> > the KIP's Test Plan to capture this.
>> >
>> > AS2
>> >
>> > Good point. The name I proposed
>> > (share.coordinator.snapshot.update.bytes.per.snapshot) was derived from
>> the
>> > existing config and swapped "records" for "bytes".
>> >
>> > But this bytes-based design is modeled on
>> > metadata.log.max.record.bytes.between.snapshots, so happy to rename.
>> > I can rename both configs:
>> >
>> > - broker: share.coordinator.snapshot.update.bytes.per.snapshot to
>> > share.coordinator.max.record.bytes.between.snapshots
>> > - per-group: share.snapshot.update.bytes.per.snapshot to
>> > share.max.record.bytes.between.snapshots
>> >
>> > AS3
>> >
>> > Broker-level HIGH: agree
>> > Group-level LOW: I see the reasoning, but every existing per-group
>> config
>> > in GroupConfig currently is MEDIUM.
>> > (including the other share.* group configs like
>> > share.record.lock.duration.ms). So LOW would make this the only LOW
>> group
>> > config. Slight lean toward MEDIUM for consistency, but no strong
>> objection
>> > to LOW.
>> >
>> > Pls let me know your thoughts.
>> >
>> > Thanks,
>> > Murali
>> >
>> > On Thu, Jul 16, 2026 at 3:55 PM Andrew Schofield <schofielaj@apache.org
>> >
>> > wrote:
>> >
>> > > Hi Murali,
>> > > Thanks for the KIP. I understand the proposal.
>> > >
>> > > AS1: What performance testing have you done to validate whether there
>> is a
>> > > performance impact due to this change? When we get to the point of
>> removing
>> > > the deprecated configuration, there is a potential behavioural cliff
>> which
>> > > we should have investigated before we get there.
>> > >
>> > > AS2: Is there any reason why you did not follow a similar naming
>> scheme as
>> > > the existing metadata log configs, such as
>> > > share.coordinator.max.record.bytes.between.snapshots?
>> > >
>> > > AS3: I suggest that the broker-level config would be HIGH and the
>> > > group-level config would be LOW importance.
>> > >
>> > > Thanks,
>> > > Andrew
>> > >
>> > > On 2026/07/16 05:13:29 Sushant Mahajan wrote:
>> > > > Thanks for the change.
>> > > >
>> > > > All comments from my side are addressed.
>> > > >
>> > > > Regards,
>> > > > Sushant Mahajan
>> > > >
>> > > > On Wed, 15 Jul 2026, 00:37 Muralidhar Basani via dev, <
>> > > dev@kafka.apache.org>
>> > > > wrote:
>> > > >
>> > > > > Thanks Sushanth.
>> > > > >
>> > > > > I updated KIP to remove the module dependency, create a helper
>> > > interface in
>> > > > > share-coordinator, and add a new metric.
>> > > > > Please take a look.
>> > > > >
>> > > > > Thanks,
>> > > > > Murali
>> > > > >
>> > > > > On Mon, Jul 13, 2026 at 5:39 PM Sushant Mahajan <
>> sushmahajn@gmail.com>
>> > > > > wrote:
>> > > > >
>> > > > > > Hi,
>> > > > > > Thanks for the response.
>> > > > > >
>> > > > > > Regarding
>> > > > > >
>> > > > > > sm01 - I would prefer not a add group coordinator dependency on
>> share
>> > > > > > coordinator. Thats why we created coordinator-common.
>> > > > > >
>> > > > > > Could we instead create a helper interface in share-coordinator
>> to
>> > > access
>> > > > > > the relevant configs and pass the impl from BrokerServer (which
>> has
>> > > > > access
>> > > > > > to relevant group config managers) to the share coordinator
>> service?
>> > > > > >
>> > > > > > sm04 - yes, lets add that one.
>> > > > > >
>> > > > > > Rest of the answers are good.
>> > > > > >
>> > > > > > Regards,
>> > > > > > Sushant Mahajan
>> > > > > >
>> > > > > > On Fri, 10 Jul 2026, 22:55 Muralidhar Basani via dev, <
>> > > > > > dev@kafka.apache.org>
>> > > > > > wrote:
>> > > > > >
>> > > > > > > Thanks again Sushant for the nice review.
>> > > > > > >
>> > > > > > > sm01
>> > > > > > >
>> > > > > > > True. share-coordinator has no compile dependency on
>> > > group-coordinator.
>> > > > > > The
>> > > > > > > KIP adds implementation project(':group-coordinator') to the
>> > > > > > > share-coordinator build. There is no cyclic dep.
>> > > > > > >
>> > > > > > > ShareGroupConfigProvider gets a new accessor
>> > > > > > > snapshotUpdateBytesPerSnapshotOrDefault(groupId,
>> brokerDefault)
>> > > > > > > ShareCoordinatorShard.Builder gets a new setter
>> > > > > > > withShareGroupConfigProvider(.....) and
>> > > ShareCoordinatorService.Builder
>> > > > > > > gets the same setter.
>> > > > > > > With these, BrokerServer can now construct the provider and
>> pass
>> > > it in.
>> > > > > > The
>> > > > > > > shard can then communicate with the provider on both the
>> write path
>> > > > > > > (generateShareStateRecord) and the replay path
>> > > (handleShareSnapshot),
>> > > > > > > falling back to the broker-level default (when the group has
>> no
>> > > > > > override).
>> > > > > > >
>> > > > > > > sm02
>> > > > > > >
>> > > > > > > Good point. A default value is not distinguishable from an
>> explicit
>> > > > > value
>> > > > > > > via getLong() alone.
>> > > > > > >
>> > > > > > > I have updated KIP Public interfaces -> deprecated config
>> section.
>> > > > > > > 1. If share.coordinator.snapshot.update.bytes.per.snapshot is
>> in
>> > > > > > > originals() -> bytes mode.
>> > > > > > > 2. Else if
>> share.coordinator.snapshot.update.records.per.snapshot
>> > > is in
>> > > > > > > originals() -> records mode (deprecated path).
>> > > > > > > 3. Else (neither explicitly set) -> bytes mode at the 20 mb
>> > > default.
>> > > > > > > 4. If both are in originals() -> bytes mode wins and a warn is
>> > > logged.
>> > > > > > >
>> > > > > > > This resolution happens once at startup in
>> ShareCoordinatorConfig,
>> > > not
>> > > > > > > per-record.
>> > > > > > >
>> > > > > > > sm03
>> > > > > > >
>> > > > > > > Cluster-roll backward compatibility
>> > > > > > >
>> > > > > > > During a rolling upgrade, yes brokers run mixed versions. Each
>> > > broker's
>> > > > > > > shard independently decides snapshot cadence, from its own
>> local
>> > > > > config.
>> > > > > > A
>> > > > > > > newer broker replaying records written by an older broker (or
>> > > other way
>> > > > > > > around) interprets them identically. So cadence may differ in
>> the
>> > > > > > mid-roll,
>> > > > > > > but there is no on-disk format change and no coordination
>> required
>> > > I
>> > > > > > see. A
>> > > > > > > broker that hasn't yet had the new config set, will simply
>> use the
>> > > 20
>> > > > > MB
>> > > > > > > default. I have updated Compatibility section in the kip.
>> > > > > > >
>> > > > > > > sm04
>> > > > > > >
>> > > > > > > Currently the records-based didn't have any. A new metric
>> > > > > > > bytes-since-snapshot (per __share_group_state partition)
>> could be
>> > > > > useful.
>> > > > > > > Let me know if this is good, or any other suggestion?
>> > > > > > >
>> > > > > > > sm05
>> > > > > > >
>> > > > > > > The threshold is measured on uncompressed serialized record
>> bytes.
>> > > > > There
>> > > > > > is
>> > > > > > > a little note in the kip mentioning that "Compression is not
>> > > accounted
>> > > > > > for;
>> > > > > > > the threshold is on uncompressed record bytes."
>> > > > > > > Actually share.coordinator.state.topic.compression.codec
>> defaults
>> > > to
>> > > > > > NONE.
>> > > > > > > So by default on-disk bytes equal the measured bytes; So it
>> will
>> > > not
>> > > > > > affect
>> > > > > > > the 20mb limit, in default deploymnt.
>> > > > > > >
>> > > > > > > Thanks,
>> > > > > > > Murali
>> > > > > > >
>> > > > > > > On Thu, Jul 9, 2026 at 7:25 PM Sushant Mahajan <
>> > > sushmahajn@gmail.com>
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > Hi,
>> > > > > > > > Went over the new byte based approach.
>> > > > > > > >
>> > > > > > > > Few questions:
>> > > > > > > >
>> > > > > > > > sm01 - currently share coord module does not depend on the
>> group
>> > > > > > > > coordinator. Could you detail out how the new group level
>> config
>> > > will
>> > > > > > be
>> > > > > > > > injected in the share coordinator?
>> > > > > > > >
>> > > > > > > > sm02 - since we have default defined as 20MB, how to
>> > > differentiate
>> > > > > > > between
>> > > > > > > > older update count set vs byte limit set? The kip mentions
>> if set
>> > > > > byte
>> > > > > > > > level will be honored. How to determine set vs default?
>> > > > > > > >
>> > > > > > > > sm03 - could you drop a line about backward compatibility in
>> > > case of
>> > > > > > > > cluster roll?
>> > > > > > > >
>> > > > > > > > sm04 - should we also add any new metrics?
>> > > > > > > >
>> > > > > > > > sm05 - do measurements need to take compression into
>> account?
>> > > Does it
>> > > > > > > > affect the 20 MB limit?
>> > > > > > > >
>> > > > > > > > Regards,
>> > > > > > > > Sushant Mahajan
>> > > > > > > >
>> > > > > > > > On Sun, 14 Jun 2026, 18:50 Muralidhar Basani via dev, <
>> > > > > > > > dev@kafka.apache.org>
>> > > > > > > > wrote:
>> > > > > > > >
>> > > > > > > > > Hi,
>> > > > > > > > > Apologies for the delayed reply.
>> > > > > > > > >
>> > > > > > > > > Sushant, thanks for the reply. Just to flag the KIP has
>> > > switched to
>> > > > > > > > > bytes-based control (pls see below), which supersedes
>> most of
>> > > the
>> > > > > > > > > records-based tuning discussion.
>> > > > > > > > >
>> > > > > > > > > Chia, thank you for the suggestions. Both the points are
>> now
>> > > in the
>> > > > > > > KIP:
>> > > > > > > > > - chia_00: broker floor stays at 0 (no compatibility
>> break)
>> > > > > > > > > - chia_01: bytes-based control adopted, matching
>> > > > > > > > > metadata.log.max.record.bytes.between.snapshots, default
>> 20 MB.
>> > > > > > > > >
>> > > > > > > > > Records-based config will be marked as deprecated.
>> > > > > > > > >
>> > > > > > > > > Updated KIP:
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Bytes-based+configurable+snapshot+frequency+for+share+groups
>> > > > > > > > >
>> > > > > > > > > Please take a look again.
>> > > > > > > > >
>> > > > > > > > > Thanks,
>> > > > > > > > > Murali
>> > > > > > > > >
>> > > > > > > > > On Thu, Jun 11, 2026 at 5:20 AM Chia-Ping Tsai <
>> > > > > chia7712@apache.org>
>> > > > > > > > > wrote:
>> > > > > > > > >
>> > > > > > > > > > hi
>> > > > > > > > > >
>> > > > > > > > > > chia_00: Changing the lower bound of the existing
>> config is a
>> > > > > > > > > > compatibility issue. If we want to avoid heavy
>> snapshots, we
>> > > can
>> > > > > > set
>> > > > > > > > the
>> > > > > > > > > > lower bound of the new group-level configuration to 200
>> by
>> > > > > default.
>> > > > > > > > Also,
>> > > > > > > > > > the broker-level constraint should be changed to 0-1000,
>> > > which
>> > > > > > won’t
>> > > > > > > > > bring
>> > > > > > > > > > compatibility issues
>> > > > > > > > > >
>> > > > > > > > > > chia_01: Have you considered using bytes instead of
>> records
>> > > as
>> > > > > the
>> > > > > > > > > > controlled unit?
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > On 2026/05/22 19:30:38 Muralidhar Basani via dev wrote:
>> > > > > > > > > > > Hi all,
>> > > > > > > > > > >
>> > > > > > > > > > > I would like to start a discussion on KIP-1349, which
>> > > allows
>> > > > > > > > > configurable
>> > > > > > > > > > > snapshot frequency of share groups.
>> > > > > > > > > > >
>> > > > > > > > > > > KIP :
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Configurable+snapshot+frequency+for+share+groups
>> > > > > > > > > > >
>> > > > > > > > > > > Thanks,
>> > > > > > > > > > > Murali
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
Re: [DISCUSS] KIP-1349 : Configurable snapshot frequency for share groups
Posted by Muralidhar Basani via dev <de...@kafka.apache.org>.
Thanks Andrew,
I have updated kip - added the performance info to the motivation, renamed
the configs to the max.record.bytes.between.snapshots form, and changed the
per-group config importance level to LOW as you suggested.
Pls let me know if you have any further feedback.
Thanks,
Murali
On Fri, Jul 24, 2026 at 4:24 PM Andrew Schofield <sc...@apache.org>
wrote:
> Hi Murali,
> Thanks for your response.
>
> AS1: Thanks for the performance information. Adding this into the KIP is
> definitely helpful in the motivation.
>
> AS2: Thanks. Sounds good to me.
>
> AS3: HIGH and MEDIUM is fine with me, but my suggestion for LOW was
> because I think it will be rare for people to adjust the per-group config.
> It's a small point and happy for you to choose.
>
> Thanks,
> Andrew
>
> On 2026/07/16 21:12:58 Muralidhar Basani via dev wrote:
> > Thanks Sushant & Andrew.
> >
> > AS1
> > On performance testing so far:
> >
> > I ran a test on a local 4.3 cluster to see how big the snapshot related
> > records actually are.
> >
> > +++++++++++
> >
> config_value,record_count,snapshot_bytes,update_bytes,snapshot_count,update_count,snapshot_byte_ratio,avg_update_bytes,avg_snapshot_bytes,updates_per_20mb,cliff_ratio_vs_N,broker_restart_load_secs
> > 50,200000,1284,51561,8,337,0.024,153.0,160.5,137069,2741.4,3
> > 100,200000,592,52206,4,339,0.011,154.0,148.0,136179,1361.8,3
> > 200,200000,592,52360,4,340,0.011,154.0,148.0,136179,680.9,4
> > 300,200000,592,52514,4,341,0.011,154.0,148.0,136179,453.9,4
> > 500,200000,592,52206,4,339,0.011,154.0,148.0,136179,272.4,4
> > +++++++++++
> >
> > (Note: update_count is low because acks were in-order. pls see the caveat
> > below.)
> >
> > The main finding: each "update" record is tiny, about 154 bytes on
> average.
> > This stayed the same no matter what I changed, because these records only
> > store small (bookkeeping) data (which messages were delivered/acked), not
> > the message contents.
> >
> > Currently the old setting takes a snapshot every N update records (max
> > 500). The new setting takes a snapshot every 20 MB. Since each update is
> > only ~154 bytes, 20 MB works out to about 136,000 updates between
> snapshots.
> >
> > So when we eventually remove the old config, a group that used to
> snapshot
> > every 500 updates would instead snapshot every ~136,000, roughly ~270×
> less
> > often at the old maximum of 500, and even more dramatic at lower settings
> > (up to ~2700× at N=50). Fewer snapshots means recovery after a restart
> > takes longer and more log is kept on disk before cleanup, because there
> are
> > more update records to replay before reaching a snapshot. It does not
> > affect throughput.
> >
> > In my test, the consumer acknowledged messages in order, so 200,000
> > messages collapsed into only ~340 update records. That was enough to
> > measure the size of an update, but it did not trigger the snapshot
> limits,
> > so restart time stayed flat (~3–4s) in every run. To properly measure the
> > recovery-time difference, I'll use a consumer that acknowledges out of
> > order, which produces many more (and larger) update records and actually
> > exercises the snapshot limits, then compare restart time and on-disk log
> > size at the old vs. new settings.
> >
> > Agree, the new code does a little extra work per update (one more
> counter,
> > plus measuring each record's size). I'll benchmark that against trunk
> > during impl, before merge. I will add a performance investigation note to
> > the KIP's Test Plan to capture this.
> >
> > AS2
> >
> > Good point. The name I proposed
> > (share.coordinator.snapshot.update.bytes.per.snapshot) was derived from
> the
> > existing config and swapped "records" for "bytes".
> >
> > But this bytes-based design is modeled on
> > metadata.log.max.record.bytes.between.snapshots, so happy to rename.
> > I can rename both configs:
> >
> > - broker: share.coordinator.snapshot.update.bytes.per.snapshot to
> > share.coordinator.max.record.bytes.between.snapshots
> > - per-group: share.snapshot.update.bytes.per.snapshot to
> > share.max.record.bytes.between.snapshots
> >
> > AS3
> >
> > Broker-level HIGH: agree
> > Group-level LOW: I see the reasoning, but every existing per-group config
> > in GroupConfig currently is MEDIUM.
> > (including the other share.* group configs like
> > share.record.lock.duration.ms). So LOW would make this the only LOW
> group
> > config. Slight lean toward MEDIUM for consistency, but no strong
> objection
> > to LOW.
> >
> > Pls let me know your thoughts.
> >
> > Thanks,
> > Murali
> >
> > On Thu, Jul 16, 2026 at 3:55 PM Andrew Schofield <sc...@apache.org>
> > wrote:
> >
> > > Hi Murali,
> > > Thanks for the KIP. I understand the proposal.
> > >
> > > AS1: What performance testing have you done to validate whether there
> is a
> > > performance impact due to this change? When we get to the point of
> removing
> > > the deprecated configuration, there is a potential behavioural cliff
> which
> > > we should have investigated before we get there.
> > >
> > > AS2: Is there any reason why you did not follow a similar naming
> scheme as
> > > the existing metadata log configs, such as
> > > share.coordinator.max.record.bytes.between.snapshots?
> > >
> > > AS3: I suggest that the broker-level config would be HIGH and the
> > > group-level config would be LOW importance.
> > >
> > > Thanks,
> > > Andrew
> > >
> > > On 2026/07/16 05:13:29 Sushant Mahajan wrote:
> > > > Thanks for the change.
> > > >
> > > > All comments from my side are addressed.
> > > >
> > > > Regards,
> > > > Sushant Mahajan
> > > >
> > > > On Wed, 15 Jul 2026, 00:37 Muralidhar Basani via dev, <
> > > dev@kafka.apache.org>
> > > > wrote:
> > > >
> > > > > Thanks Sushanth.
> > > > >
> > > > > I updated KIP to remove the module dependency, create a helper
> > > interface in
> > > > > share-coordinator, and add a new metric.
> > > > > Please take a look.
> > > > >
> > > > > Thanks,
> > > > > Murali
> > > > >
> > > > > On Mon, Jul 13, 2026 at 5:39 PM Sushant Mahajan <
> sushmahajn@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi,
> > > > > > Thanks for the response.
> > > > > >
> > > > > > Regarding
> > > > > >
> > > > > > sm01 - I would prefer not a add group coordinator dependency on
> share
> > > > > > coordinator. Thats why we created coordinator-common.
> > > > > >
> > > > > > Could we instead create a helper interface in share-coordinator
> to
> > > access
> > > > > > the relevant configs and pass the impl from BrokerServer (which
> has
> > > > > access
> > > > > > to relevant group config managers) to the share coordinator
> service?
> > > > > >
> > > > > > sm04 - yes, lets add that one.
> > > > > >
> > > > > > Rest of the answers are good.
> > > > > >
> > > > > > Regards,
> > > > > > Sushant Mahajan
> > > > > >
> > > > > > On Fri, 10 Jul 2026, 22:55 Muralidhar Basani via dev, <
> > > > > > dev@kafka.apache.org>
> > > > > > wrote:
> > > > > >
> > > > > > > Thanks again Sushant for the nice review.
> > > > > > >
> > > > > > > sm01
> > > > > > >
> > > > > > > True. share-coordinator has no compile dependency on
> > > group-coordinator.
> > > > > > The
> > > > > > > KIP adds implementation project(':group-coordinator') to the
> > > > > > > share-coordinator build. There is no cyclic dep.
> > > > > > >
> > > > > > > ShareGroupConfigProvider gets a new accessor
> > > > > > > snapshotUpdateBytesPerSnapshotOrDefault(groupId, brokerDefault)
> > > > > > > ShareCoordinatorShard.Builder gets a new setter
> > > > > > > withShareGroupConfigProvider(.....) and
> > > ShareCoordinatorService.Builder
> > > > > > > gets the same setter.
> > > > > > > With these, BrokerServer can now construct the provider and
> pass
> > > it in.
> > > > > > The
> > > > > > > shard can then communicate with the provider on both the write
> path
> > > > > > > (generateShareStateRecord) and the replay path
> > > (handleShareSnapshot),
> > > > > > > falling back to the broker-level default (when the group has no
> > > > > > override).
> > > > > > >
> > > > > > > sm02
> > > > > > >
> > > > > > > Good point. A default value is not distinguishable from an
> explicit
> > > > > value
> > > > > > > via getLong() alone.
> > > > > > >
> > > > > > > I have updated KIP Public interfaces -> deprecated config
> section.
> > > > > > > 1. If share.coordinator.snapshot.update.bytes.per.snapshot is
> in
> > > > > > > originals() -> bytes mode.
> > > > > > > 2. Else if
> share.coordinator.snapshot.update.records.per.snapshot
> > > is in
> > > > > > > originals() -> records mode (deprecated path).
> > > > > > > 3. Else (neither explicitly set) -> bytes mode at the 20 mb
> > > default.
> > > > > > > 4. If both are in originals() -> bytes mode wins and a warn is
> > > logged.
> > > > > > >
> > > > > > > This resolution happens once at startup in
> ShareCoordinatorConfig,
> > > not
> > > > > > > per-record.
> > > > > > >
> > > > > > > sm03
> > > > > > >
> > > > > > > Cluster-roll backward compatibility
> > > > > > >
> > > > > > > During a rolling upgrade, yes brokers run mixed versions. Each
> > > broker's
> > > > > > > shard independently decides snapshot cadence, from its own
> local
> > > > > config.
> > > > > > A
> > > > > > > newer broker replaying records written by an older broker (or
> > > other way
> > > > > > > around) interprets them identically. So cadence may differ in
> the
> > > > > > mid-roll,
> > > > > > > but there is no on-disk format change and no coordination
> required
> > > I
> > > > > > see. A
> > > > > > > broker that hasn't yet had the new config set, will simply use
> the
> > > 20
> > > > > MB
> > > > > > > default. I have updated Compatibility section in the kip.
> > > > > > >
> > > > > > > sm04
> > > > > > >
> > > > > > > Currently the records-based didn't have any. A new metric
> > > > > > > bytes-since-snapshot (per __share_group_state partition) could
> be
> > > > > useful.
> > > > > > > Let me know if this is good, or any other suggestion?
> > > > > > >
> > > > > > > sm05
> > > > > > >
> > > > > > > The threshold is measured on uncompressed serialized record
> bytes.
> > > > > There
> > > > > > is
> > > > > > > a little note in the kip mentioning that "Compression is not
> > > accounted
> > > > > > for;
> > > > > > > the threshold is on uncompressed record bytes."
> > > > > > > Actually share.coordinator.state.topic.compression.codec
> defaults
> > > to
> > > > > > NONE.
> > > > > > > So by default on-disk bytes equal the measured bytes; So it
> will
> > > not
> > > > > > affect
> > > > > > > the 20mb limit, in default deploymnt.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Murali
> > > > > > >
> > > > > > > On Thu, Jul 9, 2026 at 7:25 PM Sushant Mahajan <
> > > sushmahajn@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi,
> > > > > > > > Went over the new byte based approach.
> > > > > > > >
> > > > > > > > Few questions:
> > > > > > > >
> > > > > > > > sm01 - currently share coord module does not depend on the
> group
> > > > > > > > coordinator. Could you detail out how the new group level
> config
> > > will
> > > > > > be
> > > > > > > > injected in the share coordinator?
> > > > > > > >
> > > > > > > > sm02 - since we have default defined as 20MB, how to
> > > differentiate
> > > > > > > between
> > > > > > > > older update count set vs byte limit set? The kip mentions
> if set
> > > > > byte
> > > > > > > > level will be honored. How to determine set vs default?
> > > > > > > >
> > > > > > > > sm03 - could you drop a line about backward compatibility in
> > > case of
> > > > > > > > cluster roll?
> > > > > > > >
> > > > > > > > sm04 - should we also add any new metrics?
> > > > > > > >
> > > > > > > > sm05 - do measurements need to take compression into account?
> > > Does it
> > > > > > > > affect the 20 MB limit?
> > > > > > > >
> > > > > > > > Regards,
> > > > > > > > Sushant Mahajan
> > > > > > > >
> > > > > > > > On Sun, 14 Jun 2026, 18:50 Muralidhar Basani via dev, <
> > > > > > > > dev@kafka.apache.org>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > > Apologies for the delayed reply.
> > > > > > > > >
> > > > > > > > > Sushant, thanks for the reply. Just to flag the KIP has
> > > switched to
> > > > > > > > > bytes-based control (pls see below), which supersedes most
> of
> > > the
> > > > > > > > > records-based tuning discussion.
> > > > > > > > >
> > > > > > > > > Chia, thank you for the suggestions. Both the points are
> now
> > > in the
> > > > > > > KIP:
> > > > > > > > > - chia_00: broker floor stays at 0 (no compatibility break)
> > > > > > > > > - chia_01: bytes-based control adopted, matching
> > > > > > > > > metadata.log.max.record.bytes.between.snapshots, default
> 20 MB.
> > > > > > > > >
> > > > > > > > > Records-based config will be marked as deprecated.
> > > > > > > > >
> > > > > > > > > Updated KIP:
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Bytes-based+configurable+snapshot+frequency+for+share+groups
> > > > > > > > >
> > > > > > > > > Please take a look again.
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Murali
> > > > > > > > >
> > > > > > > > > On Thu, Jun 11, 2026 at 5:20 AM Chia-Ping Tsai <
> > > > > chia7712@apache.org>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > hi
> > > > > > > > > >
> > > > > > > > > > chia_00: Changing the lower bound of the existing config
> is a
> > > > > > > > > > compatibility issue. If we want to avoid heavy
> snapshots, we
> > > can
> > > > > > set
> > > > > > > > the
> > > > > > > > > > lower bound of the new group-level configuration to 200
> by
> > > > > default.
> > > > > > > > Also,
> > > > > > > > > > the broker-level constraint should be changed to 0-1000,
> > > which
> > > > > > won’t
> > > > > > > > > bring
> > > > > > > > > > compatibility issues
> > > > > > > > > >
> > > > > > > > > > chia_01: Have you considered using bytes instead of
> records
> > > as
> > > > > the
> > > > > > > > > > controlled unit?
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On 2026/05/22 19:30:38 Muralidhar Basani via dev wrote:
> > > > > > > > > > > Hi all,
> > > > > > > > > > >
> > > > > > > > > > > I would like to start a discussion on KIP-1349, which
> > > allows
> > > > > > > > > configurable
> > > > > > > > > > > snapshot frequency of share groups.
> > > > > > > > > > >
> > > > > > > > > > > KIP :
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Configurable+snapshot+frequency+for+share+groups
> > > > > > > > > > >
> > > > > > > > > > > Thanks,
> > > > > > > > > > > Murali
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-1349 : Configurable snapshot frequency for share groups
Posted by Andrew Schofield <sc...@apache.org>.
Hi Murali,
Thanks for your response.
AS1: Thanks for the performance information. Adding this into the KIP is definitely helpful in the motivation.
AS2: Thanks. Sounds good to me.
AS3: HIGH and MEDIUM is fine with me, but my suggestion for LOW was because I think it will be rare for people to adjust the per-group config. It's a small point and happy for you to choose.
Thanks,
Andrew
On 2026/07/16 21:12:58 Muralidhar Basani via dev wrote:
> Thanks Sushant & Andrew.
>
> AS1
> On performance testing so far:
>
> I ran a test on a local 4.3 cluster to see how big the snapshot related
> records actually are.
>
> +++++++++++
> config_value,record_count,snapshot_bytes,update_bytes,snapshot_count,update_count,snapshot_byte_ratio,avg_update_bytes,avg_snapshot_bytes,updates_per_20mb,cliff_ratio_vs_N,broker_restart_load_secs
> 50,200000,1284,51561,8,337,0.024,153.0,160.5,137069,2741.4,3
> 100,200000,592,52206,4,339,0.011,154.0,148.0,136179,1361.8,3
> 200,200000,592,52360,4,340,0.011,154.0,148.0,136179,680.9,4
> 300,200000,592,52514,4,341,0.011,154.0,148.0,136179,453.9,4
> 500,200000,592,52206,4,339,0.011,154.0,148.0,136179,272.4,4
> +++++++++++
>
> (Note: update_count is low because acks were in-order. pls see the caveat
> below.)
>
> The main finding: each "update" record is tiny, about 154 bytes on average.
> This stayed the same no matter what I changed, because these records only
> store small (bookkeeping) data (which messages were delivered/acked), not
> the message contents.
>
> Currently the old setting takes a snapshot every N update records (max
> 500). The new setting takes a snapshot every 20 MB. Since each update is
> only ~154 bytes, 20 MB works out to about 136,000 updates between snapshots.
>
> So when we eventually remove the old config, a group that used to snapshot
> every 500 updates would instead snapshot every ~136,000, roughly ~270× less
> often at the old maximum of 500, and even more dramatic at lower settings
> (up to ~2700× at N=50). Fewer snapshots means recovery after a restart
> takes longer and more log is kept on disk before cleanup, because there are
> more update records to replay before reaching a snapshot. It does not
> affect throughput.
>
> In my test, the consumer acknowledged messages in order, so 200,000
> messages collapsed into only ~340 update records. That was enough to
> measure the size of an update, but it did not trigger the snapshot limits,
> so restart time stayed flat (~3–4s) in every run. To properly measure the
> recovery-time difference, I'll use a consumer that acknowledges out of
> order, which produces many more (and larger) update records and actually
> exercises the snapshot limits, then compare restart time and on-disk log
> size at the old vs. new settings.
>
> Agree, the new code does a little extra work per update (one more counter,
> plus measuring each record's size). I'll benchmark that against trunk
> during impl, before merge. I will add a performance investigation note to
> the KIP's Test Plan to capture this.
>
> AS2
>
> Good point. The name I proposed
> (share.coordinator.snapshot.update.bytes.per.snapshot) was derived from the
> existing config and swapped "records" for "bytes".
>
> But this bytes-based design is modeled on
> metadata.log.max.record.bytes.between.snapshots, so happy to rename.
> I can rename both configs:
>
> - broker: share.coordinator.snapshot.update.bytes.per.snapshot to
> share.coordinator.max.record.bytes.between.snapshots
> - per-group: share.snapshot.update.bytes.per.snapshot to
> share.max.record.bytes.between.snapshots
>
> AS3
>
> Broker-level HIGH: agree
> Group-level LOW: I see the reasoning, but every existing per-group config
> in GroupConfig currently is MEDIUM.
> (including the other share.* group configs like
> share.record.lock.duration.ms). So LOW would make this the only LOW group
> config. Slight lean toward MEDIUM for consistency, but no strong objection
> to LOW.
>
> Pls let me know your thoughts.
>
> Thanks,
> Murali
>
> On Thu, Jul 16, 2026 at 3:55 PM Andrew Schofield <sc...@apache.org>
> wrote:
>
> > Hi Murali,
> > Thanks for the KIP. I understand the proposal.
> >
> > AS1: What performance testing have you done to validate whether there is a
> > performance impact due to this change? When we get to the point of removing
> > the deprecated configuration, there is a potential behavioural cliff which
> > we should have investigated before we get there.
> >
> > AS2: Is there any reason why you did not follow a similar naming scheme as
> > the existing metadata log configs, such as
> > share.coordinator.max.record.bytes.between.snapshots?
> >
> > AS3: I suggest that the broker-level config would be HIGH and the
> > group-level config would be LOW importance.
> >
> > Thanks,
> > Andrew
> >
> > On 2026/07/16 05:13:29 Sushant Mahajan wrote:
> > > Thanks for the change.
> > >
> > > All comments from my side are addressed.
> > >
> > > Regards,
> > > Sushant Mahajan
> > >
> > > On Wed, 15 Jul 2026, 00:37 Muralidhar Basani via dev, <
> > dev@kafka.apache.org>
> > > wrote:
> > >
> > > > Thanks Sushanth.
> > > >
> > > > I updated KIP to remove the module dependency, create a helper
> > interface in
> > > > share-coordinator, and add a new metric.
> > > > Please take a look.
> > > >
> > > > Thanks,
> > > > Murali
> > > >
> > > > On Mon, Jul 13, 2026 at 5:39 PM Sushant Mahajan <su...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi,
> > > > > Thanks for the response.
> > > > >
> > > > > Regarding
> > > > >
> > > > > sm01 - I would prefer not a add group coordinator dependency on share
> > > > > coordinator. Thats why we created coordinator-common.
> > > > >
> > > > > Could we instead create a helper interface in share-coordinator to
> > access
> > > > > the relevant configs and pass the impl from BrokerServer (which has
> > > > access
> > > > > to relevant group config managers) to the share coordinator service?
> > > > >
> > > > > sm04 - yes, lets add that one.
> > > > >
> > > > > Rest of the answers are good.
> > > > >
> > > > > Regards,
> > > > > Sushant Mahajan
> > > > >
> > > > > On Fri, 10 Jul 2026, 22:55 Muralidhar Basani via dev, <
> > > > > dev@kafka.apache.org>
> > > > > wrote:
> > > > >
> > > > > > Thanks again Sushant for the nice review.
> > > > > >
> > > > > > sm01
> > > > > >
> > > > > > True. share-coordinator has no compile dependency on
> > group-coordinator.
> > > > > The
> > > > > > KIP adds implementation project(':group-coordinator') to the
> > > > > > share-coordinator build. There is no cyclic dep.
> > > > > >
> > > > > > ShareGroupConfigProvider gets a new accessor
> > > > > > snapshotUpdateBytesPerSnapshotOrDefault(groupId, brokerDefault)
> > > > > > ShareCoordinatorShard.Builder gets a new setter
> > > > > > withShareGroupConfigProvider(.....) and
> > ShareCoordinatorService.Builder
> > > > > > gets the same setter.
> > > > > > With these, BrokerServer can now construct the provider and pass
> > it in.
> > > > > The
> > > > > > shard can then communicate with the provider on both the write path
> > > > > > (generateShareStateRecord) and the replay path
> > (handleShareSnapshot),
> > > > > > falling back to the broker-level default (when the group has no
> > > > > override).
> > > > > >
> > > > > > sm02
> > > > > >
> > > > > > Good point. A default value is not distinguishable from an explicit
> > > > value
> > > > > > via getLong() alone.
> > > > > >
> > > > > > I have updated KIP Public interfaces -> deprecated config section.
> > > > > > 1. If share.coordinator.snapshot.update.bytes.per.snapshot is in
> > > > > > originals() -> bytes mode.
> > > > > > 2. Else if share.coordinator.snapshot.update.records.per.snapshot
> > is in
> > > > > > originals() -> records mode (deprecated path).
> > > > > > 3. Else (neither explicitly set) -> bytes mode at the 20 mb
> > default.
> > > > > > 4. If both are in originals() -> bytes mode wins and a warn is
> > logged.
> > > > > >
> > > > > > This resolution happens once at startup in ShareCoordinatorConfig,
> > not
> > > > > > per-record.
> > > > > >
> > > > > > sm03
> > > > > >
> > > > > > Cluster-roll backward compatibility
> > > > > >
> > > > > > During a rolling upgrade, yes brokers run mixed versions. Each
> > broker's
> > > > > > shard independently decides snapshot cadence, from its own local
> > > > config.
> > > > > A
> > > > > > newer broker replaying records written by an older broker (or
> > other way
> > > > > > around) interprets them identically. So cadence may differ in the
> > > > > mid-roll,
> > > > > > but there is no on-disk format change and no coordination required
> > I
> > > > > see. A
> > > > > > broker that hasn't yet had the new config set, will simply use the
> > 20
> > > > MB
> > > > > > default. I have updated Compatibility section in the kip.
> > > > > >
> > > > > > sm04
> > > > > >
> > > > > > Currently the records-based didn't have any. A new metric
> > > > > > bytes-since-snapshot (per __share_group_state partition) could be
> > > > useful.
> > > > > > Let me know if this is good, or any other suggestion?
> > > > > >
> > > > > > sm05
> > > > > >
> > > > > > The threshold is measured on uncompressed serialized record bytes.
> > > > There
> > > > > is
> > > > > > a little note in the kip mentioning that "Compression is not
> > accounted
> > > > > for;
> > > > > > the threshold is on uncompressed record bytes."
> > > > > > Actually share.coordinator.state.topic.compression.codec defaults
> > to
> > > > > NONE.
> > > > > > So by default on-disk bytes equal the measured bytes; So it will
> > not
> > > > > affect
> > > > > > the 20mb limit, in default deploymnt.
> > > > > >
> > > > > > Thanks,
> > > > > > Murali
> > > > > >
> > > > > > On Thu, Jul 9, 2026 at 7:25 PM Sushant Mahajan <
> > sushmahajn@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi,
> > > > > > > Went over the new byte based approach.
> > > > > > >
> > > > > > > Few questions:
> > > > > > >
> > > > > > > sm01 - currently share coord module does not depend on the group
> > > > > > > coordinator. Could you detail out how the new group level config
> > will
> > > > > be
> > > > > > > injected in the share coordinator?
> > > > > > >
> > > > > > > sm02 - since we have default defined as 20MB, how to
> > differentiate
> > > > > > between
> > > > > > > older update count set vs byte limit set? The kip mentions if set
> > > > byte
> > > > > > > level will be honored. How to determine set vs default?
> > > > > > >
> > > > > > > sm03 - could you drop a line about backward compatibility in
> > case of
> > > > > > > cluster roll?
> > > > > > >
> > > > > > > sm04 - should we also add any new metrics?
> > > > > > >
> > > > > > > sm05 - do measurements need to take compression into account?
> > Does it
> > > > > > > affect the 20 MB limit?
> > > > > > >
> > > > > > > Regards,
> > > > > > > Sushant Mahajan
> > > > > > >
> > > > > > > On Sun, 14 Jun 2026, 18:50 Muralidhar Basani via dev, <
> > > > > > > dev@kafka.apache.org>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi,
> > > > > > > > Apologies for the delayed reply.
> > > > > > > >
> > > > > > > > Sushant, thanks for the reply. Just to flag the KIP has
> > switched to
> > > > > > > > bytes-based control (pls see below), which supersedes most of
> > the
> > > > > > > > records-based tuning discussion.
> > > > > > > >
> > > > > > > > Chia, thank you for the suggestions. Both the points are now
> > in the
> > > > > > KIP:
> > > > > > > > - chia_00: broker floor stays at 0 (no compatibility break)
> > > > > > > > - chia_01: bytes-based control adopted, matching
> > > > > > > > metadata.log.max.record.bytes.between.snapshots, default 20 MB.
> > > > > > > >
> > > > > > > > Records-based config will be marked as deprecated.
> > > > > > > >
> > > > > > > > Updated KIP:
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Bytes-based+configurable+snapshot+frequency+for+share+groups
> > > > > > > >
> > > > > > > > Please take a look again.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Murali
> > > > > > > >
> > > > > > > > On Thu, Jun 11, 2026 at 5:20 AM Chia-Ping Tsai <
> > > > chia7712@apache.org>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > hi
> > > > > > > > >
> > > > > > > > > chia_00: Changing the lower bound of the existing config is a
> > > > > > > > > compatibility issue. If we want to avoid heavy snapshots, we
> > can
> > > > > set
> > > > > > > the
> > > > > > > > > lower bound of the new group-level configuration to 200 by
> > > > default.
> > > > > > > Also,
> > > > > > > > > the broker-level constraint should be changed to 0-1000,
> > which
> > > > > won’t
> > > > > > > > bring
> > > > > > > > > compatibility issues
> > > > > > > > >
> > > > > > > > > chia_01: Have you considered using bytes instead of records
> > as
> > > > the
> > > > > > > > > controlled unit?
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On 2026/05/22 19:30:38 Muralidhar Basani via dev wrote:
> > > > > > > > > > Hi all,
> > > > > > > > > >
> > > > > > > > > > I would like to start a discussion on KIP-1349, which
> > allows
> > > > > > > > configurable
> > > > > > > > > > snapshot frequency of share groups.
> > > > > > > > > >
> > > > > > > > > > KIP :
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Configurable+snapshot+frequency+for+share+groups
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > > Murali
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-1349 : Configurable snapshot frequency for share groups
Posted by Muralidhar Basani via dev <de...@kafka.apache.org>.
Thanks Sushant & Andrew.
AS1
On performance testing so far:
I ran a test on a local 4.3 cluster to see how big the snapshot related
records actually are.
+++++++++++
config_value,record_count,snapshot_bytes,update_bytes,snapshot_count,update_count,snapshot_byte_ratio,avg_update_bytes,avg_snapshot_bytes,updates_per_20mb,cliff_ratio_vs_N,broker_restart_load_secs
50,200000,1284,51561,8,337,0.024,153.0,160.5,137069,2741.4,3
100,200000,592,52206,4,339,0.011,154.0,148.0,136179,1361.8,3
200,200000,592,52360,4,340,0.011,154.0,148.0,136179,680.9,4
300,200000,592,52514,4,341,0.011,154.0,148.0,136179,453.9,4
500,200000,592,52206,4,339,0.011,154.0,148.0,136179,272.4,4
+++++++++++
(Note: update_count is low because acks were in-order. pls see the caveat
below.)
The main finding: each "update" record is tiny, about 154 bytes on average.
This stayed the same no matter what I changed, because these records only
store small (bookkeeping) data (which messages were delivered/acked), not
the message contents.
Currently the old setting takes a snapshot every N update records (max
500). The new setting takes a snapshot every 20 MB. Since each update is
only ~154 bytes, 20 MB works out to about 136,000 updates between snapshots.
So when we eventually remove the old config, a group that used to snapshot
every 500 updates would instead snapshot every ~136,000, roughly ~270× less
often at the old maximum of 500, and even more dramatic at lower settings
(up to ~2700× at N=50). Fewer snapshots means recovery after a restart
takes longer and more log is kept on disk before cleanup, because there are
more update records to replay before reaching a snapshot. It does not
affect throughput.
In my test, the consumer acknowledged messages in order, so 200,000
messages collapsed into only ~340 update records. That was enough to
measure the size of an update, but it did not trigger the snapshot limits,
so restart time stayed flat (~3–4s) in every run. To properly measure the
recovery-time difference, I'll use a consumer that acknowledges out of
order, which produces many more (and larger) update records and actually
exercises the snapshot limits, then compare restart time and on-disk log
size at the old vs. new settings.
Agree, the new code does a little extra work per update (one more counter,
plus measuring each record's size). I'll benchmark that against trunk
during impl, before merge. I will add a performance investigation note to
the KIP's Test Plan to capture this.
AS2
Good point. The name I proposed
(share.coordinator.snapshot.update.bytes.per.snapshot) was derived from the
existing config and swapped "records" for "bytes".
But this bytes-based design is modeled on
metadata.log.max.record.bytes.between.snapshots, so happy to rename.
I can rename both configs:
- broker: share.coordinator.snapshot.update.bytes.per.snapshot to
share.coordinator.max.record.bytes.between.snapshots
- per-group: share.snapshot.update.bytes.per.snapshot to
share.max.record.bytes.between.snapshots
AS3
Broker-level HIGH: agree
Group-level LOW: I see the reasoning, but every existing per-group config
in GroupConfig currently is MEDIUM.
(including the other share.* group configs like
share.record.lock.duration.ms). So LOW would make this the only LOW group
config. Slight lean toward MEDIUM for consistency, but no strong objection
to LOW.
Pls let me know your thoughts.
Thanks,
Murali
On Thu, Jul 16, 2026 at 3:55 PM Andrew Schofield <sc...@apache.org>
wrote:
> Hi Murali,
> Thanks for the KIP. I understand the proposal.
>
> AS1: What performance testing have you done to validate whether there is a
> performance impact due to this change? When we get to the point of removing
> the deprecated configuration, there is a potential behavioural cliff which
> we should have investigated before we get there.
>
> AS2: Is there any reason why you did not follow a similar naming scheme as
> the existing metadata log configs, such as
> share.coordinator.max.record.bytes.between.snapshots?
>
> AS3: I suggest that the broker-level config would be HIGH and the
> group-level config would be LOW importance.
>
> Thanks,
> Andrew
>
> On 2026/07/16 05:13:29 Sushant Mahajan wrote:
> > Thanks for the change.
> >
> > All comments from my side are addressed.
> >
> > Regards,
> > Sushant Mahajan
> >
> > On Wed, 15 Jul 2026, 00:37 Muralidhar Basani via dev, <
> dev@kafka.apache.org>
> > wrote:
> >
> > > Thanks Sushanth.
> > >
> > > I updated KIP to remove the module dependency, create a helper
> interface in
> > > share-coordinator, and add a new metric.
> > > Please take a look.
> > >
> > > Thanks,
> > > Murali
> > >
> > > On Mon, Jul 13, 2026 at 5:39 PM Sushant Mahajan <su...@gmail.com>
> > > wrote:
> > >
> > > > Hi,
> > > > Thanks for the response.
> > > >
> > > > Regarding
> > > >
> > > > sm01 - I would prefer not a add group coordinator dependency on share
> > > > coordinator. Thats why we created coordinator-common.
> > > >
> > > > Could we instead create a helper interface in share-coordinator to
> access
> > > > the relevant configs and pass the impl from BrokerServer (which has
> > > access
> > > > to relevant group config managers) to the share coordinator service?
> > > >
> > > > sm04 - yes, lets add that one.
> > > >
> > > > Rest of the answers are good.
> > > >
> > > > Regards,
> > > > Sushant Mahajan
> > > >
> > > > On Fri, 10 Jul 2026, 22:55 Muralidhar Basani via dev, <
> > > > dev@kafka.apache.org>
> > > > wrote:
> > > >
> > > > > Thanks again Sushant for the nice review.
> > > > >
> > > > > sm01
> > > > >
> > > > > True. share-coordinator has no compile dependency on
> group-coordinator.
> > > > The
> > > > > KIP adds implementation project(':group-coordinator') to the
> > > > > share-coordinator build. There is no cyclic dep.
> > > > >
> > > > > ShareGroupConfigProvider gets a new accessor
> > > > > snapshotUpdateBytesPerSnapshotOrDefault(groupId, brokerDefault)
> > > > > ShareCoordinatorShard.Builder gets a new setter
> > > > > withShareGroupConfigProvider(.....) and
> ShareCoordinatorService.Builder
> > > > > gets the same setter.
> > > > > With these, BrokerServer can now construct the provider and pass
> it in.
> > > > The
> > > > > shard can then communicate with the provider on both the write path
> > > > > (generateShareStateRecord) and the replay path
> (handleShareSnapshot),
> > > > > falling back to the broker-level default (when the group has no
> > > > override).
> > > > >
> > > > > sm02
> > > > >
> > > > > Good point. A default value is not distinguishable from an explicit
> > > value
> > > > > via getLong() alone.
> > > > >
> > > > > I have updated KIP Public interfaces -> deprecated config section.
> > > > > 1. If share.coordinator.snapshot.update.bytes.per.snapshot is in
> > > > > originals() -> bytes mode.
> > > > > 2. Else if share.coordinator.snapshot.update.records.per.snapshot
> is in
> > > > > originals() -> records mode (deprecated path).
> > > > > 3. Else (neither explicitly set) -> bytes mode at the 20 mb
> default.
> > > > > 4. If both are in originals() -> bytes mode wins and a warn is
> logged.
> > > > >
> > > > > This resolution happens once at startup in ShareCoordinatorConfig,
> not
> > > > > per-record.
> > > > >
> > > > > sm03
> > > > >
> > > > > Cluster-roll backward compatibility
> > > > >
> > > > > During a rolling upgrade, yes brokers run mixed versions. Each
> broker's
> > > > > shard independently decides snapshot cadence, from its own local
> > > config.
> > > > A
> > > > > newer broker replaying records written by an older broker (or
> other way
> > > > > around) interprets them identically. So cadence may differ in the
> > > > mid-roll,
> > > > > but there is no on-disk format change and no coordination required
> I
> > > > see. A
> > > > > broker that hasn't yet had the new config set, will simply use the
> 20
> > > MB
> > > > > default. I have updated Compatibility section in the kip.
> > > > >
> > > > > sm04
> > > > >
> > > > > Currently the records-based didn't have any. A new metric
> > > > > bytes-since-snapshot (per __share_group_state partition) could be
> > > useful.
> > > > > Let me know if this is good, or any other suggestion?
> > > > >
> > > > > sm05
> > > > >
> > > > > The threshold is measured on uncompressed serialized record bytes.
> > > There
> > > > is
> > > > > a little note in the kip mentioning that "Compression is not
> accounted
> > > > for;
> > > > > the threshold is on uncompressed record bytes."
> > > > > Actually share.coordinator.state.topic.compression.codec defaults
> to
> > > > NONE.
> > > > > So by default on-disk bytes equal the measured bytes; So it will
> not
> > > > affect
> > > > > the 20mb limit, in default deploymnt.
> > > > >
> > > > > Thanks,
> > > > > Murali
> > > > >
> > > > > On Thu, Jul 9, 2026 at 7:25 PM Sushant Mahajan <
> sushmahajn@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi,
> > > > > > Went over the new byte based approach.
> > > > > >
> > > > > > Few questions:
> > > > > >
> > > > > > sm01 - currently share coord module does not depend on the group
> > > > > > coordinator. Could you detail out how the new group level config
> will
> > > > be
> > > > > > injected in the share coordinator?
> > > > > >
> > > > > > sm02 - since we have default defined as 20MB, how to
> differentiate
> > > > > between
> > > > > > older update count set vs byte limit set? The kip mentions if set
> > > byte
> > > > > > level will be honored. How to determine set vs default?
> > > > > >
> > > > > > sm03 - could you drop a line about backward compatibility in
> case of
> > > > > > cluster roll?
> > > > > >
> > > > > > sm04 - should we also add any new metrics?
> > > > > >
> > > > > > sm05 - do measurements need to take compression into account?
> Does it
> > > > > > affect the 20 MB limit?
> > > > > >
> > > > > > Regards,
> > > > > > Sushant Mahajan
> > > > > >
> > > > > > On Sun, 14 Jun 2026, 18:50 Muralidhar Basani via dev, <
> > > > > > dev@kafka.apache.org>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi,
> > > > > > > Apologies for the delayed reply.
> > > > > > >
> > > > > > > Sushant, thanks for the reply. Just to flag the KIP has
> switched to
> > > > > > > bytes-based control (pls see below), which supersedes most of
> the
> > > > > > > records-based tuning discussion.
> > > > > > >
> > > > > > > Chia, thank you for the suggestions. Both the points are now
> in the
> > > > > KIP:
> > > > > > > - chia_00: broker floor stays at 0 (no compatibility break)
> > > > > > > - chia_01: bytes-based control adopted, matching
> > > > > > > metadata.log.max.record.bytes.between.snapshots, default 20 MB.
> > > > > > >
> > > > > > > Records-based config will be marked as deprecated.
> > > > > > >
> > > > > > > Updated KIP:
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Bytes-based+configurable+snapshot+frequency+for+share+groups
> > > > > > >
> > > > > > > Please take a look again.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Murali
> > > > > > >
> > > > > > > On Thu, Jun 11, 2026 at 5:20 AM Chia-Ping Tsai <
> > > chia7712@apache.org>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > hi
> > > > > > > >
> > > > > > > > chia_00: Changing the lower bound of the existing config is a
> > > > > > > > compatibility issue. If we want to avoid heavy snapshots, we
> can
> > > > set
> > > > > > the
> > > > > > > > lower bound of the new group-level configuration to 200 by
> > > default.
> > > > > > Also,
> > > > > > > > the broker-level constraint should be changed to 0-1000,
> which
> > > > won’t
> > > > > > > bring
> > > > > > > > compatibility issues
> > > > > > > >
> > > > > > > > chia_01: Have you considered using bytes instead of records
> as
> > > the
> > > > > > > > controlled unit?
> > > > > > > >
> > > > > > > >
> > > > > > > > On 2026/05/22 19:30:38 Muralidhar Basani via dev wrote:
> > > > > > > > > Hi all,
> > > > > > > > >
> > > > > > > > > I would like to start a discussion on KIP-1349, which
> allows
> > > > > > > configurable
> > > > > > > > > snapshot frequency of share groups.
> > > > > > > > >
> > > > > > > > > KIP :
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Configurable+snapshot+frequency+for+share+groups
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Murali
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-1349 : Configurable snapshot frequency for share groups
Posted by Andrew Schofield <sc...@apache.org>.
Hi Murali,
Thanks for the KIP. I understand the proposal.
AS1: What performance testing have you done to validate whether there is a performance impact due to this change? When we get to the point of removing the deprecated configuration, there is a potential behavioural cliff which we should have investigated before we get there.
AS2: Is there any reason why you did not follow a similar naming scheme as the existing metadata log configs, such as share.coordinator.max.record.bytes.between.snapshots?
AS3: I suggest that the broker-level config would be HIGH and the group-level config would be LOW importance.
Thanks,
Andrew
On 2026/07/16 05:13:29 Sushant Mahajan wrote:
> Thanks for the change.
>
> All comments from my side are addressed.
>
> Regards,
> Sushant Mahajan
>
> On Wed, 15 Jul 2026, 00:37 Muralidhar Basani via dev, <de...@kafka.apache.org>
> wrote:
>
> > Thanks Sushanth.
> >
> > I updated KIP to remove the module dependency, create a helper interface in
> > share-coordinator, and add a new metric.
> > Please take a look.
> >
> > Thanks,
> > Murali
> >
> > On Mon, Jul 13, 2026 at 5:39 PM Sushant Mahajan <su...@gmail.com>
> > wrote:
> >
> > > Hi,
> > > Thanks for the response.
> > >
> > > Regarding
> > >
> > > sm01 - I would prefer not a add group coordinator dependency on share
> > > coordinator. Thats why we created coordinator-common.
> > >
> > > Could we instead create a helper interface in share-coordinator to access
> > > the relevant configs and pass the impl from BrokerServer (which has
> > access
> > > to relevant group config managers) to the share coordinator service?
> > >
> > > sm04 - yes, lets add that one.
> > >
> > > Rest of the answers are good.
> > >
> > > Regards,
> > > Sushant Mahajan
> > >
> > > On Fri, 10 Jul 2026, 22:55 Muralidhar Basani via dev, <
> > > dev@kafka.apache.org>
> > > wrote:
> > >
> > > > Thanks again Sushant for the nice review.
> > > >
> > > > sm01
> > > >
> > > > True. share-coordinator has no compile dependency on group-coordinator.
> > > The
> > > > KIP adds implementation project(':group-coordinator') to the
> > > > share-coordinator build. There is no cyclic dep.
> > > >
> > > > ShareGroupConfigProvider gets a new accessor
> > > > snapshotUpdateBytesPerSnapshotOrDefault(groupId, brokerDefault)
> > > > ShareCoordinatorShard.Builder gets a new setter
> > > > withShareGroupConfigProvider(.....) and ShareCoordinatorService.Builder
> > > > gets the same setter.
> > > > With these, BrokerServer can now construct the provider and pass it in.
> > > The
> > > > shard can then communicate with the provider on both the write path
> > > > (generateShareStateRecord) and the replay path (handleShareSnapshot),
> > > > falling back to the broker-level default (when the group has no
> > > override).
> > > >
> > > > sm02
> > > >
> > > > Good point. A default value is not distinguishable from an explicit
> > value
> > > > via getLong() alone.
> > > >
> > > > I have updated KIP Public interfaces -> deprecated config section.
> > > > 1. If share.coordinator.snapshot.update.bytes.per.snapshot is in
> > > > originals() -> bytes mode.
> > > > 2. Else if share.coordinator.snapshot.update.records.per.snapshot is in
> > > > originals() -> records mode (deprecated path).
> > > > 3. Else (neither explicitly set) -> bytes mode at the 20 mb default.
> > > > 4. If both are in originals() -> bytes mode wins and a warn is logged.
> > > >
> > > > This resolution happens once at startup in ShareCoordinatorConfig, not
> > > > per-record.
> > > >
> > > > sm03
> > > >
> > > > Cluster-roll backward compatibility
> > > >
> > > > During a rolling upgrade, yes brokers run mixed versions. Each broker's
> > > > shard independently decides snapshot cadence, from its own local
> > config.
> > > A
> > > > newer broker replaying records written by an older broker (or other way
> > > > around) interprets them identically. So cadence may differ in the
> > > mid-roll,
> > > > but there is no on-disk format change and no coordination required I
> > > see. A
> > > > broker that hasn't yet had the new config set, will simply use the 20
> > MB
> > > > default. I have updated Compatibility section in the kip.
> > > >
> > > > sm04
> > > >
> > > > Currently the records-based didn't have any. A new metric
> > > > bytes-since-snapshot (per __share_group_state partition) could be
> > useful.
> > > > Let me know if this is good, or any other suggestion?
> > > >
> > > > sm05
> > > >
> > > > The threshold is measured on uncompressed serialized record bytes.
> > There
> > > is
> > > > a little note in the kip mentioning that "Compression is not accounted
> > > for;
> > > > the threshold is on uncompressed record bytes."
> > > > Actually share.coordinator.state.topic.compression.codec defaults to
> > > NONE.
> > > > So by default on-disk bytes equal the measured bytes; So it will not
> > > affect
> > > > the 20mb limit, in default deploymnt.
> > > >
> > > > Thanks,
> > > > Murali
> > > >
> > > > On Thu, Jul 9, 2026 at 7:25 PM Sushant Mahajan <su...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi,
> > > > > Went over the new byte based approach.
> > > > >
> > > > > Few questions:
> > > > >
> > > > > sm01 - currently share coord module does not depend on the group
> > > > > coordinator. Could you detail out how the new group level config will
> > > be
> > > > > injected in the share coordinator?
> > > > >
> > > > > sm02 - since we have default defined as 20MB, how to differentiate
> > > > between
> > > > > older update count set vs byte limit set? The kip mentions if set
> > byte
> > > > > level will be honored. How to determine set vs default?
> > > > >
> > > > > sm03 - could you drop a line about backward compatibility in case of
> > > > > cluster roll?
> > > > >
> > > > > sm04 - should we also add any new metrics?
> > > > >
> > > > > sm05 - do measurements need to take compression into account? Does it
> > > > > affect the 20 MB limit?
> > > > >
> > > > > Regards,
> > > > > Sushant Mahajan
> > > > >
> > > > > On Sun, 14 Jun 2026, 18:50 Muralidhar Basani via dev, <
> > > > > dev@kafka.apache.org>
> > > > > wrote:
> > > > >
> > > > > > Hi,
> > > > > > Apologies for the delayed reply.
> > > > > >
> > > > > > Sushant, thanks for the reply. Just to flag the KIP has switched to
> > > > > > bytes-based control (pls see below), which supersedes most of the
> > > > > > records-based tuning discussion.
> > > > > >
> > > > > > Chia, thank you for the suggestions. Both the points are now in the
> > > > KIP:
> > > > > > - chia_00: broker floor stays at 0 (no compatibility break)
> > > > > > - chia_01: bytes-based control adopted, matching
> > > > > > metadata.log.max.record.bytes.between.snapshots, default 20 MB.
> > > > > >
> > > > > > Records-based config will be marked as deprecated.
> > > > > >
> > > > > > Updated KIP:
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Bytes-based+configurable+snapshot+frequency+for+share+groups
> > > > > >
> > > > > > Please take a look again.
> > > > > >
> > > > > > Thanks,
> > > > > > Murali
> > > > > >
> > > > > > On Thu, Jun 11, 2026 at 5:20 AM Chia-Ping Tsai <
> > chia7712@apache.org>
> > > > > > wrote:
> > > > > >
> > > > > > > hi
> > > > > > >
> > > > > > > chia_00: Changing the lower bound of the existing config is a
> > > > > > > compatibility issue. If we want to avoid heavy snapshots, we can
> > > set
> > > > > the
> > > > > > > lower bound of the new group-level configuration to 200 by
> > default.
> > > > > Also,
> > > > > > > the broker-level constraint should be changed to 0-1000, which
> > > won’t
> > > > > > bring
> > > > > > > compatibility issues
> > > > > > >
> > > > > > > chia_01: Have you considered using bytes instead of records as
> > the
> > > > > > > controlled unit?
> > > > > > >
> > > > > > >
> > > > > > > On 2026/05/22 19:30:38 Muralidhar Basani via dev wrote:
> > > > > > > > Hi all,
> > > > > > > >
> > > > > > > > I would like to start a discussion on KIP-1349, which allows
> > > > > > configurable
> > > > > > > > snapshot frequency of share groups.
> > > > > > > >
> > > > > > > > KIP :
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Configurable+snapshot+frequency+for+share+groups
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Murali
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-1349 : Configurable snapshot frequency for share groups
Posted by Sushant Mahajan <su...@gmail.com>.
Thanks for the change.
All comments from my side are addressed.
Regards,
Sushant Mahajan
On Wed, 15 Jul 2026, 00:37 Muralidhar Basani via dev, <de...@kafka.apache.org>
wrote:
> Thanks Sushanth.
>
> I updated KIP to remove the module dependency, create a helper interface in
> share-coordinator, and add a new metric.
> Please take a look.
>
> Thanks,
> Murali
>
> On Mon, Jul 13, 2026 at 5:39 PM Sushant Mahajan <su...@gmail.com>
> wrote:
>
> > Hi,
> > Thanks for the response.
> >
> > Regarding
> >
> > sm01 - I would prefer not a add group coordinator dependency on share
> > coordinator. Thats why we created coordinator-common.
> >
> > Could we instead create a helper interface in share-coordinator to access
> > the relevant configs and pass the impl from BrokerServer (which has
> access
> > to relevant group config managers) to the share coordinator service?
> >
> > sm04 - yes, lets add that one.
> >
> > Rest of the answers are good.
> >
> > Regards,
> > Sushant Mahajan
> >
> > On Fri, 10 Jul 2026, 22:55 Muralidhar Basani via dev, <
> > dev@kafka.apache.org>
> > wrote:
> >
> > > Thanks again Sushant for the nice review.
> > >
> > > sm01
> > >
> > > True. share-coordinator has no compile dependency on group-coordinator.
> > The
> > > KIP adds implementation project(':group-coordinator') to the
> > > share-coordinator build. There is no cyclic dep.
> > >
> > > ShareGroupConfigProvider gets a new accessor
> > > snapshotUpdateBytesPerSnapshotOrDefault(groupId, brokerDefault)
> > > ShareCoordinatorShard.Builder gets a new setter
> > > withShareGroupConfigProvider(.....) and ShareCoordinatorService.Builder
> > > gets the same setter.
> > > With these, BrokerServer can now construct the provider and pass it in.
> > The
> > > shard can then communicate with the provider on both the write path
> > > (generateShareStateRecord) and the replay path (handleShareSnapshot),
> > > falling back to the broker-level default (when the group has no
> > override).
> > >
> > > sm02
> > >
> > > Good point. A default value is not distinguishable from an explicit
> value
> > > via getLong() alone.
> > >
> > > I have updated KIP Public interfaces -> deprecated config section.
> > > 1. If share.coordinator.snapshot.update.bytes.per.snapshot is in
> > > originals() -> bytes mode.
> > > 2. Else if share.coordinator.snapshot.update.records.per.snapshot is in
> > > originals() -> records mode (deprecated path).
> > > 3. Else (neither explicitly set) -> bytes mode at the 20 mb default.
> > > 4. If both are in originals() -> bytes mode wins and a warn is logged.
> > >
> > > This resolution happens once at startup in ShareCoordinatorConfig, not
> > > per-record.
> > >
> > > sm03
> > >
> > > Cluster-roll backward compatibility
> > >
> > > During a rolling upgrade, yes brokers run mixed versions. Each broker's
> > > shard independently decides snapshot cadence, from its own local
> config.
> > A
> > > newer broker replaying records written by an older broker (or other way
> > > around) interprets them identically. So cadence may differ in the
> > mid-roll,
> > > but there is no on-disk format change and no coordination required I
> > see. A
> > > broker that hasn't yet had the new config set, will simply use the 20
> MB
> > > default. I have updated Compatibility section in the kip.
> > >
> > > sm04
> > >
> > > Currently the records-based didn't have any. A new metric
> > > bytes-since-snapshot (per __share_group_state partition) could be
> useful.
> > > Let me know if this is good, or any other suggestion?
> > >
> > > sm05
> > >
> > > The threshold is measured on uncompressed serialized record bytes.
> There
> > is
> > > a little note in the kip mentioning that "Compression is not accounted
> > for;
> > > the threshold is on uncompressed record bytes."
> > > Actually share.coordinator.state.topic.compression.codec defaults to
> > NONE.
> > > So by default on-disk bytes equal the measured bytes; So it will not
> > affect
> > > the 20mb limit, in default deploymnt.
> > >
> > > Thanks,
> > > Murali
> > >
> > > On Thu, Jul 9, 2026 at 7:25 PM Sushant Mahajan <su...@gmail.com>
> > > wrote:
> > >
> > > > Hi,
> > > > Went over the new byte based approach.
> > > >
> > > > Few questions:
> > > >
> > > > sm01 - currently share coord module does not depend on the group
> > > > coordinator. Could you detail out how the new group level config will
> > be
> > > > injected in the share coordinator?
> > > >
> > > > sm02 - since we have default defined as 20MB, how to differentiate
> > > between
> > > > older update count set vs byte limit set? The kip mentions if set
> byte
> > > > level will be honored. How to determine set vs default?
> > > >
> > > > sm03 - could you drop a line about backward compatibility in case of
> > > > cluster roll?
> > > >
> > > > sm04 - should we also add any new metrics?
> > > >
> > > > sm05 - do measurements need to take compression into account? Does it
> > > > affect the 20 MB limit?
> > > >
> > > > Regards,
> > > > Sushant Mahajan
> > > >
> > > > On Sun, 14 Jun 2026, 18:50 Muralidhar Basani via dev, <
> > > > dev@kafka.apache.org>
> > > > wrote:
> > > >
> > > > > Hi,
> > > > > Apologies for the delayed reply.
> > > > >
> > > > > Sushant, thanks for the reply. Just to flag the KIP has switched to
> > > > > bytes-based control (pls see below), which supersedes most of the
> > > > > records-based tuning discussion.
> > > > >
> > > > > Chia, thank you for the suggestions. Both the points are now in the
> > > KIP:
> > > > > - chia_00: broker floor stays at 0 (no compatibility break)
> > > > > - chia_01: bytes-based control adopted, matching
> > > > > metadata.log.max.record.bytes.between.snapshots, default 20 MB.
> > > > >
> > > > > Records-based config will be marked as deprecated.
> > > > >
> > > > > Updated KIP:
> > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Bytes-based+configurable+snapshot+frequency+for+share+groups
> > > > >
> > > > > Please take a look again.
> > > > >
> > > > > Thanks,
> > > > > Murali
> > > > >
> > > > > On Thu, Jun 11, 2026 at 5:20 AM Chia-Ping Tsai <
> chia7712@apache.org>
> > > > > wrote:
> > > > >
> > > > > > hi
> > > > > >
> > > > > > chia_00: Changing the lower bound of the existing config is a
> > > > > > compatibility issue. If we want to avoid heavy snapshots, we can
> > set
> > > > the
> > > > > > lower bound of the new group-level configuration to 200 by
> default.
> > > > Also,
> > > > > > the broker-level constraint should be changed to 0-1000, which
> > won’t
> > > > > bring
> > > > > > compatibility issues
> > > > > >
> > > > > > chia_01: Have you considered using bytes instead of records as
> the
> > > > > > controlled unit?
> > > > > >
> > > > > >
> > > > > > On 2026/05/22 19:30:38 Muralidhar Basani via dev wrote:
> > > > > > > Hi all,
> > > > > > >
> > > > > > > I would like to start a discussion on KIP-1349, which allows
> > > > > configurable
> > > > > > > snapshot frequency of share groups.
> > > > > > >
> > > > > > > KIP :
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Configurable+snapshot+frequency+for+share+groups
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Murali
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-1349 : Configurable snapshot frequency for share groups
Posted by Muralidhar Basani via dev <de...@kafka.apache.org>.
Thanks Sushanth.
I updated KIP to remove the module dependency, create a helper interface in
share-coordinator, and add a new metric.
Please take a look.
Thanks,
Murali
On Mon, Jul 13, 2026 at 5:39 PM Sushant Mahajan <su...@gmail.com>
wrote:
> Hi,
> Thanks for the response.
>
> Regarding
>
> sm01 - I would prefer not a add group coordinator dependency on share
> coordinator. Thats why we created coordinator-common.
>
> Could we instead create a helper interface in share-coordinator to access
> the relevant configs and pass the impl from BrokerServer (which has access
> to relevant group config managers) to the share coordinator service?
>
> sm04 - yes, lets add that one.
>
> Rest of the answers are good.
>
> Regards,
> Sushant Mahajan
>
> On Fri, 10 Jul 2026, 22:55 Muralidhar Basani via dev, <
> dev@kafka.apache.org>
> wrote:
>
> > Thanks again Sushant for the nice review.
> >
> > sm01
> >
> > True. share-coordinator has no compile dependency on group-coordinator.
> The
> > KIP adds implementation project(':group-coordinator') to the
> > share-coordinator build. There is no cyclic dep.
> >
> > ShareGroupConfigProvider gets a new accessor
> > snapshotUpdateBytesPerSnapshotOrDefault(groupId, brokerDefault)
> > ShareCoordinatorShard.Builder gets a new setter
> > withShareGroupConfigProvider(.....) and ShareCoordinatorService.Builder
> > gets the same setter.
> > With these, BrokerServer can now construct the provider and pass it in.
> The
> > shard can then communicate with the provider on both the write path
> > (generateShareStateRecord) and the replay path (handleShareSnapshot),
> > falling back to the broker-level default (when the group has no
> override).
> >
> > sm02
> >
> > Good point. A default value is not distinguishable from an explicit value
> > via getLong() alone.
> >
> > I have updated KIP Public interfaces -> deprecated config section.
> > 1. If share.coordinator.snapshot.update.bytes.per.snapshot is in
> > originals() -> bytes mode.
> > 2. Else if share.coordinator.snapshot.update.records.per.snapshot is in
> > originals() -> records mode (deprecated path).
> > 3. Else (neither explicitly set) -> bytes mode at the 20 mb default.
> > 4. If both are in originals() -> bytes mode wins and a warn is logged.
> >
> > This resolution happens once at startup in ShareCoordinatorConfig, not
> > per-record.
> >
> > sm03
> >
> > Cluster-roll backward compatibility
> >
> > During a rolling upgrade, yes brokers run mixed versions. Each broker's
> > shard independently decides snapshot cadence, from its own local config.
> A
> > newer broker replaying records written by an older broker (or other way
> > around) interprets them identically. So cadence may differ in the
> mid-roll,
> > but there is no on-disk format change and no coordination required I
> see. A
> > broker that hasn't yet had the new config set, will simply use the 20 MB
> > default. I have updated Compatibility section in the kip.
> >
> > sm04
> >
> > Currently the records-based didn't have any. A new metric
> > bytes-since-snapshot (per __share_group_state partition) could be useful.
> > Let me know if this is good, or any other suggestion?
> >
> > sm05
> >
> > The threshold is measured on uncompressed serialized record bytes. There
> is
> > a little note in the kip mentioning that "Compression is not accounted
> for;
> > the threshold is on uncompressed record bytes."
> > Actually share.coordinator.state.topic.compression.codec defaults to
> NONE.
> > So by default on-disk bytes equal the measured bytes; So it will not
> affect
> > the 20mb limit, in default deploymnt.
> >
> > Thanks,
> > Murali
> >
> > On Thu, Jul 9, 2026 at 7:25 PM Sushant Mahajan <su...@gmail.com>
> > wrote:
> >
> > > Hi,
> > > Went over the new byte based approach.
> > >
> > > Few questions:
> > >
> > > sm01 - currently share coord module does not depend on the group
> > > coordinator. Could you detail out how the new group level config will
> be
> > > injected in the share coordinator?
> > >
> > > sm02 - since we have default defined as 20MB, how to differentiate
> > between
> > > older update count set vs byte limit set? The kip mentions if set byte
> > > level will be honored. How to determine set vs default?
> > >
> > > sm03 - could you drop a line about backward compatibility in case of
> > > cluster roll?
> > >
> > > sm04 - should we also add any new metrics?
> > >
> > > sm05 - do measurements need to take compression into account? Does it
> > > affect the 20 MB limit?
> > >
> > > Regards,
> > > Sushant Mahajan
> > >
> > > On Sun, 14 Jun 2026, 18:50 Muralidhar Basani via dev, <
> > > dev@kafka.apache.org>
> > > wrote:
> > >
> > > > Hi,
> > > > Apologies for the delayed reply.
> > > >
> > > > Sushant, thanks for the reply. Just to flag the KIP has switched to
> > > > bytes-based control (pls see below), which supersedes most of the
> > > > records-based tuning discussion.
> > > >
> > > > Chia, thank you for the suggestions. Both the points are now in the
> > KIP:
> > > > - chia_00: broker floor stays at 0 (no compatibility break)
> > > > - chia_01: bytes-based control adopted, matching
> > > > metadata.log.max.record.bytes.between.snapshots, default 20 MB.
> > > >
> > > > Records-based config will be marked as deprecated.
> > > >
> > > > Updated KIP:
> > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Bytes-based+configurable+snapshot+frequency+for+share+groups
> > > >
> > > > Please take a look again.
> > > >
> > > > Thanks,
> > > > Murali
> > > >
> > > > On Thu, Jun 11, 2026 at 5:20 AM Chia-Ping Tsai <ch...@apache.org>
> > > > wrote:
> > > >
> > > > > hi
> > > > >
> > > > > chia_00: Changing the lower bound of the existing config is a
> > > > > compatibility issue. If we want to avoid heavy snapshots, we can
> set
> > > the
> > > > > lower bound of the new group-level configuration to 200 by default.
> > > Also,
> > > > > the broker-level constraint should be changed to 0-1000, which
> won’t
> > > > bring
> > > > > compatibility issues
> > > > >
> > > > > chia_01: Have you considered using bytes instead of records as the
> > > > > controlled unit?
> > > > >
> > > > >
> > > > > On 2026/05/22 19:30:38 Muralidhar Basani via dev wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > I would like to start a discussion on KIP-1349, which allows
> > > > configurable
> > > > > > snapshot frequency of share groups.
> > > > > >
> > > > > > KIP :
> > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Configurable+snapshot+frequency+for+share+groups
> > > > > >
> > > > > > Thanks,
> > > > > > Murali
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-1349 : Configurable snapshot frequency for share groups
Posted by Sushant Mahajan <su...@gmail.com>.
Hi,
Thanks for the response.
Regarding
sm01 - I would prefer not a add group coordinator dependency on share
coordinator. Thats why we created coordinator-common.
Could we instead create a helper interface in share-coordinator to access
the relevant configs and pass the impl from BrokerServer (which has access
to relevant group config managers) to the share coordinator service?
sm04 - yes, lets add that one.
Rest of the answers are good.
Regards,
Sushant Mahajan
On Fri, 10 Jul 2026, 22:55 Muralidhar Basani via dev, <de...@kafka.apache.org>
wrote:
> Thanks again Sushant for the nice review.
>
> sm01
>
> True. share-coordinator has no compile dependency on group-coordinator. The
> KIP adds implementation project(':group-coordinator') to the
> share-coordinator build. There is no cyclic dep.
>
> ShareGroupConfigProvider gets a new accessor
> snapshotUpdateBytesPerSnapshotOrDefault(groupId, brokerDefault)
> ShareCoordinatorShard.Builder gets a new setter
> withShareGroupConfigProvider(.....) and ShareCoordinatorService.Builder
> gets the same setter.
> With these, BrokerServer can now construct the provider and pass it in. The
> shard can then communicate with the provider on both the write path
> (generateShareStateRecord) and the replay path (handleShareSnapshot),
> falling back to the broker-level default (when the group has no override).
>
> sm02
>
> Good point. A default value is not distinguishable from an explicit value
> via getLong() alone.
>
> I have updated KIP Public interfaces -> deprecated config section.
> 1. If share.coordinator.snapshot.update.bytes.per.snapshot is in
> originals() -> bytes mode.
> 2. Else if share.coordinator.snapshot.update.records.per.snapshot is in
> originals() -> records mode (deprecated path).
> 3. Else (neither explicitly set) -> bytes mode at the 20 mb default.
> 4. If both are in originals() -> bytes mode wins and a warn is logged.
>
> This resolution happens once at startup in ShareCoordinatorConfig, not
> per-record.
>
> sm03
>
> Cluster-roll backward compatibility
>
> During a rolling upgrade, yes brokers run mixed versions. Each broker's
> shard independently decides snapshot cadence, from its own local config. A
> newer broker replaying records written by an older broker (or other way
> around) interprets them identically. So cadence may differ in the mid-roll,
> but there is no on-disk format change and no coordination required I see. A
> broker that hasn't yet had the new config set, will simply use the 20 MB
> default. I have updated Compatibility section in the kip.
>
> sm04
>
> Currently the records-based didn't have any. A new metric
> bytes-since-snapshot (per __share_group_state partition) could be useful.
> Let me know if this is good, or any other suggestion?
>
> sm05
>
> The threshold is measured on uncompressed serialized record bytes. There is
> a little note in the kip mentioning that "Compression is not accounted for;
> the threshold is on uncompressed record bytes."
> Actually share.coordinator.state.topic.compression.codec defaults to NONE.
> So by default on-disk bytes equal the measured bytes; So it will not affect
> the 20mb limit, in default deploymnt.
>
> Thanks,
> Murali
>
> On Thu, Jul 9, 2026 at 7:25 PM Sushant Mahajan <su...@gmail.com>
> wrote:
>
> > Hi,
> > Went over the new byte based approach.
> >
> > Few questions:
> >
> > sm01 - currently share coord module does not depend on the group
> > coordinator. Could you detail out how the new group level config will be
> > injected in the share coordinator?
> >
> > sm02 - since we have default defined as 20MB, how to differentiate
> between
> > older update count set vs byte limit set? The kip mentions if set byte
> > level will be honored. How to determine set vs default?
> >
> > sm03 - could you drop a line about backward compatibility in case of
> > cluster roll?
> >
> > sm04 - should we also add any new metrics?
> >
> > sm05 - do measurements need to take compression into account? Does it
> > affect the 20 MB limit?
> >
> > Regards,
> > Sushant Mahajan
> >
> > On Sun, 14 Jun 2026, 18:50 Muralidhar Basani via dev, <
> > dev@kafka.apache.org>
> > wrote:
> >
> > > Hi,
> > > Apologies for the delayed reply.
> > >
> > > Sushant, thanks for the reply. Just to flag the KIP has switched to
> > > bytes-based control (pls see below), which supersedes most of the
> > > records-based tuning discussion.
> > >
> > > Chia, thank you for the suggestions. Both the points are now in the
> KIP:
> > > - chia_00: broker floor stays at 0 (no compatibility break)
> > > - chia_01: bytes-based control adopted, matching
> > > metadata.log.max.record.bytes.between.snapshots, default 20 MB.
> > >
> > > Records-based config will be marked as deprecated.
> > >
> > > Updated KIP:
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Bytes-based+configurable+snapshot+frequency+for+share+groups
> > >
> > > Please take a look again.
> > >
> > > Thanks,
> > > Murali
> > >
> > > On Thu, Jun 11, 2026 at 5:20 AM Chia-Ping Tsai <ch...@apache.org>
> > > wrote:
> > >
> > > > hi
> > > >
> > > > chia_00: Changing the lower bound of the existing config is a
> > > > compatibility issue. If we want to avoid heavy snapshots, we can set
> > the
> > > > lower bound of the new group-level configuration to 200 by default.
> > Also,
> > > > the broker-level constraint should be changed to 0-1000, which won’t
> > > bring
> > > > compatibility issues
> > > >
> > > > chia_01: Have you considered using bytes instead of records as the
> > > > controlled unit?
> > > >
> > > >
> > > > On 2026/05/22 19:30:38 Muralidhar Basani via dev wrote:
> > > > > Hi all,
> > > > >
> > > > > I would like to start a discussion on KIP-1349, which allows
> > > configurable
> > > > > snapshot frequency of share groups.
> > > > >
> > > > > KIP :
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Configurable+snapshot+frequency+for+share+groups
> > > > >
> > > > > Thanks,
> > > > > Murali
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-1349 : Configurable snapshot frequency for share groups
Posted by Muralidhar Basani via dev <de...@kafka.apache.org>.
Thanks again Sushant for the nice review.
sm01
True. share-coordinator has no compile dependency on group-coordinator. The
KIP adds implementation project(':group-coordinator') to the
share-coordinator build. There is no cyclic dep.
ShareGroupConfigProvider gets a new accessor
snapshotUpdateBytesPerSnapshotOrDefault(groupId, brokerDefault)
ShareCoordinatorShard.Builder gets a new setter
withShareGroupConfigProvider(.....) and ShareCoordinatorService.Builder
gets the same setter.
With these, BrokerServer can now construct the provider and pass it in. The
shard can then communicate with the provider on both the write path
(generateShareStateRecord) and the replay path (handleShareSnapshot),
falling back to the broker-level default (when the group has no override).
sm02
Good point. A default value is not distinguishable from an explicit value
via getLong() alone.
I have updated KIP Public interfaces -> deprecated config section.
1. If share.coordinator.snapshot.update.bytes.per.snapshot is in
originals() -> bytes mode.
2. Else if share.coordinator.snapshot.update.records.per.snapshot is in
originals() -> records mode (deprecated path).
3. Else (neither explicitly set) -> bytes mode at the 20 mb default.
4. If both are in originals() -> bytes mode wins and a warn is logged.
This resolution happens once at startup in ShareCoordinatorConfig, not
per-record.
sm03
Cluster-roll backward compatibility
During a rolling upgrade, yes brokers run mixed versions. Each broker's
shard independently decides snapshot cadence, from its own local config. A
newer broker replaying records written by an older broker (or other way
around) interprets them identically. So cadence may differ in the mid-roll,
but there is no on-disk format change and no coordination required I see. A
broker that hasn't yet had the new config set, will simply use the 20 MB
default. I have updated Compatibility section in the kip.
sm04
Currently the records-based didn't have any. A new metric
bytes-since-snapshot (per __share_group_state partition) could be useful.
Let me know if this is good, or any other suggestion?
sm05
The threshold is measured on uncompressed serialized record bytes. There is
a little note in the kip mentioning that "Compression is not accounted for;
the threshold is on uncompressed record bytes."
Actually share.coordinator.state.topic.compression.codec defaults to NONE.
So by default on-disk bytes equal the measured bytes; So it will not affect
the 20mb limit, in default deploymnt.
Thanks,
Murali
On Thu, Jul 9, 2026 at 7:25 PM Sushant Mahajan <su...@gmail.com> wrote:
> Hi,
> Went over the new byte based approach.
>
> Few questions:
>
> sm01 - currently share coord module does not depend on the group
> coordinator. Could you detail out how the new group level config will be
> injected in the share coordinator?
>
> sm02 - since we have default defined as 20MB, how to differentiate between
> older update count set vs byte limit set? The kip mentions if set byte
> level will be honored. How to determine set vs default?
>
> sm03 - could you drop a line about backward compatibility in case of
> cluster roll?
>
> sm04 - should we also add any new metrics?
>
> sm05 - do measurements need to take compression into account? Does it
> affect the 20 MB limit?
>
> Regards,
> Sushant Mahajan
>
> On Sun, 14 Jun 2026, 18:50 Muralidhar Basani via dev, <
> dev@kafka.apache.org>
> wrote:
>
> > Hi,
> > Apologies for the delayed reply.
> >
> > Sushant, thanks for the reply. Just to flag the KIP has switched to
> > bytes-based control (pls see below), which supersedes most of the
> > records-based tuning discussion.
> >
> > Chia, thank you for the suggestions. Both the points are now in the KIP:
> > - chia_00: broker floor stays at 0 (no compatibility break)
> > - chia_01: bytes-based control adopted, matching
> > metadata.log.max.record.bytes.between.snapshots, default 20 MB.
> >
> > Records-based config will be marked as deprecated.
> >
> > Updated KIP:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Bytes-based+configurable+snapshot+frequency+for+share+groups
> >
> > Please take a look again.
> >
> > Thanks,
> > Murali
> >
> > On Thu, Jun 11, 2026 at 5:20 AM Chia-Ping Tsai <ch...@apache.org>
> > wrote:
> >
> > > hi
> > >
> > > chia_00: Changing the lower bound of the existing config is a
> > > compatibility issue. If we want to avoid heavy snapshots, we can set
> the
> > > lower bound of the new group-level configuration to 200 by default.
> Also,
> > > the broker-level constraint should be changed to 0-1000, which won’t
> > bring
> > > compatibility issues
> > >
> > > chia_01: Have you considered using bytes instead of records as the
> > > controlled unit?
> > >
> > >
> > > On 2026/05/22 19:30:38 Muralidhar Basani via dev wrote:
> > > > Hi all,
> > > >
> > > > I would like to start a discussion on KIP-1349, which allows
> > configurable
> > > > snapshot frequency of share groups.
> > > >
> > > > KIP :
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1349%3A+Configurable+snapshot+frequency+for+share+groups
> > > >
> > > > Thanks,
> > > > Murali
> > > >
> > >
> >
>