[DISCUSS] KIP-591: Add Kafka Streams config to set default state store
Hi devs, I'd like to propose a KIP to allow users to set default store implementation class (built-in RocksDB/InMemory, or custom one), and default to RocksDB state store, to keep backward compatibility. Detailed description can be found here: https://cwiki.apache.org/confluence/display/KAFKA/KIP-591%3A+Add+Kafka+Streams+config+to+set+default+state+store Any feedback and comments are welcome. Thank you. Luke
Re: [VOTE] KIP-805: Add range and scan query support in IQ v2
Thanks for the KIP, Vicky! I’m +1 (binding) -John On Tue, Nov 30, 2021, at 14:51, Vasiliki Papavasileiou wrote: > Hello everyone, > > I would like to start a vote for KIP-805 that adds range and scan KeyValue > queries in IQ2. > > The KIP can be found here: > https://cwiki.apache.org/confluence/display/KAFKA/KIP-805%3A+Add+range+and+scan+query+support+in+IQ+v2 > > Cheers! > Vicky
[jira] [Resolved] (KAFKA-13498) IQv2: Track Position in remaining stores
[ https://issues.apache.org/jira/browse/KAFKA-13498?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] John Roesler resolved KAFKA-13498. -- Resolution: Fixed > IQv2: Track Position in remaining stores > > > Key: KAFKA-13498 > URL: https://issues.apache.org/jira/browse/KAFKA-13498 > Project: Kafka > Issue Type: Sub-task >Reporter: John Roesler >Assignee: Patrick Stuedi >Priority: Major > -- This message was sent by Atlassian Jira (v8.20.1#820001)
[jira] [Created] (KAFKA-13498) IQv2: Track Position in remaining stores
John Roesler created KAFKA-13498: Summary: IQv2: Track Position in remaining stores Key: KAFKA-13498 URL: https://issues.apache.org/jira/browse/KAFKA-13498 Project: Kafka Issue Type: Sub-task Reporter: John Roesler -- This message was sent by Atlassian Jira (v8.20.1#820001)
Re: KIP-769: Connect API to retrieve connector configuration definitions
Thanks for the KIP, Mickael. And thanks to everyone else for the good discussion so far. Overall this is pretty good, but I do have a few questions/comments: 1. Can we include the HTTP method/verb in each of the endpoints in the "Public Interfaces" section? 2. The sample output for `GET /connector-plugins?connectorsOnly=false` includes a `version` field for the connector plugin, but not for the transform or converter plugins. Neither the `Transformation` or `Converter` interfaces have a version nor implement the `Versioned` interface, so should we add the `Versioned` interface to the `Transformation` and `Converter` interfaces and add a default method that returns "unknown"? Yes, we could do that in a separate KIP, but I think it's appropriate to do it in this KIP for a few reasons. First, we're already modifying the `Converter` interface in this KIP and it would be quite straightforward to also extend the `Versioned` interface and add a default method; then doing that for `Transformation` would also be relatively straightforward. Second, this KIP provides the justification for adding a version to the `Transformation` and `Converter` interfaces. 3. In the "Public Interfaces" section, there are two inconsistencies between the `GET /connector-plugins` and `GET /connector-plugins//config` methods. Both will make it difficult to use the second method, since the input to the second method is not directly found in the response from the first method: a. The example for `GET /connector-plugins/Cast$Value/config` uses short name, or what Connect often refers to as the alias, "Cast$Value" for the plugin, rather than the fully-qualified name "org.apache.kafka.connect.transforms.Cast$Value". The KIP should specify whether it will support both the shortened name and the fully-qualified name, or whether only one of these will be supported. Not supporting the fully-qualified name will make it impossible to use the response from the first method as an input to the second. a. The example for the list method returns the `Transformation` base and abstract classes (e.g., "org.apache.kafka.connect.transforms.Cast") rather than the concrete classes (e.g., "org.apache.kafka.connect.transforms.Cast$Value" and "org.apache.kafka.connect.transforms.Cast$Key"), while the `GET /connector-plugins//config` will require the concrete class. 4. The "Public Interfaces" section's description of the `GET /connector-plugins//config` method should probably specify the expected behavior for `Converter` implementations that don't override the new `config()` method. 5. The "Compatibility, Deprecation, and Migration Plan" section should describe whether older `Converter` implementations (compiled with older versions of the Connect API without Converter extending Configurable) will be able to be installed into newer Connect runtimes, and what the behavior will be for the `GET /connector-plugins//config` method. (Although my suggestion for #5 will partially address the latter, we still need to talk about it in terms of compatibility.) 6. Can we add a small section addressing the security implications of these new APIs? KIP-507 [1] applied to APIs that existed at the time, and this KIP should at least mention how it aligns with that mechanism. 7. Do we really think adding `GET /worker-plugins` is necessary? While I understand the reasoning, I'm less convinced that it's worth adding to the public API. First, an administrator still has to modify the worker config for all workers to take advantage of that. Second, adding this method increases the surface area and therefore risk of additional attack vectors, despite not really being able to use the resulting information in any other part of the API. Best regards, Randall [1] https://cwiki.apache.org/confluence/display/KAFKA/KIP-507%3A+Securing+Internal+Connect+REST+Endpoints On Wed, Dec 1, 2021 at 7:52 AM Mickael Maison wrote: > > Hi Chris, > > Thanks again for the feedback! I've updated the KIP based on our last > discussions. I've decided to include the new endpoint for worker > plugins. > > 1. Yes I agree, it's best to gate the new behavior. > 2. Yes, it was a remnant from the original proposal. I've now removed > the location field. > > Thanks, > Mickael > > On Tue, Nov 30, 2021 at 3:22 AM Chris Egerton > wrote: > > > > Hi Mickael, > > > > I think that's a great idea! I especially like how we can establish the > > expectation that any plugin type that appears in the response from the GET > > /connector-plugins endpoint will have a corresponding GET > > /connector-plugins//config endpoint, but (if we decide to add them in > > the future), worker plugins won't be expected to expose this kind of > > information and the different root path helps give a decent hint about this. > > > > I also like the choice to return an empty ConfigDef from Converter::config > > instead of null. > > > > Two things come to mind: > > > > 1. We may want to gate this behind a URL query parameter (maybe something >
[jira] [Created] (KAFKA-13497) Debug Log RegexRouter transform
Spencer Carlson created KAFKA-13497: --- Summary: Debug Log RegexRouter transform Key: KAFKA-13497 URL: https://issues.apache.org/jira/browse/KAFKA-13497 Project: Kafka Issue Type: Improvement Components: KafkaConnect Reporter: Spencer Carlson It is hard to troubleshoot regex transformations without knowing exactly what the new topic transformation is. Can we add a debug log in the [RegexRouter.java#L59|https://github.com/apache/kafka/blob/99b9b3e84f4e98c3f07714e1de6a139a004cbc5b/connect/transforms/src/main/java/org/apache/kafka/connect/transforms/RegexRouter.java#L59] to print out the transformed topic -- This message was sent by Atlassian Jira (v8.20.1#820001)
Re: KIP-769: Connect API to retrieve connector configuration definitions
Hi Mickael, Looks good to me! +1 non-binding when the vote thread opens :) Cheers, Chris On Wed, Dec 1, 2021 at 8:53 AM Mickael Maison wrote: > Hi Chris, > > Thanks again for the feedback! I've updated the KIP based on our last > discussions. I've decided to include the new endpoint for worker > plugins. > > 1. Yes I agree, it's best to gate the new behavior. > 2. Yes, it was a remnant from the original proposal. I've now removed > the location field. > > Thanks, > Mickael > > On Tue, Nov 30, 2021 at 3:22 AM Chris Egerton > wrote: > > > > Hi Mickael, > > > > I think that's a great idea! I especially like how we can establish the > > expectation that any plugin type that appears in the response from the > GET > > /connector-plugins endpoint will have a corresponding GET > > /connector-plugins//config endpoint, but (if we decide to add them > in > > the future), worker plugins won't be expected to expose this kind of > > information and the different root path helps give a decent hint about > this. > > > > I also like the choice to return an empty ConfigDef from > Converter::config > > instead of null. > > > > Two things come to mind: > > > > 1. We may want to gate this behind a URL query parameter (maybe something > > like "connectorsOnly") that defaults to the old behavior in order to > avoid > > breaking existing tools such as programmatic UIs that use the endpoint > > today to discover the connectors that can be created by the user. We can > > even plan to change the default for that parameter to the newly-proposed > > behavior in the next major release, which should give people enough time > to > > either adapt to the expanded response format or add the query parameter > to > > their tooling. > > > > 2. The existing GET /connector-plugins endpoint doesn't contain > information > > on the location of the plugin on the worker's file system. Do you think > we > > should still include this info in the new response format? Correct me if > > I'm wrong but it seems it may have been proposed originally to help > prevent > > already-addressed bugs in Connect classloading from striking; all else > > equal, I'd personally err on the side of leaving this info out or at > least > > reducing permitted values for it to just "classpath" or "plugin path" in > > order to avoid leaking worker file paths into the REST API, which might > > bother super security-conscious users. > > > > Cheers, > > > > Chris > > > > On Mon, Nov 29, 2021 at 5:52 AM Mickael Maison > > > wrote: > > > > > Hi Chris, > > > > > > Yes to keep compatibility we want a default implementation for > > > Converter.configs(), I've updated the KIP. > > > > > > Regarding worker plugins, the use case you described seems valuable. > > > I'd prefer not mixing worker and connector plugins on the same > > > endpoint but I agree using /plugins and /worker-plugins could be > > > confusing. > > > > > > One alternative is to expose all connector-level plugins via the > > > existing /connector-plugins endpoint. In that case, we'd need to keep > > > the current JSON schema and not group plugins by type. As the current > > > schema already has a type field for each entry, we'll still be able to > > > tell them apart. Then we can have /worker-plugins and a relatively > > > clean API. What do you think? > > > > > > Thanks, > > > Mickael > > > > > > On Sun, Nov 28, 2021 at 8:21 PM Chris Egerton > > > wrote: > > > > > > > > Hi Mickael, > > > > > > > > I think one potential use case for exposing worker-level plugins is > that > > > it > > > > may make it easier to determine whether a worker is set up correctly > (the > > > > current alternative requires looking through log files and can be a > > > little > > > > tedious), and might even make it possible to automatically identify > > > > discrepancies within a cluster by diffing the contents of that > endpoint > > > > across each worker. But I don't think this has to be addressed by the > > > > current KIP; the only thing that bothers me a little is that > "plugins" is > > > > generic and it may confuse people down the road if we add an > endpoint for > > > > worker-level plugins ("why is one just called 'plugins' and the > other one > > > > is 'worker-plugins'?"). Probably not worth blocking on, though. > > > > > > > > Agreed that the suggestion for improved validation should be made on > the > > > > KIP-802 thread. > > > > > > > > I also noticed that the newly-proposed config method for the > Converter > > > > interface doesn't have a default implementation, making it > > > > backwards-incompatible. Should we add a default implementation that > > > returns > > > > either null or an empty ConfigDef? > > > > > > > > Cheers, > > > > > > > > Chris > > > > > > > > On Fri, Nov 26, 2021 at 8:35 AM Mickael Maison < > mickael.mai...@gmail.com > > > > > > > > wrote: > > > > > > > > > Hi Chris, > > > > > > > > > > 1. If we want to expose worker plugins, I think we should do it > via a > > > > > separate endpoint. But to be
Re: KIP-769: Connect API to retrieve connector configuration definitions
Hi Chris, Thanks again for the feedback! I've updated the KIP based on our last discussions. I've decided to include the new endpoint for worker plugins. 1. Yes I agree, it's best to gate the new behavior. 2. Yes, it was a remnant from the original proposal. I've now removed the location field. Thanks, Mickael On Tue, Nov 30, 2021 at 3:22 AM Chris Egerton wrote: > > Hi Mickael, > > I think that's a great idea! I especially like how we can establish the > expectation that any plugin type that appears in the response from the GET > /connector-plugins endpoint will have a corresponding GET > /connector-plugins//config endpoint, but (if we decide to add them in > the future), worker plugins won't be expected to expose this kind of > information and the different root path helps give a decent hint about this. > > I also like the choice to return an empty ConfigDef from Converter::config > instead of null. > > Two things come to mind: > > 1. We may want to gate this behind a URL query parameter (maybe something > like "connectorsOnly") that defaults to the old behavior in order to avoid > breaking existing tools such as programmatic UIs that use the endpoint > today to discover the connectors that can be created by the user. We can > even plan to change the default for that parameter to the newly-proposed > behavior in the next major release, which should give people enough time to > either adapt to the expanded response format or add the query parameter to > their tooling. > > 2. The existing GET /connector-plugins endpoint doesn't contain information > on the location of the plugin on the worker's file system. Do you think we > should still include this info in the new response format? Correct me if > I'm wrong but it seems it may have been proposed originally to help prevent > already-addressed bugs in Connect classloading from striking; all else > equal, I'd personally err on the side of leaving this info out or at least > reducing permitted values for it to just "classpath" or "plugin path" in > order to avoid leaking worker file paths into the REST API, which might > bother super security-conscious users. > > Cheers, > > Chris > > On Mon, Nov 29, 2021 at 5:52 AM Mickael Maison > wrote: > > > Hi Chris, > > > > Yes to keep compatibility we want a default implementation for > > Converter.configs(), I've updated the KIP. > > > > Regarding worker plugins, the use case you described seems valuable. > > I'd prefer not mixing worker and connector plugins on the same > > endpoint but I agree using /plugins and /worker-plugins could be > > confusing. > > > > One alternative is to expose all connector-level plugins via the > > existing /connector-plugins endpoint. In that case, we'd need to keep > > the current JSON schema and not group plugins by type. As the current > > schema already has a type field for each entry, we'll still be able to > > tell them apart. Then we can have /worker-plugins and a relatively > > clean API. What do you think? > > > > Thanks, > > Mickael > > > > On Sun, Nov 28, 2021 at 8:21 PM Chris Egerton > > wrote: > > > > > > Hi Mickael, > > > > > > I think one potential use case for exposing worker-level plugins is that > > it > > > may make it easier to determine whether a worker is set up correctly (the > > > current alternative requires looking through log files and can be a > > little > > > tedious), and might even make it possible to automatically identify > > > discrepancies within a cluster by diffing the contents of that endpoint > > > across each worker. But I don't think this has to be addressed by the > > > current KIP; the only thing that bothers me a little is that "plugins" is > > > generic and it may confuse people down the road if we add an endpoint for > > > worker-level plugins ("why is one just called 'plugins' and the other one > > > is 'worker-plugins'?"). Probably not worth blocking on, though. > > > > > > Agreed that the suggestion for improved validation should be made on the > > > KIP-802 thread. > > > > > > I also noticed that the newly-proposed config method for the Converter > > > interface doesn't have a default implementation, making it > > > backwards-incompatible. Should we add a default implementation that > > returns > > > either null or an empty ConfigDef? > > > > > > Cheers, > > > > > > Chris > > > > > > On Fri, Nov 26, 2021 at 8:35 AM Mickael Maison > > > > > wrote: > > > > > > > Hi Chris, > > > > > > > > 1. If we want to expose worker plugins, I think we should do it via a > > > > separate endpoint. But to be honest, I'm not even sure I see strong > > > > use cases for exposing them as they are either enabled or not and > > > > can't be changed at runtime. So I'd prefer to stick to "connector > > > > level" plugins in this KIP. Let me now if you have use cases, I'm open > > > > to reconsider this choice. > > > > I'll add that in the rejected alternatives section for now > > > > > > > > 2. I remembered seeing issues in the past with multiple
[jira] [Created] (KAFKA-13496) Add reason to LeaveGroupRequest
David Jacot created KAFKA-13496: --- Summary: Add reason to LeaveGroupRequest Key: KAFKA-13496 URL: https://issues.apache.org/jira/browse/KAFKA-13496 Project: Kafka Issue Type: Sub-task Reporter: David Jacot Assignee: David Jacot -- This message was sent by Atlassian Jira (v8.20.1#820001)
[jira] [Created] (KAFKA-13495) Add reason to JoinGroupRequest
David Jacot created KAFKA-13495: --- Summary: Add reason to JoinGroupRequest Key: KAFKA-13495 URL: https://issues.apache.org/jira/browse/KAFKA-13495 Project: Kafka Issue Type: Sub-task Reporter: David Jacot Assignee: David Jacot -- This message was sent by Atlassian Jira (v8.20.1#820001)
Re: [VOTE] KIP-800: Add reason to LeaveGroupRequest
Hi all, With 4 binding +1 votes (Gwen, Mickael, Tom, Bill) and 1 non-binding +1 vote (Luke), the vote passes. Thanks everyone! Best, David On Fri, Nov 26, 2021 at 4:43 PM Bill Bejeck wrote: > > Thanks for the KIP, David this seems like it will be very helpful. > > +1(binding) > > -Bill > > On Thu, Nov 25, 2021 at 10:02 AM David Jacot > wrote: > > > Hi Tom, > > > > I do agree with you. For context, this is the current reason/message logged > > by the consumer when enforceRebalance is called so I just kept it as it is. > > I guess that we can revise it during the implementation. > > > > Thanks, > > David > > > > On Thu, Nov 25, 2021 at 3:52 PM Tom Bentley wrote: > > > > > > Thanks for the KIP David. > > > > > > It's a very trivial point, but I did wonder whether "caused" or > > "requested" > > > might be a better word than "enforced" in the default messages. > > > > > > In any case, +1 (binding). > > > > > > Kind regards, > > > > > > Tom > > > > > > > > > > > > On Thu, Nov 25, 2021 at 2:36 PM David Jacot > > > > > wrote: > > > > > > > Thanks, Mickael. I have removed it. > > > > > > > > On Thu, Nov 25, 2021 at 3:22 PM Mickael Maison < > > mickael.mai...@gmail.com> > > > > wrote: > > > > > > > > > > +1 (binding) > > > > > Thanks for the KIP > > > > > > > > > > Just a small suggestion: in other Options classes we don't seem to > > > > > prefix setters with "set", so I think we could use void reason(final > > > > > String reason). > > > > > > > > > > > > > > > On Thu, Nov 25, 2021 at 1:48 PM Luke Chen wrote: > > > > > > > > > > > > Hi David, > > > > > > Thanks for the KIP! > > > > > > It is good to have the joinGroup/leaveGroup reason sent to brokers > > for > > > > > > better troubleshooting. > > > > > > > > > > > > +1 (non-binding) > > > > > > > > > > > > Thank you. > > > > > > Luke > > > > > > > > > > > > On Thu, Nov 25, 2021 at 8:14 AM Gwen Shapira > > > > > > > > > > > > wrote: > > > > > > > > > > > > > +1 > > > > > > > > > > > > > > Thanks for driving David. Super useful. > > > > > > > > > > > > > > On Wed, Nov 24, 2021 at 8:53 AM David Jacot > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > Hi folks, > > > > > > > > > > > > > > > > I'd like to start a vote on KIP-800: Add reason to > > > > LeaveGroupRequest. > > > > > > > > > > > > > > > > KIP: https://cwiki.apache.org/confluence/x/eYyqCw > > > > > > > > > > > > > > > > Please let me know what you think. > > > > > > > > > > > > > > > > Best, > > > > > > > > David > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Gwen Shapira > > > > > > > Engineering Manager | Confluent > > > > > > > 650.450.2760 | @gwenshap > > > > > > > Follow us: Twitter | blog > > > > > > > > > > > > > > > > >
[jira] [Created] (KAFKA-13494) Session and Window Queries for IQv2
Patrick Stuedi created KAFKA-13494: -- Summary: Session and Window Queries for IQv2 Key: KAFKA-13494 URL: https://issues.apache.org/jira/browse/KAFKA-13494 Project: Kafka Issue Type: Sub-task Reporter: Patrick Stuedi Assignee: Patrick Stuedi -- This message was sent by Atlassian Jira (v8.20.1#820001)
[jira] [Created] (KAFKA-13493) Session and Window Queries for IQv2
Patrick Stuedi created KAFKA-13493: -- Summary: Session and Window Queries for IQv2 Key: KAFKA-13493 URL: https://issues.apache.org/jira/browse/KAFKA-13493 Project: Kafka Issue Type: Improvement Reporter: Patrick Stuedi Assignee: Patrick Stuedi -- This message was sent by Atlassian Jira (v8.20.1#820001)