multiple TLS listeners

2022-11-03 Thread Shuxin Yang

Hi,

   I'm new to Java-SSL and Kafka. Is it possible to configure multiple 
SSL listeners (say, one for inter-cluster, the other one for 
intra-cluster communication)? How does Kafka pick up a appropriate 
certificate for incoming request.


Thanks!

Shuxin



Build failed in Jenkins: Kafka » Kafka Branch Builder » trunk #1334

2022-11-03 Thread Apache Jenkins Server
See 


Changes:


--
[...truncated 511193 lines...]
[2022-11-03T19:48:01.838Z] > Task 
:clients:publishMavenJavaPublicationToMavenLocal
[2022-11-03T19:48:01.838Z] > Task :clients:publishToMavenLocal
[2022-11-03T19:48:18.246Z] > Task :core:compileScala
[2022-11-03T19:49:25.396Z] > Task :core:classes
[2022-11-03T19:49:25.396Z] > Task :core:compileTestJava NO-SOURCE
[2022-11-03T19:49:51.166Z] > Task :core:compileTestScala
[2022-11-03T19:50:32.572Z] > Task :core:testClasses
[2022-11-03T19:50:48.994Z] > Task :streams:compileTestJava
[2022-11-03T19:50:48.994Z] > Task :streams:testClasses
[2022-11-03T19:50:48.994Z] > Task :streams:testJar
[2022-11-03T19:50:49.933Z] > Task :streams:testSrcJar
[2022-11-03T19:50:49.933Z] > Task 
:streams:publishMavenJavaPublicationToMavenLocal
[2022-11-03T19:50:49.933Z] > Task :streams:publishToMavenLocal
[2022-11-03T19:50:49.933Z] 
[2022-11-03T19:50:49.933Z] Deprecated Gradle features were used in this build, 
making it incompatible with Gradle 8.0.
[2022-11-03T19:50:49.933Z] 
[2022-11-03T19:50:49.933Z] You can use '--warning-mode all' to show the 
individual deprecation warnings and determine if they come from your own 
scripts or plugins.
[2022-11-03T19:50:49.933Z] 
[2022-11-03T19:50:49.933Z] See 
https://docs.gradle.org/7.5.1/userguide/command_line_interface.html#sec:command_line_warnings
[2022-11-03T19:50:49.933Z] 
[2022-11-03T19:50:49.933Z] Execution optimizations have been disabled for 2 
invalid unit(s) of work during this build to ensure correctness.
[2022-11-03T19:50:49.933Z] Please consult deprecation warnings for more details.
[2022-11-03T19:50:49.933Z] 
[2022-11-03T19:50:49.933Z] BUILD SUCCESSFUL in 3m 28s
[2022-11-03T19:50:49.933Z] 79 actionable tasks: 37 executed, 42 up-to-date
[Pipeline] sh
[2022-11-03T19:50:53.366Z] + grep ^version= gradle.properties
[2022-11-03T19:50:53.366Z] + cut -d= -f 2
[Pipeline] dir
[2022-11-03T19:50:54.056Z] Running in 
/home/jenkins/jenkins-agent/workspace/Kafka_kafka_trunk/streams/quickstart
[Pipeline] {
[Pipeline] sh
[2022-11-03T19:50:56.033Z] + mvn clean install -Dgpg.skip
[2022-11-03T19:50:57.797Z] [INFO] Scanning for projects...
[2022-11-03T19:50:57.797Z] [INFO] 

[2022-11-03T19:50:57.797Z] [INFO] Reactor Build Order:
[2022-11-03T19:50:57.797Z] [INFO] 
[2022-11-03T19:50:57.797Z] [INFO] Kafka Streams :: Quickstart   
 [pom]
[2022-11-03T19:50:57.797Z] [INFO] streams-quickstart-java   
 [maven-archetype]
[2022-11-03T19:50:57.797Z] [INFO] 
[2022-11-03T19:50:57.797Z] [INFO] < 
org.apache.kafka:streams-quickstart >-
[2022-11-03T19:50:57.797Z] [INFO] Building Kafka Streams :: Quickstart 
3.4.0-SNAPSHOT[1/2]
[2022-11-03T19:50:57.797Z] [INFO] [ pom 
]-
[2022-11-03T19:50:57.797Z] [INFO] 
[2022-11-03T19:50:57.797Z] [INFO] --- maven-clean-plugin:3.0.0:clean 
(default-clean) @ streams-quickstart ---
[2022-11-03T19:50:58.735Z] [INFO] 
[2022-11-03T19:50:58.735Z] [INFO] --- maven-remote-resources-plugin:1.5:process 
(process-resource-bundles) @ streams-quickstart ---
[2022-11-03T19:50:59.673Z] [INFO] 
[2022-11-03T19:50:59.673Z] [INFO] --- maven-site-plugin:3.5.1:attach-descriptor 
(attach-descriptor) @ streams-quickstart ---
[2022-11-03T19:51:01.427Z] [INFO] 
[2022-11-03T19:51:01.427Z] [INFO] --- maven-gpg-plugin:1.6:sign 
(sign-artifacts) @ streams-quickstart ---
[2022-11-03T19:51:01.427Z] [INFO] 
[2022-11-03T19:51:01.427Z] [INFO] --- maven-install-plugin:2.5.2:install 
(default-install) @ streams-quickstart ---
[2022-11-03T19:51:01.427Z] [INFO] Installing 
/home/jenkins/jenkins-agent/workspace/Kafka_kafka_trunk/streams/quickstart/pom.xml
 to 
/home/jenkins/.m2/repository/org/apache/kafka/streams-quickstart/3.4.0-SNAPSHOT/streams-quickstart-3.4.0-SNAPSHOT.pom
[2022-11-03T19:51:01.427Z] [INFO] 
[2022-11-03T19:51:01.427Z] [INFO] --< 
org.apache.kafka:streams-quickstart-java >--
[2022-11-03T19:51:01.427Z] [INFO] Building streams-quickstart-java 
3.4.0-SNAPSHOT[2/2]
[2022-11-03T19:51:01.427Z] [INFO] --[ maven-archetype 
]---
[2022-11-03T19:51:01.427Z] [INFO] 
[2022-11-03T19:51:01.427Z] [INFO] --- maven-clean-plugin:3.0.0:clean 
(default-clean) @ streams-quickstart-java ---
[2022-11-03T19:51:01.427Z] [INFO] 
[2022-11-03T19:51:01.427Z] [INFO] --- maven-remote-resources-plugin:1.5:process 
(process-resource-bundles) @ streams-quickstart-java ---
[2022-11-03T19:51:01.427Z] [INFO] 
[2022-11-03T19:51:01.427Z] [INFO] --- maven-resources-plugin:2.7:resources 
(default-resources) @ streams-quickstart-java ---
[2022-11-03T19:51:01.427Z] [INFO] Using 'UTF-8' encoding to copy filtered 
resources.
[2022-11-03T19:5

[DISCUSS] KIP-883: Add delete callback method to Connector API

2022-11-03 Thread Hector Geraldino (BLOOMBERG/ 919 3RD A)
Hi everyone,

I've submitted KIP-883, which introduces a callback to the public Connector API 
called when deleting a connector:

https://cwiki.apache.org/confluence/display/KAFKA/KIP-883%3A+Add+delete+callback+method+to+Connector+API

It adds a new `deleted()` method (open to better naming suggestions) to the 
org.apache.kafka.connect.connector.Connector abstract class, which will be 
invoked by connect Workers when a connector is being deleted. 

Feedback and comments are welcome.

Thank you!
Hector



[jira] [Created] (KAFKA-14354) Add delete callback method to Connector API

2022-11-03 Thread Hector Geraldino (Jira)
Hector Geraldino created KAFKA-14354:


 Summary: Add delete callback method to Connector API
 Key: KAFKA-14354
 URL: https://issues.apache.org/jira/browse/KAFKA-14354
 Project: Kafka
  Issue Type: Improvement
  Components: clients
Reporter: Hector Geraldino
Assignee: Hector Geraldino


KIP-795: 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-795%3A+Add+public+APIs+for+AbstractCoordinator

The AbstractCoordinator should have a companion public interface that is part 
of Kafka's public API, so backwards compatibility can be maintained in future 
versions of the client libraries



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Resolved] (KAFKA-13434) Add a public API for AbstractCoordinator

2022-11-03 Thread Hector Geraldino (Jira)


 [ 
https://issues.apache.org/jira/browse/KAFKA-13434?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Hector Geraldino resolved KAFKA-13434.
--
Resolution: Won't Do

KIP has been discarded

> Add a public API for AbstractCoordinator
> 
>
> Key: KAFKA-13434
> URL: https://issues.apache.org/jira/browse/KAFKA-13434
> Project: Kafka
>  Issue Type: Improvement
>  Components: clients
>Reporter: Hector Geraldino
>Assignee: Hector Geraldino
>Priority: Major
>
> KIP-795: 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-795%3A+Add+public+APIs+for+AbstractCoordinator
> The AbstractCoordinator should have a companion public interface that is part 
> of Kafka's public API, so backwards compatibility can be maintained in future 
> versions of the client libraries



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [DISCUSS] KIP-882: Make Kafka Connect REST API request timeouts configurable

2022-11-03 Thread Sagar
Hey Yash,

Thanks for the KIP! This looks like a useful feature.

I think the discussion thread already has some great points by Chris. Just
a couple of points/clarifications=>

Regarding, pt#2 , I guess it might be better to forward to the leader as
suggested by Yash. Having said that, why does the worker forward to the
leader? I am thinking if the worker can perform the validation on it's own,
we could let it do the validation instead of forwarding everything to the
leader(even though it might be cheap to forward all requests to the leader).

Pt#3 => I think a bound is certainly needed but IMO it shouldn't go beyond
10 mins considering this is just validation. We shouldn't end up in a
situation where a few faulty connectors end up blocking a lot of request
processing threads, so while increasing the config is certainly helpful, we
shouldn't set too high a value IMO. Of course I am also open to suggestions
here.

Thanks!
Sagar.

On Thu, Nov 3, 2022 at 9:01 PM Chris Egerton 
wrote:

> Hi Yash,
>
> RE 2: That's a great point about validations already being performed by the
> leader. For completeness's sake, I'd like to note that this only holds for
> valid configurations; invalid ones are caught right now before being
> forwarded to the leader. Still, I think it's fine to forward to the leader
> for now and optimize further in the future if necessary. If frequent
> validations are taking place they should be conducted via the `PUT
> /connector-plugins/{pluginName}/config/validate` endpoint, which won't do
> any forwarding at all.
>
> RE 3: Yes, those endpoints LGTM. And yes, bounds on the timeout also seem
> reasonable... maybe a low-importance worker property could work for that?
> Not sure what would make sense for a default; probably somewhere in the
> 10-60 minute range but would be interested in others' thoughts.
>
> Thanks for the clarification on the zombie fencing logic. I think we might
> want to have some more subtle logic around the interaction between calls to
> Admin::fenceProducers and a worker-level timeout property if we go that
> route, but we can cross that particular bridge if we get back to it.
>
> Cheers,
>
> Chris
>
> On Wed, Nov 2, 2022 at 1:48 PM Yash Mayya  wrote:
>
> > Hi Chris,
> >
> > Thanks a lot for the super quick response and the great feedback!
> >
> > 1. I think that makes a lot of sense, and I'd be happy to update the KIP
> to
> > include this change in the scope. The current behavior where the API
> > response indicates a time out but the connector is created/updated
> > eventually anyway can be pretty confusing and is generally not a good
> user
> > experience IMO.
> >
> > 2. Wow, thanks for pointing this out - it's a really good catch and
> > something I hadn't noticed was happening with the current implementation.
> > While I do like the idea of having a query parameter that determines
> > whether validations can be skipped, I'm wondering if it might not be
> easier
> > and cleaner to just do the leader check earlier and avoid doing the
> > unnecessary config validation on the first worker? Since each config
> > validation happens on its own thread, I'm not so sure about the concern
> of
> > overloading the leader even on larger clusters, especially since
> > validations aren't typically long running operations. Furthermore, even
> > with the current implementation, the leader will always be doing a config
> > validation for connector create / update REST API requests on any worker.
> >
> > 3. That's a good point, and this way we can also restrict the APIs whose
> > timeouts are configurable - I'm thinking `PUT
> > /connector-plugins/{pluginName}/config/validate`, `POST /connectors` and
> > `PUT /connectors/{connector}/config` are the ones where such a timeout
> > parameter could be useful. Also, do you think we should enforce some
> > reasonable bounds for the timeout config?
> >
> > On the zombie fencing point, the implication was that the new worker
> > property would not control the timeout used for the call to
> > Admin::fenceProducers. However, if we go with a timeout query parameter
> > approach, even the timeout for the `PUT /connectors/{connector}/fence'
> > endpoint will remain unaffected.
> >
> > Thanks,
> > Yash
> >
> > On Wed, Nov 2, 2022 at 8:13 PM Chris Egerton 
> > wrote:
> >
> > > Hi Yash,
> > >
> > > Thanks for the KIP. It's a nice, focused change. Initially I was
> hesitant
> > > to support cases where connector validation takes this long, but
> > > considering the alternative is that we give users a 500 error response
> > but
> > > leave the request to create/modify the connector queued up in the
> > herder, I
> > > think I can get behind the motivation here. There's also an argument to
> > be
> > > made about keeping Kafka Connect available even when the systems that
> it
> > > connects to are in a degraded state.
> > >
> > > I have a few alternatives I'd be interested in your thoughts on:
> > >
> > > 1. Since the primary concern here seems to be t

Re: [DISCUSS] KIP-866 ZooKeeper to KRaft Migration

2022-11-03 Thread Jun Rao
Hi, David,

Thanks for the reply.

20/21 Yes, but separate listeners are optional. It's possible for the nodes
to use a single port for both client and server side communications.

Thanks,

Jun

On Thu, Nov 3, 2022 at 9:59 AM David Arthur
 wrote:

> 20/21, in combined mode we still have a separate listener for the
> controller APIs, e.g.,
>
> listeners=PLAINTEXT://:9092,CONTROLLER://:9093
>
> inter.broker.listener.name=PLAINTEXT
>
> controller.listener.names=CONTROLLER
>
> advertised.listeners=PLAINTEXT://localhost:9092
>
>
>
> Clients still talk to the broker through the advertised listener, and only
> brokers and other controllers will communicate over the controller
> listener.
>
> 40. Sounds good, I updated the KIP
>
> Thanks!
> David
>
>
>
> On Thu, Nov 3, 2022 at 12:14 PM Jun Rao  wrote:
>
> > Hi, David,
> >
> > Thanks for the reply.
> >
> > 20/21. When KRaft runs in the combined mode, does a controller know
> whether
> > an ApiRequest is from a client or another broker?
> >
> > 40. Adding a "None" state sounds reasonable.
> >
> > Thanks,
> >
> > Jun
> >
> > On Thu, Nov 3, 2022 at 8:39 AM David Arthur
> >  wrote:
> >
> > > Jun,
> > >
> > > 20/21 If we use a tagged field, then I don't think clients need to be
> > > concerned with it, right?. In ApiVersionsResponse sent by brokers to
> > > clients, this field would be omitted. Clients can't connect directly to
> > the
> > > KRaft controller nodes. Also, we have a precedent of sending controller
> > > node state between controllers through ApiVersions ("metadata.version"
> > > feature), so I think it fits well.
> > >
> > > 40. For new KRaft clusters, we could add another state to indicate it
> was
> > > not migrated from ZK, but created as a KRaft cluster. Maybe
> > > "kafka.controller:type=KafkaController,name=MigrationState" => "None" ?
> > We
> > > could also omit that metric for unmigrated clusters, but I'm not a fan
> of
> > > using the absence of a value to signal something.
> > >
> > > -
> > >
> > > Akhilesh, thanks for reviewing the KIP!
> > >
> > > 1. MigrationState and MetadataType are mostly the same on the
> controller,
> > > but we do have the "MigratingZkData" state that seems useful to report
> > as a
> > > metric. Aside from looking at logs, observing the controller in this
> > state
> > > is the only way to see how long its taking to copy data from ZK.
> > >
> > > "KRaftMode" instead of "MigrationFinalized" is similar to Jun's
> question
> > > about non-migrated clusters. I think it's useful to have a distinct
> > > MigrationState for clusters that have been migrated and those that were
> > > never migrated. This does mean we'll report the MigrationState long
> after
> > > the migration is complete, but we can drop these metrics in 4.0 once ZK
> > is
> > > removed.
> > >
> > > 2. The "ZkMigrationReady" will indicate that the controller has
> > > "kafka.metadata.migration.enable" _and_ the ZK configs set. We need
> some
> > > way to indicate that the whole quorum is correctly configured to handle
> > the
> > > migration so we don't failover to a controller that's not configured
> for
> > > ZK. Did I understand your question correctly?
> > >
> > > 3. Yea, good idea. While the KRaft controller has
> > > MigrationState=MigrationIneligible, we could also report
> > >
> >
> "kafka.controller:type=KafkaController,name=MigrationInelgibleBrokerCount".
> > > It might be useful to report ineligible controllers as well since that
> > can
> > > prevent the migration from starting.
> > >
> > > 4. I think I covered this in "Incompatible Brokers". We effectively
> fence
> > > these brokers by not sending them metadata RPCs.
> > >
> > > Thanks!
> > > David
> > >
> > >
> > > On Tue, Nov 1, 2022 at 3:58 PM Akhilesh Chaganti <
> akhilesh@gmail.com
> > >
> > > wrote:
> > >
> > > > Hi David,
> > > >
> > > >
> > > > Thanks for the KIP. I have some questions/suggestions.
> > > >
> > > >
> > > > 1) I see two new metrics:
> > > > kafka.controller:type=KafkaController,name=MetadataType and
> > > > kafka.controller:type=KafkaController,name=MigrationState. Won't the
> > > second
> > > > metric already cover the cases of the first metric? Also, instead of
> > > > MigrationFinalized, we could directly say the state is KRaftMode. So
> we
> > > can
> > > > use the same value for default KRaft clusters.
> > > >
> > > >
> > > > 2) ZkMigrationReady in ApiVersionsResponse from KRaft Controller. By
> > > > default, we plan to start the Controller quorum in "
> > > > *kafka.metadata.migration.enable*" config set to true. Then do we
> need
> > > this
> > > > additional information again to make sure The controllers are ready
> for
> > > > migration? What would happen if the Controller assumes it is ready
> for
> > > > migration from 3.4 by default if it doesn't see both
> MigrationMetadata
> > > > records?
> > > >
> > > >
> > > > 3) I see that we do not impose order on rolling the brokers with
> > > migration
> > > > flags and provisioning the controller quorum. Alon

Re: [DISCUSS] KIP-866 ZooKeeper to KRaft Migration

2022-11-03 Thread David Arthur
20/21, in combined mode we still have a separate listener for the
controller APIs, e.g.,

listeners=PLAINTEXT://:9092,CONTROLLER://:9093

inter.broker.listener.name=PLAINTEXT

controller.listener.names=CONTROLLER

advertised.listeners=PLAINTEXT://localhost:9092



Clients still talk to the broker through the advertised listener, and only
brokers and other controllers will communicate over the controller listener.

40. Sounds good, I updated the KIP

Thanks!
David



On Thu, Nov 3, 2022 at 12:14 PM Jun Rao  wrote:

> Hi, David,
>
> Thanks for the reply.
>
> 20/21. When KRaft runs in the combined mode, does a controller know whether
> an ApiRequest is from a client or another broker?
>
> 40. Adding a "None" state sounds reasonable.
>
> Thanks,
>
> Jun
>
> On Thu, Nov 3, 2022 at 8:39 AM David Arthur
>  wrote:
>
> > Jun,
> >
> > 20/21 If we use a tagged field, then I don't think clients need to be
> > concerned with it, right?. In ApiVersionsResponse sent by brokers to
> > clients, this field would be omitted. Clients can't connect directly to
> the
> > KRaft controller nodes. Also, we have a precedent of sending controller
> > node state between controllers through ApiVersions ("metadata.version"
> > feature), so I think it fits well.
> >
> > 40. For new KRaft clusters, we could add another state to indicate it was
> > not migrated from ZK, but created as a KRaft cluster. Maybe
> > "kafka.controller:type=KafkaController,name=MigrationState" => "None" ?
> We
> > could also omit that metric for unmigrated clusters, but I'm not a fan of
> > using the absence of a value to signal something.
> >
> > -
> >
> > Akhilesh, thanks for reviewing the KIP!
> >
> > 1. MigrationState and MetadataType are mostly the same on the controller,
> > but we do have the "MigratingZkData" state that seems useful to report
> as a
> > metric. Aside from looking at logs, observing the controller in this
> state
> > is the only way to see how long its taking to copy data from ZK.
> >
> > "KRaftMode" instead of "MigrationFinalized" is similar to Jun's question
> > about non-migrated clusters. I think it's useful to have a distinct
> > MigrationState for clusters that have been migrated and those that were
> > never migrated. This does mean we'll report the MigrationState long after
> > the migration is complete, but we can drop these metrics in 4.0 once ZK
> is
> > removed.
> >
> > 2. The "ZkMigrationReady" will indicate that the controller has
> > "kafka.metadata.migration.enable" _and_ the ZK configs set. We need some
> > way to indicate that the whole quorum is correctly configured to handle
> the
> > migration so we don't failover to a controller that's not configured for
> > ZK. Did I understand your question correctly?
> >
> > 3. Yea, good idea. While the KRaft controller has
> > MigrationState=MigrationIneligible, we could also report
> >
> "kafka.controller:type=KafkaController,name=MigrationInelgibleBrokerCount".
> > It might be useful to report ineligible controllers as well since that
> can
> > prevent the migration from starting.
> >
> > 4. I think I covered this in "Incompatible Brokers". We effectively fence
> > these brokers by not sending them metadata RPCs.
> >
> > Thanks!
> > David
> >
> >
> > On Tue, Nov 1, 2022 at 3:58 PM Akhilesh Chaganti  >
> > wrote:
> >
> > > Hi David,
> > >
> > >
> > > Thanks for the KIP. I have some questions/suggestions.
> > >
> > >
> > > 1) I see two new metrics:
> > > kafka.controller:type=KafkaController,name=MetadataType and
> > > kafka.controller:type=KafkaController,name=MigrationState. Won't the
> > second
> > > metric already cover the cases of the first metric? Also, instead of
> > > MigrationFinalized, we could directly say the state is KRaftMode. So we
> > can
> > > use the same value for default KRaft clusters.
> > >
> > >
> > > 2) ZkMigrationReady in ApiVersionsResponse from KRaft Controller. By
> > > default, we plan to start the Controller quorum in "
> > > *kafka.metadata.migration.enable*" config set to true. Then do we need
> > this
> > > additional information again to make sure The controllers are ready for
> > > migration? What would happen if the Controller assumes it is ready for
> > > migration from 3.4 by default if it doesn't see both MigrationMetadata
> > > records?
> > >
> > >
> > > 3) I see that we do not impose order on rolling the brokers with
> > migration
> > > flags and provisioning the controller quorum. Along with the KRaft
> > > controller emitting migration state metrics, it may be better to emit
> the
> > > broker count for the brokers not ready for migration yet. This will
> give
> > us
> > > more insight into any roll issues.
> > >
> > >
> > > 4) Once the KRaft controller is in migration mode, we should also
> > > prevent/handle ZkBrokerRegistrations that don't enable migration mode.
> > >
> > >
> > > Thanks
> > > Akhilesh
> > >
> > >
> > > On Tue, Nov 1, 2022 at 10:49 AM Jun Rao 
> > wrote:
> > >
> > > > Hi, David,
> > > >
> > > > Thanks for the repl

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

2022-11-03 Thread Viktor Somogyi-Vass
Hi Rajini,

If I understand correctly, the client.rack config would stay supported
after KIP-848 but does it expand the scope of that KIP too with this
config? I mean that currently you propose ConsumerProtocolSubscription to
be used but this protocol won't be available and we need to transfer the
config to the coordinator via other means. Should this be added to that KIP?

Thanks,
Viktor

On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram 
wrote:

> Hi Jun,
>
> Thank you for the review. Yes, we should add rack id to Subscription, had
> missed that part. Updated the KIP, thank you for pointing that out!
>
> Regards,
>
> Rajini
>
> On Wed, Nov 2, 2022 at 7:06 PM Jun Rao  wrote:
>
> > Hi, Rajini,
> >
> > Thanks for the KIP. Just one comment.
> >
> > Should we add rackId to GroupSubscription.Subscription for each member?
> >
> > Thanks,
> >
> > Jun
> >
> > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram 
> > wrote:
> >
> > > Hi all,
> > >
> > > I have submitted KIP-881 to implement rack-aware partition assignment
> for
> > > consumers:
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > .
> > > It adds rack id to the consumer group protocol to propagate rack
> > > information so that rack-aware assignors can be added to benefit from
> > > locality.
> > >
> > > Feedback and suggestions are welcome!
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> >
>


[jira] [Resolved] (KAFKA-14058) Replace EasyMock and PowerMock with Mockito in ExactlyOnceWorkerSourceTaskTest

2022-11-03 Thread Chris Egerton (Jira)


 [ 
https://issues.apache.org/jira/browse/KAFKA-14058?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chris Egerton resolved KAFKA-14058.
---
Fix Version/s: 3.4.0
   Resolution: Fixed

> Replace EasyMock and PowerMock with Mockito in ExactlyOnceWorkerSourceTaskTest
> --
>
> Key: KAFKA-14058
> URL: https://issues.apache.org/jira/browse/KAFKA-14058
> Project: Kafka
>  Issue Type: Sub-task
>  Components: KafkaConnect
>Reporter: Chris Egerton
>Assignee: Chris Egerton
>Priority: Minor
> Fix For: 3.4.0
>
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [DISCUSS] KIP-866 ZooKeeper to KRaft Migration

2022-11-03 Thread Jun Rao
Hi, David,

Thanks for the reply.

20/21. When KRaft runs in the combined mode, does a controller know whether
an ApiRequest is from a client or another broker?

40. Adding a "None" state sounds reasonable.

Thanks,

Jun

On Thu, Nov 3, 2022 at 8:39 AM David Arthur
 wrote:

> Jun,
>
> 20/21 If we use a tagged field, then I don't think clients need to be
> concerned with it, right?. In ApiVersionsResponse sent by brokers to
> clients, this field would be omitted. Clients can't connect directly to the
> KRaft controller nodes. Also, we have a precedent of sending controller
> node state between controllers through ApiVersions ("metadata.version"
> feature), so I think it fits well.
>
> 40. For new KRaft clusters, we could add another state to indicate it was
> not migrated from ZK, but created as a KRaft cluster. Maybe
> "kafka.controller:type=KafkaController,name=MigrationState" => "None" ? We
> could also omit that metric for unmigrated clusters, but I'm not a fan of
> using the absence of a value to signal something.
>
> -
>
> Akhilesh, thanks for reviewing the KIP!
>
> 1. MigrationState and MetadataType are mostly the same on the controller,
> but we do have the "MigratingZkData" state that seems useful to report as a
> metric. Aside from looking at logs, observing the controller in this state
> is the only way to see how long its taking to copy data from ZK.
>
> "KRaftMode" instead of "MigrationFinalized" is similar to Jun's question
> about non-migrated clusters. I think it's useful to have a distinct
> MigrationState for clusters that have been migrated and those that were
> never migrated. This does mean we'll report the MigrationState long after
> the migration is complete, but we can drop these metrics in 4.0 once ZK is
> removed.
>
> 2. The "ZkMigrationReady" will indicate that the controller has
> "kafka.metadata.migration.enable" _and_ the ZK configs set. We need some
> way to indicate that the whole quorum is correctly configured to handle the
> migration so we don't failover to a controller that's not configured for
> ZK. Did I understand your question correctly?
>
> 3. Yea, good idea. While the KRaft controller has
> MigrationState=MigrationIneligible, we could also report
> "kafka.controller:type=KafkaController,name=MigrationInelgibleBrokerCount".
> It might be useful to report ineligible controllers as well since that can
> prevent the migration from starting.
>
> 4. I think I covered this in "Incompatible Brokers". We effectively fence
> these brokers by not sending them metadata RPCs.
>
> Thanks!
> David
>
>
> On Tue, Nov 1, 2022 at 3:58 PM Akhilesh Chaganti 
> wrote:
>
> > Hi David,
> >
> >
> > Thanks for the KIP. I have some questions/suggestions.
> >
> >
> > 1) I see two new metrics:
> > kafka.controller:type=KafkaController,name=MetadataType and
> > kafka.controller:type=KafkaController,name=MigrationState. Won't the
> second
> > metric already cover the cases of the first metric? Also, instead of
> > MigrationFinalized, we could directly say the state is KRaftMode. So we
> can
> > use the same value for default KRaft clusters.
> >
> >
> > 2) ZkMigrationReady in ApiVersionsResponse from KRaft Controller. By
> > default, we plan to start the Controller quorum in "
> > *kafka.metadata.migration.enable*" config set to true. Then do we need
> this
> > additional information again to make sure The controllers are ready for
> > migration? What would happen if the Controller assumes it is ready for
> > migration from 3.4 by default if it doesn't see both MigrationMetadata
> > records?
> >
> >
> > 3) I see that we do not impose order on rolling the brokers with
> migration
> > flags and provisioning the controller quorum. Along with the KRaft
> > controller emitting migration state metrics, it may be better to emit the
> > broker count for the brokers not ready for migration yet. This will give
> us
> > more insight into any roll issues.
> >
> >
> > 4) Once the KRaft controller is in migration mode, we should also
> > prevent/handle ZkBrokerRegistrations that don't enable migration mode.
> >
> >
> > Thanks
> > Akhilesh
> >
> >
> > On Tue, Nov 1, 2022 at 10:49 AM Jun Rao 
> wrote:
> >
> > > Hi, David,
> > >
> > > Thanks for the reply.
> > >
> > > 20/21. Regarding the new ZkMigrationReady field in ApiVersionsResponse,
> > it
> > > seems that this is a bit intrusive since it exposes unneeded info to
> the
> > > clients. Another option is to add that field as part of the Fetch
> > request.
> > > We can choose to only set that field in the very first Fetch request
> > from a
> > > Quorum follower.
> > >
> > > 40. For kafka.controller:type=KafkaController,name=MigrationState, what
> > is
> > > the value for a brand new KRaft cluster?
> > >
> > > Jun
> > >
> > > On Mon, Oct 31, 2022 at 2:35 PM David Arthur
> > >  wrote:
> > >
> > > > 30. I think we can keep the single ControllerId field in those
> requests
> > > > since they are only used for fencing (as far as I know). Internally,
> > the
> > 

Re: [DISCUSS] KIP-866 ZooKeeper to KRaft Migration

2022-11-03 Thread David Arthur
Jun,

20/21 If we use a tagged field, then I don't think clients need to be
concerned with it, right?. In ApiVersionsResponse sent by brokers to
clients, this field would be omitted. Clients can't connect directly to the
KRaft controller nodes. Also, we have a precedent of sending controller
node state between controllers through ApiVersions ("metadata.version"
feature), so I think it fits well.

40. For new KRaft clusters, we could add another state to indicate it was
not migrated from ZK, but created as a KRaft cluster. Maybe
"kafka.controller:type=KafkaController,name=MigrationState" => "None" ? We
could also omit that metric for unmigrated clusters, but I'm not a fan of
using the absence of a value to signal something.

-

Akhilesh, thanks for reviewing the KIP!

1. MigrationState and MetadataType are mostly the same on the controller,
but we do have the "MigratingZkData" state that seems useful to report as a
metric. Aside from looking at logs, observing the controller in this state
is the only way to see how long its taking to copy data from ZK.

"KRaftMode" instead of "MigrationFinalized" is similar to Jun's question
about non-migrated clusters. I think it's useful to have a distinct
MigrationState for clusters that have been migrated and those that were
never migrated. This does mean we'll report the MigrationState long after
the migration is complete, but we can drop these metrics in 4.0 once ZK is
removed.

2. The "ZkMigrationReady" will indicate that the controller has
"kafka.metadata.migration.enable" _and_ the ZK configs set. We need some
way to indicate that the whole quorum is correctly configured to handle the
migration so we don't failover to a controller that's not configured for
ZK. Did I understand your question correctly?

3. Yea, good idea. While the KRaft controller has
MigrationState=MigrationIneligible, we could also report
"kafka.controller:type=KafkaController,name=MigrationInelgibleBrokerCount".
It might be useful to report ineligible controllers as well since that can
prevent the migration from starting.

4. I think I covered this in "Incompatible Brokers". We effectively fence
these brokers by not sending them metadata RPCs.

Thanks!
David


On Tue, Nov 1, 2022 at 3:58 PM Akhilesh Chaganti 
wrote:

> Hi David,
>
>
> Thanks for the KIP. I have some questions/suggestions.
>
>
> 1) I see two new metrics:
> kafka.controller:type=KafkaController,name=MetadataType and
> kafka.controller:type=KafkaController,name=MigrationState. Won't the second
> metric already cover the cases of the first metric? Also, instead of
> MigrationFinalized, we could directly say the state is KRaftMode. So we can
> use the same value for default KRaft clusters.
>
>
> 2) ZkMigrationReady in ApiVersionsResponse from KRaft Controller. By
> default, we plan to start the Controller quorum in "
> *kafka.metadata.migration.enable*" config set to true. Then do we need this
> additional information again to make sure The controllers are ready for
> migration? What would happen if the Controller assumes it is ready for
> migration from 3.4 by default if it doesn't see both MigrationMetadata
> records?
>
>
> 3) I see that we do not impose order on rolling the brokers with migration
> flags and provisioning the controller quorum. Along with the KRaft
> controller emitting migration state metrics, it may be better to emit the
> broker count for the brokers not ready for migration yet. This will give us
> more insight into any roll issues.
>
>
> 4) Once the KRaft controller is in migration mode, we should also
> prevent/handle ZkBrokerRegistrations that don't enable migration mode.
>
>
> Thanks
> Akhilesh
>
>
> On Tue, Nov 1, 2022 at 10:49 AM Jun Rao  wrote:
>
> > Hi, David,
> >
> > Thanks for the reply.
> >
> > 20/21. Regarding the new ZkMigrationReady field in ApiVersionsResponse,
> it
> > seems that this is a bit intrusive since it exposes unneeded info to the
> > clients. Another option is to add that field as part of the Fetch
> request.
> > We can choose to only set that field in the very first Fetch request
> from a
> > Quorum follower.
> >
> > 40. For kafka.controller:type=KafkaController,name=MigrationState, what
> is
> > the value for a brand new KRaft cluster?
> >
> > Jun
> >
> > On Mon, Oct 31, 2022 at 2:35 PM David Arthur
> >  wrote:
> >
> > > 30. I think we can keep the single ControllerId field in those requests
> > > since they are only used for fencing (as far as I know). Internally,
> the
> > > broker components that handle those requests will compare the
> > ControllerId
> > > with that of MetadataCache (which is updated via UMR).
> > >
> > > The reason we need the separate KRaftControllerId in the UpdateMetadata
> > > code path so that we can have different connection behavior for a KRaft
> > > controller vs ZK controller.
> > >
> > > 31. It seems reasonable to keep the MigrationRecord in the snapshot. I
> > was
> > > thinking the same thing in terms of understanding the loss for a
> > > migrati

Re: [DISCUSS] KIP-882: Make Kafka Connect REST API request timeouts configurable

2022-11-03 Thread Chris Egerton
Hi Yash,

RE 2: That's a great point about validations already being performed by the
leader. For completeness's sake, I'd like to note that this only holds for
valid configurations; invalid ones are caught right now before being
forwarded to the leader. Still, I think it's fine to forward to the leader
for now and optimize further in the future if necessary. If frequent
validations are taking place they should be conducted via the `PUT
/connector-plugins/{pluginName}/config/validate` endpoint, which won't do
any forwarding at all.

RE 3: Yes, those endpoints LGTM. And yes, bounds on the timeout also seem
reasonable... maybe a low-importance worker property could work for that?
Not sure what would make sense for a default; probably somewhere in the
10-60 minute range but would be interested in others' thoughts.

Thanks for the clarification on the zombie fencing logic. I think we might
want to have some more subtle logic around the interaction between calls to
Admin::fenceProducers and a worker-level timeout property if we go that
route, but we can cross that particular bridge if we get back to it.

Cheers,

Chris

On Wed, Nov 2, 2022 at 1:48 PM Yash Mayya  wrote:

> Hi Chris,
>
> Thanks a lot for the super quick response and the great feedback!
>
> 1. I think that makes a lot of sense, and I'd be happy to update the KIP to
> include this change in the scope. The current behavior where the API
> response indicates a time out but the connector is created/updated
> eventually anyway can be pretty confusing and is generally not a good user
> experience IMO.
>
> 2. Wow, thanks for pointing this out - it's a really good catch and
> something I hadn't noticed was happening with the current implementation.
> While I do like the idea of having a query parameter that determines
> whether validations can be skipped, I'm wondering if it might not be easier
> and cleaner to just do the leader check earlier and avoid doing the
> unnecessary config validation on the first worker? Since each config
> validation happens on its own thread, I'm not so sure about the concern of
> overloading the leader even on larger clusters, especially since
> validations aren't typically long running operations. Furthermore, even
> with the current implementation, the leader will always be doing a config
> validation for connector create / update REST API requests on any worker.
>
> 3. That's a good point, and this way we can also restrict the APIs whose
> timeouts are configurable - I'm thinking `PUT
> /connector-plugins/{pluginName}/config/validate`, `POST /connectors` and
> `PUT /connectors/{connector}/config` are the ones where such a timeout
> parameter could be useful. Also, do you think we should enforce some
> reasonable bounds for the timeout config?
>
> On the zombie fencing point, the implication was that the new worker
> property would not control the timeout used for the call to
> Admin::fenceProducers. However, if we go with a timeout query parameter
> approach, even the timeout for the `PUT /connectors/{connector}/fence'
> endpoint will remain unaffected.
>
> Thanks,
> Yash
>
> On Wed, Nov 2, 2022 at 8:13 PM Chris Egerton 
> wrote:
>
> > Hi Yash,
> >
> > Thanks for the KIP. It's a nice, focused change. Initially I was hesitant
> > to support cases where connector validation takes this long, but
> > considering the alternative is that we give users a 500 error response
> but
> > leave the request to create/modify the connector queued up in the
> herder, I
> > think I can get behind the motivation here. There's also an argument to
> be
> > made about keeping Kafka Connect available even when the systems that it
> > connects to are in a degraded state.
> >
> > I have a few alternatives I'd be interested in your thoughts on:
> >
> > 1. Since the primary concern here seems to be that custom connector
> > validation logic can take too long, do we have any thoughts on adding
> logic
> > to check for request timeout after validation has completed and, if it
> has,
> > aborting the attempt to create/modify the connector?
> >
> > 2. Right now it's possible that we'll perform two connector config
> > validations per create/modify request; once on the worker that initially
> > receives the request, and then again if that worker is not the leader of
> > the cluster and has to forward the request to the leader. Any thoughts on
> > optimizing this to only require a single validation per request? We
> > probably wouldn't want to force all validations to take place on the
> leader
> > (could lead to overloading it pretty quickly in large clusters), but we
> > could add an internal-only query parameter to skip validation and then
> use
> > that parameter when forwarding requests from followers to the leader.
> >
> > 3. A worker property is pretty coarse-grained, and difficult to change.
> We
> > might allow per-request toggling of the timeout by adding a URL query
> > parameter like '?timeout=90s' to the REST API to allow tweaking of the
> >

[GitHub] [kafka-site] cadonna merged pull request #460: Add PMC member role to Bruno

2022-11-03 Thread GitBox


cadonna merged PR #460:
URL: https://github.com/apache/kafka-site/pull/460


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [kafka-site] cadonna commented on pull request #460: Add PMC member role to Bruno

2022-11-03 Thread GitBox


cadonna commented on PR #460:
URL: https://github.com/apache/kafka-site/pull/460#issuecomment-1301727479

   The change renders as follows:
   
   https://user-images.githubusercontent.com/1042170/199666188-a123b334-1a36-4355-8a6d-505064e27ba4.png";>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org