Re: [DISCUSS] KIP-298: Error Handling in Connect

[Ewen Cheslack-Postava]

A few more thoughts -- might not change things enough to affect a vote, but
still some things to consider:

* errors.retry.delay.max.ms -- this defines the max, but I'm not seeing
where we define the actual behavior. Is this intentional, or should we just
say that it is something like exponential, based on a starting delay value?
* I'm not sure I understand tolerance vs retries? They sound generally the
same -- tolerance sounds like # of retries since it is defined in terms of
failures.
* errors.log.enable -- it's unclear why this shouldn't just be
errors.log.include.configs
|| errors.log.include.messages (and include clauses for any other flags).
If there's just some base info, that's fine, but the explanation of the
config should make that clear.
* errors.deadletterqueue.enable - similar question here about just enabling
based on other relevant configs. seems like additional config complexity
for users when the topic name is absolutely going to be a basic requirement
anyway.
* more generally related to dlq, it seems we're trying to support multiple
clusters here -- is there a reason for this? it's not that costly, but one
thing supporting this requires is an entirely separate set of configs,
ACLs, etc. in contrast, assuming an additional topic on the same cluster
we're already working with keeps things quite simple. do we think this
complexity is worth it? elsewhere, we've seen the complexity of multiple
clusters result in a lot of config confusion.
* It's not obvious throughout that the format is JSON, and I assume in many
cases it uses JsonConverter. This should be clear at the highest level, not
just in the case of things like SchemaAndValue fields. This also seems to
introduce possibly complications for DLQs -- instead of delivering the raw
data, we potentially lose raw data & schema info because we're rendering it
as JSON. Not sure that's a good idea...

I think that last item might be the biggest concern to me -- DLQ formats
and control over content & reprocessing seems a bit unclear to me here, so
I'd assume users could also end up confused.

-Ewen


On Thu, May 17, 2018 at 8:53 PM Arjun Satish  wrote:

> Konstantine,
>
> Thanks for pointing out the typos. Fixed them.
>
> I had added the JSON schema which should now include key and header configs
> in there too. This should have been in the public interfaces section.
>
> Thanks very much,
>
> On Thu, May 17, 2018 at 9:13 AM, Konstantine Karantasis <
> konstant...@confluent.io> wrote:
>
> > Thanks Arjun for your quick response.
> >
> > Adding an example for the failure log improves things, but I think it'd
> be
> > better to also add the schema definition of these Json entries. And I'll
> > agree with Magesh that this format should be public API.
> >
> > Also, does the current example have a copy/paste typo? Seems that the
> > TRANSFORMATION stage in the end has the config of a converter.
> > Similar to the above, fields for 'key' and 'headers' (and their
> conversion
> > stages) are skipped when they are not defined? Or should they present and
> > empty? A schema definition would help to know what a consumer of such
> logs
> > should expect.
> >
> > Also, thanks for adding some info for error on the source side. However,
> I
> > feel the current description might be a little bit ambiguous. I read:
> > "For errors in a source connector, the process is similar, but care needs
> > to be taken while writing back to the source." and sounds like it's
> > suggested that Connect will write records back to the source, which can't
> > be correct.
> >
> > Finally, a nit: " adds store the row information "... typo?
> >
> > Thanks,
> > - Konstantine
> >
> >
> >
> > On Thu, May 17, 2018 at 12:48 AM, Arjun Satish 
> > wrote:
> >
> > > On Wed, May 16, 2018 at 7:13 PM, Matt Farmer  wrote:
> > >
> > > > Hey Arjun,
> > > >
> > > > I like deadletterqueue all lower case, so I'm +1 on that.
> > > >
> > >
> > > Super! updated the KIP.
> > >
> > >
> > > >
> > > > Yes, in the case we were seeing there were external system failures.
> > > > We had issues connecting to S3. While the connector does include
> > > > some retry functionality, however setting these values sufficiently
> > > > high seemed to cause us to hit timeouts and cause the entire
> > > > task to fail anyway. (I think I was using something like 100 retries
> > > > during the brief test of this behavior?)
> > > >
> > >
> > > I am guessing these issues come up with trying to write to S3. Do you
> > think
> > > the S3 connector can detect the safe situations where it can throw
> > > RetriableExceptions instead of ConnectExceptions here (when the
> connector
> > > think it is safe to do so)?
> > >
> > >
> > > >
> > > > Yeah, totally understand that there could be unintended concequences
> > > > from this. I guess the use case I'm trying to optimize for is to give
> > > > folks some bubblegum to keep a high volume system limping
> > > > along until the software engineers get time to address it. So I'm
> > > > im

[jira] [Created] (KAFKA-6915) MirrorMaker: avoid duplicates when source cluster is unreachable for more than session.timeout.ms

[Fabien LD (JIRA)]

Fabien LD created KAFKA-6915:


 Summary: MirrorMaker: avoid duplicates when source cluster is 
unreachable for more than session.timeout.ms
 Key: KAFKA-6915
 URL: https://issues.apache.org/jira/browse/KAFKA-6915
 Project: Kafka
  Issue Type: Improvement
Affects Versions: 1.1.0
Reporter: Fabien LD


According to doc, see 
[https://kafka.apache.org/11/documentation.html#semantics], the exactly-once 
delivery can be achieved by storing offsets in the same store as produced data:
{quote}
When writing to an external system, the limitation is in the need to coordinate 
the consumer's position with what is actually stored as output. The classic way 
of achieving this would be to introduce a two-phase commit between the storage 
of the consumer position and the storage of the consumers output. But this can 
be handled more simply and generally by letting the consumer store its offset 
in the same place as its output
{quote}

Indeed, with current implementation where the consumer stores the offsets in 
the source cluster, we can have duplicates if networks makes source cluster 
unreachable for more than {{session.timeout.ms}}.
Indeed, once that amount of time has passed, the source cluster will rebalance 
the consumer group and later, when network is back, the generation has changed 
and consumers cannot commit the offsets for the last batches of records 
consumed (actually all records processed during the last 
{{auto.commit.interval.ms}}). So all those records are processed again when 
consumers from group are coming back.

Storing the offsets in the target cluster would resolve this risk of duplicate 
records and would be a nice feature to have.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: [VOTE] KIP-285: Connect Rest Extension Plugin

[Ewen Cheslack-Postava]

+1 (binding)

Thanks,
Ewen

On Thu, May 17, 2018 at 12:16 PM Ted Yu  wrote:

> +1
>  Original message From: Gwen Shapira 
> Date: 5/17/18  12:02 PM  (GMT-08:00) To: dev 
> Subject: Re: [VOTE] KIP-285: Connect Rest Extension Plugin
> LGTM. +1.
>
> On Wed, May 16, 2018 at 8:19 PM, Magesh Nandakumar 
> wrote:
>
> > Hello everyone,
> >
> > After a good round of discussions with excellent feedback and no major
> > objections, I would like to start a vote on KIP-285: Connect Rest
> Extension
> > Plugin.
> >
> > KIP: <
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 285%3A+Connect+Rest+Extension+Plugin
> > >
> >
> >
> > JIRA: <*https://issues.apache.org/jira/browse/KAFKA-6776
> > *>
> >
> > Discussion thread: <
> > https://www.mail-archive.com/dev@kafka.apache.org/msg86910.html>
> >
> > Thanks,
> > Magesh
> >
>
>
>
> --
> *Gwen Shapira*
> Product Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter  | blog
> 
>

Re: [DISCUSS] KIP-285: Connect Rest Extension Plugin

[Ewen Cheslack-Postava]

Yup, thanks for the changes. The 'health' package in particular feels like
a nice fit given the way we expect it to be used.

-Ewen

On Wed, May 16, 2018 at 7:02 PM Randall Hauch  wrote:

> Looks good to me. Thanks for quickly making the changes! Great work!
>
> Best regards,
>
> Randall
>
> > On May 16, 2018, at 8:07 PM, Magesh Nandakumar 
> wrote:
> >
> > Randall,
> >
> > I have adjusted the package names per Ewen's suggestions and also made
> some
> > minor edits per your suggestions. Since there are no major outstanding
> > issues, i'm moving this to vote.
> >
> > Thanks
> > Magesh
> >
> >> On Wed, May 16, 2018 at 4:38 PM, Randall Hauch 
> wrote:
> >>
> >> A few very minor suggestions:
> >>
> >>
> >>   1. There are a few formatting issues with paragraphs that use a
> >>   monospace font. Minor, but it would be nice to fix.
> >>   2. Would be nice to link to the PR
> >>   3. Do we need the org.apache.kafka.connect.rest.extension.entities
> >>   package? Could we just move the two classes into the parent
> >>   org.apache.kafka.connect.rest.extension package?
> >>   4. This sentence "The above approach helps alleviate any issues that
> >>   could arise if Extension accidentally reregister the" is cut off.
> >>   5. The "ConnectRestExtensionContext.configure(...)" method's JavaDoc
> >>   should describe the behaviors that are mentioned in the "Rest
> Extension
> >>   Integration with Connect" section; e.g., behavior when an extension
> >> adds a
> >>   resource that is already registered, whether unregistering works, etc.
> >>   Also, ideally the "close()" method would have JavaDoc that explained
> >> when
> >>   it is called (e.g., no other methods will be called on the extension
> >> after
> >>   this, etc.).
> >>   6. Packaging requirements are different for this component vs
> >>   connectors, transformations, and converters, since this now mandates
> the
> >>   Service Loader manifest file. This should be called out more
> explicitly.
> >>   7. It'd be nice if the example included how extension-specific config
> >>   properties are to be defined in the worker configuration file.
> >>
> >> As I said, these are all minor suggestions that only affect the KIP
> >> document. Once these are fixed, I think this is ready to move to voting.
> >>
> >> Best regards,
> >>
> >> Randall
> >>
> >> On Tue, May 15, 2018 at 11:30 AM, Magesh Nandakumar <
> mage...@confluent.io>
> >> wrote:
> >>
> >>> Randall- I think I have addressed all the comments. Let me know if we
> can
> >>> take this to Vote.
> >>>
> >>> Thanks
> >>> Magesh
> >>>
> >>> On Tue, May 8, 2018 at 10:12 PM, Magesh Nandakumar <
> mage...@confluent.io
> >>>
> >>> wrote:
> >>>
>  Hi All,
> 
>  I have updated the KIP to reflect changes based on the PR
>  https://github.com/apache/kafka/pull/4931. Its mostly has minor
> >> changes
>  to the interfaces and includes details on packages for the interfaces
> >> and
>  the classes. Let me know your thoughts.
> 
>  Thanks
>  Magesh
> 
>  On Fri, Apr 27, 2018 at 12:03 PM, Randall Hauch 
> >>> wrote:
> 
> > Great work, Magesh. I like the overall approach a lot, so I left some
> > pretty nuanced comments about specific details.
> >
> > Best regards,
> >
> > Randall
> >
> > On Wed, Apr 25, 2018 at 3:03 PM, Magesh Nandakumar <
> >>> mage...@confluent.io>
> > wrote:
> >
> >> Thanks Randall for your thoughts. I have created a replica of the
> > required
> >> entities in the draft implementation. If you can take a look at the
> >> PR
> > and
> >> let me know your thoughts, I will update the KIP to reflect the same
> >>
> >> https://github.com/apache/kafka/pull/4931
> >>
> >> On Tue, Apr 24, 2018 at 11:44 AM, Randall Hauch 
> > wrote:
> >>
> >>> Magesh, I think our last emails cross in mid-stream.
> >>>
> >>> We definitely want to put the new public interfaces/classes in the
> >>> API
> >>> module, and implementation in the runtime module. Yes, this will
> > affect
> >> the
> >>> design, since for example we don't want to expose runtime types to
> >>> the
> >> API,
> >>> and we want to prevent breaking changes. We don't really want to
> >>> move
> > the
> >>> REST entities if we don't have to, since that may break projects
> >>> that
> > are
> >>> extending the runtime module -- even though the runtime module is
> >>> not
> > a
> >>> public API we still want to _try_ to change things.
> >>>
> >>> Do you want to try to create a prototype to see what kind of
> >> impact
> > and
> >>> choices we'll have to make?
> >>>
> >>> Best regards,
> >>>
> >>> Randall
> >>>
> >>> On Tue, Apr 24, 2018 at 12:48 PM, Randall Hauch  >>>
> >> wrote:
> >>>
>  Thanks for updating the KIP, Magesh. You've resolved all of my
> >> concerns,
>  though I have one more: we should specify the pa

Jenkins build is back to normal : kafka-trunk-jdk8 #2652

[Apache Jenkins Server]

See 

Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Ewen Cheslack-Postava]

Thanks for addressing this Robert, it's a pretty common user need.

First, +1 (binding) generally.

Two very minor comments that I think could be clarified but wouldn't affect
votes:

* Let's list in the KIP what package the ConfigProvider,
ConfigChangeCallback, ConfigData and ConfigTransformer interfaces are
defined in. Very, very minor, but given the aim to possibly reuse elsewhere
and the fact that it'll likely end up in the common packages might mean
devs focused more on the common/core packages will have strong opinions
where they should be. I think it'd definitely be good to get input from
folks focusing on the broker on where they think it should go since I think
it would be very natural to extend this to security settings there. (Also,
I think ConfigData is left out of the list of new interfaces by accident,
but I think it's clear what's being added anyway.)
* I may have glanced past it, but we're not shipping any ConfigProviders
out of the box? This mentions file and vault, but just as examples. Just
want to make sure everyone knows up front that this is a pluggable API, but
you need to add more jars to take advantage of it. I think this is fine as
I don't think there are truly common secrets provider
formats/apis/protocols, just want to make sure it is clear.

Thanks,
Ewen

On Thu, May 17, 2018 at 6:19 PM Ted Yu  wrote:

> +1
>  Original message From: Magesh Nandakumar <
> mage...@confluent.io> Date: 5/17/18  6:05 PM  (GMT-08:00) To:
> dev@kafka.apache.org Subject: Re: [VOTE] KIP-297: Externalizing Secrets
> for Connect Configurations
> Thanks Robert, this looks great
>
> +1 (non-binding)
>
> On Thu, May 17, 2018 at 5:35 PM, Colin McCabe  wrote:
>
> > Thanks, Robert!
> >
> > +1 (non-binding)
> >
> > Colin
> >
> >
> > On Thu, May 17, 2018, at 14:15, Robert Yokota wrote:
> > > Hi Colin,
> > >
> > > I've changed the KIP to have a composite object returned from get().
> > It's
> > > probably the most straightforward option.  Please let me know if you
> have
> > > any other concerns.
> > >
> > > Thanks,
> > > Robert
> > >
> > > On Thu, May 17, 2018 at 11:44 AM, Robert Yokota 
> > wrote:
> > >
> > > >
> > > >
> > > > Hi Colin,
> > > >
> > > > My last response was not that clear, so let me back up and explain a
> > bit
> > > > more.
> > > >
> > > > Some secret managers, such as Vault (and maybe Keywhiz) have the
> > notion of
> > > > a lease duration or a TTL for a path.  Every path can have a
> different
> > > > TTL.  This is period after which the value of the keys at the given
> > path
> > > > may be invalid.  It can be used to indicate a rotation will be done.
> > In
> > > > the cause of the Vault integration with AWS, Vault will actually
> > delete the
> > > > secrets from AWS at the moment the TTL expires.  A TTL could be used
> by
> > > > other ConfigProviders, such as a FileConfigProvider, to indicate that
> > all
> > > > the secrets at a given path (file), will be rotated on a regular
> basis.
> > > >
> > > > I would like to expose the TTL in the APIs somewhere.  The TTL can be
> > made
> > > > available at the time get() is called.  Connect already has a built
> in
> > > > ScheduledExecutor, so Connect can just use the TTL to schedule a
> > Connector
> > > > restart.  Originally, I had exposed the TTL in a ConfigContext
> > interface
> > > > passed to the get() method.  To reduce the number of APIs, I placed
> it
> > on
> > > > the onChange() method.  This means at the time of get(), onChange()
> > would
> > > > be called with a TTL.  The Connector's implementation of the callback
> > would
> > > > use onChange() with the TTL to schedule a restart.
> > > >
> > > > If you think this is overloading onChange() too much, I could add the
> > > > ConfigContext back to get():
> > > >
> > > >
> > > > Map get(ConfigContext ctx, String path);
> > > >
> > > > public interface ConfigContext {
> > > >
> > > > void willExpire(String path, long ttl);
> > > >
> > > > }
> > > >
> > > >
> > > >
> > > > or I could separate out the TTL method in the callback:
> > > >
> > > >
> > > > public interface ConfigChangeCallback {
> > > >
> > > > void willExpire(String path, long ttl);
> > > >
> > > > void onChange(String path, Map values);
> > > > }
> > > >
> > > >
> > > >
> > > > Or we could return a composite object from get():
> > > >
> > > > ConfigData get(String path);
> > > >
> > > > public class ConfigData {
> > > >
> > > >   Map data;
> > > >   long ttl;
> > > >
> > > > }
> > > >
> > > >
> > > > Do you have a preference Colin?
> > > >
> > > > Thanks,
> > > > Robert
> > > >
> > > >
> > > > On Thu, May 17, 2018 at 9:27 AM, Colin McCabe 
> > wrote:
> > > >
> > > >> Hi Robert,
> > > >>
> > > >> Hmm.  I thought that if you're using ConfigChangeCallback, you are
> > > >> relying on the ConfigProvider to make a callback to you when the
> > > >> configuration has changed.  So isn't that always the "push model"
> > (where
> > > >> the ConfigProvider pushes changes to Connect).  If you want the

Re: [DISCUSS] KIP-298: Error Handling in Connect

[Arjun Satish]

Konstantine,

Thanks for pointing out the typos. Fixed them.

I had added the JSON schema which should now include key and header configs
in there too. This should have been in the public interfaces section.

Thanks very much,

On Thu, May 17, 2018 at 9:13 AM, Konstantine Karantasis <
konstant...@confluent.io> wrote:

> Thanks Arjun for your quick response.
>
> Adding an example for the failure log improves things, but I think it'd be
> better to also add the schema definition of these Json entries. And I'll
> agree with Magesh that this format should be public API.
>
> Also, does the current example have a copy/paste typo? Seems that the
> TRANSFORMATION stage in the end has the config of a converter.
> Similar to the above, fields for 'key' and 'headers' (and their conversion
> stages) are skipped when they are not defined? Or should they present and
> empty? A schema definition would help to know what a consumer of such logs
> should expect.
>
> Also, thanks for adding some info for error on the source side. However, I
> feel the current description might be a little bit ambiguous. I read:
> "For errors in a source connector, the process is similar, but care needs
> to be taken while writing back to the source." and sounds like it's
> suggested that Connect will write records back to the source, which can't
> be correct.
>
> Finally, a nit: " adds store the row information "... typo?
>
> Thanks,
> - Konstantine
>
>
>
> On Thu, May 17, 2018 at 12:48 AM, Arjun Satish 
> wrote:
>
> > On Wed, May 16, 2018 at 7:13 PM, Matt Farmer  wrote:
> >
> > > Hey Arjun,
> > >
> > > I like deadletterqueue all lower case, so I'm +1 on that.
> > >
> >
> > Super! updated the KIP.
> >
> >
> > >
> > > Yes, in the case we were seeing there were external system failures.
> > > We had issues connecting to S3. While the connector does include
> > > some retry functionality, however setting these values sufficiently
> > > high seemed to cause us to hit timeouts and cause the entire
> > > task to fail anyway. (I think I was using something like 100 retries
> > > during the brief test of this behavior?)
> > >
> >
> > I am guessing these issues come up with trying to write to S3. Do you
> think
> > the S3 connector can detect the safe situations where it can throw
> > RetriableExceptions instead of ConnectExceptions here (when the connector
> > think it is safe to do so)?
> >
> >
> > >
> > > Yeah, totally understand that there could be unintended concequences
> > > from this. I guess the use case I'm trying to optimize for is to give
> > > folks some bubblegum to keep a high volume system limping
> > > along until the software engineers get time to address it. So I'm
> > > imagining the situation that I'm paged on a Saturday night because of
> > > an intermittent network issue. With a config flag like this I could
> push
> > > a config change to cause Connect to treat that as retriable and allow
> > > me to wait until the following Monday to make changes to the code.
> > > That may not be a sensible concern for Kafka writ large, but Connect
> > > is a bit weird when compared with Streams or the Clients. It's almost
> > > more of a piece of infrastructure than a library, and I generally like
> > > infrastructure to have escape hatches like that. Just my 0.02 though.
> :)
> > >
> >
> > haha yes, it would be good to avoid those Saturday night pagers. Again, I
> > am hesitant to imply retries on ConnectExceptions. We could definitely
> > define new Exceptions in the Connector, which can be thrown to retry if
> the
> > connector thinks it is safe to do so. We need to know that a retry can be
> > super dangerous in a Task.put(List). Duplicate records can
> > easily creep in, and can be notoriously hard to detect and clean up.
> >
> >
> >
> > > Thanks,
> > > Matt
> > >
> > > On Tue, May 15, 2018 at 8:46 PM, Arjun Satish 
> > > wrote:
> > >
> > > > Matt,
> > > >
> > > > Thanks so much for your comments. Really appreciate it!
> > > >
> > > > 1. Good point about the acronym. I can use deadletterqueue instead of
> > dlq
> > > > (using all lowercase to be consistent with the other configs in
> Kafka).
> > > > What do you think?
> > > >
> > > > 2. Could you please tell us what errors caused these tasks to fail?
> > Were
> > > > they because of external system failures? And if so, could they be
> > > > implemented in the Connector itself? Or using retries with backoffs?
> > > >
> > > > 3. I like this idea. But did not include it here since it might be a
> > > > stretch. One thing to note is that ConnectExceptions can be thrown
> > from a
> > > > variety of places in a connector. I think it should be OK for the
> > > Connector
> > > > to throw RetriableException or something that extends it for the
> > > operation
> > > > to be retried. By changing this behavior, a lot of existing
> connectors
> > > > would have to be updated so that they don't rewrite messages into
> this
> > > > sink. For example, a sink connector might write some data into the
>

Re: [DISCUSS] KIP-305: Add Connect primitive number converters

[Ewen Cheslack-Postava]

Just a couple of minor points that don't really affect the implementation:

* For nulls, let's just mention the underlying serializers already support
this. I'm actually not sure why they should/need to, but given they do,
let's just defer to that implementation.
* I'm not sure where Float and Double converters are actually useful. The
use cases I know for integer serdes is for keys, but floats seem like a bad
choice for keys. These aren't a lot of overhead to build and maintain, but
if we don't know use cases for the specific types, it might be silly to
spend time and effort building and maintaining them.

Otherwise, this seems simple and straightforward. Generally +1 on the
proposal.

-Ewen

On Thu, May 17, 2018 at 6:04 PM Magesh Nandakumar 
wrote:

> Thanks Randall for the KIP. I think it will be super useful and looks
> pretty straightforward to me.
>
> Thanks
> Magesh
>
> On Thu, May 17, 2018 at 4:15 PM, Randall Hauch  wrote:
>
> > I'd like to start discussion of a very straightforward proposal for
> Connect
> > to add converters for the basic primitive number types: integer, short,
> > long, double, and float. Here is the KIP:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 305%3A+Add+Connect+primitive+number+converters
> >
> > As mentioned in the KIP, I've created a pull request (
> > https://github.com/apache/kafka/pull/5034) for those looking for
> > implementation details.
> >
> > Any feedback is appreciated.
> >
> > Best regards,
> >
> > Randall
> >
>

Jenkins build is back to normal : kafka-trunk-jdk7 #3435

[Apache Jenkins Server]

See 

Build failed in Jenkins: kafka-trunk-jdk10 #112

[Apache Jenkins Server]

See 


Changes:

[wangguoz] HOTFIX: use ConsumedInternal in StreamsBuilder

--
[...truncated 1.50 MB...]

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToEarliest PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsExportImportPlan 
STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsExportImportPlan 
PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToSpecificOffset 
STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToSpecificOffset 
PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsShiftPlus STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsShiftPlus PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLatest STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLatest PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsNewConsumerExistingTopic STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsNewConsumerExistingTopic PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsShiftByLowerThanEarliest STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsShiftByLowerThanEarliest PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsByDuration STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsByDuration PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLocalDateTime 
STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLocalDateTime 
PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsToEarliestOnTopicsAndPartitions STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsToEarliestOnTopicsAndPartitions PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToEarliestOnTopics 
STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToEarliestOnTopics 
PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfBlankArg STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfBlankArg PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowVerifyWithoutReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowVerifyWithoutReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowTopicsOptionWithVerify STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowTopicsOptionWithVerify PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithThrottleOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithThrottleOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfNoArgs STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfNoArgs PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithoutReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithoutReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowBrokersListWithVerifyOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowBrokersListWithVerifyOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumExecuteOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumExecuteOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumGenerateOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumGenerateOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersAndTopicsOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersAndTopicsOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowThrottleWithVerifyOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowThrottleWithVerifyOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldUseDefaultsIfEnabled 
STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldUseDefaultsIfEnabled 
PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldAllowThrottleOptionOnExecute STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldAllowThrottleOptionOnExecute PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowEx

Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Ted Yu]

+1
 Original message From: Magesh Nandakumar 
 Date: 5/17/18  6:05 PM  (GMT-08:00) To: 
dev@kafka.apache.org Subject: Re: [VOTE] KIP-297: Externalizing Secrets for 
Connect Configurations 
Thanks Robert, this looks great

+1 (non-binding)

On Thu, May 17, 2018 at 5:35 PM, Colin McCabe  wrote:

> Thanks, Robert!
>
> +1 (non-binding)
>
> Colin
>
>
> On Thu, May 17, 2018, at 14:15, Robert Yokota wrote:
> > Hi Colin,
> >
> > I've changed the KIP to have a composite object returned from get().
> It's
> > probably the most straightforward option.  Please let me know if you have
> > any other concerns.
> >
> > Thanks,
> > Robert
> >
> > On Thu, May 17, 2018 at 11:44 AM, Robert Yokota 
> wrote:
> >
> > >
> > >
> > > Hi Colin,
> > >
> > > My last response was not that clear, so let me back up and explain a
> bit
> > > more.
> > >
> > > Some secret managers, such as Vault (and maybe Keywhiz) have the
> notion of
> > > a lease duration or a TTL for a path.  Every path can have a different
> > > TTL.  This is period after which the value of the keys at the given
> path
> > > may be invalid.  It can be used to indicate a rotation will be done.
> In
> > > the cause of the Vault integration with AWS, Vault will actually
> delete the
> > > secrets from AWS at the moment the TTL expires.  A TTL could be used by
> > > other ConfigProviders, such as a FileConfigProvider, to indicate that
> all
> > > the secrets at a given path (file), will be rotated on a regular basis.
> > >
> > > I would like to expose the TTL in the APIs somewhere.  The TTL can be
> made
> > > available at the time get() is called.  Connect already has a built in
> > > ScheduledExecutor, so Connect can just use the TTL to schedule a
> Connector
> > > restart.  Originally, I had exposed the TTL in a ConfigContext
> interface
> > > passed to the get() method.  To reduce the number of APIs, I placed it
> on
> > > the onChange() method.  This means at the time of get(), onChange()
> would
> > > be called with a TTL.  The Connector's implementation of the callback
> would
> > > use onChange() with the TTL to schedule a restart.
> > >
> > > If you think this is overloading onChange() too much, I could add the
> > > ConfigContext back to get():
> > >
> > >
> > > Map get(ConfigContext ctx, String path);
> > >
> > > public interface ConfigContext {
> > >
> > > void willExpire(String path, long ttl);
> > >
> > > }
> > >
> > >
> > >
> > > or I could separate out the TTL method in the callback:
> > >
> > >
> > > public interface ConfigChangeCallback {
> > >
> > > void willExpire(String path, long ttl);
> > >
> > > void onChange(String path, Map values);
> > > }
> > >
> > >
> > >
> > > Or we could return a composite object from get():
> > >
> > > ConfigData get(String path);
> > >
> > > public class ConfigData {
> > >
> > >   Map data;
> > >   long ttl;
> > >
> > > }
> > >
> > >
> > > Do you have a preference Colin?
> > >
> > > Thanks,
> > > Robert
> > >
> > >
> > > On Thu, May 17, 2018 at 9:27 AM, Colin McCabe 
> wrote:
> > >
> > >> Hi Robert,
> > >>
> > >> Hmm.  I thought that if you're using ConfigChangeCallback, you are
> > >> relying on the ConfigProvider to make a callback to you when the
> > >> configuration has changed.  So isn't that always the "push model"
> (where
> > >> the ConfigProvider pushes changes to Connect).  If you want the "pull
> > >> model" where you initiate updates, you can simply call
> ConfigProvider#get
> > >> directly, right?
> > >>
> > >> The actual implementation of ConfigProvider subclasses will depend on
> the
> > >> type of configuration storage mechanism on the backend.  In the case
> of
> > >> Vault, it sounds like we need to have something like a
> ScheduledExecutor
> > >> which re-fetches keys after a certain amount of time.
> > >>
> > >> As an aside, what does a "lease duration" mean for a configuration
> key?
> > >> Does that mean Vault will reject changes to the configuration key if
> I try
> > >> to make them within the lease duration?  Or is this like a period
> after
> > >> which a password is automatically rotated?
> > >>
> > >> On Wed, May 16, 2018, at 22:25, Robert Yokota wrote:
> > >> > Hi Colin,
> > >> >
> > >> > > With regard to delayMs, can’t we just restart the
> > >> > > Connector when the keys are actually changed?
> > >> >
> > >> > Currently the VaultConfigProvider does not find out when values for
> keys
> > >> > have changed.  You could do this with a poll model (with a
> > >> > background thread in the ConfigProvider), but since for each
> key-value
> > >> > pair, Vault provides a lease duration stating exactly when a value
> for a
> > >> > key will change in the future, this is an alternative model of just
> > >> passing
> > >> > the lease duration to the client (in this case the Connector), to
> allow
> > >> it
> > >> > to determine what to do (such as schedule a restart).   This may
> allow
> > >> one
> > >> > to avoid the complexity of figuring out a proper poll interval (with
>

Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Magesh Nandakumar]

Thanks Robert, this looks great

+1 (non-binding)

On Thu, May 17, 2018 at 5:35 PM, Colin McCabe  wrote:

> Thanks, Robert!
>
> +1 (non-binding)
>
> Colin
>
>
> On Thu, May 17, 2018, at 14:15, Robert Yokota wrote:
> > Hi Colin,
> >
> > I've changed the KIP to have a composite object returned from get().
> It's
> > probably the most straightforward option.  Please let me know if you have
> > any other concerns.
> >
> > Thanks,
> > Robert
> >
> > On Thu, May 17, 2018 at 11:44 AM, Robert Yokota 
> wrote:
> >
> > >
> > >
> > > Hi Colin,
> > >
> > > My last response was not that clear, so let me back up and explain a
> bit
> > > more.
> > >
> > > Some secret managers, such as Vault (and maybe Keywhiz) have the
> notion of
> > > a lease duration or a TTL for a path.  Every path can have a different
> > > TTL.  This is period after which the value of the keys at the given
> path
> > > may be invalid.  It can be used to indicate a rotation will be done.
> In
> > > the cause of the Vault integration with AWS, Vault will actually
> delete the
> > > secrets from AWS at the moment the TTL expires.  A TTL could be used by
> > > other ConfigProviders, such as a FileConfigProvider, to indicate that
> all
> > > the secrets at a given path (file), will be rotated on a regular basis.
> > >
> > > I would like to expose the TTL in the APIs somewhere.  The TTL can be
> made
> > > available at the time get() is called.  Connect already has a built in
> > > ScheduledExecutor, so Connect can just use the TTL to schedule a
> Connector
> > > restart.  Originally, I had exposed the TTL in a ConfigContext
> interface
> > > passed to the get() method.  To reduce the number of APIs, I placed it
> on
> > > the onChange() method.  This means at the time of get(), onChange()
> would
> > > be called with a TTL.  The Connector's implementation of the callback
> would
> > > use onChange() with the TTL to schedule a restart.
> > >
> > > If you think this is overloading onChange() too much, I could add the
> > > ConfigContext back to get():
> > >
> > >
> > > Map get(ConfigContext ctx, String path);
> > >
> > > public interface ConfigContext {
> > >
> > > void willExpire(String path, long ttl);
> > >
> > > }
> > >
> > >
> > >
> > > or I could separate out the TTL method in the callback:
> > >
> > >
> > > public interface ConfigChangeCallback {
> > >
> > > void willExpire(String path, long ttl);
> > >
> > > void onChange(String path, Map values);
> > > }
> > >
> > >
> > >
> > > Or we could return a composite object from get():
> > >
> > > ConfigData get(String path);
> > >
> > > public class ConfigData {
> > >
> > >   Map data;
> > >   long ttl;
> > >
> > > }
> > >
> > >
> > > Do you have a preference Colin?
> > >
> > > Thanks,
> > > Robert
> > >
> > >
> > > On Thu, May 17, 2018 at 9:27 AM, Colin McCabe 
> wrote:
> > >
> > >> Hi Robert,
> > >>
> > >> Hmm.  I thought that if you're using ConfigChangeCallback, you are
> > >> relying on the ConfigProvider to make a callback to you when the
> > >> configuration has changed.  So isn't that always the "push model"
> (where
> > >> the ConfigProvider pushes changes to Connect).  If you want the "pull
> > >> model" where you initiate updates, you can simply call
> ConfigProvider#get
> > >> directly, right?
> > >>
> > >> The actual implementation of ConfigProvider subclasses will depend on
> the
> > >> type of configuration storage mechanism on the backend.  In the case
> of
> > >> Vault, it sounds like we need to have something like a
> ScheduledExecutor
> > >> which re-fetches keys after a certain amount of time.
> > >>
> > >> As an aside, what does a "lease duration" mean for a configuration
> key?
> > >> Does that mean Vault will reject changes to the configuration key if
> I try
> > >> to make them within the lease duration?  Or is this like a period
> after
> > >> which a password is automatically rotated?
> > >>
> > >> On Wed, May 16, 2018, at 22:25, Robert Yokota wrote:
> > >> > Hi Colin,
> > >> >
> > >> > > With regard to delayMs, can’t we just restart the
> > >> > > Connector when the keys are actually changed?
> > >> >
> > >> > Currently the VaultConfigProvider does not find out when values for
> keys
> > >> > have changed.  You could do this with a poll model (with a
> > >> > background thread in the ConfigProvider), but since for each
> key-value
> > >> > pair, Vault provides a lease duration stating exactly when a value
> for a
> > >> > key will change in the future, this is an alternative model of just
> > >> passing
> > >> > the lease duration to the client (in this case the Connector), to
> allow
> > >> it
> > >> > to determine what to do (such as schedule a restart).   This may
> allow
> > >> one
> > >> > to avoid the complexity of figuring out a proper poll interval (with
> > >> lease
> > >> > durations of varying periods), or worrying about putting too much
> load
> > >> on
> > >> > the secrets manager by polling too often.
> > >>
> > >> Those things are still concerns if t

Re: [DISCUSS] KIP-305: Add Connect primitive number converters

[Magesh Nandakumar]

Thanks Randall for the KIP. I think it will be super useful and looks
pretty straightforward to me.

Thanks
Magesh

On Thu, May 17, 2018 at 4:15 PM, Randall Hauch  wrote:

> I'd like to start discussion of a very straightforward proposal for Connect
> to add converters for the basic primitive number types: integer, short,
> long, double, and float. Here is the KIP:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 305%3A+Add+Connect+primitive+number+converters
>
> As mentioned in the KIP, I've created a pull request (
> https://github.com/apache/kafka/pull/5034) for those looking for
> implementation details.
>
> Any feedback is appreciated.
>
> Best regards,
>
> Randall
>

Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Colin McCabe]

Thanks, Robert!

+1 (non-binding)

Colin


On Thu, May 17, 2018, at 14:15, Robert Yokota wrote:
> Hi Colin,
> 
> I've changed the KIP to have a composite object returned from get().  It's
> probably the most straightforward option.  Please let me know if you have
> any other concerns.
> 
> Thanks,
> Robert
> 
> On Thu, May 17, 2018 at 11:44 AM, Robert Yokota  wrote:
> 
> >
> >
> > Hi Colin,
> >
> > My last response was not that clear, so let me back up and explain a bit
> > more.
> >
> > Some secret managers, such as Vault (and maybe Keywhiz) have the notion of
> > a lease duration or a TTL for a path.  Every path can have a different
> > TTL.  This is period after which the value of the keys at the given path
> > may be invalid.  It can be used to indicate a rotation will be done.  In
> > the cause of the Vault integration with AWS, Vault will actually delete the
> > secrets from AWS at the moment the TTL expires.  A TTL could be used by
> > other ConfigProviders, such as a FileConfigProvider, to indicate that all
> > the secrets at a given path (file), will be rotated on a regular basis.
> >
> > I would like to expose the TTL in the APIs somewhere.  The TTL can be made
> > available at the time get() is called.  Connect already has a built in
> > ScheduledExecutor, so Connect can just use the TTL to schedule a Connector
> > restart.  Originally, I had exposed the TTL in a ConfigContext interface
> > passed to the get() method.  To reduce the number of APIs, I placed it on
> > the onChange() method.  This means at the time of get(), onChange() would
> > be called with a TTL.  The Connector's implementation of the callback would
> > use onChange() with the TTL to schedule a restart.
> >
> > If you think this is overloading onChange() too much, I could add the
> > ConfigContext back to get():
> >
> >
> > Map get(ConfigContext ctx, String path);
> >
> > public interface ConfigContext {
> >
> > void willExpire(String path, long ttl);
> >
> > }
> >
> >
> >
> > or I could separate out the TTL method in the callback:
> >
> >
> > public interface ConfigChangeCallback {
> >
> > void willExpire(String path, long ttl);
> >
> > void onChange(String path, Map values);
> > }
> >
> >
> >
> > Or we could return a composite object from get():
> >
> > ConfigData get(String path);
> >
> > public class ConfigData {
> >
> >   Map data;
> >   long ttl;
> >
> > }
> >
> >
> > Do you have a preference Colin?
> >
> > Thanks,
> > Robert
> >
> >
> > On Thu, May 17, 2018 at 9:27 AM, Colin McCabe  wrote:
> >
> >> Hi Robert,
> >>
> >> Hmm.  I thought that if you're using ConfigChangeCallback, you are
> >> relying on the ConfigProvider to make a callback to you when the
> >> configuration has changed.  So isn't that always the "push model" (where
> >> the ConfigProvider pushes changes to Connect).  If you want the "pull
> >> model" where you initiate updates, you can simply call ConfigProvider#get
> >> directly, right?
> >>
> >> The actual implementation of ConfigProvider subclasses will depend on the
> >> type of configuration storage mechanism on the backend.  In the case of
> >> Vault, it sounds like we need to have something like a ScheduledExecutor
> >> which re-fetches keys after a certain amount of time.
> >>
> >> As an aside, what does a "lease duration" mean for a configuration key?
> >> Does that mean Vault will reject changes to the configuration key if I try
> >> to make them within the lease duration?  Or is this like a period after
> >> which a password is automatically rotated?
> >>
> >> On Wed, May 16, 2018, at 22:25, Robert Yokota wrote:
> >> > Hi Colin,
> >> >
> >> > > With regard to delayMs, can’t we just restart the
> >> > > Connector when the keys are actually changed?
> >> >
> >> > Currently the VaultConfigProvider does not find out when values for keys
> >> > have changed.  You could do this with a poll model (with a
> >> > background thread in the ConfigProvider), but since for each key-value
> >> > pair, Vault provides a lease duration stating exactly when a value for a
> >> > key will change in the future, this is an alternative model of just
> >> passing
> >> > the lease duration to the client (in this case the Connector), to allow
> >> it
> >> > to determine what to do (such as schedule a restart).   This may allow
> >> one
> >> > to avoid the complexity of figuring out a proper poll interval (with
> >> lease
> >> > durations of varying periods), or worrying about putting too much load
> >> on
> >> > the secrets manager by polling too often.
> >>
> >> Those things are still concerns if the Connector is polling, right?
> >> Perhaps the connector poll too often and puts too much load on Vault.  And
> >> so forth.  It seems like this problem needs to be solved either way (and
> >> probably can be solved with reasonable default minimum fetch intervals).
> >>
> >> best,
> >> Colin
> >>
> >>
> >> >  In other words, by adding this
> >> > one additional parameter, a ConfigProvider can provide both push an

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

Actually, I just realized that this won't work.  The AlterConfigs API is kind 
of broken right now.  DescribeConfigs won't return the "sensitive" 
configurations like passwords.  So doing describe + edit + alter will wipe out 
all sensitive configs. :(

I think we should probably just create a flag for alterConfigs which marks it 
as incremental, like we discussed earlier, and do this as a compatible change 
that is needed for the shell command.

best,
Colin


On Thu, May 17, 2018, at 09:32, Colin McCabe wrote:
> Hi Viktor,
> 
> Since the KIP freeze is coming up really soon, maybe we should just drop 
> the section about changes to AlterConfigs from KIP-248.  We don't really 
> need it here, since ConfigCommand can use AlterConfigs as-is.
> 
> We can pick up the discussion about improving AlterConfigs in a future KIP.
> 
> cheers,
> Colin
> 
> On Wed, May 16, 2018, at 22:06, Colin McCabe wrote:
> > Hi Viktor,
> > 
> > The shell command isn’t that easy to integrate into applications.
> > AdminClient will get integrated  into a lot more stuff, which
> > increases the potential for conflicts.  I would argue that we should
> > fix this soon.
> > If we do want to reduce the scope in this KIP, we could do the merge in
> > the ConfigCommand  tool for now, and leave AC unchanged.
> > Best,
> > Colin
> > 
> > 
> > On Wed, May 16, 2018, at 04:57, Viktor Somogyi wrote:
> > > Hi Colin,
> > >
> > > > Doing get-merge-set is buggy, though.  If someone else does get-merge-
> > > > set at the same time as you, you might overwrite that person's
> > > > changes, or vice versa.  So I really don't think we should try to do
> > > > this.  Also, having both an incremental and a full API is useful,
> > > > and it's just a single boolean at the protocol and API level.>
> > > Overwriting somebody's change is currently possible with the
> > > ConfigCommand, as it will do this get-merge-set behavior on the client> 
> > > side, in the command. From this perspective I think it's not much
> > > different to do this with the admin client. Also I think admins don't> 
> > > modify the quotas/configs of a client/user/topic/broker often (and
> > > multiple admins would do it even more rarely), so I don't think it is> a 
> > > big issue. What I think would be useful here but may be out of scope> is 
> > > to version the changes similarly to leader epochs. So when an admin> 
> > > updates the configs, it will increment a version number and won't let> 
> > > other admins to push changes in with lower than that. Instead it would> 
> > > return an error.
> > >
> > > I would be also interested what others think about this?
> > >
> > > Cheers,
> > > Viktor
> > >
> > >
> > > On Sat, May 12, 2018 at 2:29 AM, Colin McCabe
> > >  wrote:> > On Wed, May 9, 2018, at 05:41, Viktor 
> > > Somogyi wrote:
> > > >> Hi Colin,
> > > >>
> > > >> > We are going to need to create a new version of
> > > >> > AlterConfigsRequest to add the "incremental" boolean.  So while
> > > >> > we're doing that, maybe we can change the type to
> > > >> > NULLABLE_STRING.> >>
> > > >> I was just talking to a colleague yesterday and we came to the
> > > >> conclusion that we should keep the boolean flag only on the client> >> 
> > > >> side (as you may have suggested earlier?) and not make part of the> >> 
> > > >> protocol as it might lead to a very complicated API on the long
> > > >> term.> >> Also we would keep the server side API simpler. Instead of 
> > > >> the
> > > >> protocol change we could just simply have the boolean flag in
> > > >> AlterConfigOptions and the AdminClient should do the get-merge-set> >> 
> > > >> logic which corresponds to the behavior of the current
> > > >> ConfigCommand.> >> That way we won't need to change the protocol for 
> > > >> now but
> > > >> still have> >> both functionality. What do you think?
> > > >
> > > >  Hi Viktor,
> > > >
> > > > Doing get-merge-set is buggy, though.  If someone else does get-merge-
> > > > set at the same time as you, you might overwrite that person's
> > > > changes, or vice versa.  So I really don't think we should try to do
> > > > this.  Also, having both an incremental and a full API is useful,
> > > > and it's just a single boolean at the protocol and API level.> >
> > > >>
> > > >> > Hmm.  Not sure I follow.  KIP-133 doesn't use the empty string or
> > > >> > "" to indicate defaults, does it?> >>
> > > >> No it doesn't. It was just my early idea to indicate "delete"
> > > >> on the> >> protocol level. (We are using  for denoting the 
> > > >> default
> > > >> client id or user in zookeeper.) Rajini was referring that we
> > > >> shouldn't expose this to the protocol level but instead denote
> > > >> delete> >> with an empty string.
> > > >>
> > > >> > This comes from DescribeConfigsResponse.
> > > >> > Unless I'm missing something, I think your suggestion to not
> > > >> > expose "" is already implemented?> >>
> > > >> In some way, yes. Although this one is used in describe and not in> >> 
> > > >> alter. F

[jira] [Created] (KAFKA-6914) Kafka Connect - Plugins class should have a constructor that can take in parent ClassLoader

[Sriram KS (JIRA)]

Sriram KS created KAFKA-6914:


 Summary: Kafka Connect - Plugins class should have a constructor 
that can take in parent ClassLoader
 Key: KAFKA-6914
 URL: https://issues.apache.org/jira/browse/KAFKA-6914
 Project: Kafka
  Issue Type: Bug
Reporter: Sriram KS


Currently Plugins class has a single constructor that takes in map of props.

Please make Plugin class to have a constructor that takes in a classLoader as 
well and use it to set DelegationClassLoader's parent classLoader.

Reason:

This will be useful if i am already having a managed class Loader environment 
like a Spring boot app which resolves my class dependencies using my 
maven/gradle dependency management.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[DISCUSS] KIP-305: Add Connect primitive number converters

[Randall Hauch]

I'd like to start discussion of a very straightforward proposal for Connect
to add converters for the basic primitive number types: integer, short,
long, double, and float. Here is the KIP:

https://cwiki.apache.org/confluence/display/KAFKA/KIP-305%3A+Add+Connect+primitive+number+converters

As mentioned in the KIP, I've created a pull request (
https://github.com/apache/kafka/pull/5034) for those looking for
implementation details.

Any feedback is appreciated.

Best regards,

Randall

Re: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS resolved IP addresses

[Edoardo Comar]

Hi Jonathan,

> A solution might be to expose to users the choice of using hostname or 
> canonical host name on both sides.
> Say having one setting that collapses functionalities from both KIPs 
> (bootstrap expansion + advertised lookup)
> and an additional parameter that defines how the resolution is performed, 
> using getCanonicalHostName() or not.

thanks sounds to me *less* simple than independent config options, sorry.

I would like to say once again that by itself  KIP-302 only speeds up
the client behavior that can happen anyway when the client restarts
multiple times,
as every time there is no guarantee that - in presence of multiple A
DNS records - the same IP is returned. Attempting to use additiona IPs
if the first fail just makes client recovery faster.

cheers
Edo

On 17 May 2018 at 12:12, Skrzypek, Jonathan  wrote:
> Yes, makes sense.
> You mentioned multiple times you see no overlap and no issue with your KIP, 
> and that they solve different use cases.
>
> Appreciate you have an existing use case that would work, but we need to make 
> sure this isn't confusing to users and that any combination will always work, 
> across security protocols.
>
> A solution might be to expose to users the choice of using hostname or 
> canonical host name on both sides.
> Say having one setting that collapses functionalities from both KIPs 
> (bootstrap expansion + advertised lookup) and an additional parameter that 
> defines how the resolution is performed, using getCanonicalHostName() or not.
>
> Maybe that gives less flexibility as users wouldn't be able to decide to only 
> perform DNS lookup on bootstrap.servers or on advertised listeners.
> But this would ensure consistency so that a user can decide to use cnames or 
> not (depending on their certificates and Kerberos principals in their 
> environment) and it would work.
>
> Jonathan Skrzypek
>
> -Original Message-
> From: Edoardo Comar [mailto:edoco...@gmail.com]
> Sent: 16 May 2018 21:59
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS resolved 
> IP addresses
>
> Hi Jonathan,
> I am afraid that may not work for everybody.
>
> It would not work for us.
> With our current DNS, my Kafka clients are perfectly happy to use any IPs -
> DNS has multiple A records for the 'myhostname.mydomain' used for
> bootstrap and advertised listeners.
> The hosts all serve TLS certificates that include
> 'myhostname.mydomain'  and the clients are happy.
>
> However, applying getCanonicalHostName to those IPs would return
> hostnames that would not match the TLS certificates.
>
> So once again I believe your solution and ours solve different use cases.
>
> cheers
> Edo
>
> On 16 May 2018 at 18:29, Skrzypek, Jonathan  wrote:
>> I think there are combinations that will break SASL and SSL auth.
>> Could the trick be to have a single parameter that triggers dns resolve both 
>> for bootstrap and advertised listeners, both using getCanonicalHostName() ?
>>
>> Jonathan Skrzypek
>>
>> -Original Message-
>> From: Edoardo Comar [mailto:edoco...@gmail.com]
>> Sent: 16 May 2018 17:03
>> To: dev@kafka.apache.org
>> Subject: Re: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS 
>> resolved IP addresses
>>
>> Hi Rajini,
>>
>> In your example KIP-302 would attempt to connect to the first address
>> returned, let's say
>>
>> www.apache.org/195.154.151.36
>>
>> then, only if that fails, will in turn try the remaining:
>>
>> www.apache.org/40.79.78.1
>> www.apache.org/140.211.11.105
>> www.apache.org/2001:bc8:2142:300:0:0:0:0
>>
>> You're right to say that we expect certificates served by those
>> endpoints to be valid for "www.apache.org"
>>
>> Without KIP-302, only one would be attempted.
>> Which is the first one, that can change every time
>> (typically changes on every Java process restart,
>> but may change also any time InetAddress.getAllByName it's invoked
>> depending on the caching).
>>
>> The behavioral change that KIP-302 may introduce is that in the example 
>> above,
>> also an IPv6 connection may be attempted after some IPv4 ones.
>>
>> InetAddress.getAllByName() implementation uses a system property
>> "java.net.preferIPv6Addresses"
>> to decide which type of address to return first (default is still IPv4
>> in java 10)
>>
>> We will amend the KIP and PR so that the loop only uses IPs of the
>> same type as the first one returned.
>>
>> A part from the above, KIP 302 does not seem to change any existing
>> client behaviour, as any one of multiple IP addresses (of a given
>> v4/v6 type) can currently be picked.
>> We're happy however to keep the looping behavior optional with the
>> discussed config property, disabled by default.
>>
>> As for KIP-235 that may introduce new hostnames in the bootstrap list
>> (the current PR rewrites the bootstrap list)
>> and we fail to see the conflict with KIP-302, whatever the set of
>> configs chosen.
>>
>> We'd be happy to try understand what we are missing i

Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology Sink

[John Roesler]

+1 non-binding

On Thu, May 17, 2018 at 4:44 PM, Matthias J. Sax 
wrote:

> +1 (binding)
>
>
> On 5/17/18 12:18 PM, Ted Yu wrote:
> > +1
> >  Original message From: Gwen Shapira 
> Date: 5/17/18  11:53 AM  (GMT-08:00) To: dev 
> Subject: Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams'
> Topology Sink
> > Yay, its about time :)
> >
> > +1
> >
> > On Thu, May 17, 2018 at 12:38 PM, Guozhang Wang 
> wrote:
> >
> >> Hello folks,
> >>
> >> I'd like to start a voting thread on adding dynamic routing
> functionality
> >> in Streams sink node. Please find a KIP here:
> >>
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> 303%3A+Add+Dynamic+Routing+in+Streams+Sink
> >>
> >>
> >> And the PR itself ready for review as well under KAFKA-4936:
> >>
> >> https://github.com/apache/kafka/pull/5018
> >>
> >>
> >>
> >> Thanks!
> >> -- Guozhang
> >>
> >
> >
> >
>
>

Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology Sink

[Matthias J. Sax]

+1 (binding)


On 5/17/18 12:18 PM, Ted Yu wrote:
> +1
>  Original message From: Gwen Shapira  
> Date: 5/17/18  11:53 AM  (GMT-08:00) To: dev  Subject: 
> Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology 
> Sink 
> Yay, its about time :)
> 
> +1
> 
> On Thu, May 17, 2018 at 12:38 PM, Guozhang Wang  wrote:
> 
>> Hello folks,
>>
>> I'd like to start a voting thread on adding dynamic routing functionality
>> in Streams sink node. Please find a KIP here:
>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> 303%3A+Add+Dynamic+Routing+in+Streams+Sink
>>
>>
>> And the PR itself ready for review as well under KAFKA-4936:
>>
>> https://github.com/apache/kafka/pull/5018
>>
>>
>>
>> Thanks!
>> -- Guozhang
>>
> 
> 
> 



signature.asc
Description: OpenPGP digital signature

Build failed in Jenkins: kafka-trunk-jdk8 #2651

[Apache Jenkins Server]

See 


Changes:

[github] KAFKA-6729: Reuse source topics for source KTable's materialized 
store's

[github] HOTFIX: move Conusmed to o.a.k.streams.kstream (#5033)

--
[...truncated 1.48 MB...]
org.apache.kafka.clients.admin.KafkaAdminClientTest > 
testDescribeConsumerGroups PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testDescribeConfigs 
STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testDescribeConfigs PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testCreateAcls STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testCreateAcls PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testCloseAdminClient 
STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testCloseAdminClient 
PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > 
testPropagatedMetadataFetchException STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > 
testPropagatedMetadataFetchException PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testCreatePartitions 
STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testCreatePartitions 
PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > 
testAdminClientApisAuthenticationFailure STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > 
testAdminClientApisAuthenticationFailure PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testListConsumerGroups 
STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testListConsumerGroups 
PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testDeleteConsumerGroups 
STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testDeleteConsumerGroups 
PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testDeleteAcls STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testDeleteAcls PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testHandleTimeout STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testHandleTimeout SKIPPED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testGenerateClientId 
STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > testGenerateClientId 
PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > 
testDescribeConsumerGroupOffsets STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > 
testDescribeConsumerGroupOffsets PASSED

org.apache.kafka.clients.admin.KafkaAdminClientTest > 
testCalcTimeoutMsRemainingAsInt STARTED

org.apache.kafka.clients.admin.KafkaAdminClientTest > 
testCalcTimeoutMsRemainingAsInt PASSED

org.apache.kafka.clients.CommonClientConfigsTest > 
testExponentialBackoffDefaults STARTED

org.apache.kafka.clients.CommonClientConfigsTest > 
testExponentialBackoffDefaults PASSED
:clients:determineCommitId UP-TO-DATE
:clients:createVersionFile
:clients:jar UP-TO-DATE
:core:compileJava NO-SOURCE
:core:compileScala UP-TO-DATE
:core:processResources NO-SOURCE
:core:classes UP-TO-DATE
:core:copyDependantLibs
:core:jar
:examples:compileJavawarning: [options] bootstrap class path not set in 
conjunction with -source 1.7
:19:
 warning: [deprecation] FetchRequest in kafka.api has been deprecated
import kafka.api.FetchRequest;
^
:20:
 warning: [deprecation] FetchRequestBuilder in kafka.api has been deprecated
import kafka.api.FetchRequestBuilder;
^
:22:
 warning: [deprecation] SimpleConsumer in kafka.javaapi.consumer has been 
deprecated
import kafka.javaapi.consumer.SimpleConsumer;
 ^
4 warnings

:examples:processResources NO-SOURCE
:examples:classes
:examples:checkstyleMain
:examples:compileTestJava NO-SOURCE
:examples:processTestResources NO-SOURCE
:examples:testClasses UP-TO-DATE
:examples:checkstyleTest NO-SOURCE
:examples:findbugsMain
:examples:test NO-SOURCE
:log4j-appender:compileJavawarning: [options] bootstrap class path not set in 
conjunction with -source 1.7
1 warning

:log4j-appender:processResources NO-SOURCE
:log4j-appender:classes
:log4j-appender:checkstyleMain
:log4j-appender:compileTestJavawarning: [options] bootstrap class path not set 
in conjunction with -source 1.7
1 warning

:log4j-appender:processTestResources NO-SOURCE
:log4j-appender:testClasses
:log4j-appender:checkstyleTest
:log4j-appender:findbugsMain
:log4j-appender:test

org.apache.kafka.log4jappender.KafkaLog4jAppenderTest > testLog4jAppends STARTED

org.apache.kafka.log4jappender.KafkaLog4jAppenderTest > testLog4jAppends PASSED

org.apache.kafka.log4jappender.KafkaLog4jAppenderTest > testKafka

[jira] [Created] (KAFKA-6913) Add primitive numeric converters to Connect

[Randall Hauch (JIRA)]

Randall Hauch created KAFKA-6913:


 Summary: Add primitive numeric converters to Connect
 Key: KAFKA-6913
 URL: https://issues.apache.org/jira/browse/KAFKA-6913
 Project: Kafka
  Issue Type: Improvement
  Components: KafkaConnect
Affects Versions: 1.1.0
Reporter: Randall Hauch
Assignee: Randall Hauch


Kafka common includes serdes for long, int, short, float, and double types, but 
Connect does not have converters for these. They should support null.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Robert Yokota]

Hi Colin,

I've changed the KIP to have a composite object returned from get().  It's
probably the most straightforward option.  Please let me know if you have
any other concerns.

Thanks,
Robert

On Thu, May 17, 2018 at 11:44 AM, Robert Yokota  wrote:

>
>
> Hi Colin,
>
> My last response was not that clear, so let me back up and explain a bit
> more.
>
> Some secret managers, such as Vault (and maybe Keywhiz) have the notion of
> a lease duration or a TTL for a path.  Every path can have a different
> TTL.  This is period after which the value of the keys at the given path
> may be invalid.  It can be used to indicate a rotation will be done.  In
> the cause of the Vault integration with AWS, Vault will actually delete the
> secrets from AWS at the moment the TTL expires.  A TTL could be used by
> other ConfigProviders, such as a FileConfigProvider, to indicate that all
> the secrets at a given path (file), will be rotated on a regular basis.
>
> I would like to expose the TTL in the APIs somewhere.  The TTL can be made
> available at the time get() is called.  Connect already has a built in
> ScheduledExecutor, so Connect can just use the TTL to schedule a Connector
> restart.  Originally, I had exposed the TTL in a ConfigContext interface
> passed to the get() method.  To reduce the number of APIs, I placed it on
> the onChange() method.  This means at the time of get(), onChange() would
> be called with a TTL.  The Connector's implementation of the callback would
> use onChange() with the TTL to schedule a restart.
>
> If you think this is overloading onChange() too much, I could add the
> ConfigContext back to get():
>
>
> Map get(ConfigContext ctx, String path);
>
> public interface ConfigContext {
>
> void willExpire(String path, long ttl);
>
> }
>
>
>
> or I could separate out the TTL method in the callback:
>
>
> public interface ConfigChangeCallback {
>
> void willExpire(String path, long ttl);
>
> void onChange(String path, Map values);
> }
>
>
>
> Or we could return a composite object from get():
>
> ConfigData get(String path);
>
> public class ConfigData {
>
>   Map data;
>   long ttl;
>
> }
>
>
> Do you have a preference Colin?
>
> Thanks,
> Robert
>
>
> On Thu, May 17, 2018 at 9:27 AM, Colin McCabe  wrote:
>
>> Hi Robert,
>>
>> Hmm.  I thought that if you're using ConfigChangeCallback, you are
>> relying on the ConfigProvider to make a callback to you when the
>> configuration has changed.  So isn't that always the "push model" (where
>> the ConfigProvider pushes changes to Connect).  If you want the "pull
>> model" where you initiate updates, you can simply call ConfigProvider#get
>> directly, right?
>>
>> The actual implementation of ConfigProvider subclasses will depend on the
>> type of configuration storage mechanism on the backend.  In the case of
>> Vault, it sounds like we need to have something like a ScheduledExecutor
>> which re-fetches keys after a certain amount of time.
>>
>> As an aside, what does a "lease duration" mean for a configuration key?
>> Does that mean Vault will reject changes to the configuration key if I try
>> to make them within the lease duration?  Or is this like a period after
>> which a password is automatically rotated?
>>
>> On Wed, May 16, 2018, at 22:25, Robert Yokota wrote:
>> > Hi Colin,
>> >
>> > > With regard to delayMs, can’t we just restart the
>> > > Connector when the keys are actually changed?
>> >
>> > Currently the VaultConfigProvider does not find out when values for keys
>> > have changed.  You could do this with a poll model (with a
>> > background thread in the ConfigProvider), but since for each key-value
>> > pair, Vault provides a lease duration stating exactly when a value for a
>> > key will change in the future, this is an alternative model of just
>> passing
>> > the lease duration to the client (in this case the Connector), to allow
>> it
>> > to determine what to do (such as schedule a restart).   This may allow
>> one
>> > to avoid the complexity of figuring out a proper poll interval (with
>> lease
>> > durations of varying periods), or worrying about putting too much load
>> on
>> > the secrets manager by polling too often.
>>
>> Those things are still concerns if the Connector is polling, right?
>> Perhaps the connector poll too often and puts too much load on Vault.  And
>> so forth.  It seems like this problem needs to be solved either way (and
>> probably can be solved with reasonable default minimum fetch intervals).
>>
>> best,
>> Colin
>>
>>
>> >  In other words, by adding this
>> > one additional parameter, a ConfigProvider can provide both push and
>> pull
>> > models to clients, perhaps with an additional configuration parameter to
>> > the ConfigProvider to determine which model (push or poll) to use.
>> >
>> > Thanks,
>> > Robert
>> >
>> > On Wed, May 16, 2018 at 9:56 PM, Colin McCabe 
>> wrote:
>> >
>> > > Thanks, Robert.  With regard to delayMs, can’t we just restart the
>> > > Connector when the ke

Build failed in Jenkins: kafka-trunk-jdk7 #3434

[Apache Jenkins Server]

See 


Changes:

[github] KAFKA-6729: Reuse source topics for source KTable's materialized 
store's

[github] HOTFIX: move Conusmed to o.a.k.streams.kstream (#5033)

--
[...truncated 1.48 MB...]

org.apache.kafka.common.KafkaFutureTest > testAllOfFuturesHandlesZeroFutures 
STARTED

org.apache.kafka.common.KafkaFutureTest > testAllOfFuturesHandlesZeroFutures 
PASSED

org.apache.kafka.common.KafkaFutureTest > testFutureTimeoutWithZeroWait STARTED

org.apache.kafka.common.KafkaFutureTest > testFutureTimeoutWithZeroWait PASSED

org.apache.kafka.common.KafkaFutureTest > testAllOfFutures STARTED

org.apache.kafka.common.KafkaFutureTest > testAllOfFutures PASSED

org.apache.kafka.common.header.internals.RecordHeadersTest > testAdd STARTED

org.apache.kafka.common.header.internals.RecordHeadersTest > testAdd PASSED

org.apache.kafka.common.header.internals.RecordHeadersTest > testNew STARTED

org.apache.kafka.common.header.internals.RecordHeadersTest > testNew PASSED

org.apache.kafka.common.header.internals.RecordHeadersTest > testReadOnly 
STARTED

org.apache.kafka.common.header.internals.RecordHeadersTest > testReadOnly PASSED

org.apache.kafka.common.header.internals.RecordHeadersTest > testHeaders STARTED

org.apache.kafka.common.header.internals.RecordHeadersTest > testHeaders PASSED

org.apache.kafka.common.header.internals.RecordHeadersTest > testLastHeader 
STARTED

org.apache.kafka.common.header.internals.RecordHeadersTest > testLastHeader 
PASSED

org.apache.kafka.common.header.internals.RecordHeadersTest > testRemove STARTED

org.apache.kafka.common.header.internals.RecordHeadersTest > testRemove PASSED

org.apache.kafka.common.header.internals.RecordHeadersTest > 
testAddRemoveInterleaved STARTED

org.apache.kafka.common.header.internals.RecordHeadersTest > 
testAddRemoveInterleaved PASSED
:clients:determineCommitId UP-TO-DATE
:clients:createVersionFile
:clients:jar UP-TO-DATE
:core:compileJava NO-SOURCE
:core:compileScala UP-TO-DATE
:core:processResources NO-SOURCE
:core:classes UP-TO-DATE
:core:copyDependantLibs
:core:jar
:examples:compileJava:19:
 warning: [deprecation] FetchRequest in kafka.api has been deprecated
import kafka.api.FetchRequest;
^
:20:
 warning: [deprecation] FetchRequestBuilder in kafka.api has been deprecated
import kafka.api.FetchRequestBuilder;
^
:22:
 warning: [deprecation] SimpleConsumer in kafka.javaapi.consumer has been 
deprecated
import kafka.javaapi.consumer.SimpleConsumer;
 ^
3 warnings

:examples:processResources NO-SOURCE
:examples:classes
:examples:checkstyleMain
:examples:compileTestJava NO-SOURCE
:examples:processTestResources NO-SOURCE
:examples:testClasses UP-TO-DATE
:examples:checkstyleTest NO-SOURCE
:examples:findbugsMain
:examples:test NO-SOURCE
:log4j-appender:compileJava
:log4j-appender:processResources NO-SOURCE
:log4j-appender:classes
:log4j-appender:checkstyleMain
:log4j-appender:compileTestJava
:log4j-appender:processTestResources NO-SOURCE
:log4j-appender:testClasses
:log4j-appender:checkstyleTest
:log4j-appender:findbugsMain
:log4j-appender:test

org.apache.kafka.log4jappender.KafkaLog4jAppenderTest > testLog4jAppends STARTED

org.apache.kafka.log4jappender.KafkaLog4jAppenderTest > testLog4jAppends PASSED

org.apache.kafka.log4jappender.KafkaLog4jAppenderTest > testKafkaLog4jConfigs 
STARTED

org.apache.kafka.log4jappender.KafkaLog4jAppenderTest > testKafkaLog4jConfigs 
PASSED
:core:compileTestJava NO-SOURCE
:core:compileTestScala UP-TO-DATE
:core:processTestResources UP-TO-DATE
:core:testClasses UP-TO-DATE
:connect:api:compileJava
:connect:api:processResources NO-SOURCE
:connect:api:classes
:connect:api:copyDependantLibs
:connect:api:jar
:connect:json:compileJava
:connect:json:processResources NO-SOURCE
:connect:json:classes
:connect:json:copyDependantLibs
:connect:json:jar
:streams:compileJava:21:
 warning: [deprecation] StateStoreSupplier in 
org.apache.kafka.streams.processor has been deprecated
import org.apache.kafka.streams.processor.StateStoreSupplier;
 ^
:21:
 warning: [deprecation] StateStoreSupplier in 
org.apache.kafka.streams.processor has been deprecated
import org.apache.kafka.streams.processor.StateStoreSupp

Build failed in Jenkins: kafka-trunk-jdk10 #111

[Apache Jenkins Server]

See 


Changes:

[github] KAFKA-6729: Reuse source topics for source KTable's materialized 
store's

[github] HOTFIX: move Conusmed to o.a.k.streams.kstream (#5033)

--
[...truncated 1.51 MB...]

kafka.network.SocketServerTest > testGracefulClose PASSED

kafka.network.SocketServerTest > controlThrowable STARTED

kafka.network.SocketServerTest > controlThrowable PASSED

kafka.network.SocketServerTest > testRequestMetricsAfterStop STARTED

kafka.network.SocketServerTest > testRequestMetricsAfterStop PASSED

kafka.network.SocketServerTest > testConnectionIdReuse STARTED

kafka.network.SocketServerTest > testConnectionIdReuse PASSED

kafka.network.SocketServerTest > testClientDisconnectionUpdatesRequestMetrics 
STARTED

kafka.network.SocketServerTest > testClientDisconnectionUpdatesRequestMetrics 
PASSED

kafka.network.SocketServerTest > testProcessorMetricsTags STARTED

kafka.network.SocketServerTest > testProcessorMetricsTags PASSED

kafka.network.SocketServerTest > testMaxConnectionsPerIp STARTED

kafka.network.SocketServerTest > testMaxConnectionsPerIp PASSED

kafka.network.SocketServerTest > testConnectionId STARTED

kafka.network.SocketServerTest > testConnectionId PASSED

kafka.network.SocketServerTest > 
testBrokerSendAfterChannelClosedUpdatesRequestMetrics STARTED

kafka.network.SocketServerTest > 
testBrokerSendAfterChannelClosedUpdatesRequestMetrics PASSED

kafka.network.SocketServerTest > testNoOpAction STARTED

kafka.network.SocketServerTest > testNoOpAction PASSED

kafka.network.SocketServerTest > simpleRequest STARTED

kafka.network.SocketServerTest > simpleRequest PASSED

kafka.network.SocketServerTest > closingChannelException STARTED

kafka.network.SocketServerTest > closingChannelException PASSED

kafka.network.SocketServerTest > testIdleConnection STARTED

kafka.network.SocketServerTest > testIdleConnection PASSED

kafka.network.SocketServerTest > 
testClientDisconnectionWithStagedReceivesFullyProcessed STARTED

kafka.network.SocketServerTest > 
testClientDisconnectionWithStagedReceivesFullyProcessed PASSED

kafka.network.SocketServerTest > testZeroMaxConnectionsPerIp STARTED

kafka.network.SocketServerTest > testZeroMaxConnectionsPerIp PASSED

kafka.network.SocketServerTest > testMetricCollectionAfterShutdown STARTED

kafka.network.SocketServerTest > testMetricCollectionAfterShutdown PASSED

kafka.network.SocketServerTest > testSessionPrincipal STARTED

kafka.network.SocketServerTest > testSessionPrincipal PASSED

kafka.network.SocketServerTest > configureNewConnectionException STARTED

kafka.network.SocketServerTest > configureNewConnectionException PASSED

kafka.network.SocketServerTest > testMaxConnectionsPerIpOverrides STARTED

kafka.network.SocketServerTest > testMaxConnectionsPerIpOverrides PASSED

kafka.network.SocketServerTest > processNewResponseException STARTED

kafka.network.SocketServerTest > processNewResponseException PASSED

kafka.network.SocketServerTest > processCompletedSendException STARTED

kafka.network.SocketServerTest > processCompletedSendException PASSED

kafka.network.SocketServerTest > processDisconnectedException STARTED

kafka.network.SocketServerTest > processDisconnectedException PASSED

kafka.network.SocketServerTest > sendCancelledKeyException STARTED

kafka.network.SocketServerTest > sendCancelledKeyException PASSED

kafka.network.SocketServerTest > processCompletedReceiveException STARTED

kafka.network.SocketServerTest > processCompletedReceiveException PASSED

kafka.network.SocketServerTest > testSocketsCloseOnShutdown STARTED

kafka.network.SocketServerTest > testSocketsCloseOnShutdown PASSED

kafka.network.SocketServerTest > pollException STARTED

kafka.network.SocketServerTest > pollException PASSED

kafka.network.SocketServerTest > testSslSocketServer STARTED

kafka.network.SocketServerTest > testSslSocketServer PASSED

kafka.network.SocketServerTest > tooBigRequestIsRejected STARTED

kafka.network.SocketServerTest > tooBigRequestIsRejected PASSED

kafka.integration.PrimitiveApiTest > testMultiProduce STARTED

kafka.integration.PrimitiveApiTest > testMultiProduce PASSED

kafka.integration.PrimitiveApiTest > testDefaultEncoderProducerAndFetch STARTED

kafka.integration.PrimitiveApiTest > testDefaultEncoderProducerAndFetch PASSED

kafka.integration.PrimitiveApiTest > testFetchRequestCanProperlySerialize 
STARTED

kafka.integration.PrimitiveApiTest > testFetchRequestCanProperlySerialize PASSED

kafka.integration.PrimitiveApiTest > testPipelinedProduceRequests STARTED

kafka.integration.PrimitiveApiTest > testPipelinedProduceRequests PASSED

kafka.integration.PrimitiveApiTest > testProduceAndMultiFetch STARTED

kafka.integration.PrimitiveApiTest > testProduceAndMultiFetch PASSED

kafka.integration.PrimitiveApiTest > 
testDefaultEncoderProducerAndFetchWithCompression STARTED

kafka.integration.PrimitiveApiTest > 
testDefaultEncoderP

Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs

[Andy Coates]

Hey Piyush - my bad. Sorry.

On 17 May 2018 at 13:23, Piyush Vijay  wrote:

> It's still not complete. I'll drop a message here when I'm done with the
> updates.
>
> Thanks
>
>
> Piyush Vijay
>
> On Thu, May 17, 2018 at 12:04 PM, Andy Coates  wrote:
>
> > Thanks for the update to the KIP Piyush!
> >
> > Reading it through again, I've a couple of questions:
> >
> > 1. Why is there a need for a new 'getMatchingAcls' method, over the
> > existing getAcls method? They both take a Resource instance and return a
> > set of Acls. What is the difference in their behaviour?
> > 2. It's not clear to me from the KIP alone what will change, from a users
> > perspective, on how they add / list / delete ACLs.  I'm assuming this
> won't
> > change.
> > 3. Writing ACLs to a new location to get around the issues of embedded
> > wildcards in existing group ACLs makes sense to me - but just a thought,
> > will we be writing all new ACLs under this new path, or just those that
> are
> > partial wildcards?  I'm assuming its the latter, but it could just be
> 'all'
> > right? As we could escape illegal chars.  So we could just make this new
> > path 'v2' rather wildcard.
> >
> > Andy
> >
> > On 17 May 2018 at 09:32, Colin McCabe  wrote:
> >
> > > On Thu, May 17, 2018, at 09:28, Piyush Vijay wrote:
> > > > I was planning to do that.
> > > >
> > > > Another unrelated detail is the presence of the support for ‘*’ ACL
> > > > currently. Looks like we’ll have to keep supporting this as a special
> > > case,
> > > > even though using a different location for wildcard-suffix ACLs on
> Zk.
> > >
> > > +1.
> > >
> > > Thanks, Piyush.
> > >
> > > Colin
> > >
> > > >
> > > >
> > > >
> > > > On Thu, May 17, 2018 at 9:15 AM Colin McCabe 
> > wrote:
> > > >
> > > > > Thanks, Piyush.  +1 for starting the vote soon.
> > > > >
> > > > > Can you please also add a discussion about escaping?  For example,
> > > earlier
> > > > > we discussed using backslashes to escape special characters.  So
> that
> > > users
> > > > > can create an ACL referring to a literal "foo*" group by creating
> an
> > > ACL
> > > > > for "foo\*"  Similarly, you can get a literal backslash with "\\".
> > > This is
> > > > > the standard UNIX escaping mechanism.
> > > > >
> > > > > Also, for the section that says "Changes to AdminClient (needs
> > > > > discussion)", we need a new method that will allow users to escape
> > > consumer
> > > > > group names and other names.  So you can feed this method your
> > "foo\*"
> > > > > consumer group name, and it will give you "foo\\\*", which is what
> > you
> > > > > would need to use to create an ACL for this consumer group in
> > > AdminClient.
> > > > > I think that's the only change we need to admin client
> > > > >
> > > > > regards,
> > > > > Colin
> > > > >
> > > > >
> > > > > On Thu, May 17, 2018, at 08:55, Piyush Vijay wrote:
> > > > > > Hi Rajini/Colin,
> > > > > >
> > > > > > I will remove the wildcard principals from the scope for now,
> > > updating
> > > > > KIP
> > > > > > right now and will open it for vote.
> > > > > >
> > > > > > Thanks
> > > > > >
> > > > > >
> > > > > > Piyush Vijay
> > > > > >
> > > > > > On Thu, May 17, 2018 at 6:59 AM, Rajini Sivaram <
> > > rajinisiva...@gmail.com
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Piyush,
> > > > > > >
> > > > > > > I have added a PR (https://github.com/apache/kafka/pull/5030)
> > with
> > > > > tests
> > > > > > > to
> > > > > > > show how group principals can be used for authorization with
> > custom
> > > > > > > principal builders. One of the tests uses SASL. It is not quite
> > the
> > > > > same as
> > > > > > > a full-fledged user groups, but since it works with all
> security
> > > > > protocols,
> > > > > > > it could be an alternative to wildcarded principals.
> > > > > > >
> > > > > > > Let us know if we can help in any way to get this KIP updated
> and
> > > > > ready for
> > > > > > > voting to include in 2.0.0.
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Rajini
> > > > > > >
> > > > > > >
> > > > > > > On Wed, May 16, 2018 at 10:21 PM, Colin McCabe <
> > cmcc...@apache.org
> > > >
> > > > > wrote:
> > > > > > >
> > > > > > > > > On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram <
> > > > > > > rajinisiva...@gmail.com
> > > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Piyush,
> > > > > > > > > >
> > > > > > > > > > It is possible to configure PrincipalBuilder for SASL (
> > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > > > 189%3A+Improve+principal+builder+interface+and+add+
> > > > > > > support+for+SASL).
> > > > > > > > If
> > > > > > > > > > that satisfies your requirements, perhaps we can move
> > > wildcarded
> > > > > > > > principals
> > > > > > > > > > out of this KIP and focus on wildcarded resources?
> > > > > > > >
> > > > > > > > +1.
> > > > > > > >
> > > > > > > > We also need to determine which characters will be reserved
> for
> 

Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs

[Piyush Vijay]

It's still not complete. I'll drop a message here when I'm done with the
updates.

Thanks


Piyush Vijay

On Thu, May 17, 2018 at 12:04 PM, Andy Coates  wrote:

> Thanks for the update to the KIP Piyush!
>
> Reading it through again, I've a couple of questions:
>
> 1. Why is there a need for a new 'getMatchingAcls' method, over the
> existing getAcls method? They both take a Resource instance and return a
> set of Acls. What is the difference in their behaviour?
> 2. It's not clear to me from the KIP alone what will change, from a users
> perspective, on how they add / list / delete ACLs.  I'm assuming this won't
> change.
> 3. Writing ACLs to a new location to get around the issues of embedded
> wildcards in existing group ACLs makes sense to me - but just a thought,
> will we be writing all new ACLs under this new path, or just those that are
> partial wildcards?  I'm assuming its the latter, but it could just be 'all'
> right? As we could escape illegal chars.  So we could just make this new
> path 'v2' rather wildcard.
>
> Andy
>
> On 17 May 2018 at 09:32, Colin McCabe  wrote:
>
> > On Thu, May 17, 2018, at 09:28, Piyush Vijay wrote:
> > > I was planning to do that.
> > >
> > > Another unrelated detail is the presence of the support for ‘*’ ACL
> > > currently. Looks like we’ll have to keep supporting this as a special
> > case,
> > > even though using a different location for wildcard-suffix ACLs on Zk.
> >
> > +1.
> >
> > Thanks, Piyush.
> >
> > Colin
> >
> > >
> > >
> > >
> > > On Thu, May 17, 2018 at 9:15 AM Colin McCabe 
> wrote:
> > >
> > > > Thanks, Piyush.  +1 for starting the vote soon.
> > > >
> > > > Can you please also add a discussion about escaping?  For example,
> > earlier
> > > > we discussed using backslashes to escape special characters.  So that
> > users
> > > > can create an ACL referring to a literal "foo*" group by creating an
> > ACL
> > > > for "foo\*"  Similarly, you can get a literal backslash with "\\".
> > This is
> > > > the standard UNIX escaping mechanism.
> > > >
> > > > Also, for the section that says "Changes to AdminClient (needs
> > > > discussion)", we need a new method that will allow users to escape
> > consumer
> > > > group names and other names.  So you can feed this method your
> "foo\*"
> > > > consumer group name, and it will give you "foo\\\*", which is what
> you
> > > > would need to use to create an ACL for this consumer group in
> > AdminClient.
> > > > I think that's the only change we need to admin client
> > > >
> > > > regards,
> > > > Colin
> > > >
> > > >
> > > > On Thu, May 17, 2018, at 08:55, Piyush Vijay wrote:
> > > > > Hi Rajini/Colin,
> > > > >
> > > > > I will remove the wildcard principals from the scope for now,
> > updating
> > > > KIP
> > > > > right now and will open it for vote.
> > > > >
> > > > > Thanks
> > > > >
> > > > >
> > > > > Piyush Vijay
> > > > >
> > > > > On Thu, May 17, 2018 at 6:59 AM, Rajini Sivaram <
> > rajinisiva...@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Piyush,
> > > > > >
> > > > > > I have added a PR (https://github.com/apache/kafka/pull/5030)
> with
> > > > tests
> > > > > > to
> > > > > > show how group principals can be used for authorization with
> custom
> > > > > > principal builders. One of the tests uses SASL. It is not quite
> the
> > > > same as
> > > > > > a full-fledged user groups, but since it works with all security
> > > > protocols,
> > > > > > it could be an alternative to wildcarded principals.
> > > > > >
> > > > > > Let us know if we can help in any way to get this KIP updated and
> > > > ready for
> > > > > > voting to include in 2.0.0.
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Rajini
> > > > > >
> > > > > >
> > > > > > On Wed, May 16, 2018 at 10:21 PM, Colin McCabe <
> cmcc...@apache.org
> > >
> > > > wrote:
> > > > > >
> > > > > > > > On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram <
> > > > > > rajinisiva...@gmail.com
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Piyush,
> > > > > > > > >
> > > > > > > > > It is possible to configure PrincipalBuilder for SASL (
> > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > > 189%3A+Improve+principal+builder+interface+and+add+
> > > > > > support+for+SASL).
> > > > > > > If
> > > > > > > > > that satisfies your requirements, perhaps we can move
> > wildcarded
> > > > > > > principals
> > > > > > > > > out of this KIP and focus on wildcarded resources?
> > > > > > >
> > > > > > > +1.
> > > > > > >
> > > > > > > We also need to determine which characters will be reserved for
> > the
> > > > > > > future.  I think previously we thought about @, #, $, %, ^, &,
> *.
> > > > > > >
> > > > > > > > > On Tue, May 15, 2018 at 7:15 PM, Piyush Vijay <
> > > > > > piyushvij...@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > >> Hi Colin,
> > > > > > > > >>
> > > > > > > > >> Escaping at this level is making sense to me but let me
> > think

Re: [DISCUSS] KIP-273 Kafka to support using ETCD beside Zookeeper

[Jakub Scholz]

Hi Balint,

Out of curiosity - have you ever tried this project
https://github.com/coreos/zetcd which tries to provide Zookeeper API for
Etcd?

Thanks & Regards
Jakub

On Wed, Mar 28, 2018 at 6:18 PM Molnár Bálint 
wrote:

> Hi all,
>
> I have created KIP-273: Kafka to support using ETCD beside Zookeeper
>
> Here is the link to the KIP:
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-273+-+Kafka+to+support+using+ETCD+beside+Zookeeper
>
> Looking forward to the discussion.
>
> Thanks,
> Balint
>

Re: [DISCUSS] KIP-273 Kafka to support using ETCD beside Zookeeper

[Jakub Scholz]

Hi,

To be honest, I'm not sure I understand who is the "we" in the "We're going
to post ...". If there are any discussions or plans to replace Zookeeper,
then I think they should be done here on the developer list instead of
teasing people for several months with some secretive projects.

Thanks & Regards
Jakub


On Mon, May 14, 2018 at 9:57 AM Molnár Bálint 
wrote:

> Hi Colin,
>
> Do you have a rough estimate on that?
>
> Thanks,
> Balint
>
> Colin McCabe  ezt írta (időpont: 2018. máj. 9., Sze,
> 19:53):
>
> > Hi Molnar,
> >
> > The points Ismael brought up earlier (and that were brought up on KIP-30)
> > are still relevant here.  As Ismael said, the goal is to get rid of
> > external dependencies here.   We're going to post more about this soon
> > (sorry for the delay)
> >
> > thanks,
> > Colin
> >
> >
> > On Wed, May 9, 2018, at 07:29, Molnár Bálint wrote:
> > > Hi,
> > > I just rebased the Etcd implementation proposal on trunk. Pinging to
> see
> > if
> > > anyone has feedback on my questions from my previous email.
> > >
> > > Molnár Bálint  ezt írta (időpont: 2018. ápr.
> 4.,
> > > Sze, 10:08):
> > >
> > > > Hi,
> > > > Thanks again for the feedback.
> > > >
> > > > Is there already ongoing work for having an own consensus
> > implementation
> > > > within Kafka?
> > > > If that work haven't started yet, we think there is value in having
> an
> > > > interim solution, that allows the use of another consensus system
> > besides
> > > > Zookeeper.
> > > >
> > > > We ask the community to take a look at the Etcd implementation
> proposal
> > > > we created and provide feedback on that.
> > > > This helps to asses rather this approach is viable at all.
> > > >
> > > > We are open to collaborate on integrating our proposed Etcd
> > implementation
> > > > into any integration test system, to certify that all use cases works
> > as
> > > > expected.
> > > >
> > > > Balint
> > > >
> > > > 2018-03-30 22:21 GMT+02:00 Gwen Shapira :
> > > >
> > > >> Hi,
> > > >>
> > > >> I had an offline discussion with Ismael and wanted to summarize the
> > > >> comments and questions he raised so we are all on the same page.
> > > >>
> > > >> The core issue is that this change adds a new public API. Since we
> > already
> > > >> know that the goal for the next 1-2 years is to get rid of ZK
> > completely.
> > > >> Do we want to go to the effort of adding (and discussing and
> > reviewing) a
> > > >> new public API, knowing that it will be completely removed in a
> year?
> > And
> > > >> since building and testing a plugin also involves effort, will
> anyone
> > do
> > > >> it
> > > >> for something that is going to be temporary by design?
> > > >>
> > > >> Ismael, correct me if this isn't a fair representation of your
> > concerns.
> > > >>
> > > >> Gwen
> > > >>
> > > >>
> > > >>
> > > >> On Thu, Mar 29, 2018 at 9:33 AM, Gwen Shapira 
> > wrote:
> > > >>
> > > >> > Few other concerns that were raised in the previous discussion
> were
> > > >> around
> > > >> > the challenges both to maintainers and users in making this API
> > > >> pluggable
> > > >> > and how does making the interface pluggable aligns with future
> > goals for
> > > >> > the project. At the time this was difficult to discuss because
> there
> > > >> wasn't
> > > >> > a concrete proposal. I want to discuss these points in the context
> > of
> > > >> this
> > > >> > specific proposal:
> > > >> >
> > > >> > 1. Problem: Pluggable APIs mean larger surface testing area and
> > multiple
> > > >> > implementations to cover.
> > > >> > In this case: At the time, the Kafka project didn't have much
> > > >> > experience with pluggable APIs and components, so the concerns
> were
> > very
> > > >> > valid. Right now Kafka has multiple pluggable components -
> > Connectors,
> > > >> > converters, transformations, authentication protocols,
> authorization
> > > >> > database, coordination protocol, serializers, etc. I think that
> as a
> > > >> > community we gotten better at testing the interface, testing the
> > very
> > > >> few
> > > >> > implementations that are included in Apache Kafka itself and
> > allowing
> > > >> the
> > > >> > community to innovate and validate outside of the Kafka project. I
> > don't
> > > >> > recall major issues either from lack of testing or from usability
> > > >> > perspective.
> > > >> >
> > > >> > 2. Problem: Users don't want to choose a consensus implementation,
> > they
> > > >> > just don't want ZK.
> > > >> > In this case: I agree that users don't actually want to spend
> > time
> > > >> > choosing consensus implementation and a simpler deployment model
> > would
> > > >> > serve them better. IMO, if Apache Kafka ships with our well-tested
> > ZK
> > > >> > implementation, 99% of the users will choose to use that (a vast
> > > >> majority
> > > >> > uses our less-than-amazing authorization plugin), and the few that
> > > >> really
> > > >> > need something else for whatever reason, will be able to get what
> > they
> > >

Jenkins build is back to normal : kafka-trunk-jdk8 #2650

[Apache Jenkins Server]

See 

Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology Sink

[Ted Yu]

+1
 Original message From: Gwen Shapira  Date: 
5/17/18  11:53 AM  (GMT-08:00) To: dev  Subject: Re: 
[VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology Sink 
Yay, its about time :)

+1

On Thu, May 17, 2018 at 12:38 PM, Guozhang Wang  wrote:

> Hello folks,
>
> I'd like to start a voting thread on adding dynamic routing functionality
> in Streams sink node. Please find a KIP here:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 303%3A+Add+Dynamic+Routing+in+Streams+Sink
>
>
> And the PR itself ready for review as well under KAFKA-4936:
>
> https://github.com/apache/kafka/pull/5018
>
>
>
> Thanks!
> -- Guozhang
>



-- 
*Gwen Shapira*
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter  | blog

Re: [VOTE] KIP-285: Connect Rest Extension Plugin

[Ted Yu]

+1
 Original message From: Gwen Shapira  Date: 
5/17/18  12:02 PM  (GMT-08:00) To: dev  Subject: Re: 
[VOTE] KIP-285: Connect Rest Extension Plugin 
LGTM. +1.

On Wed, May 16, 2018 at 8:19 PM, Magesh Nandakumar 
wrote:

> Hello everyone,
>
> After a good round of discussions with excellent feedback and no major
> objections, I would like to start a vote on KIP-285: Connect Rest Extension
> Plugin.
>
> KIP: <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 285%3A+Connect+Rest+Extension+Plugin
> >
>
>
> JIRA: <*https://issues.apache.org/jira/browse/KAFKA-6776
> *>
>
> Discussion thread: <
> https://www.mail-archive.com/dev@kafka.apache.org/msg86910.html>
>
> Thanks,
> Magesh
>



-- 
*Gwen Shapira*
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter  | blog

Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs

[Andy Coates]

Thanks for the update to the KIP Piyush!

Reading it through again, I've a couple of questions:

1. Why is there a need for a new 'getMatchingAcls' method, over the
existing getAcls method? They both take a Resource instance and return a
set of Acls. What is the difference in their behaviour?
2. It's not clear to me from the KIP alone what will change, from a users
perspective, on how they add / list / delete ACLs.  I'm assuming this won't
change.
3. Writing ACLs to a new location to get around the issues of embedded
wildcards in existing group ACLs makes sense to me - but just a thought,
will we be writing all new ACLs under this new path, or just those that are
partial wildcards?  I'm assuming its the latter, but it could just be 'all'
right? As we could escape illegal chars.  So we could just make this new
path 'v2' rather wildcard.

Andy

On 17 May 2018 at 09:32, Colin McCabe  wrote:

> On Thu, May 17, 2018, at 09:28, Piyush Vijay wrote:
> > I was planning to do that.
> >
> > Another unrelated detail is the presence of the support for ‘*’ ACL
> > currently. Looks like we’ll have to keep supporting this as a special
> case,
> > even though using a different location for wildcard-suffix ACLs on Zk.
>
> +1.
>
> Thanks, Piyush.
>
> Colin
>
> >
> >
> >
> > On Thu, May 17, 2018 at 9:15 AM Colin McCabe  wrote:
> >
> > > Thanks, Piyush.  +1 for starting the vote soon.
> > >
> > > Can you please also add a discussion about escaping?  For example,
> earlier
> > > we discussed using backslashes to escape special characters.  So that
> users
> > > can create an ACL referring to a literal "foo*" group by creating an
> ACL
> > > for "foo\*"  Similarly, you can get a literal backslash with "\\".
> This is
> > > the standard UNIX escaping mechanism.
> > >
> > > Also, for the section that says "Changes to AdminClient (needs
> > > discussion)", we need a new method that will allow users to escape
> consumer
> > > group names and other names.  So you can feed this method your "foo\*"
> > > consumer group name, and it will give you "foo\\\*", which is what you
> > > would need to use to create an ACL for this consumer group in
> AdminClient.
> > > I think that's the only change we need to admin client
> > >
> > > regards,
> > > Colin
> > >
> > >
> > > On Thu, May 17, 2018, at 08:55, Piyush Vijay wrote:
> > > > Hi Rajini/Colin,
> > > >
> > > > I will remove the wildcard principals from the scope for now,
> updating
> > > KIP
> > > > right now and will open it for vote.
> > > >
> > > > Thanks
> > > >
> > > >
> > > > Piyush Vijay
> > > >
> > > > On Thu, May 17, 2018 at 6:59 AM, Rajini Sivaram <
> rajinisiva...@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > Hi Piyush,
> > > > >
> > > > > I have added a PR (https://github.com/apache/kafka/pull/5030) with
> > > tests
> > > > > to
> > > > > show how group principals can be used for authorization with custom
> > > > > principal builders. One of the tests uses SASL. It is not quite the
> > > same as
> > > > > a full-fledged user groups, but since it works with all security
> > > protocols,
> > > > > it could be an alternative to wildcarded principals.
> > > > >
> > > > > Let us know if we can help in any way to get this KIP updated and
> > > ready for
> > > > > voting to include in 2.0.0.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Rajini
> > > > >
> > > > >
> > > > > On Wed, May 16, 2018 at 10:21 PM, Colin McCabe  >
> > > wrote:
> > > > >
> > > > > > > On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram <
> > > > > rajinisiva...@gmail.com
> > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Piyush,
> > > > > > > >
> > > > > > > > It is possible to configure PrincipalBuilder for SASL (
> > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > 189%3A+Improve+principal+builder+interface+and+add+
> > > > > support+for+SASL).
> > > > > > If
> > > > > > > > that satisfies your requirements, perhaps we can move
> wildcarded
> > > > > > principals
> > > > > > > > out of this KIP and focus on wildcarded resources?
> > > > > >
> > > > > > +1.
> > > > > >
> > > > > > We also need to determine which characters will be reserved for
> the
> > > > > > future.  I think previously we thought about @, #, $, %, ^, &, *.
> > > > > >
> > > > > > > > On Tue, May 15, 2018 at 7:15 PM, Piyush Vijay <
> > > > > piyushvij...@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > >> Hi Colin,
> > > > > > > >>
> > > > > > > >> Escaping at this level is making sense to me but let me
> think
> > > more
> > > > > > and get
> > > > > > > >> back to you.
> > > > > >
> > > > > > Thanks, Piyush.  What questions do you think are still open
> regarding
> > > > > > escape characters?
> > > > > > As Rajini mentioned, we have to get this in soon in order to make
> > > the KIP
> > > > > > freeze.
> > > > > >
> > > > > > > >>
> > > > > > > >> But should we not just get rid of one of AclBinding or
> > > > > > AclBindingFilter
> > > > > > > >> then? Is there a reason 

Re: [VOTE] KIP-285: Connect Rest Extension Plugin

[Gwen Shapira]

LGTM. +1.

On Wed, May 16, 2018 at 8:19 PM, Magesh Nandakumar 
wrote:

> Hello everyone,
>
> After a good round of discussions with excellent feedback and no major
> objections, I would like to start a vote on KIP-285: Connect Rest Extension
> Plugin.
>
> KIP: <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 285%3A+Connect+Rest+Extension+Plugin
> >
>
>
> JIRA: <*https://issues.apache.org/jira/browse/KAFKA-6776
> *>
>
> Discussion thread: <
> https://www.mail-archive.com/dev@kafka.apache.org/msg86910.html>
>
> Thanks,
> Magesh
>



-- 
*Gwen Shapira*
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter  | blog

Jenkins build is back to normal : kafka-trunk-jdk7 #3433

[Apache Jenkins Server]

See 

Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology Sink

[Gwen Shapira]

Google splits the URL into two lines and only the first half applies when
you click, so it looks very broken. It is easy enough to find though.

On Thu, May 17, 2018 at 12:54 PM, Guozhang Wang  wrote:

> https://cwiki.apache.org/confluence/display/KAFKA/KIP-303%
> 3A+Add+Dynamic+Routing+in+Streams+Sink works fine on my browser.. anyone
> else have the same issue?
>
> On Thu, May 17, 2018 at 11:51 AM, Damian Guy  wrote:
>
> > Thanks Guozhang. +1
> >
> > Note: the link to the KIP is broken
> >
> > On Thu, 17 May 2018 at 11:38 Guozhang Wang  wrote:
> >
> > > Hello folks,
> > >
> > > I'd like to start a voting thread on adding dynamic routing
> functionality
> > > in Streams sink node. Please find a KIP here:
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 303%3A+Add+Dynamic+Routing+in+Streams+Sink
> > >
> > >
> > > And the PR itself ready for review as well under KAFKA-4936:
> > >
> > > https://github.com/apache/kafka/pull/5018
> > >
> > >
> > >
> > > Thanks!
> > > -- Guozhang
> > >
> >
>
>
>
> --
> -- Guozhang
>



-- 
*Gwen Shapira*
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter  | blog

Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology Sink

[Guozhang Wang]

https://cwiki.apache.org/confluence/display/KAFKA/KIP-303%
3A+Add+Dynamic+Routing+in+Streams+Sink works fine on my browser.. anyone
else have the same issue?

On Thu, May 17, 2018 at 11:51 AM, Damian Guy  wrote:

> Thanks Guozhang. +1
>
> Note: the link to the KIP is broken
>
> On Thu, 17 May 2018 at 11:38 Guozhang Wang  wrote:
>
> > Hello folks,
> >
> > I'd like to start a voting thread on adding dynamic routing functionality
> > in Streams sink node. Please find a KIP here:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 303%3A+Add+Dynamic+Routing+in+Streams+Sink
> >
> >
> > And the PR itself ready for review as well under KAFKA-4936:
> >
> > https://github.com/apache/kafka/pull/5018
> >
> >
> >
> > Thanks!
> > -- Guozhang
> >
>



-- 
-- Guozhang

Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology Sink

[Gwen Shapira]

Yay, its about time :)

+1

On Thu, May 17, 2018 at 12:38 PM, Guozhang Wang  wrote:

> Hello folks,
>
> I'd like to start a voting thread on adding dynamic routing functionality
> in Streams sink node. Please find a KIP here:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 303%3A+Add+Dynamic+Routing+in+Streams+Sink
>
>
> And the PR itself ready for review as well under KAFKA-4936:
>
> https://github.com/apache/kafka/pull/5018
>
>
>
> Thanks!
> -- Guozhang
>



-- 
*Gwen Shapira*
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter  | blog

Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology Sink

[Bill Bejeck]

Thanks for the KIP.

+1

-Bill

On Thu, May 17, 2018 at 2:51 PM, Damian Guy  wrote:

> Thanks Guozhang. +1
>
> Note: the link to the KIP is broken
>
> On Thu, 17 May 2018 at 11:38 Guozhang Wang  wrote:
>
> > Hello folks,
> >
> > I'd like to start a voting thread on adding dynamic routing functionality
> > in Streams sink node. Please find a KIP here:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 303%3A+Add+Dynamic+Routing+in+Streams+Sink
> >
> >
> > And the PR itself ready for review as well under KAFKA-4936:
> >
> > https://github.com/apache/kafka/pull/5018
> >
> >
> >
> > Thanks!
> > -- Guozhang
> >
>

Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology Sink

[Damian Guy]

Thanks Guozhang. +1

Note: the link to the KIP is broken

On Thu, 17 May 2018 at 11:38 Guozhang Wang  wrote:

> Hello folks,
>
> I'd like to start a voting thread on adding dynamic routing functionality
> in Streams sink node. Please find a KIP here:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 303%3A+Add+Dynamic+Routing+in+Streams+Sink
>
>
> And the PR itself ready for review as well under KAFKA-4936:
>
> https://github.com/apache/kafka/pull/5018
>
>
>
> Thanks!
> -- Guozhang
>

Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Robert Yokota]

Hi Colin,

My last response was not that clear, so let me back up and explain a bit
more.

Some secret managers, such as Vault (and maybe Keywhiz) have the notion of
a lease duration or a TTL for a path.  Every path can have a different
TTL.  This is period after which the value of the keys at the given path
may be invalid.  It can be used to indicate a rotation will be done.  In
the cause of the Vault integration with AWS, Vault will actually delete the
secrets from AWS at the moment the TTL expires.  A TTL could be used by
other ConfigProviders, such as a FileConfigProvider, to indicate that all
the secrets at a given path (file), will be rotated on a regular basis.

I would like to expose the TTL in the APIs somewhere.  The TTL can be made
available at the time get() is called.  Connect already has a built in
ScheduledExecutor, so Connect can just use the TTL to schedule a Connector
restart.  Originally, I had exposed the TTL in a ConfigContext interface
passed to the get() method.  To reduce the number of APIs, I placed it on
the onChange() method.  This means at the time of get(), onChange() would
be called with a TTL.  The Connector's implementation of the callback would
use onChange() with the TTL to schedule a restart.

If you think this is overloading onChange() too much, I could add the
ConfigContext back to get():


Map get(ConfigContext ctx, String path);

public interface ConfigContext {

void willExpire(String path, long ttl);

}



or I could separate out the TTL method in the callback:


public interface ConfigChangeCallback {

void willExpire(String path, long ttl);

void onChange(String path, Map values);
}



Or we could return a composite object from get():

ConfigData get(String path);

public class ConfigData {

  Map data;
  long ttl;

}


Do you have a preference Colin?

Thanks,
Robert


On Thu, May 17, 2018 at 9:27 AM, Colin McCabe  wrote:

> Hi Robert,
>
> Hmm.  I thought that if you're using ConfigChangeCallback, you are relying
> on the ConfigProvider to make a callback to you when the configuration has
> changed.  So isn't that always the "push model" (where the ConfigProvider
> pushes changes to Connect).  If you want the "pull model" where you
> initiate updates, you can simply call ConfigProvider#get directly, right?
>
> The actual implementation of ConfigProvider subclasses will depend on the
> type of configuration storage mechanism on the backend.  In the case of
> Vault, it sounds like we need to have something like a ScheduledExecutor
> which re-fetches keys after a certain amount of time.
>
> As an aside, what does a "lease duration" mean for a configuration key?
> Does that mean Vault will reject changes to the configuration key if I try
> to make them within the lease duration?  Or is this like a period after
> which a password is automatically rotated?
>
> On Wed, May 16, 2018, at 22:25, Robert Yokota wrote:
> > Hi Colin,
> >
> > > With regard to delayMs, can’t we just restart the
> > > Connector when the keys are actually changed?
> >
> > Currently the VaultConfigProvider does not find out when values for keys
> > have changed.  You could do this with a poll model (with a
> > background thread in the ConfigProvider), but since for each key-value
> > pair, Vault provides a lease duration stating exactly when a value for a
> > key will change in the future, this is an alternative model of just
> passing
> > the lease duration to the client (in this case the Connector), to allow
> it
> > to determine what to do (such as schedule a restart).   This may allow
> one
> > to avoid the complexity of figuring out a proper poll interval (with
> lease
> > durations of varying periods), or worrying about putting too much load on
> > the secrets manager by polling too often.
>
> Those things are still concerns if the Connector is polling, right?
> Perhaps the connector poll too often and puts too much load on Vault.  And
> so forth.  It seems like this problem needs to be solved either way (and
> probably can be solved with reasonable default minimum fetch intervals).
>
> best,
> Colin
>
>
> >  In other words, by adding this
> > one additional parameter, a ConfigProvider can provide both push and pull
> > models to clients, perhaps with an additional configuration parameter to
> > the ConfigProvider to determine which model (push or poll) to use.
> >
> > Thanks,
> > Robert
> >
> > On Wed, May 16, 2018 at 9:56 PM, Colin McCabe 
> wrote:
> >
> > > Thanks, Robert.  With regard to delayMs, can’t we just restart the
> > > Connector when the keys are actually changed?  Or is the concern that
> > > this would lengthen the effective key rotation time?  Can’t the user
> > > just configure a slightly shorter key rotation time to counteract
> > > this concern?
> > > Regards,
> > > Colin
> > >
> > > On Wed, May 16, 2018, at 19:13, Robert Yokota wrote:
> > > > Hi Colin,
> > > >
> > > > Good questions.
> > > >
> > > >
> > > > > As a clarification about the indirections, what if I 

[VOTE] KIP-303: Add Dynamic Routing Support in Kafka Streams' Topology Sink

[Guozhang Wang]

Hello folks,

I'd like to start a voting thread on adding dynamic routing functionality
in Streams sink node. Please find a KIP here:

https://cwiki.apache.org/confluence/display/KAFKA/KIP-
303%3A+Add+Dynamic+Routing+in+Streams+Sink


And the PR itself ready for review as well under KAFKA-4936:

https://github.com/apache/kafka/pull/5018



Thanks!
-- Guozhang

Re: [DISCUSS] KIP-303: Add Dynamic Routing in Streams Sink

[Guozhang Wang]

Thanks Damian.

I think I'm convinced to introduce a new class for the sake of semantics
clarity plus being able to expose record context along with it. I'm going
to update the KIP and start the voting thread now.

On Thu, May 17, 2018 at 10:49 AM, Damian Guy  wrote:

> Hi Guozhang, yes i think it would make sense to add this now as having the
> additional record context would be valuable. Though i'm happy either way.
>
> On Wed, 16 May 2018 at 23:07 Guozhang Wang  wrote:
>
>> Hi Damian,
>>
>> My current plan is to add the "RichKeyValueMapper" when getting in
>> KIP-159 to include the record context in this dynamic routing feature. So
>> just to clarify: are you more concerning that if we are going to do that
>> anyways in the future, we should not add overloaded functions with
>> "KeyValueMapper" as of now since they will be subsumed soon?
>>
>>
>> Guozhang
>>
>>
>> On Wed, May 16, 2018 at 2:59 PM, Damian Guy  wrote:
>>
>>> Overall i'm a +1 on this, but i'm not a big fan of using the
>>> KeyValueMapper
>>> to choose the topic. It is a bit counter-intuitve to me. I'd prefer to
>>> add
>>> a class specifically for it and possibly pass in the RecordContext
>>>
>>> On Wed, 16 May 2018 at 13:22 Guozhang Wang  wrote:
>>>
>>> > Hello folks,
>>> >
>>> > Please let me know if you have further feedbacks; if there is no more
>>> > feedbacks I'm going to start the voting thread soon.
>>> >
>>> >
>>> > Guozhang
>>> >
>>> >
>>> > On Wed, May 16, 2018 at 8:31 AM, Guozhang Wang 
>>> wrote:
>>> >
>>> > > I have thought about exposing record context as well, and in the end
>>> I
>>> > > decided to piggy-back it with KIP-159. And if we want to indeed
>>> reuse the
>>> > > class it will be:
>>> > >
>>> > > ```
>>> > > public interface RichKeyValueMapper {
>>> > > VR apply(final K key, final V value, final RecordContext
>>> > > recordContext);
>>> > > }
>>> > > ```
>>> > >
>>> > >
>>> > >
>>> > > Guozhang
>>> > >
>>> > >
>>> > > On Tue, May 15, 2018 at 10:04 PM, Matthias J. Sax <
>>> matth...@confluent.io
>>> > >
>>> > > wrote:
>>> > >
>>> > >> Just my 2 cents:
>>> > >>
>>> > >> I am fine with `KeyValueMapper` (+1 for code reusage) -- the
>>> JavaDocs
>>> > >> will explain what the `KeyValueMapper` is supposed to do, ie,
>>> extract
>>> > >> and return the sink topic name from the key-value pair.
>>> > >>
>>> > >> A side remark though: do we think that accessing key/value is
>>> > >> sufficient? Or should we provide access to the full metadata? We
>>> could
>>> > >> also do this with KIP-159 of course -- but this would come earliest
>>> in
>>> > >> 2.1. As an alternative we could add a `TopicNameExtractor` to
>>> expose the
>>> > >> whole record context. The advantage would be, that we don't need to
>>> > >> change it via KIP-159 later again. WDYT?
>>> > >>
>>> > >> -Matthias
>>> > >>
>>> > >> On 5/15/18 5:57 PM, Bill Bejeck wrote:
>>> > >> > Thanks for the KIP Guozhang, it's a +1 for me.
>>> > >> >
>>> > >> > As for re-using the KeyValueMapper for choosing the topic, I am
>>> on the
>>> > >> > fence, a more explicitly named class would be more clear, but I'm
>>> not
>>> > >> sure
>>> > >> > it's worth a new class that will primarily perform the same
>>> actions as
>>> > >> the
>>> > >> > KeyValueMapper.
>>> > >> >
>>> > >> > Thanks,
>>> > >> > Bill
>>> > >> >
>>> > >> > On Tue, May 15, 2018 at 5:52 PM, Guozhang Wang <
>>> wangg...@gmail.com>
>>> > >> wrote:
>>> > >> >
>>> > >> >> Hello John:
>>> > >> >>
>>> > >> >> * As for the type superclass, it is mainly for allowing super
>>> class
>>> > >> serdes.
>>> > >> >> More details can be found here:
>>> > >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>> > >> >> 100+-+Relax+Type+constraints+in+Kafka+Streams+API
>>> > >> >>
>>> > >> >> * I may have slight preference on reusing existing classes but I
>>> > think
>>> > >> most
>>> > >> >> of my rationales are quite subjective. Personally I do not find
>>> `self
>>> > >> >> documenting` worth a new class but I can be convinced if people
>>> have
>>> > >> other
>>> > >> >> motivations doing it :)
>>> > >> >>
>>> > >> >>
>>> > >> >> Guozhang
>>> > >> >>
>>> > >> >>
>>> > >> >> On Tue, May 15, 2018 at 11:19 AM, John Roesler <
>>> j...@confluent.io>
>>> > >> wrote:
>>> > >> >>
>>> > >> >>> Thanks for the KIP, Guozhang.
>>> > >> >>>
>>> > >> >>> It looks good overall to me; I just have one question:
>>> > >> >>> * Why do we bound the generics of KVMapper in KStream to be
>>> > >> >> superclass-of-K
>>> > >> >>> and superclass-of-V instead of exactly K and V, as in Topology?
>>> I
>>> > >> might
>>> > >> >> be
>>> > >> >>> thinking about it wrong, but that seems backwards to me. If
>>> > anything,
>>> > >> >>> bounding to be a subclass-of-K or subclass-of-V would seem
>>> right to
>>> > >> me.
>>> > >> >>>
>>> > >> >>> One extra thought: I agree that KVMapper>> name*/>
>>> > >> is an
>>> > >> >>> applicable type for extracting the topic name, but I wonder
>>> what the
>>> > >> >> value
>>> > >> >>> of reusing the KVMapp

[jira] [Resolved] (KAFKA-6729) KTable should use user source topics if possible and not create changelog topic

[Guozhang Wang (JIRA)]

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

Guozhang Wang resolved KAFKA-6729.
--
Resolution: Fixed

> KTable should use user source topics if possible and not create changelog 
> topic
> ---
>
> Key: KAFKA-6729
> URL: https://issues.apache.org/jira/browse/KAFKA-6729
> Project: Kafka
>  Issue Type: Bug
>  Components: streams
>Affects Versions: 1.0.0
>Reporter: Matthias J. Sax
>Assignee: Guozhang Wang
>Priority: Blocker
> Fix For: 2.0.0
>
>
> With KIP-182 we reworked Streams API largely and introduced a regression into 
> 1.0 code base. If a KTable is populated from a source topic, ie, 
> StreamsBuilder.table() -- the KTable does create its own changelog topic. 
> However, in older releases (0.11 or older), we don't create a changelog topic 
> but use the user specified source topic instead.
> We want to reintroduce this optimization to reduce the load (storage and 
> write) on the broker side for this case.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Resolved] (KAFKA-6426) Kafka SASL/SCRAM authentication does not fail for incorrect username or password.

[Manikumar (JIRA)]

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

Manikumar resolved KAFKA-6426.
--
Resolution: Cannot Reproduce

This looks like configuration issue. Please reopen if you think the issue still 
exists. 

> Kafka SASL/SCRAM authentication does not fail for incorrect username or 
> password.
> -
>
> Key: KAFKA-6426
> URL: https://issues.apache.org/jira/browse/KAFKA-6426
> Project: Kafka
>  Issue Type: Bug
>Affects Versions: 0.10.2.1
> Environment: Ubuntu 16.04, JDK 1.8, Kafka_2.10-0.10.2.1
>Reporter: Menaka Madushanka
>Priority: Major
> Attachments: broker-jaas.conf, client-jaas.conf, consumer.properties, 
> producer.properties, server.properties
>
>
> Hi,
> I configured Kafka 0.10.2.1 for SASL/SCRAM by following the documentation 
> [1]. 
> But it does work when I use incorrect username or password in the client as 
> well. 
> I have attached the server.properties, consumer.properties, 
> producer.properties, jass config files for broker and client. 
> Also, in my producer, I have set
>  {{props.put("sasl.mechanism", "SCRAM-SHA-256");}}
> but when running, it shows,
> {{kafka.utils.VerifiableProperties  - Property sasl.mechanism is not valid}}
> [1] 
> [https://kafka.apache.org/documentation/#security_sasl_scram|https://kafka.apache.org/documentation/#security_sasl_scram]
> Thanks and Regards,
> Menaka



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Resolved] (KAFKA-6682) Kafka reconnection after broker restart

[Manikumar (JIRA)]

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

Manikumar resolved KAFKA-6682.
--
Resolution: Duplicate

Resolving this as duplicate of KAFKA-6260.  Please reopen if the issue still 
exists.

> Kafka reconnection after broker restart
> ---
>
> Key: KAFKA-6682
> URL: https://issues.apache.org/jira/browse/KAFKA-6682
> Project: Kafka
>  Issue Type: Bug
>  Components: clients
>Affects Versions: 1.0.0
>Reporter: madi
>Priority: Major
>
> I am using kafka producer plugin for logback (danielwegener) with the clients 
> library 1.0.0 and after restart of broker all my JVMs connected to it get 
> tons of the exceptions:
> {code:java}
> 11:22:48.738 [kafka-producer-network-thread | app-logback-relaxed] cid: 
> clid: E [    @] a: o.a.k.c.p.internals.Sender - [Producer 
> clientId=id-id-logback-relaxed] Uncaught error in kafka producer I/O 
> thread:  ex:java.lang.NullPointerException: null
>     at 
> org.apache.kafka.common.network.Selector.pollSelectionKeys(Selector.java:436)
>     at org.apache.kafka.common.network.Selector.poll(Selector.java:399)
>     at org.apache.kafka.clients.NetworkClient.poll(NetworkClient.java:460)
>     at org.apache.kafka.clients.producer.internals.Sender.run(Sender.java:239)
>     at org.apache.kafka.clients.producer.internals.Sender.run(Sender.java:163)
>     at java.lang.Thread.run(Thread.java:798){code}
> During restart there are still other brokers available behind LB.    
> Dosen't matter kafka is up again, only restarting JVM helps
> {code:java}
>      class="com.github.danielwegener.logback.kafka.KafkaAppender">
>     
>     
>    
>  %date{"-MM-dd'T'HH:mm:ss.SSS'Z'"} ${HOSTNAME} 
> [%thread] %logger{32} - %message ex:%exf%n
>     
>     mytopichere
>     
>      class="com.github.danielwegener.logback.kafka.keying.HostNameKeyingStrategy" 
> />
>     
>      class="com.github.danielwegener.logback.kafka.delivery.AsynchronousDeliveryStrategy"
>  />
>     
>    
>  
>     
>     bootstrap.servers=10.99.99.1:9092
>     
>     acks=0
>     
>     block.on.buffer.full=false
>     
>     
> client.id=${HOSTNAME}-${CONTEXT_NAME}-logback-relaxed
>     
>     
>     compression.type=none
>    
>  
>     max.block.ms=0
>     {code}
> I provide loadbalancer address in bootstrap servers here. There are three 
> kafka brokers behind.
> {code:java}
> java version "1.7.0"
> Java(TM) SE Runtime Environment (build pap6470sr9fp60ifix-20161110_01(SR9 
> FP60)+IV90630+IV90578))
> IBM J9 VM (build 2.6, JRE 1.7.0 AIX ppc64-64 Compressed References 
> 20161005_321282 (JIT enabled, AOT enabled)
> J9VM - R26_Java726_SR9_20161005_1259_B321282
> JIT  - tr.r11_20161001_125404
> GC   - R26_Java726_SR9_20161005_1259_B321282_CMPRSS
> J9CL - 20161005_321282)
> JCL - 20161021_01 based on Oracle jdk7u121-b15{code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Build failed in Jenkins: kafka-trunk-jdk10 #110

[Apache Jenkins Server]

See 


Changes:

[wangguoz] MINOR: Use Set instead of List for multiple topics (#5024)

--
[...truncated 1.50 MB...]

kafka.zk.KafkaZkClientTest > testClusterIdMethods STARTED

kafka.zk.KafkaZkClientTest > testClusterIdMethods PASSED

kafka.zk.KafkaZkClientTest > testEntityConfigManagementMethods STARTED

kafka.zk.KafkaZkClientTest > testEntityConfigManagementMethods PASSED

kafka.zk.KafkaZkClientTest > testUpdateLeaderAndIsr STARTED

kafka.zk.KafkaZkClientTest > testUpdateLeaderAndIsr PASSED

kafka.zk.KafkaZkClientTest > testUpdateBrokerInfo STARTED

kafka.zk.KafkaZkClientTest > testUpdateBrokerInfo PASSED

kafka.zk.KafkaZkClientTest > testCreateRecursive STARTED

kafka.zk.KafkaZkClientTest > testCreateRecursive PASSED

kafka.zk.KafkaZkClientTest > testGetConsumerOffsetNoData STARTED

kafka.zk.KafkaZkClientTest > testGetConsumerOffsetNoData PASSED

kafka.zk.KafkaZkClientTest > testDeleteTopicPathMethods STARTED

kafka.zk.KafkaZkClientTest > testDeleteTopicPathMethods PASSED

kafka.zk.KafkaZkClientTest > testSetTopicPartitionStatesRaw STARTED

kafka.zk.KafkaZkClientTest > testSetTopicPartitionStatesRaw PASSED

kafka.zk.KafkaZkClientTest > testAclManagementMethods STARTED

kafka.zk.KafkaZkClientTest > testAclManagementMethods PASSED

kafka.zk.KafkaZkClientTest > testPreferredReplicaElectionMethods STARTED

kafka.zk.KafkaZkClientTest > testPreferredReplicaElectionMethods PASSED

kafka.zk.KafkaZkClientTest > testPropagateLogDir STARTED

kafka.zk.KafkaZkClientTest > testPropagateLogDir PASSED

kafka.zk.KafkaZkClientTest > testGetDataAndStat STARTED

kafka.zk.KafkaZkClientTest > testGetDataAndStat PASSED

kafka.zk.KafkaZkClientTest > testReassignPartitionsInProgress STARTED

kafka.zk.KafkaZkClientTest > testReassignPartitionsInProgress PASSED

kafka.zk.KafkaZkClientTest > testCreateTopLevelPaths STARTED

kafka.zk.KafkaZkClientTest > testCreateTopLevelPaths PASSED

kafka.zk.KafkaZkClientTest > testIsrChangeNotificationGetters STARTED

kafka.zk.KafkaZkClientTest > testIsrChangeNotificationGetters PASSED

kafka.zk.KafkaZkClientTest > testLogDirEventNotificationsDeletion STARTED

kafka.zk.KafkaZkClientTest > testLogDirEventNotificationsDeletion PASSED

kafka.zk.KafkaZkClientTest > testGetLogConfigs STARTED

kafka.zk.KafkaZkClientTest > testGetLogConfigs PASSED

kafka.zk.KafkaZkClientTest > testBrokerSequenceIdMethods STARTED

kafka.zk.KafkaZkClientTest > testBrokerSequenceIdMethods PASSED

kafka.zk.KafkaZkClientTest > testCreateSequentialPersistentPath STARTED

kafka.zk.KafkaZkClientTest > testCreateSequentialPersistentPath PASSED

kafka.zk.KafkaZkClientTest > testConditionalUpdatePath STARTED

kafka.zk.KafkaZkClientTest > testConditionalUpdatePath PASSED

kafka.zk.KafkaZkClientTest > testDeleteTopicZNode STARTED

kafka.zk.KafkaZkClientTest > testDeleteTopicZNode PASSED

kafka.zk.KafkaZkClientTest > testDeletePath STARTED

kafka.zk.KafkaZkClientTest > testDeletePath PASSED

kafka.zk.KafkaZkClientTest > testGetBrokerMethods STARTED

kafka.zk.KafkaZkClientTest > testGetBrokerMethods PASSED

kafka.zk.KafkaZkClientTest > testCreateTokenChangeNotification STARTED

kafka.zk.KafkaZkClientTest > testCreateTokenChangeNotification PASSED

kafka.zk.KafkaZkClientTest > testGetTopicsAndPartitions STARTED

kafka.zk.KafkaZkClientTest > testGetTopicsAndPartitions PASSED

kafka.zk.KafkaZkClientTest > testRegisterBrokerInfo STARTED

kafka.zk.KafkaZkClientTest > testRegisterBrokerInfo PASSED

kafka.zk.KafkaZkClientTest > testControllerManagementMethods STARTED

kafka.zk.KafkaZkClientTest > testControllerManagementMethods PASSED

kafka.zk.KafkaZkClientTest > testTopicAssignmentMethods STARTED

kafka.zk.KafkaZkClientTest > testTopicAssignmentMethods PASSED

kafka.zk.KafkaZkClientTest > testPropagateIsrChanges STARTED

kafka.zk.KafkaZkClientTest > testPropagateIsrChanges PASSED

kafka.zk.KafkaZkClientTest > testControllerEpochMethods STARTED

kafka.zk.KafkaZkClientTest > testControllerEpochMethods PASSED

kafka.zk.KafkaZkClientTest > testDeleteRecursive STARTED

kafka.zk.KafkaZkClientTest > testDeleteRecursive PASSED

kafka.zk.KafkaZkClientTest > testGetTopicPartitionStates STARTED

kafka.zk.KafkaZkClientTest > testGetTopicPartitionStates PASSED

kafka.zk.KafkaZkClientTest > testCreateConfigChangeNotification STARTED

kafka.zk.KafkaZkClientTest > testCreateConfigChangeNotification PASSED

kafka.zk.KafkaZkClientTest > testDelegationTokenMethods STARTED

kafka.zk.KafkaZkClientTest > testDelegationTokenMethods PASSED

kafka.zk.ReassignPartitionsZNodeTest > testDecodeInvalidJson STARTED

kafka.zk.ReassignPartitionsZNodeTest > testDecodeInvalidJson PASSED

kafka.zk.ReassignPartitionsZNodeTest > testEncode STARTED

kafka.zk.ReassignPartitionsZNodeTest > testEncode PASSED

kafka.zk.ReassignPartitionsZNodeTest > testDecodeValidJson STARTED

kafka.zk.ReassignPartitionsZNodeTest > testDecodeValidJson PASSED

kafka.zk.ZKPathTe

Re: [DISCUSS] KIP-303: Add Dynamic Routing in Streams Sink

[Damian Guy]

Hi Guozhang, yes i think it would make sense to add this now as having the
additional record context would be valuable. Though i'm happy either way.

On Wed, 16 May 2018 at 23:07 Guozhang Wang  wrote:

> Hi Damian,
>
> My current plan is to add the "RichKeyValueMapper" when getting in KIP-159
> to include the record context in this dynamic routing feature. So just to
> clarify: are you more concerning that if we are going to do that anyways in
> the future, we should not add overloaded functions with "KeyValueMapper" as
> of now since they will be subsumed soon?
>
>
> Guozhang
>
>
> On Wed, May 16, 2018 at 2:59 PM, Damian Guy  wrote:
>
>> Overall i'm a +1 on this, but i'm not a big fan of using the
>> KeyValueMapper
>> to choose the topic. It is a bit counter-intuitve to me. I'd prefer to add
>> a class specifically for it and possibly pass in the RecordContext
>>
>> On Wed, 16 May 2018 at 13:22 Guozhang Wang  wrote:
>>
>> > Hello folks,
>> >
>> > Please let me know if you have further feedbacks; if there is no more
>> > feedbacks I'm going to start the voting thread soon.
>> >
>> >
>> > Guozhang
>> >
>> >
>> > On Wed, May 16, 2018 at 8:31 AM, Guozhang Wang 
>> wrote:
>> >
>> > > I have thought about exposing record context as well, and in the end I
>> > > decided to piggy-back it with KIP-159. And if we want to indeed reuse
>> the
>> > > class it will be:
>> > >
>> > > ```
>> > > public interface RichKeyValueMapper {
>> > > VR apply(final K key, final V value, final RecordContext
>> > > recordContext);
>> > > }
>> > > ```
>> > >
>> > >
>> > >
>> > > Guozhang
>> > >
>> > >
>> > > On Tue, May 15, 2018 at 10:04 PM, Matthias J. Sax <
>> matth...@confluent.io
>> > >
>> > > wrote:
>> > >
>> > >> Just my 2 cents:
>> > >>
>> > >> I am fine with `KeyValueMapper` (+1 for code reusage) -- the JavaDocs
>> > >> will explain what the `KeyValueMapper` is supposed to do, ie, extract
>> > >> and return the sink topic name from the key-value pair.
>> > >>
>> > >> A side remark though: do we think that accessing key/value is
>> > >> sufficient? Or should we provide access to the full metadata? We
>> could
>> > >> also do this with KIP-159 of course -- but this would come earliest
>> in
>> > >> 2.1. As an alternative we could add a `TopicNameExtractor` to expose
>> the
>> > >> whole record context. The advantage would be, that we don't need to
>> > >> change it via KIP-159 later again. WDYT?
>> > >>
>> > >> -Matthias
>> > >>
>> > >> On 5/15/18 5:57 PM, Bill Bejeck wrote:
>> > >> > Thanks for the KIP Guozhang, it's a +1 for me.
>> > >> >
>> > >> > As for re-using the KeyValueMapper for choosing the topic, I am on
>> the
>> > >> > fence, a more explicitly named class would be more clear, but I'm
>> not
>> > >> sure
>> > >> > it's worth a new class that will primarily perform the same
>> actions as
>> > >> the
>> > >> > KeyValueMapper.
>> > >> >
>> > >> > Thanks,
>> > >> > Bill
>> > >> >
>> > >> > On Tue, May 15, 2018 at 5:52 PM, Guozhang Wang > >
>> > >> wrote:
>> > >> >
>> > >> >> Hello John:
>> > >> >>
>> > >> >> * As for the type superclass, it is mainly for allowing super
>> class
>> > >> serdes.
>> > >> >> More details can be found here:
>> > >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > >> >> 100+-+Relax+Type+constraints+in+Kafka+Streams+API
>> > >> >>
>> > >> >> * I may have slight preference on reusing existing classes but I
>> > think
>> > >> most
>> > >> >> of my rationales are quite subjective. Personally I do not find
>> `self
>> > >> >> documenting` worth a new class but I can be convinced if people
>> have
>> > >> other
>> > >> >> motivations doing it :)
>> > >> >>
>> > >> >>
>> > >> >> Guozhang
>> > >> >>
>> > >> >>
>> > >> >> On Tue, May 15, 2018 at 11:19 AM, John Roesler > >
>> > >> wrote:
>> > >> >>
>> > >> >>> Thanks for the KIP, Guozhang.
>> > >> >>>
>> > >> >>> It looks good overall to me; I just have one question:
>> > >> >>> * Why do we bound the generics of KVMapper in KStream to be
>> > >> >> superclass-of-K
>> > >> >>> and superclass-of-V instead of exactly K and V, as in Topology? I
>> > >> might
>> > >> >> be
>> > >> >>> thinking about it wrong, but that seems backwards to me. If
>> > anything,
>> > >> >>> bounding to be a subclass-of-K or subclass-of-V would seem right
>> to
>> > >> me.
>> > >> >>>
>> > >> >>> One extra thought: I agree that KVMapper> name*/>
>> > >> is an
>> > >> >>> applicable type for extracting the topic name, but I wonder what
>> the
>> > >> >> value
>> > >> >>> of reusing the KVMapper for this purpose is. Would defining a new
>> > >> class,
>> > >> >>> say TopicNameExtractor, provide the same functionality while
>> > >> being a
>> > >> >>> bit more self-documenting?
>> > >> >>>
>> > >> >>> Thanks,
>> > >> >>> -John
>> > >> >>>
>> > >> >>> On Tue, May 15, 2018 at 12:32 AM, Guozhang Wang <
>> wangg...@gmail.com
>> > >
>> > >> >>> wrote:
>> > >> >>>
>> > >>  Hello folks,
>> > >> 
>> > >>  I'd like to start a discussion on adding dynamic routin

Re: [DISCUSS] KIP-304: Connect runtime mode improvements for container platforms

[Stephane Maarek]

Fat fingered too... "Connect source should be able to achieve the same
centrality of offsets "

On Thu., 17 May 2018, 10:27 pm Stephane Maarek, <
steph...@simplemachines.com.au> wrote:

> Say you have 50 connectors all with different ACLs and service account.
> That's 50 connect clusters to maintain. So 50*3 internal connect topics to
> maintain (they can't share the same connect topics because they're
> different clusters). At default config we're talking 1500 partitions which
> is a lot for a Kafka cluster.
>
> The parallel is with consumer groups. Are all consumer groups backing
> their offset in their own topic or in a central topic ? Connect should be
> able to achieve the same centrality of Configs.
>
> Finally , Configs should go along with the jar, and not be stored in
> Kafka, especially for connectors that have secrets. There's no reason Kafka
> needs to have a database secret on its own disk
>
> On Thu., 17 May 2018, 5:55 pm Rahul Singh, 
> wrote:
>
>> First sentence fat fingered.
>>
>> “Just curious as to why there’s an issue with the backing topics for
>> Kafka Connect.”
>>
>> --
>> Rahul Singh
>> rahul.si...@anant.us
>>
>> Anant Corporation
>>
>> On May 17, 2018, 6:17 AM -0400, Stephane Maarek <
>> steph...@simplemachines.com.au>, wrote:
>> > Hi Salius
>> >
>> > I think you're on the money, but you're not pushing things too far.
>> > This is something I've hoped for a long time.
>> > Let's talk Kafka Connect v2
>> >
>> > Kafka Connect Cluster, as you said, are not convenient to work with (the
>> > KIP details drawbacks well). I'm all about containerisation just like
>> > stream apps support (and boasts!).
>> >
>> > Now, here's the problem with Kafka Connect. There are three backing
>> topics.
>> > Here's the analysis of how they can evolve:
>> > - Config topic: this one is irrelevant if each connect cluster comes
>> with a
>> > config bundled with the corresponding JAR, as you mentioned in your KIP
>> > - Status topic: this is something I wish was gone too. The consumers
>> have a
>> > coordinator, and I believe the connect workers should have a coordinator
>> > too, for task rebalancing.
>> > - Source Offset topic: only relevant for sources. I wish there was a
>> > __connect_offsets global topic just like for consumers and an
>> > "ConnectOffsetCoordinator" to talk to to retrieve latest committed
>> offset.
>> >
>> > If we look above, with a few back-end fundamental transformations, we
>> can
>> > probably make Connect "cluster-less".
>> >
>> > What the community would get out of it is huge:
>> > - Connect workers for a specific connector are independent and isolated,
>> > measurable (in CPU and Mem) and auto-scalable
>> > - CI/CD is super easy to integrate, as it's just another container /
>> jar.
>> > - You can roll restart a specific connector and upgrade a JAR without
>> > interrupting your other connectors and while keeping the current
>> connector
>> > from running.
>> > - The topics backing connect are removed except the global one, which
>> > allows you to scale easily in terms of number of connectors
>> > - Running a connector in dev or prod (for people offering connectors)
>> is as
>> > easy as doing a simple "docker run".
>> > - Each consumer / producer settings can be configured at the container
>> > level.
>> > - Each connect process is immutable in configuration.
>> > - Each connect process has its own security identity (right now, you
>> need a
>> > connect cluster per service role, which is a lot of overhead in terms of
>> > backing topic)
>> >
>> > Now, I don't have the Kafka expertise to know exactly which changes to
>> make
>> > in the code, but I believe the final idea is achievable.
>> > The change would be breaking for how Kafka Connect is run, but I think
>> > there's a chance to make the change non breaking to how Connect is
>> > programmed. I believe the same public API framework can be used.
>> >
>> > Finally, the REST API can be used for monitoring, or the JMX metrics as
>> > usual.
>> >
>> > I may be completely wrong, but I would see such a change drive the
>> > utilisation, management of Connect by a lot while lowering the barrier
>> to
>> > adoption.
>> >
>> > This change may be big to implement but probably worthwhile. I'd be
>> happy
>> > to provide more "user feedback" on a PR, but probably won't be able to
>> > implement a PR myself.
>> >
>> > More than happy to discuss this
>> >
>> > Best,
>> > Stephane
>> >
>> >
>> > Kind regards,
>> > Stephane
>> >
>> > [image: Simple Machines]
>> >
>> > Stephane Maarek | Developer
>> >
>> > +61 416 575 980
>> > steph...@simplemachines.com.au
>> > simplemachines.com.au
>> > Level 2, 145 William Street, Sydney NSW 2010
>> >
>> > On 17 May 2018 at 14:42, Saulius Valatka  wrote:
>> >
>> > > Hi,
>> > >
>> > > the only real usecase for the REST interface I can see is providing
>> > > health/liveness checks for mesos/kubernetes. It's also true that the
>> API
>> > > can be left as is and e.g. not exposed publicly on the platfor

Re: [DISCUSS] KIP-304: Connect runtime mode improvements for container platforms

[Stephane Maarek]

Say you have 50 connectors all with different ACLs and service account.
That's 50 connect clusters to maintain. So 50*3 internal connect topics to
maintain (they can't share the same connect topics because they're
different clusters). At default config we're talking 1500 partitions which
is a lot for a Kafka cluster.

The parallel is with consumer groups. Are all consumer groups backing their
offset in their own topic or in a central topic ? Connect should be able to
achieve the same centrality of Configs.

Finally , Configs should go along with the jar, and not be stored in Kafka,
especially for connectors that have secrets. There's no reason Kafka needs
to have a database secret on its own disk

On Thu., 17 May 2018, 5:55 pm Rahul Singh, 
wrote:

> First sentence fat fingered.
>
> “Just curious as to why there’s an issue with the backing topics for Kafka
> Connect.”
>
> --
> Rahul Singh
> rahul.si...@anant.us
>
> Anant Corporation
>
> On May 17, 2018, 6:17 AM -0400, Stephane Maarek <
> steph...@simplemachines.com.au>, wrote:
> > Hi Salius
> >
> > I think you're on the money, but you're not pushing things too far.
> > This is something I've hoped for a long time.
> > Let's talk Kafka Connect v2
> >
> > Kafka Connect Cluster, as you said, are not convenient to work with (the
> > KIP details drawbacks well). I'm all about containerisation just like
> > stream apps support (and boasts!).
> >
> > Now, here's the problem with Kafka Connect. There are three backing
> topics.
> > Here's the analysis of how they can evolve:
> > - Config topic: this one is irrelevant if each connect cluster comes
> with a
> > config bundled with the corresponding JAR, as you mentioned in your KIP
> > - Status topic: this is something I wish was gone too. The consumers
> have a
> > coordinator, and I believe the connect workers should have a coordinator
> > too, for task rebalancing.
> > - Source Offset topic: only relevant for sources. I wish there was a
> > __connect_offsets global topic just like for consumers and an
> > "ConnectOffsetCoordinator" to talk to to retrieve latest committed
> offset.
> >
> > If we look above, with a few back-end fundamental transformations, we can
> > probably make Connect "cluster-less".
> >
> > What the community would get out of it is huge:
> > - Connect workers for a specific connector are independent and isolated,
> > measurable (in CPU and Mem) and auto-scalable
> > - CI/CD is super easy to integrate, as it's just another container / jar.
> > - You can roll restart a specific connector and upgrade a JAR without
> > interrupting your other connectors and while keeping the current
> connector
> > from running.
> > - The topics backing connect are removed except the global one, which
> > allows you to scale easily in terms of number of connectors
> > - Running a connector in dev or prod (for people offering connectors) is
> as
> > easy as doing a simple "docker run".
> > - Each consumer / producer settings can be configured at the container
> > level.
> > - Each connect process is immutable in configuration.
> > - Each connect process has its own security identity (right now, you
> need a
> > connect cluster per service role, which is a lot of overhead in terms of
> > backing topic)
> >
> > Now, I don't have the Kafka expertise to know exactly which changes to
> make
> > in the code, but I believe the final idea is achievable.
> > The change would be breaking for how Kafka Connect is run, but I think
> > there's a chance to make the change non breaking to how Connect is
> > programmed. I believe the same public API framework can be used.
> >
> > Finally, the REST API can be used for monitoring, or the JMX metrics as
> > usual.
> >
> > I may be completely wrong, but I would see such a change drive the
> > utilisation, management of Connect by a lot while lowering the barrier to
> > adoption.
> >
> > This change may be big to implement but probably worthwhile. I'd be happy
> > to provide more "user feedback" on a PR, but probably won't be able to
> > implement a PR myself.
> >
> > More than happy to discuss this
> >
> > Best,
> > Stephane
> >
> >
> > Kind regards,
> > Stephane
> >
> > [image: Simple Machines]
> >
> > Stephane Maarek | Developer
> >
> > +61 416 575 980
> > steph...@simplemachines.com.au
> > simplemachines.com.au
> > Level 2, 145 William Street, Sydney NSW 2010
> >
> > On 17 May 2018 at 14:42, Saulius Valatka  wrote:
> >
> > > Hi,
> > >
> > > the only real usecase for the REST interface I can see is providing
> > > health/liveness checks for mesos/kubernetes. It's also true that the
> API
> > > can be left as is and e.g. not exposed publicly on the platform level,
> but
> > > this would still leave opportunities to accidentally mess something up
> > > internally, so it's mostly a safety concern.
> > >
> > > Regarding the option renaming: I agree that it's not necessary as it's
> not
> > > clashing with anything, my reasoning is that assuming some other offset
> > > storage appears in

Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs

[Colin McCabe]

On Thu, May 17, 2018, at 09:28, Piyush Vijay wrote:
> I was planning to do that.
> 
> Another unrelated detail is the presence of the support for ‘*’ ACL
> currently. Looks like we’ll have to keep supporting this as a special case,
> even though using a different location for wildcard-suffix ACLs on Zk.

+1.

Thanks, Piyush.

Colin

> 
> 
> 
> On Thu, May 17, 2018 at 9:15 AM Colin McCabe  wrote:
> 
> > Thanks, Piyush.  +1 for starting the vote soon.
> >
> > Can you please also add a discussion about escaping?  For example, earlier
> > we discussed using backslashes to escape special characters.  So that users
> > can create an ACL referring to a literal "foo*" group by creating an ACL
> > for "foo\*"  Similarly, you can get a literal backslash with "\\".  This is
> > the standard UNIX escaping mechanism.
> >
> > Also, for the section that says "Changes to AdminClient (needs
> > discussion)", we need a new method that will allow users to escape consumer
> > group names and other names.  So you can feed this method your "foo\*"
> > consumer group name, and it will give you "foo\\\*", which is what you
> > would need to use to create an ACL for this consumer group in AdminClient.
> > I think that's the only change we need to admin client
> >
> > regards,
> > Colin
> >
> >
> > On Thu, May 17, 2018, at 08:55, Piyush Vijay wrote:
> > > Hi Rajini/Colin,
> > >
> > > I will remove the wildcard principals from the scope for now, updating
> > KIP
> > > right now and will open it for vote.
> > >
> > > Thanks
> > >
> > >
> > > Piyush Vijay
> > >
> > > On Thu, May 17, 2018 at 6:59 AM, Rajini Sivaram  > >
> > > wrote:
> > >
> > > > Hi Piyush,
> > > >
> > > > I have added a PR (https://github.com/apache/kafka/pull/5030) with
> > tests
> > > > to
> > > > show how group principals can be used for authorization with custom
> > > > principal builders. One of the tests uses SASL. It is not quite the
> > same as
> > > > a full-fledged user groups, but since it works with all security
> > protocols,
> > > > it could be an alternative to wildcarded principals.
> > > >
> > > > Let us know if we can help in any way to get this KIP updated and
> > ready for
> > > > voting to include in 2.0.0.
> > > >
> > > > Thanks,
> > > >
> > > > Rajini
> > > >
> > > >
> > > > On Wed, May 16, 2018 at 10:21 PM, Colin McCabe 
> > wrote:
> > > >
> > > > > > On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram <
> > > > rajinisiva...@gmail.com
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Piyush,
> > > > > > >
> > > > > > > It is possible to configure PrincipalBuilder for SASL (
> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > 189%3A+Improve+principal+builder+interface+and+add+
> > > > support+for+SASL).
> > > > > If
> > > > > > > that satisfies your requirements, perhaps we can move wildcarded
> > > > > principals
> > > > > > > out of this KIP and focus on wildcarded resources?
> > > > >
> > > > > +1.
> > > > >
> > > > > We also need to determine which characters will be reserved for the
> > > > > future.  I think previously we thought about @, #, $, %, ^, &, *.
> > > > >
> > > > > > > On Tue, May 15, 2018 at 7:15 PM, Piyush Vijay <
> > > > piyushvij...@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > >> Hi Colin,
> > > > > > >>
> > > > > > >> Escaping at this level is making sense to me but let me think
> > more
> > > > > and get
> > > > > > >> back to you.
> > > > >
> > > > > Thanks, Piyush.  What questions do you think are still open regarding
> > > > > escape characters?
> > > > > As Rajini mentioned, we have to get this in soon in order to make
> > the KIP
> > > > > freeze.
> > > > >
> > > > > > >>
> > > > > > >> But should we not just get rid of one of AclBinding or
> > > > > AclBindingFilter
> > > > > > >> then? Is there a reason to keep both given that
> > AclBindingFilter and
> > > > > > >> AclBinding look exact copy of each other after this change? This
> > > > will
> > > > > be a
> > > > > > >> one-time breaking change in APIs marked as "Evolving", but makes
> > > > > sense in
> > > > > > >> the long term? Am I missing something here?
> > > > >
> > > > > AclBinding represents an ACL.  AclBindingFilter is a filter which
> > can be
> > > > > used to locate AclBinding objects.  Similarly with Resource and
> > > > > ResourceFilter.  There is no reason to combine them because they
> > > > represent
> > > > > different things.  Although they contain many of the same fields,
> > they
> > > > are
> > > > > not exact copies.  Many fields can be null in AclBindingFilter--
> > fields
> > > > can
> > > > > never be null in AclBinding.
> > > > >
> > > > > For example, you can have an AclBindingFilter that matches every
> > > > > AclBinding.  There is more discussion of this on the original KIP
> > that
> > > > > added ACL support to AdminClient.
> > > > >
> > > > > best,
> > > > > Colin
> > > > >
> > > > > > >>
> > > > > > >>
> > > > > > >>
> > > > > > >> Piyush Vijay
> > > > > > >>
> > > > > > >> On 

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

Hi Viktor,

Since the KIP freeze is coming up really soon, maybe we should just drop the 
section about changes to AlterConfigs from KIP-248.  We don't really need it 
here, since ConfigCommand can use AlterConfigs as-is.

We can pick up the discussion about improving AlterConfigs in a future KIP.

cheers,
Colin

On Wed, May 16, 2018, at 22:06, Colin McCabe wrote:
> Hi Viktor,
> 
> The shell command isn’t that easy to integrate into applications.
> AdminClient will get integrated  into a lot more stuff, which
> increases the potential for conflicts.  I would argue that we should
> fix this soon.
> If we do want to reduce the scope in this KIP, we could do the merge in
> the ConfigCommand  tool for now, and leave AC unchanged.
> Best,
> Colin
> 
> 
> On Wed, May 16, 2018, at 04:57, Viktor Somogyi wrote:
> > Hi Colin,
> >
> > > Doing get-merge-set is buggy, though.  If someone else does get-merge-
> > > set at the same time as you, you might overwrite that person's
> > > changes, or vice versa.  So I really don't think we should try to do
> > > this.  Also, having both an incremental and a full API is useful,
> > > and it's just a single boolean at the protocol and API level.>
> > Overwriting somebody's change is currently possible with the
> > ConfigCommand, as it will do this get-merge-set behavior on the client> 
> > side, in the command. From this perspective I think it's not much
> > different to do this with the admin client. Also I think admins don't> 
> > modify the quotas/configs of a client/user/topic/broker often (and
> > multiple admins would do it even more rarely), so I don't think it is> a 
> > big issue. What I think would be useful here but may be out of scope> is to 
> > version the changes similarly to leader epochs. So when an admin> updates 
> > the configs, it will increment a version number and won't let> other admins 
> > to push changes in with lower than that. Instead it would> return an error.
> >
> > I would be also interested what others think about this?
> >
> > Cheers,
> > Viktor
> >
> >
> > On Sat, May 12, 2018 at 2:29 AM, Colin McCabe
> >  wrote:> > On Wed, May 9, 2018, at 05:41, Viktor 
> > Somogyi wrote:
> > >> Hi Colin,
> > >>
> > >> > We are going to need to create a new version of
> > >> > AlterConfigsRequest to add the "incremental" boolean.  So while
> > >> > we're doing that, maybe we can change the type to
> > >> > NULLABLE_STRING.> >>
> > >> I was just talking to a colleague yesterday and we came to the
> > >> conclusion that we should keep the boolean flag only on the client> >> 
> > >> side (as you may have suggested earlier?) and not make part of the> >> 
> > >> protocol as it might lead to a very complicated API on the long
> > >> term.> >> Also we would keep the server side API simpler. Instead of the
> > >> protocol change we could just simply have the boolean flag in
> > >> AlterConfigOptions and the AdminClient should do the get-merge-set> >> 
> > >> logic which corresponds to the behavior of the current
> > >> ConfigCommand.> >> That way we won't need to change the protocol for now 
> > >> but
> > >> still have> >> both functionality. What do you think?
> > >
> > >  Hi Viktor,
> > >
> > > Doing get-merge-set is buggy, though.  If someone else does get-merge-
> > > set at the same time as you, you might overwrite that person's
> > > changes, or vice versa.  So I really don't think we should try to do
> > > this.  Also, having both an incremental and a full API is useful,
> > > and it's just a single boolean at the protocol and API level.> >
> > >>
> > >> > Hmm.  Not sure I follow.  KIP-133 doesn't use the empty string or
> > >> > "" to indicate defaults, does it?> >>
> > >> No it doesn't. It was just my early idea to indicate "delete"
> > >> on the> >> protocol level. (We are using  for denoting the 
> > >> default
> > >> client id or user in zookeeper.) Rajini was referring that we
> > >> shouldn't expose this to the protocol level but instead denote
> > >> delete> >> with an empty string.
> > >>
> > >> > This comes from DescribeConfigsResponse.
> > >> > Unless I'm missing something, I think your suggestion to not
> > >> > expose "" is already implemented?> >>
> > >> In some way, yes. Although this one is used in describe and not in> >> 
> > >> alter. For alter I don't think we'd need my early "" idea.> >
> > > OK.  Thanks for the explanation.  Using an empty string to indicate
> > > delete, as Rajini suggested, seems pretty reasonable to me.  null
> > > would work as well.> >
> > >>
> > >> >> And we use STRING rather than NULLABLE_STRING in describe
> > >> >> configs etc. So we> >> >> should probably do the same for quotas."
> > >> >
> > >> > I think nearly all responses treat ERROR_MESSAGE as a nullable
> > >> > string.  CommonFields#ERROR_MESSAGE, which is used by most of
> > >> > them, is a nullable string.  It's DescribeConfigsResponse that is
> > >> > the black sheep here.> >> >
> > >> >  > public static final Field.NullableStr ERROR_MESSAGE = new

Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs

[Piyush Vijay]

I was planning to do that.

Another unrelated detail is the presence of the support for ‘*’ ACL
currently. Looks like we’ll have to keep supporting this as a special case,
even though using a different location for wildcard-suffix ACLs on Zk.



On Thu, May 17, 2018 at 9:15 AM Colin McCabe  wrote:

> Thanks, Piyush.  +1 for starting the vote soon.
>
> Can you please also add a discussion about escaping?  For example, earlier
> we discussed using backslashes to escape special characters.  So that users
> can create an ACL referring to a literal "foo*" group by creating an ACL
> for "foo\*"  Similarly, you can get a literal backslash with "\\".  This is
> the standard UNIX escaping mechanism.
>
> Also, for the section that says "Changes to AdminClient (needs
> discussion)", we need a new method that will allow users to escape consumer
> group names and other names.  So you can feed this method your "foo\*"
> consumer group name, and it will give you "foo\\\*", which is what you
> would need to use to create an ACL for this consumer group in AdminClient.
> I think that's the only change we need to admin client
>
> regards,
> Colin
>
>
> On Thu, May 17, 2018, at 08:55, Piyush Vijay wrote:
> > Hi Rajini/Colin,
> >
> > I will remove the wildcard principals from the scope for now, updating
> KIP
> > right now and will open it for vote.
> >
> > Thanks
> >
> >
> > Piyush Vijay
> >
> > On Thu, May 17, 2018 at 6:59 AM, Rajini Sivaram  >
> > wrote:
> >
> > > Hi Piyush,
> > >
> > > I have added a PR (https://github.com/apache/kafka/pull/5030) with
> tests
> > > to
> > > show how group principals can be used for authorization with custom
> > > principal builders. One of the tests uses SASL. It is not quite the
> same as
> > > a full-fledged user groups, but since it works with all security
> protocols,
> > > it could be an alternative to wildcarded principals.
> > >
> > > Let us know if we can help in any way to get this KIP updated and
> ready for
> > > voting to include in 2.0.0.
> > >
> > > Thanks,
> > >
> > > Rajini
> > >
> > >
> > > On Wed, May 16, 2018 at 10:21 PM, Colin McCabe 
> wrote:
> > >
> > > > > On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram <
> > > rajinisiva...@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Piyush,
> > > > > >
> > > > > > It is possible to configure PrincipalBuilder for SASL (
> > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > 189%3A+Improve+principal+builder+interface+and+add+
> > > support+for+SASL).
> > > > If
> > > > > > that satisfies your requirements, perhaps we can move wildcarded
> > > > principals
> > > > > > out of this KIP and focus on wildcarded resources?
> > > >
> > > > +1.
> > > >
> > > > We also need to determine which characters will be reserved for the
> > > > future.  I think previously we thought about @, #, $, %, ^, &, *.
> > > >
> > > > > > On Tue, May 15, 2018 at 7:15 PM, Piyush Vijay <
> > > piyushvij...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > >> Hi Colin,
> > > > > >>
> > > > > >> Escaping at this level is making sense to me but let me think
> more
> > > > and get
> > > > > >> back to you.
> > > >
> > > > Thanks, Piyush.  What questions do you think are still open regarding
> > > > escape characters?
> > > > As Rajini mentioned, we have to get this in soon in order to make
> the KIP
> > > > freeze.
> > > >
> > > > > >>
> > > > > >> But should we not just get rid of one of AclBinding or
> > > > AclBindingFilter
> > > > > >> then? Is there a reason to keep both given that
> AclBindingFilter and
> > > > > >> AclBinding look exact copy of each other after this change? This
> > > will
> > > > be a
> > > > > >> one-time breaking change in APIs marked as "Evolving", but makes
> > > > sense in
> > > > > >> the long term? Am I missing something here?
> > > >
> > > > AclBinding represents an ACL.  AclBindingFilter is a filter which
> can be
> > > > used to locate AclBinding objects.  Similarly with Resource and
> > > > ResourceFilter.  There is no reason to combine them because they
> > > represent
> > > > different things.  Although they contain many of the same fields,
> they
> > > are
> > > > not exact copies.  Many fields can be null in AclBindingFilter--
> fields
> > > can
> > > > never be null in AclBinding.
> > > >
> > > > For example, you can have an AclBindingFilter that matches every
> > > > AclBinding.  There is more discussion of this on the original KIP
> that
> > > > added ACL support to AdminClient.
> > > >
> > > > best,
> > > > Colin
> > > >
> > > > > >>
> > > > > >>
> > > > > >>
> > > > > >> Piyush Vijay
> > > > > >>
> > > > > >> On Tue, May 15, 2018 at 9:01 AM, Colin McCabe <
> cmcc...@apache.org>
> > > > wrote:
> > > > > >>
> > > > > >> > Hi Piyush,
> > > > > >> >
> > > > > >> > I think AclBinding should operate the same way as
> > > AclBindingFilter.
> > > > > >> >
> > > > > >> > So you should be able to do something like this:
> > > > > >> > > AclBindingFilter filter = new AclBindingFiler(new
> >

Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Colin McCabe]

Hi Robert,

Hmm.  I thought that if you're using ConfigChangeCallback, you are relying on 
the ConfigProvider to make a callback to you when the configuration has 
changed.  So isn't that always the "push model" (where the ConfigProvider 
pushes changes to Connect).  If you want the "pull model" where you initiate 
updates, you can simply call ConfigProvider#get directly, right?

The actual implementation of ConfigProvider subclasses will depend on the type 
of configuration storage mechanism on the backend.  In the case of Vault, it 
sounds like we need to have something like a ScheduledExecutor which re-fetches 
keys after a certain amount of time.

As an aside, what does a "lease duration" mean for a configuration key?  Does 
that mean Vault will reject changes to the configuration key if I try to make 
them within the lease duration?  Or is this like a period after which a 
password is automatically rotated?

On Wed, May 16, 2018, at 22:25, Robert Yokota wrote:
> Hi Colin,
> 
> > With regard to delayMs, can’t we just restart the
> > Connector when the keys are actually changed?
> 
> Currently the VaultConfigProvider does not find out when values for keys
> have changed.  You could do this with a poll model (with a
> background thread in the ConfigProvider), but since for each key-value
> pair, Vault provides a lease duration stating exactly when a value for a
> key will change in the future, this is an alternative model of just passing
> the lease duration to the client (in this case the Connector), to allow it
> to determine what to do (such as schedule a restart).   This may allow one
> to avoid the complexity of figuring out a proper poll interval (with lease
> durations of varying periods), or worrying about putting too much load on
> the secrets manager by polling too often.

Those things are still concerns if the Connector is polling, right?  Perhaps 
the connector poll too often and puts too much load on Vault.  And so forth.  
It seems like this problem needs to be solved either way (and probably can be 
solved with reasonable default minimum fetch intervals).

best,
Colin


>  In other words, by adding this
> one additional parameter, a ConfigProvider can provide both push and pull
> models to clients, perhaps with an additional configuration parameter to
> the ConfigProvider to determine which model (push or poll) to use.
> 
> Thanks,
> Robert
> 
> On Wed, May 16, 2018 at 9:56 PM, Colin McCabe  wrote:
> 
> > Thanks, Robert.  With regard to delayMs, can’t we just restart the
> > Connector when the keys are actually changed?  Or is the concern that
> > this would lengthen the effective key rotation time?  Can’t the user
> > just configure a slightly shorter key rotation time to counteract
> > this concern?
> > Regards,
> > Colin
> >
> > On Wed, May 16, 2018, at 19:13, Robert Yokota wrote:
> > > Hi Colin,
> > >
> > > Good questions.
> > >
> > >
> > > > As a clarification about the indirections, what if I have the
> > > > connect> configuration key foo set up as ${vault:bar}, and in Vault,
> > > have the bar> key set to ${file:baz}?
> > > > Does connect get foo as the contents of the baz file?  I would
> > > > argue that> it should not (and in general, we shouldn't allow
> > ConfigProviders to
> > > indirect to other
> > > > ConfigProviders) but I don't think it's spelled out right now.
> > >
> > > I've added a clarification to the KIP that further indirections are
> > > not> performed even if the values returned from ConfigProviders have the
> > > variable syntax.
> > >
> > >
> > > > What's the behavior when a config key is not found in Vault
> > > > (or other> ConfigProvider)?  Does the variable get replaced with the
> > empty
> > > string, or> with the literal ${vault:whatever} string?
> > >
> > > It would remain unresolved and still be of the form
> > > ${provider:key}.  I've> added a clarification to the KIP.
> > >
> > >
> > > > Do we really need "${provider:[path:]key}", or can it just be
> > > ${provider:key}?
> > >
> > > The path is a separate parameter in the APIs, so I think it's
> > > important to> explicitly delineate it in the variable syntax.  For
> > example, I
> > > currently> have a working VaultConfigProvider prototype and the syntax
> > for a
> > > Vault key> reference looks like
> > >
> > > db_password=${vault:secret/staging:mysql_password}
> > >
> > > I think it's important to standardize how to separate the path
> > > from the key> rather than leave it to each ConfigProvider to determine a
> > possibly
> > > different way.  This will also make it easier to move secrets from one>
> > ConfigProvider to another should one choose to do so.
> > >
> > >
> > > > Do we really need delayMs?
> > >
> > > One of the goals of this KIP is to allow for secrets rotation without>
> > having to modify existing connectors.  In the case of the
> > > VaultConfigProvider, it knows the lease durations and will be able to>
> > schedule a restart of the Connector using an API in the Herder.  The
> >

Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs

[Colin McCabe]

Thanks, Piyush.  +1 for starting the vote soon.

Can you please also add a discussion about escaping?  For example, earlier we 
discussed using backslashes to escape special characters.  So that users can 
create an ACL referring to a literal "foo*" group by creating an ACL for 
"foo\*"  Similarly, you can get a literal backslash with "\\".  This is the 
standard UNIX escaping mechanism.

Also, for the section that says "Changes to AdminClient (needs discussion)", we 
need a new method that will allow users to escape consumer group names and 
other names.  So you can feed this method your "foo\*" consumer group name, and 
it will give you "foo\\\*", which is what you would need to use to create an 
ACL for this consumer group in AdminClient.  I think that's the only change we 
need to admin client

regards,
Colin


On Thu, May 17, 2018, at 08:55, Piyush Vijay wrote:
> Hi Rajini/Colin,
> 
> I will remove the wildcard principals from the scope for now, updating KIP
> right now and will open it for vote.
> 
> Thanks
> 
> 
> Piyush Vijay
> 
> On Thu, May 17, 2018 at 6:59 AM, Rajini Sivaram 
> wrote:
> 
> > Hi Piyush,
> >
> > I have added a PR (https://github.com/apache/kafka/pull/5030) with tests
> > to
> > show how group principals can be used for authorization with custom
> > principal builders. One of the tests uses SASL. It is not quite the same as
> > a full-fledged user groups, but since it works with all security protocols,
> > it could be an alternative to wildcarded principals.
> >
> > Let us know if we can help in any way to get this KIP updated and ready for
> > voting to include in 2.0.0.
> >
> > Thanks,
> >
> > Rajini
> >
> >
> > On Wed, May 16, 2018 at 10:21 PM, Colin McCabe  wrote:
> >
> > > > On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram <
> > rajinisiva...@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > Hi Piyush,
> > > > >
> > > > > It is possible to configure PrincipalBuilder for SASL (
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > 189%3A+Improve+principal+builder+interface+and+add+
> > support+for+SASL).
> > > If
> > > > > that satisfies your requirements, perhaps we can move wildcarded
> > > principals
> > > > > out of this KIP and focus on wildcarded resources?
> > >
> > > +1.
> > >
> > > We also need to determine which characters will be reserved for the
> > > future.  I think previously we thought about @, #, $, %, ^, &, *.
> > >
> > > > > On Tue, May 15, 2018 at 7:15 PM, Piyush Vijay <
> > piyushvij...@gmail.com>
> > > > > wrote:
> > > > >
> > > > >> Hi Colin,
> > > > >>
> > > > >> Escaping at this level is making sense to me but let me think more
> > > and get
> > > > >> back to you.
> > >
> > > Thanks, Piyush.  What questions do you think are still open regarding
> > > escape characters?
> > > As Rajini mentioned, we have to get this in soon in order to make the KIP
> > > freeze.
> > >
> > > > >>
> > > > >> But should we not just get rid of one of AclBinding or
> > > AclBindingFilter
> > > > >> then? Is there a reason to keep both given that AclBindingFilter and
> > > > >> AclBinding look exact copy of each other after this change? This
> > will
> > > be a
> > > > >> one-time breaking change in APIs marked as "Evolving", but makes
> > > sense in
> > > > >> the long term? Am I missing something here?
> > >
> > > AclBinding represents an ACL.  AclBindingFilter is a filter which can be
> > > used to locate AclBinding objects.  Similarly with Resource and
> > > ResourceFilter.  There is no reason to combine them because they
> > represent
> > > different things.  Although they contain many of the same fields, they
> > are
> > > not exact copies.  Many fields can be null in AclBindingFilter-- fields
> > can
> > > never be null in AclBinding.
> > >
> > > For example, you can have an AclBindingFilter that matches every
> > > AclBinding.  There is more discussion of this on the original KIP that
> > > added ACL support to AdminClient.
> > >
> > > best,
> > > Colin
> > >
> > > > >>
> > > > >>
> > > > >>
> > > > >> Piyush Vijay
> > > > >>
> > > > >> On Tue, May 15, 2018 at 9:01 AM, Colin McCabe 
> > > wrote:
> > > > >>
> > > > >> > Hi Piyush,
> > > > >> >
> > > > >> > I think AclBinding should operate the same way as
> > AclBindingFilter.
> > > > >> >
> > > > >> > So you should be able to do something like this:
> > > > >> > > AclBindingFilter filter = new AclBindingFiler(new
> > > > >> > ResourceFilter(ResourceType.GROUP, "foo*"))
> > > > >> > > AclBinding binding = new AclBinding(new
> > > Resource(ResourceType.GROUP,
> > > > >> > "foo*"))
> > > > >> > > assertTrue(filter.matches(binding));
> > > > >> >
> > > > >> > Thinking about this more, it's starting to feel really messy to
> > > create
> > > > >> new
> > > > >> > "pattern" constructors for Resource and ResourceFilter.  I don't
> > > think
> > > > >> > people will be able to figure this out.  Maybe we should just
> > have a
> > > > >> > limited compatibility break here, where it is now required to
> > e

Re: [DISCUSS] KIP-298: Error Handling in Connect

[Konstantine Karantasis]

Thanks Arjun for your quick response.

Adding an example for the failure log improves things, but I think it'd be
better to also add the schema definition of these Json entries. And I'll
agree with Magesh that this format should be public API.

Also, does the current example have a copy/paste typo? Seems that the
TRANSFORMATION stage in the end has the config of a converter.
Similar to the above, fields for 'key' and 'headers' (and their conversion
stages) are skipped when they are not defined? Or should they present and
empty? A schema definition would help to know what a consumer of such logs
should expect.

Also, thanks for adding some info for error on the source side. However, I
feel the current description might be a little bit ambiguous. I read:
"For errors in a source connector, the process is similar, but care needs
to be taken while writing back to the source." and sounds like it's
suggested that Connect will write records back to the source, which can't
be correct.

Finally, a nit: " adds store the row information "... typo?

Thanks,
- Konstantine



On Thu, May 17, 2018 at 12:48 AM, Arjun Satish 
wrote:

> On Wed, May 16, 2018 at 7:13 PM, Matt Farmer  wrote:
>
> > Hey Arjun,
> >
> > I like deadletterqueue all lower case, so I'm +1 on that.
> >
>
> Super! updated the KIP.
>
>
> >
> > Yes, in the case we were seeing there were external system failures.
> > We had issues connecting to S3. While the connector does include
> > some retry functionality, however setting these values sufficiently
> > high seemed to cause us to hit timeouts and cause the entire
> > task to fail anyway. (I think I was using something like 100 retries
> > during the brief test of this behavior?)
> >
>
> I am guessing these issues come up with trying to write to S3. Do you think
> the S3 connector can detect the safe situations where it can throw
> RetriableExceptions instead of ConnectExceptions here (when the connector
> think it is safe to do so)?
>
>
> >
> > Yeah, totally understand that there could be unintended concequences
> > from this. I guess the use case I'm trying to optimize for is to give
> > folks some bubblegum to keep a high volume system limping
> > along until the software engineers get time to address it. So I'm
> > imagining the situation that I'm paged on a Saturday night because of
> > an intermittent network issue. With a config flag like this I could push
> > a config change to cause Connect to treat that as retriable and allow
> > me to wait until the following Monday to make changes to the code.
> > That may not be a sensible concern for Kafka writ large, but Connect
> > is a bit weird when compared with Streams or the Clients. It's almost
> > more of a piece of infrastructure than a library, and I generally like
> > infrastructure to have escape hatches like that. Just my 0.02 though. :)
> >
>
> haha yes, it would be good to avoid those Saturday night pagers. Again, I
> am hesitant to imply retries on ConnectExceptions. We could definitely
> define new Exceptions in the Connector, which can be thrown to retry if the
> connector thinks it is safe to do so. We need to know that a retry can be
> super dangerous in a Task.put(List). Duplicate records can
> easily creep in, and can be notoriously hard to detect and clean up.
>
>
>
> > Thanks,
> > Matt
> >
> > On Tue, May 15, 2018 at 8:46 PM, Arjun Satish 
> > wrote:
> >
> > > Matt,
> > >
> > > Thanks so much for your comments. Really appreciate it!
> > >
> > > 1. Good point about the acronym. I can use deadletterqueue instead of
> dlq
> > > (using all lowercase to be consistent with the other configs in Kafka).
> > > What do you think?
> > >
> > > 2. Could you please tell us what errors caused these tasks to fail?
> Were
> > > they because of external system failures? And if so, could they be
> > > implemented in the Connector itself? Or using retries with backoffs?
> > >
> > > 3. I like this idea. But did not include it here since it might be a
> > > stretch. One thing to note is that ConnectExceptions can be thrown
> from a
> > > variety of places in a connector. I think it should be OK for the
> > Connector
> > > to throw RetriableException or something that extends it for the
> > operation
> > > to be retried. By changing this behavior, a lot of existing connectors
> > > would have to be updated so that they don't rewrite messages into this
> > > sink. For example, a sink connector might write some data into the
> > external
> > > system partially, and then fail with a ConnectException. Since the
> > > framework has no way of knowing what was written and what was not, a
> > retry
> > > here might cause the same data to written again into the sink.
> > >
> > > Best,
> > >
> > >
> > > On Mon, May 14, 2018 at 12:46 PM, Matt Farmer  wrote:
> > >
> > > > Hi Arjun,
> > > >
> > > > I'm following this very closely as better error handling in Connect
> is
> > a
> > > > high priority
> > > > for MailChimp's Data Systems team.
> > > >
> > > > A few thoughts

Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs

[Piyush Vijay]

Hi Rajini/Colin,

I will remove the wildcard principals from the scope for now, updating KIP
right now and will open it for vote.

Thanks


Piyush Vijay

On Thu, May 17, 2018 at 6:59 AM, Rajini Sivaram 
wrote:

> Hi Piyush,
>
> I have added a PR (https://github.com/apache/kafka/pull/5030) with tests
> to
> show how group principals can be used for authorization with custom
> principal builders. One of the tests uses SASL. It is not quite the same as
> a full-fledged user groups, but since it works with all security protocols,
> it could be an alternative to wildcarded principals.
>
> Let us know if we can help in any way to get this KIP updated and ready for
> voting to include in 2.0.0.
>
> Thanks,
>
> Rajini
>
>
> On Wed, May 16, 2018 at 10:21 PM, Colin McCabe  wrote:
>
> > > On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram <
> rajinisiva...@gmail.com
> > >
> > > wrote:
> > >
> > > > Hi Piyush,
> > > >
> > > > It is possible to configure PrincipalBuilder for SASL (
> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > 189%3A+Improve+principal+builder+interface+and+add+
> support+for+SASL).
> > If
> > > > that satisfies your requirements, perhaps we can move wildcarded
> > principals
> > > > out of this KIP and focus on wildcarded resources?
> >
> > +1.
> >
> > We also need to determine which characters will be reserved for the
> > future.  I think previously we thought about @, #, $, %, ^, &, *.
> >
> > > > On Tue, May 15, 2018 at 7:15 PM, Piyush Vijay <
> piyushvij...@gmail.com>
> > > > wrote:
> > > >
> > > >> Hi Colin,
> > > >>
> > > >> Escaping at this level is making sense to me but let me think more
> > and get
> > > >> back to you.
> >
> > Thanks, Piyush.  What questions do you think are still open regarding
> > escape characters?
> > As Rajini mentioned, we have to get this in soon in order to make the KIP
> > freeze.
> >
> > > >>
> > > >> But should we not just get rid of one of AclBinding or
> > AclBindingFilter
> > > >> then? Is there a reason to keep both given that AclBindingFilter and
> > > >> AclBinding look exact copy of each other after this change? This
> will
> > be a
> > > >> one-time breaking change in APIs marked as "Evolving", but makes
> > sense in
> > > >> the long term? Am I missing something here?
> >
> > AclBinding represents an ACL.  AclBindingFilter is a filter which can be
> > used to locate AclBinding objects.  Similarly with Resource and
> > ResourceFilter.  There is no reason to combine them because they
> represent
> > different things.  Although they contain many of the same fields, they
> are
> > not exact copies.  Many fields can be null in AclBindingFilter-- fields
> can
> > never be null in AclBinding.
> >
> > For example, you can have an AclBindingFilter that matches every
> > AclBinding.  There is more discussion of this on the original KIP that
> > added ACL support to AdminClient.
> >
> > best,
> > Colin
> >
> > > >>
> > > >>
> > > >>
> > > >> Piyush Vijay
> > > >>
> > > >> On Tue, May 15, 2018 at 9:01 AM, Colin McCabe 
> > wrote:
> > > >>
> > > >> > Hi Piyush,
> > > >> >
> > > >> > I think AclBinding should operate the same way as
> AclBindingFilter.
> > > >> >
> > > >> > So you should be able to do something like this:
> > > >> > > AclBindingFilter filter = new AclBindingFiler(new
> > > >> > ResourceFilter(ResourceType.GROUP, "foo*"))
> > > >> > > AclBinding binding = new AclBinding(new
> > Resource(ResourceType.GROUP,
> > > >> > "foo*"))
> > > >> > > assertTrue(filter.matches(binding));
> > > >> >
> > > >> > Thinking about this more, it's starting to feel really messy to
> > create
> > > >> new
> > > >> > "pattern" constructors for Resource and ResourceFilter.  I don't
> > think
> > > >> > people will be able to figure this out.  Maybe we should just
> have a
> > > >> > limited compatibility break here, where it is now required to
> escape
> > > >> weird
> > > >> > consumer group names when creating ACLs for them.
> > > >> >
> > > >> > To future-proof this, we should reserve a bunch of characters at
> > once,
> > > >> > like *, @, $, %, ^, &, +, [, ], etc.  If these characters appear
> in
> > a
> > > >> > resource name, it should be an error, unless they are escaped
> with a
> > > >> > backslash.  That way, we can use them in the future.  We should
> > create a
> > > >> > Resource.escapeName function which adds the correct escape
> > characters to
> > > >> > resource names (so it would translate foo* into foo\*, foo+bar
> into
> > > >> > foo\+bar, etc. etc.
> > > >> >
> > > >> > best,
> > > >> > Colin
> > > >> >
> > > >> >
> > > >> > On Mon, May 14, 2018, at 17:08, Piyush Vijay wrote:
> > > >> > > Colin,
> > > >> > >
> > > >> > > createAcls take a AclBinding, however, instead of
> > AclBindingFilter.
> > > >> What
> > > >> > > are your thoughts here?
> > > >> > >
> > > >> > > public abstract DescribeAclsResult describeAcls(AclBindingFilter
> > > >> > > filter, DescribeAclsOptions options);
> > > >> > >
> > > >> > > public abstract CreateAclsRe

Re: [VOTE] KIP-255: OAuth Authentication via SASL/OAUTHBEARER

[Ron Dagostino]

Hi Jun.  I've updated the KIP to add a new section titled "Summary for
Production Use" that includes this information along with a consolidated
set of references to the applicable specifications.  Thanks for the
questions.

*We still need another binding vote* (currently there are two binding + 1
votes, from Rajini and Jun, and three non-binding +1 votes, from Mickael,
Manikumar, and myself).

Please vote before the May 22nd KIP Freeze deadline so this KIP can be
included in the 2.0.0 release.

A pull request is available and includes additional commits reflecting
initial review comments: https://github.com/apache/kafka/pull/4994

Ron

On Wed, May 16, 2018 at 8:09 PM, Jun Rao  wrote:

> Hi, Ron,
>
> Thanks. I understand now. It may be useful to add a reference to JWT in the
> KIP.
>
> Jun
>
> On Tue, May 15, 2018 at 6:51 PM, Ron Dagostino  wrote:
>
> > Hi Jun.  I think you are getting at the fact that OAuth 2 is a flexible
> > framework that allows different installations to do things differently.
> It
> > is true that the principal name in Kafka could come from any claim in the
> > token.  Most of the time it would come from the 'sub' claim, but it could
> > certainly come from another claim, or it could be only indirectly based
> on
> > a claim value (maybe certain text would be trimmed or prefixed/suffixed).
> > The point, which I think you are getting at, is that because the
> framework
> > is flexible we need to accommodate that flexibility.  The callback
> handler
> > class defined by the listener.name.sasl_ssl.oauthbearer.sasl.server.
> > callback.handler.class configuration value gives us the required
> > flexibility.  As an example, I have an implementation that leverages a
> > popular open source JOSE library to parse the compact serialization,
> > retrieve the public key if it has not yet been retrieved, verify the
> > digital signature, and map the 'sub' claim to the OAuthBearerToken's
> > principal name (which becomes the SASL authorization ID, which becomes
> the
> > Kafka principal name).  I could just as easily have mapped a different
> > claim to the OAuthBearerToken's principal name, manipulated the 'sub'
> claim
> > value in some way, etc.  I write the callback handler code, so I complete
> > flexibility to do whatever my OAuth 2 installation requires me to do.
> >
> > Ron
> >
> > On Tue, May 15, 2018 at 1:39 PM, Jun Rao  wrote:
> >
> > > Hi, Ron,
> > >
> > > Thanks for the reply. I understood your answers to #2 and #3.
> > >
> > > For #1, will the server map all clients' principal name to the value
> > > associated with "sub" claim? How do we support mapping different
> clients
> > to
> > > different principal names?
> > >
> > > Jun
> > >
> > > On Mon, May 14, 2018 at 7:02 PM, Ron Dagostino 
> > wrote:
> > >
> > > > Hi Jun.  Thanks for the +1 vote.
> > > >
> > > > Regarding the first question about token claims, yes, you have it
> > correct
> > > > about translating the OAuth token to a principle name via a JAAS
> module
> > > > option in the default unsecured case.  Specifically, the OAuth SASL
> > > Server
> > > > implementation is responsible for setting the authorization ID, and
> it
> > > gets
> > > > the authorization ID from the OAuthBearerToken's principalName()
> > method.
> > > > The listener.name.sasl_ssl.oauthbearer.sasl.server.
> > > callback.handler.class
> > > > is responsible for handling an instance of
> OAuthBearerValidatorCallback
> > > to
> > > > accept a token compact serialization from the client and return an
> > > instance
> > > > of OAuthBearerToken (assuming the compact serialization validates),
> and
> > > in
> > > > the default unsecured case the builtin unsecured validator callback
> > > handler
> > > > defines the OAuthBearerToken.principalName() method to return the
> > 'sub'
> > > > claim value by default (with the actual claim it uses being
> > configurable
> > > > via the unsecuredValidatorPrincipalClaimName JAAS module option).
> So
> > > that
> > > > is how we translate from a token to a principal name in the default
> > > > unsecured (out-of-the-box) case.
> > > >
> > > > For production use cases, the implementation associated with
> > > > listener.name.sasl_ssl.oauthbearer.sasl.server.
> callback.handler.class
> > > can
> > > > do whatever it wants.  As an example, I have written a class that
> > wraps a
> > > > com.nimbusds.jwt.SignedJWT instance (see
> > > > https://connect2id.com/products/nimbus-jose-jwt/) and presents it as
> > an
> > > > OAuthBearerToken:
> > > >
> > > > public class NimbusSignedJwtOAuthBearerToken implements
> > > OAuthBearerToken {
> > > > private final SignedJWT signedJwt;
> > > > private final String principalName;
> > > > private final Set scope;
> > > > private final Long startTimeMs;
> > > > private final long lifetimeMs;
> > > >
> > > > /**
> > > >  * Constructor
> > > >  *
> > > >  * @param signedJwt
> > > >  *the mandatory signed JWT
> > > >  * @param principalCl

Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs

[Rajini Sivaram]

Hi Piyush,

I have added a PR (https://github.com/apache/kafka/pull/5030) with tests to
show how group principals can be used for authorization with custom
principal builders. One of the tests uses SASL. It is not quite the same as
a full-fledged user groups, but since it works with all security protocols,
it could be an alternative to wildcarded principals.

Let us know if we can help in any way to get this KIP updated and ready for
voting to include in 2.0.0.

Thanks,

Rajini


On Wed, May 16, 2018 at 10:21 PM, Colin McCabe  wrote:

> > On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram  >
> > wrote:
> >
> > > Hi Piyush,
> > >
> > > It is possible to configure PrincipalBuilder for SASL (
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 189%3A+Improve+principal+builder+interface+and+add+support+for+SASL).
> If
> > > that satisfies your requirements, perhaps we can move wildcarded
> principals
> > > out of this KIP and focus on wildcarded resources?
>
> +1.
>
> We also need to determine which characters will be reserved for the
> future.  I think previously we thought about @, #, $, %, ^, &, *.
>
> > > On Tue, May 15, 2018 at 7:15 PM, Piyush Vijay 
> > > wrote:
> > >
> > >> Hi Colin,
> > >>
> > >> Escaping at this level is making sense to me but let me think more
> and get
> > >> back to you.
>
> Thanks, Piyush.  What questions do you think are still open regarding
> escape characters?
> As Rajini mentioned, we have to get this in soon in order to make the KIP
> freeze.
>
> > >>
> > >> But should we not just get rid of one of AclBinding or
> AclBindingFilter
> > >> then? Is there a reason to keep both given that AclBindingFilter and
> > >> AclBinding look exact copy of each other after this change? This will
> be a
> > >> one-time breaking change in APIs marked as "Evolving", but makes
> sense in
> > >> the long term? Am I missing something here?
>
> AclBinding represents an ACL.  AclBindingFilter is a filter which can be
> used to locate AclBinding objects.  Similarly with Resource and
> ResourceFilter.  There is no reason to combine them because they represent
> different things.  Although they contain many of the same fields, they are
> not exact copies.  Many fields can be null in AclBindingFilter-- fields can
> never be null in AclBinding.
>
> For example, you can have an AclBindingFilter that matches every
> AclBinding.  There is more discussion of this on the original KIP that
> added ACL support to AdminClient.
>
> best,
> Colin
>
> > >>
> > >>
> > >>
> > >> Piyush Vijay
> > >>
> > >> On Tue, May 15, 2018 at 9:01 AM, Colin McCabe 
> wrote:
> > >>
> > >> > Hi Piyush,
> > >> >
> > >> > I think AclBinding should operate the same way as AclBindingFilter.
> > >> >
> > >> > So you should be able to do something like this:
> > >> > > AclBindingFilter filter = new AclBindingFiler(new
> > >> > ResourceFilter(ResourceType.GROUP, "foo*"))
> > >> > > AclBinding binding = new AclBinding(new
> Resource(ResourceType.GROUP,
> > >> > "foo*"))
> > >> > > assertTrue(filter.matches(binding));
> > >> >
> > >> > Thinking about this more, it's starting to feel really messy to
> create
> > >> new
> > >> > "pattern" constructors for Resource and ResourceFilter.  I don't
> think
> > >> > people will be able to figure this out.  Maybe we should just have a
> > >> > limited compatibility break here, where it is now required to escape
> > >> weird
> > >> > consumer group names when creating ACLs for them.
> > >> >
> > >> > To future-proof this, we should reserve a bunch of characters at
> once,
> > >> > like *, @, $, %, ^, &, +, [, ], etc.  If these characters appear in
> a
> > >> > resource name, it should be an error, unless they are escaped with a
> > >> > backslash.  That way, we can use them in the future.  We should
> create a
> > >> > Resource.escapeName function which adds the correct escape
> characters to
> > >> > resource names (so it would translate foo* into foo\*, foo+bar into
> > >> > foo\+bar, etc. etc.
> > >> >
> > >> > best,
> > >> > Colin
> > >> >
> > >> >
> > >> > On Mon, May 14, 2018, at 17:08, Piyush Vijay wrote:
> > >> > > Colin,
> > >> > >
> > >> > > createAcls take a AclBinding, however, instead of
> AclBindingFilter.
> > >> What
> > >> > > are your thoughts here?
> > >> > >
> > >> > > public abstract DescribeAclsResult describeAcls(AclBindingFilter
> > >> > > filter, DescribeAclsOptions options);
> > >> > >
> > >> > > public abstract CreateAclsResult createAcls(Collection<
> AclBinding>
> > >> > > acls, CreateAclsOptions options);
> > >> > >
> > >> > > public abstract DeleteAclsResult
> > >> > > deleteAcls(Collection filters,
> DeleteAclsOptions
> > >> > > options);
> > >> > >
> > >> > >
> > >> > > Thanks
> > >> > >
> > >> > > Piyush Vijay
> > >> > >
> > >> > > On Mon, May 14, 2018 at 9:26 AM, Andy Coates 
> > >> wrote:
> > >> > >
> > >> > > > +1
> > >> > > >
> > >> > > > On 11 May 2018 at 17:14, Colin McCabe 
> wrote:
> > >> > > >
> > >> > > > > Hi Andy,
> > >> > > > >
> > >> > > > > I

[jira] [Created] (KAFKA-6912) Add authorization tests for custom principal types

[Rajini Sivaram (JIRA)]

Rajini Sivaram created KAFKA-6912:
-

 Summary: Add authorization tests for custom principal types
 Key: KAFKA-6912
 URL: https://issues.apache.org/jira/browse/KAFKA-6912
 Project: Kafka
  Issue Type: Task
  Components: core
Reporter: Rajini Sivaram
Assignee: Rajini Sivaram
 Fix For: 2.0.0


KIP-290 proposes to add prefixed-wildcarded principals to enable ACLs to be 
configured for groups of principals. This doesn't work with all security 
protocols - e.g. SSL principals are of format CN=name,O=org,C=country where 
prefixes don't fit in terms of grouping. Kafka currently doesn't support the 
concept of user groups, but it is possible to use custom KafkaPrincipalBuilders 
to generate group principals during authentication. By default, Kafka generates 
principals of type User, but custom types (e.g. Group) are supported. This does 
currently have the restriction ACLs may be defined only at group level (cannot 
combine both user & group level ACLs for a connection), but it works currently 
for all security protocols.

We don't have any tests that verify custom principal types and authorization 
based on custom principal types. It will be good to add some tests.

 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: [DISCUSS] KIP-304: Connect runtime mode improvements for container platforms

[Rahul Singh]

First sentence fat fingered.

“Just curious as to why there’s an issue with the backing topics for Kafka 
Connect.”

--
Rahul Singh
rahul.si...@anant.us

Anant Corporation

On May 17, 2018, 6:17 AM -0400, Stephane Maarek 
, wrote:
> Hi Salius
>
> I think you're on the money, but you're not pushing things too far.
> This is something I've hoped for a long time.
> Let's talk Kafka Connect v2
>
> Kafka Connect Cluster, as you said, are not convenient to work with (the
> KIP details drawbacks well). I'm all about containerisation just like
> stream apps support (and boasts!).
>
> Now, here's the problem with Kafka Connect. There are three backing topics.
> Here's the analysis of how they can evolve:
> - Config topic: this one is irrelevant if each connect cluster comes with a
> config bundled with the corresponding JAR, as you mentioned in your KIP
> - Status topic: this is something I wish was gone too. The consumers have a
> coordinator, and I believe the connect workers should have a coordinator
> too, for task rebalancing.
> - Source Offset topic: only relevant for sources. I wish there was a
> __connect_offsets global topic just like for consumers and an
> "ConnectOffsetCoordinator" to talk to to retrieve latest committed offset.
>
> If we look above, with a few back-end fundamental transformations, we can
> probably make Connect "cluster-less".
>
> What the community would get out of it is huge:
> - Connect workers for a specific connector are independent and isolated,
> measurable (in CPU and Mem) and auto-scalable
> - CI/CD is super easy to integrate, as it's just another container / jar.
> - You can roll restart a specific connector and upgrade a JAR without
> interrupting your other connectors and while keeping the current connector
> from running.
> - The topics backing connect are removed except the global one, which
> allows you to scale easily in terms of number of connectors
> - Running a connector in dev or prod (for people offering connectors) is as
> easy as doing a simple "docker run".
> - Each consumer / producer settings can be configured at the container
> level.
> - Each connect process is immutable in configuration.
> - Each connect process has its own security identity (right now, you need a
> connect cluster per service role, which is a lot of overhead in terms of
> backing topic)
>
> Now, I don't have the Kafka expertise to know exactly which changes to make
> in the code, but I believe the final idea is achievable.
> The change would be breaking for how Kafka Connect is run, but I think
> there's a chance to make the change non breaking to how Connect is
> programmed. I believe the same public API framework can be used.
>
> Finally, the REST API can be used for monitoring, or the JMX metrics as
> usual.
>
> I may be completely wrong, but I would see such a change drive the
> utilisation, management of Connect by a lot while lowering the barrier to
> adoption.
>
> This change may be big to implement but probably worthwhile. I'd be happy
> to provide more "user feedback" on a PR, but probably won't be able to
> implement a PR myself.
>
> More than happy to discuss this
>
> Best,
> Stephane
>
>
> Kind regards,
> Stephane
>
> [image: Simple Machines]
>
> Stephane Maarek | Developer
>
> +61 416 575 980
> steph...@simplemachines.com.au
> simplemachines.com.au
> Level 2, 145 William Street, Sydney NSW 2010
>
> On 17 May 2018 at 14:42, Saulius Valatka  wrote:
>
> > Hi,
> >
> > the only real usecase for the REST interface I can see is providing
> > health/liveness checks for mesos/kubernetes. It's also true that the API
> > can be left as is and e.g. not exposed publicly on the platform level, but
> > this would still leave opportunities to accidentally mess something up
> > internally, so it's mostly a safety concern.
> >
> > Regarding the option renaming: I agree that it's not necessary as it's not
> > clashing with anything, my reasoning is that assuming some other offset
> > storage appears in the future, having all config properties at the root
> > level of offset.storage.* _MIGHT_ introduce clashes in the future, so this
> > is just a suggestion for introducing a convention of
> > offset.storage.., which the existing
> > property offset.storage.file.filename already adheres to. But in general,
> > yes -- this can be left as is.
> >
> >
> >
> > 2018-05-17 1:20 GMT+03:00 Jakub Scholz :
> >
> > > Hi,
> > >
> > > What do you plan to use the read-only REST interface for? Is there
> > > something what you cannot get through metrics interface? Otherwise it
> > might
> > > be easier to just disable the REST interface (either in the code, or just
> > > on the platform level - e.g. in Kubernetes).
> > >
> > > Also, I do not know what is the usual approach in Kafka ... but do we
> > > really have to rename the offset.storage.* options? The current names do
> > > not seem to have any collision with what you are adding and they would
> > get
> > > "out of sync" with the other options used in connec

Re: [DISCUSS] KIP-304: Connect runtime mode improvements for container platforms

[Rahul Singh]

Just curious as to why there’s a pr been for the backing topics for Kafka 
Connect.

I’m a fan of decoupling and love the world that operates in containers.

I’m just wondering specifically what the issue is for having topics storing the 
connect states for source / sinks.

Wouldn’t it make sense to keep it “central” to have one state of truth even if 
a coordinator crashes or whatever. Otherwise if you move everything including 
config to separate workers — you essentially have to manage all your config for 
each worker.

I like how Kubernetes works with configmaps. You register configmaps in kubectl 
and then can use those values in different places. Otherwise we’d be back to 
environment variables as in Docker compose. Having central state in etcd allows 
kube to create and deploy stateless deployments, pods, services.


--
Rahul Singh
rahul.si...@anant.us

Anant Corporation

On May 17, 2018, 6:17 AM -0400, Stephane Maarek 
, wrote:
> Hi Salius
>
> I think you're on the money, but you're not pushing things too far.
> This is something I've hoped for a long time.
> Let's talk Kafka Connect v2
>
> Kafka Connect Cluster, as you said, are not convenient to work with (the
> KIP details drawbacks well). I'm all about containerisation just like
> stream apps support (and boasts!).
>
> Now, here's the problem with Kafka Connect. There are three backing topics.
> Here's the analysis of how they can evolve:
> - Config topic: this one is irrelevant if each connect cluster comes with a
> config bundled with the corresponding JAR, as you mentioned in your KIP
> - Status topic: this is something I wish was gone too. The consumers have a
> coordinator, and I believe the connect workers should have a coordinator
> too, for task rebalancing.
> - Source Offset topic: only relevant for sources. I wish there was a
> __connect_offsets global topic just like for consumers and an
> "ConnectOffsetCoordinator" to talk to to retrieve latest committed offset.
>
> If we look above, with a few back-end fundamental transformations, we can
> probably make Connect "cluster-less".
>
> What the community would get out of it is huge:
> - Connect workers for a specific connector are independent and isolated,
> measurable (in CPU and Mem) and auto-scalable
> - CI/CD is super easy to integrate, as it's just another container / jar.
> - You can roll restart a specific connector and upgrade a JAR without
> interrupting your other connectors and while keeping the current connector
> from running.
> - The topics backing connect are removed except the global one, which
> allows you to scale easily in terms of number of connectors
> - Running a connector in dev or prod (for people offering connectors) is as
> easy as doing a simple "docker run".
> - Each consumer / producer settings can be configured at the container
> level.
> - Each connect process is immutable in configuration.
> - Each connect process has its own security identity (right now, you need a
> connect cluster per service role, which is a lot of overhead in terms of
> backing topic)
>
> Now, I don't have the Kafka expertise to know exactly which changes to make
> in the code, but I believe the final idea is achievable.
> The change would be breaking for how Kafka Connect is run, but I think
> there's a chance to make the change non breaking to how Connect is
> programmed. I believe the same public API framework can be used.
>
> Finally, the REST API can be used for monitoring, or the JMX metrics as
> usual.
>
> I may be completely wrong, but I would see such a change drive the
> utilisation, management of Connect by a lot while lowering the barrier to
> adoption.
>
> This change may be big to implement but probably worthwhile. I'd be happy
> to provide more "user feedback" on a PR, but probably won't be able to
> implement a PR myself.
>
> More than happy to discuss this
>
> Best,
> Stephane
>
>
> Kind regards,
> Stephane
>
> [image: Simple Machines]
>
> Stephane Maarek | Developer
>
> +61 416 575 980
> steph...@simplemachines.com.au
> simplemachines.com.au
> Level 2, 145 William Street, Sydney NSW 2010
>
> On 17 May 2018 at 14:42, Saulius Valatka  wrote:
>
> > Hi,
> >
> > the only real usecase for the REST interface I can see is providing
> > health/liveness checks for mesos/kubernetes. It's also true that the API
> > can be left as is and e.g. not exposed publicly on the platform level, but
> > this would still leave opportunities to accidentally mess something up
> > internally, so it's mostly a safety concern.
> >
> > Regarding the option renaming: I agree that it's not necessary as it's not
> > clashing with anything, my reasoning is that assuming some other offset
> > storage appears in the future, having all config properties at the root
> > level of offset.storage.* _MIGHT_ introduce clashes in the future, so this
> > is just a suggestion for introducing a convention of
> > offset.storage.., which the existing
> > property offset.storage.file.filename already adheres to. Bu

RE: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS resolved IP addresses

[Skrzypek, Jonathan]

Yes, makes sense.
You mentioned multiple times you see no overlap and no issue with your KIP, and 
that they solve different use cases.

Appreciate you have an existing use case that would work, but we need to make 
sure this isn't confusing to users and that any combination will always work, 
across security protocols.

A solution might be to expose to users the choice of using hostname or 
canonical host name on both sides.
Say having one setting that collapses functionalities from both KIPs (bootstrap 
expansion + advertised lookup) and an additional parameter that defines how the 
resolution is performed, using getCanonicalHostName() or not. 

Maybe that gives less flexibility as users wouldn't be able to decide to only 
perform DNS lookup on bootstrap.servers or on advertised listeners.
But this would ensure consistency so that a user can decide to use cnames or 
not (depending on their certificates and Kerberos principals in their 
environment) and it would work.

Jonathan Skrzypek 

-Original Message-
From: Edoardo Comar [mailto:edoco...@gmail.com] 
Sent: 16 May 2018 21:59
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS resolved 
IP addresses

Hi Jonathan,
I am afraid that may not work for everybody.

It would not work for us.
With our current DNS, my Kafka clients are perfectly happy to use any IPs -
DNS has multiple A records for the 'myhostname.mydomain' used for
bootstrap and advertised listeners.
The hosts all serve TLS certificates that include
'myhostname.mydomain'  and the clients are happy.

However, applying getCanonicalHostName to those IPs would return
hostnames that would not match the TLS certificates.

So once again I believe your solution and ours solve different use cases.

cheers
Edo

On 16 May 2018 at 18:29, Skrzypek, Jonathan  wrote:
> I think there are combinations that will break SASL and SSL auth.
> Could the trick be to have a single parameter that triggers dns resolve both 
> for bootstrap and advertised listeners, both using getCanonicalHostName() ?
>
> Jonathan Skrzypek
>
> -Original Message-
> From: Edoardo Comar [mailto:edoco...@gmail.com]
> Sent: 16 May 2018 17:03
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS resolved 
> IP addresses
>
> Hi Rajini,
>
> In your example KIP-302 would attempt to connect to the first address
> returned, let's say
>
> www.apache.org/195.154.151.36
>
> then, only if that fails, will in turn try the remaining:
>
> www.apache.org/40.79.78.1
> www.apache.org/140.211.11.105
> www.apache.org/2001:bc8:2142:300:0:0:0:0
>
> You're right to say that we expect certificates served by those
> endpoints to be valid for "www.apache.org"
>
> Without KIP-302, only one would be attempted.
> Which is the first one, that can change every time
> (typically changes on every Java process restart,
> but may change also any time InetAddress.getAllByName it's invoked
> depending on the caching).
>
> The behavioral change that KIP-302 may introduce is that in the example above,
> also an IPv6 connection may be attempted after some IPv4 ones.
>
> InetAddress.getAllByName() implementation uses a system property
> "java.net.preferIPv6Addresses"
> to decide which type of address to return first (default is still IPv4
> in java 10)
>
> We will amend the KIP and PR so that the loop only uses IPs of the
> same type as the first one returned.
>
> A part from the above, KIP 302 does not seem to change any existing
> client behaviour, as any one of multiple IP addresses (of a given
> v4/v6 type) can currently be picked.
> We're happy however to keep the looping behavior optional with the
> discussed config property, disabled by default.
>
> As for KIP-235 that may introduce new hostnames in the bootstrap list
> (the current PR rewrites the bootstrap list)
> and we fail to see the conflict with KIP-302, whatever the set of
> configs chosen.
>
> We'd be happy to try understand what we are missing in a KIP call :-)
>
> cheers
> Edo
>
> On 15 May 2018 at 16:58, Rajini Sivaram  wrote:
>> Hi Edo,
>>
>> I agree that KIP-235 and KIP-302 address different scenarios. And I agree
>> that each one is not sufficient in itself to address both the scenarios.
>> But I also think that they conflict and hence they need to be looked at
>> together and perhaps use a single config.
>>
>> As an example:
>>
>> If I run:
>>
>> for (InetAddress address : InetAddress.getAllByName("www.apache.org")) {
>> System.out.printf("HostName %s canonicalHostName %s IP %s\n",
>> address.getHostName(), address.getCanonicalHostName(),
>> address.getHostAddress());
>> }
>>
>> I get:
>>
>> HostName www.apache.org canonicalHostName tlp-eu-west.apache.org IP
>> 195.154.151.36
>> HostName www.apache.org canonicalHostName 40.79.78.1 IP 40.79.78.1
>> HostName www.apache.org canonicalHostName themis.apache.org IP
>> 140.211.11.105
>> HostName www.apache.org canonicalHostName 2001:bc8:2142:300:0:0:

Re: [DISCUSS] KIP-304: Connect runtime mode improvements for container platforms

[Stephane Maarek]

Hi Salius

I think you're on the money, but you're not pushing things too far.
This is something I've hoped for a long time.
Let's talk Kafka Connect v2

Kafka Connect Cluster, as you said, are not convenient to work with (the
KIP details drawbacks well). I'm all about containerisation just like
stream apps support (and boasts!).

Now, here's the problem with Kafka Connect. There are three backing topics.
Here's the analysis of how they can evolve:
- Config topic: this one is irrelevant if each connect cluster comes with a
config bundled with the corresponding JAR, as you mentioned in your KIP
- Status topic: this is something I wish was gone too. The consumers have a
coordinator, and I believe the connect workers should have a coordinator
too, for task rebalancing.
- Source Offset topic: only relevant for sources. I wish there was a
__connect_offsets global topic just like for consumers and an
"ConnectOffsetCoordinator" to talk to to retrieve latest committed offset.

If we look above, with a few back-end fundamental transformations, we can
probably make Connect "cluster-less".

What the community would get out of it is huge:
- Connect workers for a specific connector are independent and isolated,
measurable (in CPU and Mem) and auto-scalable
- CI/CD is super easy to integrate, as it's just another container / jar.
- You can roll restart a specific connector and upgrade a JAR without
interrupting your other connectors and while keeping the current connector
from running.
- The topics backing connect are removed except the global one, which
allows you to scale easily in terms of number of connectors
- Running a connector in dev or prod (for people offering connectors) is as
easy as doing a simple "docker run".
- Each consumer / producer settings can be configured at the container
level.
- Each connect process is immutable in configuration.
- Each connect process has its own security identity (right now, you need a
connect cluster per service role, which is a lot of overhead in terms of
backing topic)

Now, I don't have the Kafka expertise to know exactly which changes to make
in the code, but I believe the final idea is achievable.
The change would be breaking for how Kafka Connect is run, but I think
there's a chance to make the change non breaking to how Connect is
programmed. I believe the same public API framework can be used.

Finally, the REST API can be used for monitoring, or the JMX metrics as
usual.

I may be completely wrong, but I would see such a change drive the
utilisation, management of Connect by a lot while lowering the barrier to
adoption.

This change may be big to implement but probably worthwhile. I'd be happy
to provide more "user feedback" on a PR, but probably won't be able to
implement a PR myself.

More than happy to discuss this

Best,
Stephane


Kind regards,
Stephane

[image: Simple Machines]

Stephane Maarek | Developer

+61 416 575 980
steph...@simplemachines.com.au
simplemachines.com.au
Level 2, 145 William Street, Sydney NSW 2010

On 17 May 2018 at 14:42, Saulius Valatka  wrote:

> Hi,
>
> the only real usecase for the REST interface I can see is providing
> health/liveness checks for mesos/kubernetes. It's also true that the API
> can be left as is and e.g. not exposed publicly on the platform level, but
> this would still leave opportunities to accidentally mess something up
> internally, so it's mostly a safety concern.
>
> Regarding the option renaming: I agree that it's not necessary as it's not
> clashing with anything, my reasoning is that assuming some other offset
> storage appears in the future, having all config properties at the root
> level of offset.storage.* _MIGHT_ introduce clashes in the future, so this
> is just a suggestion for introducing a convention of
> offset.storage.., which the existing
> property offset.storage.file.filename already adheres to. But in general,
> yes -- this can be left as is.
>
>
>
> 2018-05-17 1:20 GMT+03:00 Jakub Scholz :
>
> > Hi,
> >
> > What do you plan to use the read-only REST interface for? Is there
> > something what you cannot get through metrics interface? Otherwise it
> might
> > be easier to just disable the REST interface (either in the code, or just
> > on the platform level - e.g. in Kubernetes).
> >
> > Also, I do not know what is the usual approach in Kafka ... but do we
> > really have to rename the offset.storage.* options? The current names do
> > not seem to have any collision with what you are adding and they would
> get
> > "out of sync" with the other options used in connect (status.storage.*
> and
> > config.storage.*). So it seems a bit unnecessary change to me.
> >
> > Jakub
> >
> >
> >
> > On Wed, May 16, 2018 at 10:10 PM Saulius Valatka 
> > wrote:
> >
> > > Hi,
> > >
> > > I'd like to start a discussion on the following KIP:
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 304%3A+Connect+runtime+mode+improvements+for+container+platforms
> > >
> > > Basically the idea is to mak

[jira] [Created] (KAFKA-6911) Incorrect check for keystore/truststore dynamic update

[Rajini Sivaram (JIRA)]

Rajini Sivaram created KAFKA-6911:
-

 Summary: Incorrect check for keystore/truststore dynamic update
 Key: KAFKA-6911
 URL: https://issues.apache.org/jira/browse/KAFKA-6911
 Project: Kafka
  Issue Type: Task
  Components: core
Affects Versions: 1.1.0
Reporter: Rajini Sivaram
Assignee: Rajini Sivaram
 Fix For: 2.0.0, 1.1.1


The check to see if keystore or truststore needs updating is incorrect - it 
checks if one of the configs has not changed, rather than changed.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: [DISCUSS] KIP-304: Connect runtime mode improvements for container platforms

[Saulius Valatka]

Hi,

the only real usecase for the REST interface I can see is providing
health/liveness checks for mesos/kubernetes. It's also true that the API
can be left as is and e.g. not exposed publicly on the platform level, but
this would still leave opportunities to accidentally mess something up
internally, so it's mostly a safety concern.

Regarding the option renaming: I agree that it's not necessary as it's not
clashing with anything, my reasoning is that assuming some other offset
storage appears in the future, having all config properties at the root
level of offset.storage.* _MIGHT_ introduce clashes in the future, so this
is just a suggestion for introducing a convention of
offset.storage.., which the existing
property offset.storage.file.filename already adheres to. But in general,
yes -- this can be left as is.



2018-05-17 1:20 GMT+03:00 Jakub Scholz :

> Hi,
>
> What do you plan to use the read-only REST interface for? Is there
> something what you cannot get through metrics interface? Otherwise it might
> be easier to just disable the REST interface (either in the code, or just
> on the platform level - e.g. in Kubernetes).
>
> Also, I do not know what is the usual approach in Kafka ... but do we
> really have to rename the offset.storage.* options? The current names do
> not seem to have any collision with what you are adding and they would get
> "out of sync" with the other options used in connect (status.storage.* and
> config.storage.*). So it seems a bit unnecessary change to me.
>
> Jakub
>
>
>
> On Wed, May 16, 2018 at 10:10 PM Saulius Valatka 
> wrote:
>
> > Hi,
> >
> > I'd like to start a discussion on the following KIP:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 304%3A+Connect+runtime+mode+improvements+for+container+platforms
> >
> > Basically the idea is to make it easier to run separate instances of
> Kafka
> > Connect hosting isolated connectors on container platforms such as Mesos
> or
> > Kubernetes.
> >
> > In particular it would be interesting to hear opinions about the proposed
> > read-only REST API mode, more specifically I'm concerned about the
> > possibility to implement it in distributed mode as it appears the
> framework
> > is using it internally (
> >
> > https://github.com/apache/kafka/blob/trunk/connect/
> runtime/src/main/java/org/apache/kafka/connect/runtime/
> distributed/DistributedHerder.java#L1019
> > ),
> > however this particular API method appears to be undocumented(?).
> >
> > Looking forward for your feedback.
> >
>

[VOTE] KIP-298: Error Handling in Connect kafka

[Arjun Satish]

All,

Many thanks for all the feedback on KIP-298. Highly appreciate the time and
effort you all put into it.

I've updated the KIP accordingly, and would like to start to start a vote
on it.

KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-
298%3A+Error+Handling+in+Connect
JIRA: https://issues.apache.org/jira/browse/KAFKA-6738
Discussion Thread: https://www.mail-archive.com/
dev@kafka.apache.org/msg87660.html

Thanks very much!

Re: [DISCUSS] KIP-298: Error Handling in Connect

[Arjun Satish]

On Wed, May 16, 2018 at 7:13 PM, Matt Farmer  wrote:

> Hey Arjun,
>
> I like deadletterqueue all lower case, so I'm +1 on that.
>

Super! updated the KIP.


>
> Yes, in the case we were seeing there were external system failures.
> We had issues connecting to S3. While the connector does include
> some retry functionality, however setting these values sufficiently
> high seemed to cause us to hit timeouts and cause the entire
> task to fail anyway. (I think I was using something like 100 retries
> during the brief test of this behavior?)
>

I am guessing these issues come up with trying to write to S3. Do you think
the S3 connector can detect the safe situations where it can throw
RetriableExceptions instead of ConnectExceptions here (when the connector
think it is safe to do so)?


>
> Yeah, totally understand that there could be unintended concequences
> from this. I guess the use case I'm trying to optimize for is to give
> folks some bubblegum to keep a high volume system limping
> along until the software engineers get time to address it. So I'm
> imagining the situation that I'm paged on a Saturday night because of
> an intermittent network issue. With a config flag like this I could push
> a config change to cause Connect to treat that as retriable and allow
> me to wait until the following Monday to make changes to the code.
> That may not be a sensible concern for Kafka writ large, but Connect
> is a bit weird when compared with Streams or the Clients. It's almost
> more of a piece of infrastructure than a library, and I generally like
> infrastructure to have escape hatches like that. Just my 0.02 though. :)
>

haha yes, it would be good to avoid those Saturday night pagers. Again, I
am hesitant to imply retries on ConnectExceptions. We could definitely
define new Exceptions in the Connector, which can be thrown to retry if the
connector thinks it is safe to do so. We need to know that a retry can be
super dangerous in a Task.put(List). Duplicate records can
easily creep in, and can be notoriously hard to detect and clean up.



> Thanks,
> Matt
>
> On Tue, May 15, 2018 at 8:46 PM, Arjun Satish 
> wrote:
>
> > Matt,
> >
> > Thanks so much for your comments. Really appreciate it!
> >
> > 1. Good point about the acronym. I can use deadletterqueue instead of dlq
> > (using all lowercase to be consistent with the other configs in Kafka).
> > What do you think?
> >
> > 2. Could you please tell us what errors caused these tasks to fail? Were
> > they because of external system failures? And if so, could they be
> > implemented in the Connector itself? Or using retries with backoffs?
> >
> > 3. I like this idea. But did not include it here since it might be a
> > stretch. One thing to note is that ConnectExceptions can be thrown from a
> > variety of places in a connector. I think it should be OK for the
> Connector
> > to throw RetriableException or something that extends it for the
> operation
> > to be retried. By changing this behavior, a lot of existing connectors
> > would have to be updated so that they don't rewrite messages into this
> > sink. For example, a sink connector might write some data into the
> external
> > system partially, and then fail with a ConnectException. Since the
> > framework has no way of knowing what was written and what was not, a
> retry
> > here might cause the same data to written again into the sink.
> >
> > Best,
> >
> >
> > On Mon, May 14, 2018 at 12:46 PM, Matt Farmer  wrote:
> >
> > > Hi Arjun,
> > >
> > > I'm following this very closely as better error handling in Connect is
> a
> > > high priority
> > > for MailChimp's Data Systems team.
> > >
> > > A few thoughts (in no particular order):
> > >
> > > For the dead letter queue configuration, could we use deadLetterQueue
> > > instead of
> > > dlq? Acronyms are notoriously hard to keep straight in everyone's head
> > and
> > > unless
> > > there's a compelling reason it would be nice to use the characters and
> be
> > > explicit.
> > >
> > > Have you considered any behavior that would periodically attempt to
> > restart
> > > failed
> > > tasks after a certain amount of time? To get around our issues
> internally
> > > we've
> > > deployed a tool that monitors for failed tasks and restarts the task by
> > > hitting the
> > > REST API after the failure. Such a config would allow us to get rid of
> > this
> > > tool.
> > >
> > > Have you considered a config setting to allow-list additional classes
> as
> > > retryable? In the situation we ran into, we were getting
> > ConnectExceptions
> > > that
> > > were intermittent due to an unrelated service. With such a setting we
> > could
> > > have
> > > deployed a config that temporarily whitelisted that Exception as
> > > retry-worthy
> > > and continued attempting to make progress while the other team worked
> > > on mitigating the problem.
> > >
> > > Thanks for the KIP!
> > >
> > > On Wed, May 9, 2018 at 2:59 AM, Arjun Satish 
> > > wrote:
> > >
> > > > All,
> > >

Re: [DISCUSS] KIP-298: Error Handling in Connect

[Arjun Satish]

Magesh,

Updated the KIP. Thanks a lot!

Best,

On Wed, May 16, 2018 at 7:12 PM, Magesh Nandakumar 
wrote:

> Arjun,
>
> Thanks for all the changes. Technically, the message format used for the
> DLQ should be part of the public interface since users could consume it and
> take actions.
>
> Thanks,
> Magesh
>
> On Wed, May 16, 2018 at 6:56 PM, Arjun Satish 
> wrote:
>
> > Hi Konstantine,
> >
> > Thanks a lot for your feedback. I have made the necessary changes to the
> > KIP.
> >
> > Best,
> >
> > On Wed, May 16, 2018 at 11:38 AM, Konstantine Karantasis <
> > konstant...@confluent.io> wrote:
> >
> > > Arjun, it's exciting to see a KIP around better handling of bad-data
> and
> > > errors in Kafka Connect.
> > >
> > > I have only a few comments below, which I hope you'll find helpful.
> > >
> > > 1. I think it'd be useful to describe a bit more in detail how someone
> > can
> > > extract the raw data of a Kafka record that failed to get converted (on
> > the
> > > sink side in this example). How's the JSON schema looks like for an
> entry
> > > that is added to the dead-letter-queue and what someone should do to
> get
> > > the raw bytes?
> > >
> > > 2. Similarly, it'd be nice to describe a bit more what is placed or
> > > attempted to be placed in the dead-letter-queue in the case of source
> > > records that fail to get imported to Kafka. Currently the only
> sentence I
> > > read related to that is: "Similarly, for source connectors, the
> developer
> > > can write the corrected records back to the original source".
> > >
> > > 3. I think the plural for 'retries' in config options:
> > > 'errors.retries.limit' and 'errors.retries.delay.max.ms' doesn't read
> > very
> > > well. Should 'retry' be used same as 'tolerance' (or 'log') is used
> right
> > > below? For example:
> > > errors.retry.limit
> > > and
> > > errors.retry.delay.max.ms
> > >
> > > 4. Should the metric names be 'total-record-failures' and
> > > 'total-records-skipped' to match their metric description and also be
> > > similar to 'total-retries'?
> > >
> > > And a few minor comments:
> > >
> > > - The domain of 'errors.retries.limit' does not include 0 in the
> allowed
> > > values (even though it's the default value).
> > >
> > > - For someone unfamiliar with the term SMT, the acronym is not
> explained
> > in
> > > the text. Also the term transformations is better IMO.
> > >
> > > - typo: 'the task is to killed'
> > >
> > > - If you intend to add a link to a PR additionally to the jira ticket,
> > it'd
> > > be handy to add it to the KIP header (along with state, thread, jira,
> > etc).
> > > Now it's a bit hidden in the text and it's not clear that the KIP
> > includes
> > > a link to a PR.
> > >
> > > Thanks for working on this missing but important functionality.
> > >
> > > - Konstantine
> > >
> > >
> > > On Tue, May 15, 2018 at 10:41 PM, Arjun Satish  >
> > > wrote:
> > >
> > > > Magesh,
> > > >
> > > > Just to add to your point about retriable exceptions: the producer
> can
> > > > throw retriable exceptions which we are handling it here:
> > > >
> > > > https://github.com/apache/kafka/blob/trunk/connect/
> > > > runtime/src/main/java/org/apache/kafka/connect/runtime/
> > > > WorkerSourceTask.java#L275
> > > >
> > > > BTW, exceptions like TimeoutExceptions (which extend
> > RetriableExceptions)
> > > > are bubbled back to the application, and need to be handled as per
> > > > application requirements.
> > > >
> > > > Best,
> > > >
> > > > On Tue, May 15, 2018 at 8:30 PM, Arjun Satish <
> arjun.sat...@gmail.com>
> > > > wrote:
> > > >
> > > > > Magesh,
> > > > >
> > > > > Thanks for the feedback! Really appreciate your comments.
> > > > >
> > > > > 1. I updated the KIP to state that only the configs of the failed
> > > > > operation will be emitted. Thank you!
> > > > >
> > > > > The purpose of bundling the configs of the failed operation along
> > with
> > > > the
> > > > > error context is to have a single place to find everything relevant
> > to
> > > > the
> > > > > failure. This way, we can only look at the error logs to find the
> > most
> > > > > common pieces to "failure" puzzles: the operation, the config and
> the
> > > > input
> > > > > record. Ideally, a programmer should be able to take these pieces
> and
> > > > > reproduce the error locally.
> > > > >
> > > > > 2. Added a table to describe this in the KIP.
> > > > >
> > > > > 3. Raw bytes will be base64 encoded before being logged. Updated
> the
> > > KIP
> > > > > to state this. Thank you!
> > > > >
> > > > > 4. I'll add an example log4j config to show we can take logs from a
> > > class
> > > > > and redirect it to a different location. Made a note in the PR for
> > > this.
> > > > >
> > > > > 5. When we talk about logging messages, this could mean instances
> of
> > > > > SinkRecords or SourceRecords. When we disable logging of messages,
> > > these
> > > > > records would be replaced by a "null". If you think it makes sense,
> > > > instead
> > > > > of complete