Re: [DISCUSS] KIP-950: Tiered Storage Disablement

2024-04-09 Thread Doğuşcan Namal
rate thread. But not sure about KRaft mode.
> > >
> > > Thanks.
> > > Luke
> > >
> > >
> > > On Fri, Mar 29, 2024 at 4:14 PM Christo Lolov 
> > > wrote:
> > >
> > > > Heya everyone!
> > > >
> > > > re: Doguscan
> > > >
> > > > I believe the answer to 101 needs a bit more discussion. As far as I
> > > know,
> > > > tiered storage today has methods to update a metadata of a segment to
> > say
> > > > "hey, I would like this deleted", but actual deletion is left to
> plugin
> > > > implementations (or any background cleaners). In other words, there
> is
> > no
> > > > "immediate" deletion. In this KIP, we would like to continue doing
> the
> > > same
> > > > if the retention policy is set to delete. So I believe the answer is
> > > > actually that a) we will update the metadata of the segments to mark
> > them
> > > > as deleted and b) we will advance the log start offset. Any deletion
> of
> > > > actual files will still be delegated to plugin implementations. I
> > believe
> > > > this is further supported by "*remote.log.disable.policy=delete:*
> Logs
> > > that
> > > > are archived in the remote storage will not be part of the contiguous
> > > > "active" log and will be deleted asynchronously as part of the
> > > disablement
> > > > process"
> > > >
> > > > Following from the above, I believe for 102 it is fine to allow
> setting
> > > of
> > > > remote.log.disable.policy on a disabled topic in much the same way we
> > > allow
> > > > other remote-related configurations to be set on a topic (i.e.
> > > > local.retention.*) - it just won't have an effect. Granted, I do
> > believe
> > > we
> > > > should restrict the policy being changed while a disablement is
> > ongoing.
> > > >
> > > > re: Satish and Kamal
> > > >
> > > > 104, 1 and 2 are fair asks, I will work with Doguscan to update the
> KIP
> > > > with the information!
> > > >
> > > > Best,
> > > > Christo
> > > >
> > > > On Thu, 28 Mar 2024 at 10:31, Doğuşcan Namal <
> namal.dogus...@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > Hi Satish, I will try to answer as much as I can and the others
> could
> > > > chime
> > > > > in with further details.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > *101. For remote.log.disable.policy=delete: Does it delete the
> remote
> > > log
> > > > > data immediately and the data in remote storage will not be taken
> > into
> > > > > account by any replica? That means log-start-offset is moved to the
> > > > earlier
> > > > > local-log-start-offset.*
> > > > > *Exactly. RemoteLogData will be deleted immediately. *
> > > > > *So before the deletion starts we move LogStart offset to
> > > > > LocalLogStartOffset to ensure that no RemoteLog will be accessed
> > after
> > > > that
> > > > > point.*
> > > > >
> > > > >
> > > > > * 102. Can we update the remote.log.disable.policy after tiered
> > storage
> > > > is
> > > > > disabled on a topic?*
> > > > >
> > > > > *This is a good point. I think we should not allow modifying this
> > > > > configuration*
> > > > > *because changing the policy from Deletion to Retain when there is
> an
> > > > > ongoing Deletion will result in an undefined behaviour and where we
> > > > retain
> > > > > half of the remote log and delete the other half.*
> > > > >
> > > > > * 103. Do we plan to add any metrics related to this feature?*
> > > > >
> > > > >
> > > > >
> > > > > *Any recommendations?*
> > > > > *We may emit a gauge showing the enablement state of a topic but we
> > > could
> > > > > gather that info from the logs as well.*
> > > > > *The total duration for remote topic deletion could be added as
> well
> > > but
> > > > > this is more of a metric for the RemotePartitionRemover itself.

Re: [DISCUSS] KIP-950: Tiered Storage Disablement

2024-03-28 Thread Doğuşcan Namal
Hi Satish, I will try to answer as much as I can and the others could chime
in with further details.





*101. For remote.log.disable.policy=delete: Does it delete the remote log
data immediately and the data in remote storage will not be taken into
account by any replica? That means log-start-offset is moved to the earlier
local-log-start-offset.*
*Exactly. RemoteLogData will be deleted immediately. *
*So before the deletion starts we move LogStart offset to
LocalLogStartOffset to ensure that no RemoteLog will be accessed after that
point.*


* 102. Can we update the remote.log.disable.policy after tiered storage is
disabled on a topic?*

*This is a good point. I think we should not allow modifying this
configuration*
*because changing the policy from Deletion to Retain when there is an
ongoing Deletion will result in an undefined behaviour and where we retain
half of the remote log and delete the other half.*

* 103. Do we plan to add any metrics related to this feature?*



*Any recommendations?*
*We may emit a gauge showing the enablement state of a topic but we could
gather that info from the logs as well.*
*The total duration for remote topic deletion could be added as well but
this is more of a metric for the RemotePartitionRemover itself.*



*104. Please add configuration details about copier thread pool, expiration
thread pool and the migration of the existing
RemoteLogManagerScheduledThreadPool.*

*Will add the details.*

105. How is the behaviour with topic or partition deletion request
handled when tiered storage disablement request is still being
processed on a topic?

*If the disablement policy is Delete then a successive topic deletion
request is going to be a NOOP because RemoteLogs is already being deleted.*
*If the disablement policy is Retain, then we only moved the LogStartOffset
and didn't touch RemoteLogs anyway, so the delete topic request will result*

*in the initiation of RemoteLog deletion.*


On Tue, 26 Mar 2024 at 18:21, Kamal Chandraprakash <
kamal.chandraprak...@gmail.com> wrote:

> Hi,
>
> Thanks for the KIP! Overall the KIP looks good and covered most of the
> items.
>
> 1. Could you explain how the brokers will handle the DisableRemoteTopic API
> request?
>
> 2. Who will initiate the controller interaction sequence? Does the
> controller listens for
> topic config updates and initiate the disablement?
>
> --
> Kamal
>
> On Tue, Mar 26, 2024 at 4:40 PM Satish Duggana 
> wrote:
>
> > Thanks Mehari, Divij, Christo etal for the KIP.
> >
> > I had an initial review of the KIP and left the below comments.
> >
> > 101. For remote.log.disable.policy=delete:
> > Does it delete the remote log data immediately and the data in remote
> > storage will not be taken into account by any replica? That means
> > log-start-offset is moved to the earlier local-log-start-offset.
> >
> > 102. Can we update the remote.log.disable.policy after tiered storage
> > is disabled on a topic?
> >
> > 103. Do we plan to add any metrics related to this feature?
> >
> > 104. Please add configuration details about copier thread pool,
> > expiration thread pool and the migration of the existing
> > RemoteLogManagerScheduledThreadPool.
> >
> > 105. How is the behaviour with topic or partition deletion request
> > handled when tiered storage disablement request is still being
> > processed on a topic?
> >
> > ~Satish.
> >
> > On Mon, 25 Mar 2024 at 13:34, Doğuşcan Namal 
> > wrote:
> > >
> > > Hi Christo and Luke,
> > >
> > > I think the KRaft section of the KIP requires slight improvement. The
> > metadata propagation in KRaft is handled by the RAFT layer instead of
> > sending Controller -> Broker RPCs. In fact, KIP-631 deprecated these
> RPCs.
> > >
> > > I will come up with some recommendations on how we could improve that
> > one but until then, @Luke please feel free to review the KIP.
> > >
> > > @Satish, if we want this to make it to Kafka 3.8 I believe we need to
> > aim to get the KIP approved in the following weeks otherwise it will slip
> > and we can not support it in Zookeeper mode.
> > >
> > > I also would like to better understand what is the community's stand
> for
> > adding a new feature for Zookeeper since it is marked as deprecated
> already.
> > >
> > > Thanks.
> > >
> > >
> > >
> > > On Mon, 18 Mar 2024 at 13:42, Christo Lolov 
> > wrote:
> > >>
> > >> Heya,
> > >>
> > >> I do have some time to put into this, but to be honest I am still
> after
> > >> reviews of the KIP itself :)
> > &g

Re: [VOTE] KIP-1025: Optionally URL-encode clientID and clientSecret in authorization header

2024-03-27 Thread Doğuşcan Namal
Thanks for checking it out Nelson. Yeah I think it makes sense to leave it
for the users who want to use it for testing.

On Mon, 25 Mar 2024 at 20:44, Nelson B.  wrote:

> Hi Doğuşcan,
>
> Thanks for your vote!
>
> Currently, the usage of TLS depends on the protocol used by the
> authorization server which is configured
> through the "sasl.oauthbearer.token.endpoint.url" option. So, if the
> URL address uses simple http (not https)
> then secrets will be transmitted in plaintext. I think it's possible to
> enforce using only https but I think any
> production-grade authorization server uses https anyway and maybe users may
> want to test using http in the dev environment.
>
> Thanks,
>
> On Thu, Mar 21, 2024 at 3:56 PM Doğuşcan Namal 
> wrote:
>
> > Hi Nelson, thanks for the KIP.
> >
> > From the RFC:
> > ```
> > The authorization server MUST require the use of TLS as described in
> >Section 1.6 when sending requests using password authentication.
> > ```
> >
> > I believe we already have an enforcement for OAuth to be enabled only in
> > SSLChannel but would be good to double check. Sending secrets over
> > plaintext is a security bad practice :)
> >
> > +1 (non-binding) from me.
> >
> > On Tue, 19 Mar 2024 at 16:00, Nelson B.  wrote:
> >
> > > Hi all,
> > >
> > > I would like to start a vote on KIP-1025
> > > <
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1025%3A+Optionally+URL-encode+clientID+and+clientSecret+in+authorization+header
> > > >,
> > > which would optionally URL-encode clientID and clientSecret in the
> > > authorization header.
> > >
> > > I feel like all possible issues have been addressed in the discussion
> > > thread.
> > >
> > > Thanks,
> > >
> >
>


Re: [DISCUSS] KIP-950: Tiered Storage Disablement

2024-03-25 Thread Doğuşcan Namal
Hi Christo and Luke,

I think the KRaft section of the KIP requires slight improvement. The
metadata propagation in KRaft is handled by the RAFT layer instead of
sending Controller -> Broker RPCs. In fact, KIP-631 deprecated these RPCs
<https://cwiki.apache.org/confluence/display/KAFKA/KIP-631%3A+The+Quorum-based+Kafka+Controller#KIP631:TheQuorumbasedKafkaController-ObsoletingtheMetadataPropagationRPCs>
.

I will come up with some recommendations on how we could improve that one
but until then, @Luke please feel free to review the KIP.

@Satish, if we want this to make it to Kafka 3.8 I believe we need to aim
to get the KIP approved in the following weeks otherwise it will slip and
we can not support it in Zookeeper mode.

I also would like to better understand what is the community's stand for
adding a new feature for Zookeeper since it is marked as deprecated already.

Thanks.



On Mon, 18 Mar 2024 at 13:42, Christo Lolov  wrote:

> Heya,
>
> I do have some time to put into this, but to be honest I am still after
> reviews of the KIP itself :)
>
> After the latest changes it ought to be detailing both a Zookeeper approach
> and a KRaft approach.
>
> Do you have any thoughts on how it could be improved or should I start a
> voting thread?
>
> Best,
> Christo
>
> On Thu, 14 Mar 2024 at 06:12, Luke Chen  wrote:
>
> > Hi Christo,
> >
> > Any update with this KIP?
> > If you don't have time to complete it, I can collaborate with you to work
> > on it.
> >
> > Thanks.
> > Luke
> >
> > On Wed, Jan 17, 2024 at 11:38 PM Satish Duggana <
> satish.dugg...@gmail.com>
> > wrote:
> >
> > > Hi Christo,
> > > Thanks for volunteering to contribute to the KIP discussion. I suggest
> > > considering this KIP for both ZK and KRaft as it will be helpful for
> > > this feature to be available in 3.8.0 running with ZK clusters.
> > >
> > > Thanks,
> > > Satish.
> > >
> > > On Wed, 17 Jan 2024 at 19:04, Christo Lolov 
> > > wrote:
> > > >
> > > > Hello!
> > > >
> > > > I volunteer to get this KIP moving forward and implemented in Apache
> > > Kafka
> > > > 3.8.
> > > >
> > > > I have caught up with Mehari offline and we have agreed that given
> > Apache
> > > > Kafka 4.0 being around the corner we would like to propose this
> feature
> > > > only for KRaft clusters.
> > > >
> > > > Any and all reviews and comments are welcome!
> > > >
> > > > Best,
> > > > Christo
> > > >
> > > > On Tue, 9 Jan 2024 at 09:44, Doğuşcan Namal <
> namal.dogus...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi everyone, any progress on the status of this KIP? Overall looks
> > > good to
> > > > > me but I wonder whether we still need to support it for Zookeeper
> > mode
> > > > > given that it will be deprecated in the next 3 months.
> > > > >
> > > > > On 2023/07/21 20:16:46 "Beyene, Mehari" wrote:
> > > > > > Hi everyone,
> > > > > > I would like to start a discussion on KIP-950: Tiered Storage
> > > Disablement
> > > > > (
> > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-950%3A++Tiered+Storage+Disablement
> > > > > ).
> > > > > >
> > > > > > This KIP proposes adding the ability to disable and re-enable
> > tiered
> > > > > storage on a topic.
> > > > > >
> > > > > > Thanks,
> > > > > > Mehari
> > > > > >
> > > > >
> > >
> >
>


Re: [VOTE] KIP-1025: Optionally URL-encode clientID and clientSecret in authorization header

2024-03-21 Thread Doğuşcan Namal
Hi Nelson, thanks for the KIP.

>From the RFC:
```
The authorization server MUST require the use of TLS as described in
   Section 1.6 when sending requests using password authentication.
```

I believe we already have an enforcement for OAuth to be enabled only in
SSLChannel but would be good to double check. Sending secrets over
plaintext is a security bad practice :)

+1 (non-binding) from me.

On Tue, 19 Mar 2024 at 16:00, Nelson B.  wrote:

> Hi all,
>
> I would like to start a vote on KIP-1025
> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1025%3A+Optionally+URL-encode+clientID+and+clientSecret+in+authorization+header
> >,
> which would optionally URL-encode clientID and clientSecret in the
> authorization header.
>
> I feel like all possible issues have been addressed in the discussion
> thread.
>
> Thanks,
>


Re: [DISCUSS] Minimum constraint for segment.ms

2024-03-19 Thread Doğuşcan Namal
Hi all,

There are also message.max.bytes, replica.fetch.max.bytes and their
derivatives requires a constraint on their maximum value as the maximum
total memory on the instance. Otherwise, these could cause out of memory
errors on the instance.

Do you think this is in scope here as well?

On Thu, 14 Mar 2024 at 10:29, Haruki Okada  wrote:

> Hi, Divij.
>
> This isn't about config default value/constraint change though, I found
> there's a behavior discrepancy in max.block.ms config, which may cause
> breaking change if we change the behavior.
> The detail is described in the ticket:
> https://issues.apache.org/jira/browse/KAFKA-16372
>
> What do you think?
>
> 2024年3月14日(木) 13:09 Kamal Chandraprakash :
>
> > One use case I see for setting the `segment.bytes` to 1 is to delete all
> > the records from the topic.
> > We can mention about it in the doc to use the `kafka-delete-records` API
> > instead.
> >
> >
> >
> >
> > On Wed, Mar 13, 2024 at 6:59 PM Divij Vaidya 
> > wrote:
> >
> > > + users@kafka
> > >
> > > Hi users of Apache Kafka
> > >
> > > With the upcoming 4.0 release, we have an opportunity to improve the
> > > constraints and default values for various Kafka configurations.
> > >
> > > We are soliciting your feedback and suggestions on configurations where
> > the
> > > default values and/or constraints should be adjusted. Please reply in
> > this
> > > thread directly.
> > >
> > > --
> > > Divij Vaidya
> > > Apache Kafka PMC
> > >
> > >
> > >
> > > On Wed, Mar 13, 2024 at 12:56 PM Divij Vaidya  >
> > > wrote:
> > >
> > > > Thanks for the discussion folks. I have started a KIP
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1030%3A+Change+constraints+and+default+values+for+various+configurations
> > > > to keep track of the changes that we are discussion. Please consider
> > this
> > > > as a collaborative work-in-progress KIP and once it is ready to be
> > > > published, we can start a discussion thread on it.
> > > >
> > > > I am also going to start a thread to solicit feedback from users@
> > > mailing
> > > > list as well.
> > > >
> > > > --
> > > > Divij Vaidya
> > > >
> > > >
> > > >
> > > > On Wed, Mar 13, 2024 at 12:55 PM Christopher Shannon <
> > > > christopher.l.shan...@gmail.com> wrote:
> > > >
> > > >> I think it's a great idea to raise a KIP to look at adjusting
> defaults
> > > and
> > > >> minimum/maximum config values for version 4.0.
> > > >>
> > > >> As pointed out, the minimum values for segment.ms and segment.bytes
> > > don't
> > > >> make sense and would probably bring down a cluster pretty quickly if
> > set
> > > >> that low, so version 4.0 is a good time to fix it and to also look
> at
> > > the
> > > >> other configs as well for adjustments.
> > > >>
> > > >> On Wed, Mar 13, 2024 at 4:39 AM Sergio Daniel Troiano
> > > >>  wrote:
> > > >>
> > > >> > hey guys,
> > > >> >
> > > >> > Regarding to num.recovery.threads.per.data.dir: I agree, in our
> > > company
> > > >> we
> > > >> > use the number of vCPUs to do so as this is not competing with
> ready
> > > >> > cluster traffic.
> > > >> >
> > > >> >
> > > >> > On Wed, 13 Mar 2024 at 09:29, Luke Chen 
> wrote:
> > > >> >
> > > >> > > Hi Divij,
> > > >> > >
> > > >> > > Thanks for raising this.
> > > >> > > The valid minimum value 1 for `segment.ms` is completely
> > > >> unreasonable.
> > > >> > > Similarly for `segment.bytes`, `metadata.log.segment.ms`,
> > > >> > > `metadata.log.segment.bytes`.
> > > >> > >
> > > >> > > In addition to that, there are also some config default values
> > we'd
> > > >> like
> > > >> > to
> > > >> > > propose to change in v4.0.
> > > >> > > We can collect more comments from the community, and come out
> > with a
> > > >> KIP
> > > >> > > for them.
> > > >> > >
> > > >> > > 1. num.recovery.threads.per.data.dir:
> > > >> > > The current default value is 1. But the log recovery is
> happening
> > > >> before
> > > >> > > brokers are in ready state, which means, we should use all the
> > > >> available
> > > >> > > resource to speed up the log recovery to bring the broker to
> ready
> > > >> state
> > > >> > > soon. Default value should be... maybe 4 (to be decided)?
> > > >> > >
> > > >> > > 2. Other configs might be able to consider to change the
> default,
> > > but
> > > >> > open
> > > >> > > for comments:
> > > >> > >2.1. num.replica.fetchers: default is 1, but that's not
> enough
> > > when
> > > >> > > there are multiple partitions in the cluster
> > > >> > >2.2.
> `socket.send.buffer.bytes`/`socket.receive.buffer.bytes`:
> > > >> > > Currently, we set 100kb as default value, but that's not enough
> > for
> > > >> > > high-speed network.
> > > >> > >
> > > >> > > Thank you.
> > > >> > > Luke
> > > >> > >
> > > >> > >
> > > >> > > On Tue, Mar 12, 2024 at 1:32 AM Divij Vaidya <
> > > divijvaidy...@gmail.com
> > > >> >
> > > >> > > wrote:
> > > >> > >
> > > >> > > > Hey folks
> > > >> > > >
> > > >> > > > Before I file a KIP to change this in 4.0, I 

Re: [Discuss] KIP-1019: Expose method to determine Metric Measurability

2024-02-15 Thread Doğuşcan Namal
LGTM thanks for the KIP.

+1(non-binding)

On Wed, 14 Feb 2024 at 15:22, Andrew Schofield <
andrew_schofield_j...@outlook.com> wrote:

> Hi Apoorv,
> Thanks for the KIP. Looks like a useful change to tidy up the metrics code.
>
> Thanks,
> Andrew
>
> > On 14 Feb 2024, at 14:55, Apoorv Mittal 
> wrote:
> >
> > Hi,
> > I would like to start discussion of a small KIP which fills a gap in
> > determining Kafka Metric measurability.
> >
> > KIP-1019: Expose method to determine Metric Measurability
> > <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1019%3A+Expose+method+to+determine+Metric+Measurability
> >
> >
> > Regards,
> > Apoorv Mittal
>
>


Re: ZK vs KRaft benchmarking - latency differences?

2024-02-02 Thread Doğuşcan Namal
Hey Michael, thanks for your comments. I think the first of the
improvements you mentioned, the faster controller failover is a known
improvement to me. But the second one you suggest is a faster consumer
group failover, could you open that up a bit for me why do you think it
will be better on KRaft?

As you mentioned these are improvements on the recovery times, so from your
mail I understand you wouldn't expect an improvement on latencies as well.

On Thu, 1 Feb 2024 at 22:53, Michael K. Edwards 
wrote:

> The interesting numbers are the recovery times after 1) the Kafka broker
> currently acting as the "active" controller (or the sole controller in a
> ZooKeeper-based deployment) goes away; 2) the Kafka broker currently acting
> as the consumer group coordinator for a consumer group with many partitions
> and a high commit rate goes away.  Here "goes away" means as ugly a loss
> mode as can realistically be simulated in your test environment; I suggest
> forcing the to-be-impaired broker into heavy paging by running it inside a
> cgroups container and progressively shrinking the memory cgroup.  It's also
> fun to force high packet loss using iptables.
>
> If you're serious about testing KRaft's survivability under load, then I
> suggest you compare against a ZooKeeper deployment that's relatively
> non-broken.  That means setting up a ZooKeeper observer
> https://zookeeper.apache.org/doc/current/zookeeperObservers.html local to
> each broker.  Personally I'd want to test with a large number of partitions
> (840 or 2520 per topic, tens of thousands overall), especially in the
> coordinator-failure scenario.  I haven't been following the horizontal
> scaling work closely, but I suspect that still means porting forward the
> Dropwizard-based metrics patch I wrote years ago.  If I were doing that,
> I'd bring the shared dependencies of zookeeper and kafka up to current and
> do a custom zookeeper build off of the 3.9.x branch (compare
>
> https://github.com/mkedwards/zookeeper/commit/e608be61a3851c128088d9c9c54871f56aa05012
> and consider backporting
>
> https://github.com/apache/zookeeper/commit/5894dc88cce1f4675809fb347cc60d3e0ebf08d4
> ).
> Then I'd do https://github.com/mkedwards/kafka/tree/bitpusher-2.3 all over
> again, starting from the kafka 3.6.x branch and synchronizing the shared
> dependencies.
>
> If you'd like to outsource that work, I'm available on a consulting basis
> :D  Seriously, ZooKeeper itself has in my opinion never been the problem,
> at least since it got revived after the sad 3.14.1x / 3.5.x-alpha days.
> Inadequately resourced and improperly deployed ZooKeeper clusters have been
> a problem, as has the use of JMX to do the job of a modern metrics
> library.  The KRaft ship has sailed as far as upstream development is
> concerned; but if you're not committed to that in your production
> environment, there are other ways to scale up and out while retaining
> ZooKeeper as your reliable configuration/metadata store.  (It's also
> cost-effective and latency-feasible to run a cross-AZ ZooKeeper cluster,
> which I would not attempt with Kafka brokers in any kind of large-scale
> production setting.)
>
> Cheers,
> - Michael
>
> On Thu, Feb 1, 2024 at 7:02 AM Doğuşcan Namal 
> wrote:
>
> > Hi Paul,
> >
> > I did some benchmarking as well and couldn't find a marginal difference
> > between KRaft and Zookeeper on end to end latency from producers to
> > consumers. I tested it on Kafka version 3.5.1 and used openmessaging's
> > benchmarking framework https://openmessaging.cloud/docs/benchmarks/ .
> >
> > What I noticed was if you run the tests long enough(60 mins) the
> throughput
> > converges to the same value eventually. I also noticed some difference on
> > p99+ latencies between Zookeeper and KRaft clusters but the results were
> > not consistent on repetitive runs.
> >
> > Which version did you make the tests on and what are your findings?
> >
> > On Wed, 31 Jan 2024 at 22:57, Brebner, Paul  > .invalid>
> > wrote:
> >
> > > Hi all,
> > >
> > > We’ve previously done some benchmarking of Kafka ZooKeeper vs KRaft and
> > > found no difference in throughput (which we believed is also what
> theory
> > > predicted, as ZK/Kraft are only involved in Kafka meta-data operations,
> > not
> > > data workloads).
> > >
> > > BUT – latest tests reveal improved producer and consumer latency for
> > Kraft
> > > c.f. ZooKeeper.  So just wanted to check if Kraft is actually involved
> in
> > > any aspect of write/read workloads? For example, some documentation
> > > (possibly old) suggests that consumer offsets are stored in meta-data?
> > In
> > > which case this could explain better Kraft latencies. But if not, then
> > I’m
> > > curious to understand the difference (and if it’s documented anywhere?)
> > >
> > > Also if anyone has noticed the same regarding latency in benchmarks.
> > >
> > > Regards, Paul Brebner
> > >
> >
>


Re: ZK vs KRaft benchmarking - latency differences?

2024-02-01 Thread Doğuşcan Namal
Hi Paul,

I did some benchmarking as well and couldn't find a marginal difference
between KRaft and Zookeeper on end to end latency from producers to
consumers. I tested it on Kafka version 3.5.1 and used openmessaging's
benchmarking framework https://openmessaging.cloud/docs/benchmarks/ .

What I noticed was if you run the tests long enough(60 mins) the throughput
converges to the same value eventually. I also noticed some difference on
p99+ latencies between Zookeeper and KRaft clusters but the results were
not consistent on repetitive runs.

Which version did you make the tests on and what are your findings?

On Wed, 31 Jan 2024 at 22:57, Brebner, Paul 
wrote:

> Hi all,
>
> We’ve previously done some benchmarking of Kafka ZooKeeper vs KRaft and
> found no difference in throughput (which we believed is also what theory
> predicted, as ZK/Kraft are only involved in Kafka meta-data operations, not
> data workloads).
>
> BUT – latest tests reveal improved producer and consumer latency for Kraft
> c.f. ZooKeeper.  So just wanted to check if Kraft is actually involved in
> any aspect of write/read workloads? For example, some documentation
> (possibly old) suggests that consumer offsets are stored in meta-data?  In
> which case this could explain better Kraft latencies. But if not, then I’m
> curious to understand the difference (and if it’s documented anywhere?)
>
> Also if anyone has noticed the same regarding latency in benchmarks.
>
> Regards, Paul Brebner
>


RE: [DISCUSS] KIP-950: Tiered Storage Disablement

2024-01-09 Thread Doğuşcan Namal
Hi everyone, any progress on the status of this KIP? Overall looks good to
me but I wonder whether we still need to support it for Zookeeper mode
given that it will be deprecated in the next 3 months.

On 2023/07/21 20:16:46 "Beyene, Mehari" wrote:
> Hi everyone,
> I would like to start a discussion on KIP-950: Tiered Storage Disablement
(
https://cwiki.apache.org/confluence/display/KAFKA/KIP-950%3A++Tiered+Storage+Disablement
).
>
> This KIP proposes adding the ability to disable and re-enable tiered
storage on a topic.
>
> Thanks,
> Mehari
>


Re: [VOTE] KIP-1000: List Client Metrics Configuration Resources

2023-11-16 Thread Doğuşcan Namal
Thanks for the brief KIP Andrew. Having discussed the details in KIP-714, I
see this is a natural follow up to that.

+1(non-binding)

On Wed, 15 Nov 2023 at 15:23, Andrew Schofield <
andrew_schofield_j...@outlook.com> wrote:

> Hi,
> I’d like to start the voting for KIP-1000: List Client Metrics
> Configuration Resources.
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1000%3A+List+Client+Metrics+Configuration+Resources
>
> Thanks,
> Andrew


Re: [VOTE] KIP-982: Enhance Custom KafkaPrincipalBuilder to Access SslPrincipalMapper and KerberosShortNamer

2023-10-20 Thread Doğuşcan Namal
Hi Raghu,

Thanks for the short KIP.

+1(non-binding)

On Thu, 19 Oct 2023 at 23:56, Raghu B  wrote:

> Hi everyone,
>
> I would like to start a vote on KIP-982, which proposed enhancements to
> the Custom KafkaPrincipalBuilder to allow access to SslPrincipalMapper and
> KerberosShortNamer.
>
> This KIP
> 
> aims to improve the flexibility and usability of custom
> KafkaPrincipalBuilder implementations by enabling support for Mapping Rules
> and enhancing the overall security configuration of Kafka brokers.
>
> Thank you for your participation!
>
> Sincerely,
> Raghu
>


Re: Re: Re: [DISCUSS] KIP-972: Add the metric of the current running version of kafka

2023-10-11 Thread Doğuşcan Namal
Hello, do we have a metric showing the uptime? We could tag that metric
with version information as well.

I like the idea of adding the version as a tag as well. However, I am not
inclined to tag each metric with a KafkaVersion information. We could
discuss which metrics could be tagged but let's keep that out of scope from
this discussion.

On Wed, 11 Oct 2023 at 07:37, Sophie Blee-Goldman 
wrote:

> Just to chime in here since I recently went through a similar thing, I
> support adding the version
> as a tag instead of introducing an entirely new metric for this. In fact I
> just implemented exactly this
> in a project that uses Kafka, for these reasons:
>
> 1. Adding the version as a tag means that all metrics which are already
> collected will benefit, and lets you easily tell
> at a glance which version a specific client metric corresponds to. This is
> incredibly useful when looking at a dashboard
> covering multiple instances from different sources. For example, imagine a
> graph that plots the performance (eg bytes
> consumed rate) of many individual consumers and which shows several of them
> maxing out much lower than the rest.
> If the metric is tagged with the version already, you can easily check if
> the slow consumers are all using a specific version
> and may be displaying a performance regression. If the version info has to
> be plotted separately as its own metric, this is
> much more of a hassle to check.
> 2. Additional metrics can be expensive, but additional tags are almost
> always free (at least, that is my understanding)
> 3. As you guys already discussed, many systems (like Prometheus) require
> numeric values, and it's pretty much impossible
> to come up with a readable scheme for all the relevant versioning info --
> even if we removed the dots we're left with a rather
> unreadable representation of the version and of course will need to solve
> the "-SNAPSHOT" issue somehow. But beyond that,
> in addition to the raw version we also wanted to emit the specific commit
> id, which really needs to be a string.
>
> I'm pretty sure Kafka client metrics also include the commit id in addition
> to the version. If we add the version to the tags,
> we should consider adding the commit id as well. This is incredibly useful
> for intermediate/SNAPSHOT versions, which
> don't uniquely identify the specific code that is running.
>
> I would personally love to see a KIP start tagging the existing metrics
> with the version info, and it sounds like this would also
> solve your problem in a very natural way
>
> On Tue, Oct 10, 2023 at 5:42 AM Mickael Maison 
> wrote:
>
> > Hi Hudeqi,
> >
> > Rather than creating a gauge with a dummy value, could we add the
> > version (and commitId) as tags to an existing metric.
> > For example, the alongside the existing Version and CommitId metrics
> > we have StartTimeMs. Maybe we can have a StartTimeMs metrics with the
> > version and commitId) as tags on it? The existing metric already has
> > the brokerid (id) as tag. WDYT?
> >
> > Thanks,
> > Mickael
> >
> > On Thu, Aug 31, 2023 at 4:59 AM hudeqi <16120...@bjtu.edu.cn> wrote:
> > >
> > > Thank you for your answer, Mickael.
> > > If set the value of gauge to a constant value of 1, adding that tag key
> > is "version" and value is the version value of the obtained string type,
> > does this solve the problem? We can get the version by tag in prometheus.
> > >
> > > best,
> > > hudeqi
> > >
> > > Mickael Maison mickael.mai...@gmail.com写道:
> > > > Hi,
> > > >
> > > > Prometheus only support numeric values for metrics. This means it's
> > > > not able to handle the kafka.server:type=app-info metric since Kafka
> > > > versions are not valid numbers (3.5.0).
> > > > As a workaround we could create a metric with the version without the
> > > > dots, for example with value 350 for Kafka 3.5.0.
> > > >
> > > > Also in between releases Kafka uses the -SNAPSHOT suffix (for example
> > > > trunk is currently 3.7.0-SNAPSHOT) so we should also consider a way
> to
> > > > handle those.
> > > >
> > > > Thanks,
> > > > Mickael
> > > >
> > > > On Wed, Aug 30, 2023 at 2:51 PM hudeqi <16120...@bjtu.edu.cn> wrote:
> > > > >
> > > > > Hi, Kamal, thanks your reminding, but I have a question: It seems
> > that I can't get this metric through "jmx_prometheus"? Although I
> observed
> > this metric through other tools.
> > > > >
> > > > > best,
> > > > > hudeqi
> > > > >
> > > > > Kamal Chandraprakash &
> lt;kamal.chandraprak...@gmail.com
> > 写道:
> > > > > > Hi Hudeqi,
> > > > > >
> > > > > > Kafka already emits the version metric. Can you check whether the
> > below
> > > > > > metric satisfies your requirement?
> > > > > >
> > > > > > kafka.server:type=app-info,id=0
> > > > > >
> > > > > > --
> > > > > > Kamal
> > > > > >
> > > > > > On Mon, Aug 28, 2023 at 2:29 PM hudeqi <16120...@bjtu.edu.cn>
> > wrote:
> > > > > >
> > > > > > > Hi, all, I want to submit a minor kip to add a metric, which
> > supports to
> > > > > > > get 

KRaft Performance Improvements

2023-10-03 Thread Doğuşcan Namal
Do we have any performance test results showing the difference between
KRaft vs Zookeeper?

The one that I found online is from Redpanda comparing the tail latencies
https://redpanda.com/blog/kafka-kraft-vs-redpanda-performance-2023#the-test:-redpanda-23.1-vs.-kafka-3.4.0-with-kraft

Can I assume this is a valid comparison?

Also I heard that KRaft will be helpful for the number of partitions on a
cluster. Do we have any test showing the difference?

Are there any expected performance improvements?

Thanks
Doguscan


Re: [DISCUSS] KIP-714: Client metrics and observability

2023-08-08 Thread Doğuşcan Namal
Thanks for your answers Andrew. I share your pain that it took a while for
you to get this KIP approved and you want to reduce the scope of it, will
be happy to help you with the implementation :)

Could you help me walk through what happens if the target broker is
unreachable? Is the client going to drop these metrics or is it going to
send it to the other brokers it is connected to? This information is
crucial to understand the client side impact on leadership failovers.
Moreover, in case of partial outages, such as only the network between the
client and the broker is partitioned whereas the network within the cluster
is healthy, practically there is no other way than the client side metrics
to identify this problem.

Doguscan

On Fri, 4 Aug 2023 at 15:33, Andrew Schofield <
andrew_schofield_j...@outlook.com> wrote:

> Hi Doguscan,
> Thanks for your comments. I’m glad to hear you’re interested in this KIP.
>
> 1) It is preferred that a client sends its metrics to the same broker
> connection
> but actually it is able to send them to any broker. As a result, if a
> broker becomes
> unhealthy, the client can push its metrics to any other broker. It seems
> to me that
> pushing to KRaft controllers instead just has the effect of increasing the
> load on
> the controllers, while still having the characteristic that an unhealthy
> controller
> would present inconvenience for collecting metrics.
>
> 2) When the `PushTelemetryRequest.Terminating` flag is set, the standard
> request
> throttling is not disabled. The metrics rate-limiting based on the push
> interval is
> not applied in this case for a single request for the combination of
> client instance ID
> and subscription ID.
>
> (I have corrected the KIP text because it erroneously said “client ID and
> subscription ID”.
>
> 3) While this is a theoretical problem, I’m not keen on adding yet more
> configurations
> to the broker or client. The `interval.ms` configuration on the
> CLIENT_METRICS
> resource could perhaps have a minimum and maximum value to prevent
> accidental
> misconfiguration.
>
> 4) One of the reasons that this KIP has taken so long to get to this stage
> is that
> it tried to do many things all at once. So, it’s greatly simplified
> compared with
> 6 months ago. I can see the value of collecting client configurations for
> problem
> determination, but I don’t want to make this KIP more complicated. I think
> the
> idea has merit as a separate follow-on KIP. I would be happy to collaborate
> with you on this.
>
> 5) The default is set to 5 minutes to minimise the load on the broker for
> situations
> in which the administrator didn’t set an interval on a metrics
> subscription. To
> use an interval of 1 minute, it is only necessary to set `interval.ms` on
> the metrics
> subscription to 6ms.
>
> 6) Uncompressed data is always supported. The KIP says:
>  "The CompressionType of NONE will not be
> "present in the response from the broker, though the broker does support
> uncompressed
> "client telemetry if none of the accepted compression codecs are supported
> by the client.”
> So in your example, the client need only use CompressionType=NONE.
>
> Thanks,
> Andrew
>
> > On 4 Aug 2023, at 14:04, Doğuşcan Namal 
> wrote:
> >
> > Hi Andrew, thanks a lot for this KIP. I was thinking of something similar
> > so thanks for writing this down 
> >
> >
> >
> > Couple of questions related to the design:
> >
> >
> >
> > 1. Can we investigate the option for using the Kraft controllers instead
> of
> > the brokers for sending metrics? The disadvantage of sending these
> metrics
> > directly to the brokers tightly couples metric observability to data
> plane
> > availability. If the broker is unhealthy then the root cause of an
> incident
> > is clear however on partial failures it makes it hard to debug these
> > incidents from the brokers perspective.
> >
> >
> >
> > 2. Ratelimiting will be disable if the `PushTelemetryRequest.Terminating`
> > flag is set. However, this may cause unavailability on the broker if too
> > many clients are terminated at once, especially network threads could
> > become busy and introduce latency on the produce/consume on other
> > non-terminating clients connections. I think there is a room for
> > improvement here. If the client is gracefully shutting down, it could
> wait
> > for the request to be handled if it is being ratelimited, it doesn't need
> > to "force push" the metrics. For that reason, maybe we could define a
> > separate ratelimiting for telemetry data?
> >
> >
>

RE: [DISCUSS] KIP-714: Client metrics and observability

2023-08-04 Thread Doğuşcan Namal
Hi Andrew, thanks a lot for this KIP. I was thinking of something similar
so thanks for writing this down 



Couple of questions related to the design:



1. Can we investigate the option for using the Kraft controllers instead of
the brokers for sending metrics? The disadvantage of sending these metrics
directly to the brokers tightly couples metric observability to data plane
availability. If the broker is unhealthy then the root cause of an incident
is clear however on partial failures it makes it hard to debug these
incidents from the brokers perspective.



2. Ratelimiting will be disable if the `PushTelemetryRequest.Terminating`
flag is set. However, this may cause unavailability on the broker if too
many clients are terminated at once, especially network threads could
become busy and introduce latency on the produce/consume on other
non-terminating clients connections. I think there is a room for
improvement here. If the client is gracefully shutting down, it could wait
for the request to be handled if it is being ratelimited, it doesn't need
to "force push" the metrics. For that reason, maybe we could define a
separate ratelimiting for telemetry data?



3. `PushIntervalMs` is set on the client side by a response from
`GetTelemetrySubscriptionsResponse`. If the broker sets this value to too
low, like 1msec, this may hog all of the clients activity and cause an
impact on the client side. I think we should introduce a configuration both
on the client and the broker side for the minimum and maximum numbers for
this value to fence out misconfigurations.



4. One of the important things I face during debugging the client side
failures is to understand the client side configurations. Can the client
sends these configs during the GetTelemetrySubscriptions request as well?



Small comments:

5. Default PushIntervalMs is 5 minutes. Can we make it 1 minute instead? I
think 5 minutes of aggregated data is too not helpful in the world of
telemetry 

6. UnsupportedCompressionType: Shall we fallback to non-compression mode in
that case? I think compression is nice to have, but non-compressed
telemetry data is valuable as well. Especially for low throughput clients,
compressing telemetry data may cause more CPU load then the actual data
plane work.


Thanks again.

Doguscan



> On Jun 13, 2023, at 8:06 AM, Andrew Schofield

>  wrote:

>

> Hi,

> I would like to start a new discussion thread on KIP-714: Client metrics
and

> observability.

>

>
https://cwiki.apache.org/confluence/display/KAFKA/KIP-714%3A+Client+metrics+and+observability

>

> I have edited the proposal significantly to reduce the scope. The overall

> mechanism for client metric subscriptions is unchanged, but the

> KIP is now based on the existing client metrics, rather than introducing
new

> metrics. The purpose remains helping cluster operators

> investigate performance problems experienced by clients without requiring

> changes to the client application code or configuration.

>

> Thanks,

> Andrew


Fwd: [DISCUSS] KIP-714: Client metrics and observability

2023-08-04 Thread Doğuşcan Namal
Hi Andrew, thanks a lot for this KIP. I was thinking of something similar
so thanks for writing this down 



Couple of questions related to the design:



1. Can we investigate the option for using the Kraft controllers instead of
the brokers for sending metrics? The disadvantage of sending these metrics
directly to the brokers tightly couples metric observability to data plane
availability. If the broker is unhealthy then the root cause of an incident
is clear however on partial failures it makes it hard to debug these
incidents from the brokers perspective.



2. Ratelimiting will be disable if the `PushTelemetryRequest.Terminating`
flag is set. However, this may cause unavailability on the broker if too
many clients are terminated at once, especially network threads could
become busy and introduce latency on the produce/consume on other
non-terminating clients connections. I think there is a room for
improvement here. If the client is gracefully shutting down, it could wait
for the request to be handled if it is being ratelimited, it doesn't need
to "force push" the metrics. For that reason, maybe we could define a
separate ratelimiting for telemetry data?



3. `PushIntervalMs` is set on the client side by a response from
`GetTelemetrySubscriptionsResponse`. If the broker sets this value to too
low, like 1msec, this may hog all of the clients activity and cause an
impact on the client side. I think we should introduce a configuration both
on the client and the broker side for the minimum and maximum numbers for
this value to fence out misconfigurations.



4. One of the important things I face during debugging the client side
failures is to understand the client side configurations. Can the client
sends these configs during the GetTelemetrySubscriptions request as well?



Small comments:

5. Default PushIntervalMs is 5 minutes. Can we make it 1 minute instead? I
think 5 minutes of aggregated data is too not helpful in the world of
telemetry 

6. UnsupportedCompressionType: Shall we fallback to non-compression mode in
that case? I think compression is nice to have, but non-compressed
telemetry data is valuable as well. Especially for low throughput clients,
compressing telemetry data may cause more CPU load then the actual data
plane work.


Thanks again.

Doguscan



> On Jun 13, 2023, at 8:06 AM, Andrew Schofield

>  wrote:

>

> Hi,

> I would like to start a new discussion thread on KIP-714: Client metrics
and

> observability.

>

>
https://cwiki.apache.org/confluence/display/KAFKA/KIP-714%3A+Client+metrics+and+observability

>

> I have edited the proposal significantly to reduce the scope. The overall

> mechanism for client metric subscriptions is unchanged, but the

> KIP is now based on the existing client metrics, rather than introducing
new

> metrics. The purpose remains helping cluster operators

> investigate performance problems experienced by clients without requiring

> changes to the client application code or configuration.

>

> Thanks,

> Andrew