[jira] [Resolved] (KAFKA-4908) consumer.properties logging warnings

2018-01-08 Thread Seweryn Habdank-Wojewodzki (JIRA)

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

Seweryn Habdank-Wojewodzki resolved KAFKA-4908.
---
Resolution: Done

Not an issue for me anymore.

> consumer.properties logging warnings
> 
>
> Key: KAFKA-4908
> URL: https://issues.apache.org/jira/browse/KAFKA-4908
> Project: Kafka
>  Issue Type: Bug
>Affects Versions: 0.10.2.0
>Reporter: Seweryn Habdank-Wojewodzki
>Priority: Minor
>
> default consumer.properties at startaup of the console consumer delivered 
> with Kafka package are logging warnings:
> [2017-03-15 16:36:57,439] WARN The configuration 
> 'zookeeper.connection.timeout.ms' was supplied but isn't a known config. 
> (org.apache.kafka.clients.consumer.ConsumerConfig)
> [2017-03-15 16:36:57,455] WARN The configuration 'zookeeper.connect' was 
> supplied but isn't a known config. 
> (org.apache.kafka.clients.consumer.ConsumerConfig)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Resolved] (KAFKA-4315) Kafka Connect documentation problems

2018-01-08 Thread Seweryn Habdank-Wojewodzki (JIRA)

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

Seweryn Habdank-Wojewodzki resolved KAFKA-4315.
---
Resolution: Done

I do not care anymore about this matter.

> Kafka Connect documentation problems
> 
>
> Key: KAFKA-4315
> URL: https://issues.apache.org/jira/browse/KAFKA-4315
> Project: Kafka
>  Issue Type: Bug
>Reporter: Seweryn Habdank-Wojewodzki
>
> On the base of documentation of the Kafka Connect - 
> http://kafka.apache.org/documentation#connect, I had tried to build example 
> in Java. It was not possible. 
> The code pieces available on the webpage are taken out of any context and 
> they are not compiling. 
> Also it seems they are taken completely from other code software parts, so 
> even putting them together shows, that they are not building any reasonable 
> example. And they tend to be very complex. where I would expect that the API 
> examples are driving "Hello World" like code.
> Also there are weak connections between examples from the Kafka documentation 
> and Kafka Connect tools code parts available in the Kafka source.
> Finally I would be nice to have a kind of statement in the Kafka 
> documentation which parts of API are stable and which are unstable or 
> experimental.
> I saw much (~20) of such a remarks in the Kafka code - I mean that API is 
> unstable. This note is very important, as we will plan additional effort to 
> prepare some facades for unstable code.
> In my opinion it is nothing wrong in experimental API, but all those matters 
> when documented shall be well documented. The current status of the main 
> Kafka documentation makes impression that Kafka Connect is well tested and 
> consistent and stable feature set, but it is not. What leads to confusion on 
> the effort management.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[VOTE] KIP-174 Deprecate and remove internal converter configs in WorkerConfig

2018-01-08 Thread UMESH CHAUDHARY
Hello All,
Since there are no outstanding comments on this, so I'd like to start a
vote.

Please find the KIP here

and
the related JIRA here .

The KIP suggests to deprecate and remove the configs:
internal.key.converter, internal.value.converter

Appreciate your comments.

Regards,
Umesh


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

2018-01-08 Thread Apache Jenkins Server
See 


Changes:

[ismael] MINOR: Catch JsonMappingException subclass (#3821)

--
[...truncated 401.14 KB...]
kafka.tools.ConsoleProducerTest > testParseKeyProp STARTED

kafka.tools.ConsoleProducerTest > testParseKeyProp PASSED

kafka.tools.ConsoleProducerTest > testValidConfigsOldProducer STARTED

kafka.tools.ConsoleProducerTest > testValidConfigsOldProducer PASSED

kafka.tools.ConsoleProducerTest > testInvalidConfigs STARTED

kafka.tools.ConsoleProducerTest > testInvalidConfigs PASSED

kafka.tools.ConsoleProducerTest > testValidConfigsNewProducer STARTED

kafka.tools.ConsoleProducerTest > testValidConfigsNewProducer PASSED

kafka.tools.MirrorMakerTest > 
testDefaultMirrorMakerMessageHandlerWithNoTimestampInSourceMessage STARTED

kafka.tools.MirrorMakerTest > 
testDefaultMirrorMakerMessageHandlerWithNoTimestampInSourceMessage PASSED

kafka.tools.MirrorMakerTest > testDefaultMirrorMakerMessageHandler STARTED

kafka.tools.MirrorMakerTest > testDefaultMirrorMakerMessageHandler PASSED

kafka.tools.MirrorMakerTest > testDefaultMirrorMakerMessageHandlerWithHeaders 
STARTED

kafka.tools.MirrorMakerTest > testDefaultMirrorMakerMessageHandlerWithHeaders 
PASSED

kafka.tools.ConsumerPerformanceTest > testDetailedHeaderMatchBody STARTED

kafka.tools.ConsumerPerformanceTest > testDetailedHeaderMatchBody PASSED

kafka.tools.ConsumerPerformanceTest > testNonDetailedHeaderMatchBody STARTED

kafka.tools.ConsumerPerformanceTest > testNonDetailedHeaderMatchBody PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidOldConsumerConfigWithAutoOffsetResetSmallest STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidOldConsumerConfigWithAutoOffsetResetSmallest PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithNoOffsetReset STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithNoOffsetReset PASSED

kafka.tools.ConsoleConsumerTest > shouldLimitReadsToMaxMessageLimit STARTED

kafka.tools.ConsoleConsumerTest > shouldLimitReadsToMaxMessageLimit PASSED

kafka.tools.ConsoleConsumerTest > shouldParseValidNewConsumerValidConfig STARTED

kafka.tools.ConsoleConsumerTest > shouldParseValidNewConsumerValidConfig PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetAndMatchingFromBeginning 
STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetAndMatchingFromBeginning 
PASSED

kafka.tools.ConsoleConsumerTest > shouldStopWhenOutputCheckErrorFails STARTED

kafka.tools.ConsoleConsumerTest > shouldStopWhenOutputCheckErrorFails PASSED

kafka.tools.ConsoleConsumerTest > 
shouldExitOnInvalidConfigWithAutoOffsetResetAndConflictingFromBeginningNewConsumer
 STARTED

kafka.tools.ConsoleConsumerTest > 
shouldExitOnInvalidConfigWithAutoOffsetResetAndConflictingFromBeginningNewConsumer
 PASSED

kafka.tools.ConsoleConsumerTest > 
shouldSetAutoResetToSmallestWhenFromBeginningConfigured STARTED

kafka.tools.ConsoleConsumerTest > 
shouldSetAutoResetToSmallestWhenFromBeginningConfigured PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetEarliest STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetEarliest PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewSimpleConsumerValidConfigWithStringOffset STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewSimpleConsumerValidConfigWithStringOffset PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidOldConsumerConfigWithAutoOffsetResetLargest STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidOldConsumerConfigWithAutoOffsetResetLargest PASSED

kafka.tools.ConsoleConsumerTest > shouldParseConfigsFromFile STARTED

kafka.tools.ConsoleConsumerTest > shouldParseConfigsFromFile PASSED

kafka.tools.ConsoleConsumerTest > groupIdsProvidedInDifferentPlacesMustMatch 
STARTED

kafka.tools.ConsoleConsumerTest > groupIdsProvidedInDifferentPlacesMustMatch 
PASSED

kafka.tools.ConsoleConsumerTest > 
shouldExitOnInvalidConfigWithAutoOffsetResetAndConflictingFromBeginningOldConsumer
 STARTED

kafka.tools.ConsoleConsumerTest > 
shouldExitOnInvalidConfigWithAutoOffsetResetAndConflictingFromBeginningOldConsumer
 PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewSimpleConsumerValidConfigWithNumericOffset STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewSimpleConsumerValidConfigWithNumericOffset PASSED

kafka.tools.ConsoleConsumerTest > testDefaultConsumer STARTED

kafka.tools.ConsoleConsumerTest > testDefaultConsumer PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetLatest STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetLatest PASSED

kafka.tools.ConsoleConsumerTest > shouldParseValidOldConsumerValidConfig STARTED

kafka.tools.Conso

Re: [VOTE] KIP-174 Deprecate and remove internal converter configs in WorkerConfig

2018-01-08 Thread Ted Yu
+1

On Mon, Jan 8, 2018 at 4:27 AM, UMESH CHAUDHARY  wrote:

> Hello All,
> Since there are no outstanding comments on this, so I'd like to start a
> vote.
>
> Please find the KIP here
>  174+-+Deprecate+and+remove+internal+converter+configs+in+WorkerConfig>
> and
> the related JIRA here .
>
> The KIP suggests to deprecate and remove the configs:
> internal.key.converter, internal.value.converter
>
> Appreciate your comments.
>
> Regards,
> Umesh
>


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

2018-01-08 Thread Apache Jenkins Server
See 


Changes:

[romain_hardouin] Catch JsonMappingException subclass

[damian.guy] MINOR: Add documentation for KAFKA-6086 
(ProductionExceptionHandler)

--
[...truncated 267.71 KB...]
kafka.server.ClientQuotaManagerTest > testQuotaViolation STARTED

kafka.server.ClientQuotaManagerTest > testQuotaViolation PASSED

kafka.server.ClientQuotaManagerTest > testRequestPercentageQuotaViolation 
STARTED

kafka.server.ClientQuotaManagerTest > testRequestPercentageQuotaViolation PASSED

kafka.server.ClientQuotaManagerTest > testQuotaConfigPrecedence STARTED

kafka.server.ClientQuotaManagerTest > testQuotaConfigPrecedence PASSED

kafka.server.ClientQuotaManagerTest > testExpireQuotaSensors STARTED

kafka.server.ClientQuotaManagerTest > testExpireQuotaSensors PASSED

kafka.server.ClientQuotaManagerTest > testClientIdNotSanitized STARTED

kafka.server.ClientQuotaManagerTest > testClientIdNotSanitized PASSED

kafka.server.ClientQuotaManagerTest > testExpireThrottleTimeSensor STARTED

kafka.server.ClientQuotaManagerTest > testExpireThrottleTimeSensor PASSED

kafka.server.ClientQuotaManagerTest > testUserClientIdQuotaParsing STARTED

kafka.server.ClientQuotaManagerTest > testUserClientIdQuotaParsing PASSED

kafka.server.ClientQuotaManagerTest > 
testUserClientQuotaParsingIdWithDefaultClientIdQuota STARTED

kafka.server.ClientQuotaManagerTest > 
testUserClientQuotaParsingIdWithDefaultClientIdQuota PASSED

kafka.server.MetadataCacheTest > 
getTopicMetadataWithNonSupportedSecurityProtocol STARTED

kafka.server.MetadataCacheTest > 
getTopicMetadataWithNonSupportedSecurityProtocol PASSED

kafka.server.MetadataCacheTest > getTopicMetadataIsrNotAvailable STARTED

kafka.server.MetadataCacheTest > getTopicMetadataIsrNotAvailable PASSED

kafka.server.MetadataCacheTest > getTopicMetadata STARTED

kafka.server.MetadataCacheTest > getTopicMetadata PASSED

kafka.server.MetadataCacheTest > getTopicMetadataReplicaNotAvailable STARTED

kafka.server.MetadataCacheTest > getTopicMetadataReplicaNotAvailable PASSED

kafka.server.MetadataCacheTest > getTopicMetadataPartitionLeaderNotAvailable 
STARTED

kafka.server.MetadataCacheTest > getTopicMetadataPartitionLeaderNotAvailable 
PASSED

kafka.server.MetadataCacheTest > getAliveBrokersShouldNotBeMutatedByUpdateCache 
STARTED

kafka.server.MetadataCacheTest > getAliveBrokersShouldNotBeMutatedByUpdateCache 
PASSED

kafka.server.MetadataCacheTest > getTopicMetadataNonExistingTopics STARTED

kafka.server.MetadataCacheTest > getTopicMetadataNonExistingTopics PASSED

kafka.server.ReplicaFetcherThreadTest > shouldFetchLeaderEpochOnFirstFetchOnly 
STARTED

kafka.server.ReplicaFetcherThreadTest > shouldFetchLeaderEpochOnFirstFetchOnly 
PASSED

kafka.server.ReplicaFetcherThreadTest > 
shouldTruncateToInitialFetchOffsetIfLeaderReturnsUndefinedOffset STARTED

kafka.server.ReplicaFetcherThreadTest > 
shouldTruncateToInitialFetchOffsetIfLeaderReturnsUndefinedOffset PASSED

kafka.server.ReplicaFetcherThreadTest > 
shouldPollIndefinitelyIfLeaderReturnsAnyException STARTED

kafka.server.ReplicaFetcherThreadTest > 
shouldPollIndefinitelyIfLeaderReturnsAnyException PASSED

kafka.server.ReplicaFetcherThreadTest > 
shouldTruncateToOffsetSpecifiedInEpochOffsetResponse STARTED

kafka.server.ReplicaFetcherThreadTest > 
shouldTruncateToOffsetSpecifiedInEpochOffsetResponse PASSED

kafka.server.ReplicaFetcherThreadTest > shouldHandleExceptionFromBlockingSend 
STARTED

kafka.server.ReplicaFetcherThreadTest > shouldHandleExceptionFromBlockingSend 
PASSED

kafka.server.ReplicaFetcherThreadTest > 
shouldNotIssueLeaderEpochRequestIfInterbrokerVersionBelow11 STARTED

kafka.server.ReplicaFetcherThreadTest > 
shouldNotIssueLeaderEpochRequestIfInterbrokerVersionBelow11 PASSED

kafka.server.ReplicaFetcherThreadTest > 
shouldMovePartitionsOutOfTruncatingLogState STARTED

kafka.server.ReplicaFetcherThreadTest > 
shouldMovePartitionsOutOfTruncatingLogState PASSED

kafka.server.ReplicaFetcherThreadTest > 
shouldFilterPartitionsMadeLeaderDuringLeaderEpochRequest STARTED

kafka.server.ReplicaFetcherThreadTest > 
shouldFilterPartitionsMadeLeaderDuringLeaderEpochRequest PASSED

kafka.server.CreateTopicsRequestTest > testValidCreateTopicsRequests STARTED

kafka.server.CreateTopicsRequestTest > testValidCreateTopicsRequests PASSED

kafka.server.CreateTopicsRequestTest > testErrorCreateTopicsRequests STARTED

kafka.server.CreateTopicsRequestTest > testErrorCreateTopicsRequests FAILED
java.lang.AssertionError: Partition [error-timeout,0] metadata not 
propagated after 15000 ms
at kafka.utils.TestUtils$.fail(TestUtils.scala:350)
at kafka.utils.TestUtils$.waitUntilTrue(TestUtils.scala:860)
at 
kafka.utils.TestUtils$.waitUntilMetadataIsPropagated(TestUtils.scala:928)
at 
kafka.server.CreateTopicsRequestTest.testErrorCreateTopicsRequests(CreateTopicsRequestTest.scala:106)

kafka.server.Cre

Re: [VOTE] KIP-226 - Dynamic Broker Configuration

2018-01-08 Thread Jun Rao
Hi, Rajini,

Could password.encoder.secret be updated dynamically? If so, each broker
will still have access to the old secret when password.encoder.secret is
updated. Perhaps that's a simpler way to handle changing secret than
introducing an extra config.

Thanks,

Jun

On Fri, Jan 5, 2018 at 3:09 AM, Rajini Sivaram 
wrote:

> Hi Jun,
>
> We are using 2-way encryption. The password configs encoded are
> keystore/truststore passwords and JAAS configuration. We need to be able to
> extract the actual values for these, so we cannot use 1-way hash. So if we
> have the old secret, we can decrypt and get the original values.
>
> Thank you,
>
> Rajini
>
> On Fri, Jan 5, 2018 at 12:11 AM, Jun Rao  wrote:
>
> > Hi, Rajin,
> >
> > Does providing the old-secret help? My understanding is that the encoded
> > passwd is the result of a 1-way hash with the secret. So, one can't
> decode
> > the passwd with old-secret. If that's the case, one still needs to
> provide
> > the unencrypted paaswd to re-encode with the new secret?
> >
> > Thanks,
> >
> > Jun
> >
> > On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram 
> > wrote:
> >
> > > Hi Jun/Jason,
> > >
> > > I was wondering whether it is worth adding a new property (static
> config
> > in
> > > server.properties) to pass in the previous encoder password as well
> when
> > > changing encoder password. So you would set:
> > >
> > >- password.encoder.secret=new-password
> > >- password.encoder.old.secret=old-password
> > >
> > > When the broker starts up and loads passwords from ZK, it would check
> if
> > > old-password is being used. If so, it would re-encode all passwords in
> ZK
> > > using new-password and store them back in ZK. If the new-password is
> > > already in use in ZK, the old one will be ignored. This needs an extra
> > > property, but makes it simpler for the user since all other passwords
> can
> > > be used from ZK.
> > >
> > > What do you think?
> > >
> > >
> > >
> > > On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <
> rajinisiva...@gmail.com>
> > > wrote:
> > >
> > > > Hi Jason,
> > > >
> > > > Thank you for reviewing and voting.
> > > >
> > > > Thanks, I had missed the rename. Have updated the KIP.
> > > >
> > > > The configs can be defined in the static server.properties or in
> > > > ZooKeeper. If a ZK config cannot be decoded (or is not valid), we log
> > an
> > > > error and revert to the static config or default. When updating the
> > > secret
> > > > used by the encode, we expect all password values to be specified in
> > > > server.properties. And the decoding or sanity check of the password
> in
> > ZK
> > > > would fail with the new secret, so we would use the password values
> > from
> > > > server.properties. Once the broker starts up, the values can be reset
> > in
> > > ZK
> > > > using AdminClient and they will be encoded using the new secret.
> > > >
> > > >
> > > > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson 
> > > > wrote:
> > > >
> > > >> +1 Thanks for the KIP. One minor nit: I think we changed
> > > >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in
> the
> > > PR.
> > > >>
> > > >> As far as updating secrets, I wasn't sure I understand how that will
> > > work.
> > > >> Do the password configs accept multiple values?
> > > >>
> > > >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <
> > rajinisiva...@gmail.com
> > > >
> > > >> wrote:
> > > >>
> > > >> > Hi Jun,
> > > >> >
> > > >> > Thank you for reviewing and voting.
> > > >> >
> > > >> > 50. I have updated the KIP to describe how the secret may be
> > changed.
> > > >> All
> > > >> > dynamically configurable passwords and per-broker configs. So the
> > > secret
> > > >> > can be different across brokers and updated using rolling restart.
> > In
> > > >> order
> > > >> > to update the secret, each broker needs to be restarted with an
> > > updated
> > > >> > server.properties which contains the new secret as well as the
> > current
> > > >> > values of all the password configs. Admin client can then be used
> to
> > > >> update
> > > >> > the passwords in ZooKeeper that are encrypted using the new
> secret.
> > > >> >
> > > >> > 51. leader.replication.throttled.replicas and
> > > >> > follower.replication.throttled.replicas
> > > >> > are dynamically configurable at the topic level. But there are no
> > > >> defaults
> > > >> > for these at the broker level since they refer to partitions of
> the
> > > >> topic.
> > > >> > The rates used for throttling were already configurable at the
> > broker
> > > >> > level.
> > > >> >
> > > >> > I made a couple of other changes to the KIP:
> > > >> >
> > > >> > 1. The config names used for encoding passwords are now prefixed
> > with
> > > >> > password.encoder.
> > > >> > Also added key length as a config since this is constrained by the
> > > >> > algorithm which is also configurable.
> > > >> > 2. I moved the update of inter-broker security protocol and
> > > >> > inter-broker sasl mechanism to the follow-on KIP 

Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig constructors public

2018-01-08 Thread Ewen Cheslack-Postava
+1 (binding), though I still think the Map should be  instead of
.

I also think its better to just expose the defaults as constants on the
class. Apparently there was discussion of this before and the concern is
that people write code that rely on the default configs and we might break
their code if we change it. I don't really buy this as using the constant
allows you to to symbolically reference the value rather than making your
own copy of it. Usually if we change a default like that there is an
important reason why and having the old copied value might be worse than
having the value change out from under you. Having the defaults explicitly
exposed can also be helpful when writing tests sometimes.

-Ewen

On Wed, Jan 3, 2018 at 9:30 AM, Colin McCabe  wrote:

> On Thu, Dec 21, 2017, at 10:28, Jason Gustafson wrote:
> > Hey Matthias,
> >
> > Let me suggest an alternative. As you have mentioned, these config
> classes
> > do not give users much benefit currently. Maybe we change that? I think
> > many users would appreciate having a builder for configuration since it
> > provides type safety and is generally a much friendlier pattern to work
> > with programmatically. Users could then do something like this:
> >
> > ConsumerConfig config = ConsumerConfig.newBuilder()
> > .setBootstrapServers("localhost:9092")
> > .setGroupId("group")
> > .setRequestTimeout(15, TimeUnit.SECONDS)
> > .build();
> >
> > Consumer consumer = new KafkaConsumer(config);
> >
> > An additional benefit of this is that it gives us a better way to expose
> > config deprecations. In any case, it would make it less odd to expose the
> > public constructor without giving users anything useful to do with the
> > class.
>
> Yeah, that would be good.  The builder idea would definitely make it a lot
> easier to configure clients programmatically.
>
> I do wonder if there are some cross-version compatibility issues here.  If
> there's any configuration that needs to be set by the client, but then
> propagated to the broker to be applied, the validation of that
> configuration really needs to be done by the broker itself.  The client
> code doesn't know the broker version, so it can't validate these configs.
> One example is topic configurations (although those are not set by
> ProducerConfig).  I'm not sure how big of an issue this is with our current
> configurations.
>
> Another problem here is that all these builder functions become API, and
> cannot easily be changed.  So if we want to change a configuration key that
> formerly accepted an int to accept a long, it will be difficult to do
> that.  We would have to add a new function with a separate name.
>
> best,
> Colin
>
>
> >
> > What do you think?
> >
> > -Jason
> >
> > On Wed, Dec 20, 2017 at 5:59 PM, Matthias J. Sax 
> > wrote:
> >
> > > It's tailored for internal usage. I think client constructors don't
> > > benefit from accepting those config objects. We just want to be able to
> > > access the default values for certain parameters.
> > >
> > > From a user point of view, it's actually boiler plate code if you pass
> > > in a config object instead of a plain Properties object because the
> > > config object itself is immutable.
> > >
> > > I actually create a JIRA to remove the constructors from KafkaStreams
> > > that do accept StreamsConfig for exact this reason:
> > > https://issues.apache.org/jira/browse/KAFKA-6386
> > >
> > >
> > > -Matthias
> > >
> > >
> > > On 12/20/17 3:33 PM, Jason Gustafson wrote:
> > > > Hi Matthias,
> > > >
> > > > Isn't it a little weird to make these constructors public but not
> also
> > > > expose the corresponding client constructors that use them?
> > > >
> > > > -Jason
> > > >
> > > > On Tue, Dec 19, 2017 at 9:30 AM, Bill Bejeck 
> wrote:
> > > >
> > > >> +1
> > > >>
> > > >> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang  >
> > > >> wrote:
> > > >>
> > > >>> +1
> > > >>>
> > > >>> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <
> t.j.bent...@gmail.com>
> > > >>> wrote:
> > > >>>
> > >  +1
> > > 
> > >  On 18 December 2017 at 23:28, Vahid S Hashemian <
> > > >>> vahidhashem...@us.ibm.com
> > > >
> > >  wrote:
> > > 
> > > > +1
> > > >
> > > > Thanks for the KIP.
> > > >
> > > > --Vahid
> > > >
> > > >
> > > >
> > > > From:   Ted Yu 
> > > > To: dev@kafka.apache.org
> > > > Date:   12/18/2017 02:45 PM
> > > > Subject:Re: [VOTE] KIP-243: Make ProducerConfig and
> > >  ConsumerConfig
> > > > constructors public
> > > >
> > > >
> > > >
> > > > +1
> > > >
> > > > nit: via "copy and past" an 'e' is missing at the end.
> > > >
> > > > On Mon, Dec 18, 2017 at 2:38 PM, Matthias J. Sax <
> > > >>> matth...@confluent.io>
> > > > wrote:
> > > >
> > > >> Hi,
> > > >>
> > > >> I want to propose the following KIP:
> > > >>
> > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.
> > > > apache.or

[jira] [Resolved] (KAFKA-6387) Worker's producer and consumer configs should inherit from worker configs

2018-01-08 Thread Randall Hauch (JIRA)

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

Randall Hauch resolved KAFKA-6387.
--
Resolution: Won't Fix

Because of the inability to maintain backward compatibility of the behavior, 
I'm withdrawing this request and marking this issue as WONTFIX.

> Worker's producer and consumer configs should inherit from worker configs
> -
>
> Key: KAFKA-6387
> URL: https://issues.apache.org/jira/browse/KAFKA-6387
> Project: Kafka
>  Issue Type: Improvement
>  Components: KafkaConnect
>Affects Versions: 1.0.0
>Reporter: Randall Hauch
>  Labels: needs-kip
>
> Currently, the worker configuration file defines the connection properties 
> for the three separate types of connections being made to the Kafka cluster:
> # the worker group membership,
> # producers for source connectors,
> # the consumers for sink connectors. 
> The configs are namespaced because to properly support things like 
> interceptors where the configs for 2 and 3 would conflict (same config name, 
> different value).
> However, it would be beneficial when such control is not required for the 
> producers and consumers to inherit the top-level configurations yet be able 
> to override them with the {{producer.}} and {{consumer.}} namespaced 
> configurations. This way the producer- and consumer-specific configurations 
> need only be specified if/when they need to override the top-level 
> configurations. This may be necessary, for example, to have different ACLs 
> than the connector tasks compared to the producers and consumers.
> This will require a minimal KIP to explain the new behavior. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


Re: [DISCUSS] KIP-208: Add SSL support to Kafka Connect REST interface

2018-01-08 Thread Randall Hauch
Nice feedback, Ewen. Thanks!

On Thu, Jan 4, 2018 at 5:11 PM, Ewen Cheslack-Postava 
wrote:

> Hey Jakub,
>
> Sorry for not getting to this sooner. Overall the proposal looks good to
> me, I just had a couple of questions.
>
> 1. For the configs/overrides, does this happen on a per-setting basis or if
> one override is included do we not use any of the original settings? I
> suspect that if you need to override one setting, it probably means you're
> using an entirely different config and so the latter behavior seems better
> to me. We've talked a bit about doing something similar for the
> producer/consumer security settings as well so you don't have to specify
> security configs in 3 places in your worker config.
>

Not sure if you were referring to
https://issues.apache.org/jira/browse/KAFKA-6387, but I just withdrew that
proposal (and the corresponding KIP-246) because behavior with existing
configurations was not backward compatible, so existing configs might have
very different behavior after the "inheritance" was implemented.

But regardless, I do think that in this case if you have to override one of
the settings you probably need to override multiple. So I'd be in favor of
requiring all configs to be specified in the overridden `listeners.*`
properties.


>
> 2. For using default values from the worker config, I am wondering how
> convinced we are that it will be common for them to be the same? I really
> don't have enough experience w/ these setups to know, so just a question
> here. I think the other thing to take into account here is that even though
> we're not dealing with authorization in this KIP, we will eventually want
> it for these APIs. Would we expect to be using the same principal for Kafka
> and the Connect REST API? In a case where a company has a Connect cluster
> that, e.g., an ops team manages and they are the only ones that are
> supposed to make changes, that would make sense to me. But for a setup
> where some dev team is allowed to use the REST API to create new connectors
> but the cluster is managed by an ops team, I would think the Kafka
> credentials would be different. I'm not sure how frequent each case would
> be, so I'm a bit unsure about the default of using the worker security
> configs by default. Thoughts?
>
> 3. We should probably specify the default in the table for
> rest.advertised.security.protocol because in ConfigDef if you don't
> specify
> a default value it becomes a required config. The HTTP default will
> probably need to be in there anyway.
>
> 4. Do we want to list the existing settings as deprecated and just move to
> using listeners for consistency? We don't need to remove them anytime soon,
> but given that the broker is doing the same, maybe we should just do that
> in this KIP?
>

Marking them as deprecated in this KIP sounds good to me.

>
> I think these are mostly small details, overall it looks like a good plan!
>

+1

Randall


>
> Thanks,
> Ewen
>
> On Tue, Oct 24, 2017 at 5:19 AM, Jakub Scholz  wrote:
>
> > There has been no discussion since my last update week ago. Unless
> someone
> > has some further comments in the next 48 hours, I will start the voting
> for
> > this KIP.
> >
> > Thanks & Regards
> > Jakub
> >
> > On Tue, Oct 17, 2017 at 5:54 PM, Jakub Scholz  wrote:
> >
> > > Ok, so I updated the KIP according to what we discussed. Please have a
> > > look at the updates. Two points I'm not 100% sure about:
> > >
> > > 1) Should we mark the rest.host.name and rest.port options as
> > deprecated?
> > >
> > > 2) I needed to also address the advertised hostname / port. With
> multiple
> > > listeners it is not clear anymore which one should be used. I saw as
> one
> > > option to add advertised.listeners option and some modified version of
> > > inter.broker.listener.name option to follow what is done in Kafka
> > > brokers. But for the Connect REST interface, we do not advertise the
> > > address to the clients like in Kafka broker. So we only need to tell
> > other
> > > workers how to connect - and for that we need only one advertised
> > address.
> > > So I decided to reuse the existing rest.advertised.host.name and
> > > rest.advertised.port options and add additional option
> > > rest.advertised.security.protocol to specify whether HTTP or HTTPS
> > should
> > > be used. Does this make sense to you? DO you think this is the right
> > > approach?
> > >
> > > Thanks & Regards
> > > Jakub
> > >
> > > On Mon, Oct 16, 2017 at 6:34 PM, Randall Hauch 
> wrote:
> > >
> > >> The broker's configuration options are "listeners" (plural) and
> > >> "listeners.security.protocol.map". I agree that following the pattern
> > set
> > >> by the broker is better, so these are really good ideas. However, at
> > this
> > >> point I don't see a need for the "listeners.security.procotol.map",
> > which
> > >> for the broker must be set if the listener name is not a security
> > >> protocol.
> > >> Can we not simply just allow "HTTP" and "HTTPS" as the 

Re: [DISCUSS] KIP-212: Enforce set of legal characters for connector names

2018-01-08 Thread Randall Hauch
So are we ready to start a vote on this KIP?

On Sat, Jan 6, 2018 at 6:00 PM, Ewen Cheslack-Postava 
wrote:

> re: whitespace characters, I'm fine with the restriction since I don't see
> it becoming an issue in practice. I just don't see any reason to restrict
> it so it seems like we're going out of our way and doing extra work to be
> restrictive, but without clear motivation.
>
> In general my default approach (without context of a specific system) would
> be to accept anything that we can encode in UTF-8 and only apply
> restrictions where it becomes necessary (e.g. we need to define a delimiter
> for some reason). The constraints of URLs introduce some complexity (you
> need escaping), but probably generally still allow this. If I can use an
> emoji when naming things, then I'm probably happy :) Whitespace characters
> definitely have some other issues (e.g. you can have non-visible whitespace
> which obscures which connector you're actually working with), but despite
> the JIRA linked, I wasn't really convinced they need special handling. It
> seems like a really weird issue to encounter in the first place.
>
> -Ewen
>
> On Fri, Jan 5, 2018 at 8:10 AM, Randall Hauch  wrote:
>
> > Sönke, I'm happy with the current proposal.
> >
> > Ewen, the proposal allows any characters in the name as long as they are
> > properly escaped/encoded. That seems to adhere to the robustness
> principle.
> > The only exception is that the proposal trims leading and trailing
> > whitespace characters in an attempt to reduce user errors. Can you please
> > clarify that you're okay with this behavior? I agree that technically we
> > can (and currently do) support whitespace-only names, but users have
> > reported this as problematic, and it also would be confusing for most
> user
> > interfaces.
> >
> > Best regards,
> >
> > Randall
> >
> > On Thu, Jan 4, 2018 at 10:31 PM, Ewen Cheslack-Postava <
> e...@confluent.io>
> > wrote:
> >
> > > Very late to the game here, but a few thoughts:
> > >
> > > 1. Regarding whether KIP is necessary, I don't mind doing it for
> > > documentation sake, but I would classify any mishandling of connector
> > names
> > > here as a bug. Which doesn't require a KIP to fix.
> > >
> > > 2. For support of characters, Kafka has some history of just being
> > > restrictive (e.g., see topic name restrictions), but I personally
> > disagree
> > > with this approach. I think it is better to be liberal in what we
> accept
> > > and just document limitations. I think our default should be to accept
> > any
> > > user input and document why we can't handle certain inputs and how the
> > user
> > > should adapt if we can't. In general I try to work under the robustness
> > > principle: *Be conservative in what you do, be liberal in what you
> accept
> > > from others*
> > >
> > > 3. Related to 2, there were some cases like whitespace-only connector
> > > names. This seems extremely weird and not critical, so I'm fine not
> > > supporting it officially, but technically I don't see any reason it
> > > shouldn't be supported with any appropriate escaping (i.e. what would
> it
> > > break for us?).
> > >
> > > But in general, I think just being more explicit about expectations is
> > > great and it'd be great to set baseline expectations.
> > >
> > > -Ewen
> > >
> > >
> > >
> > > On Mon, Nov 20, 2017 at 12:33 AM, Sönke Liebau <
> > > soenke.lie...@opencore.com.invalid> wrote:
> > >
> > > > @Randall: are you happy with the KIP as it stands so I can call for a
> > > vote,
> > > > or are there any outstanding items still to discuss?
> > > >
> > > > Same question to anybody else who'd like to participate of course :)
> > > >
> > > > On Thu, Nov 16, 2017 at 5:35 PM, Sönke Liebau <
> > > soenke.lie...@opencore.com>
> > > > wrote:
> > > >
> > > > > Sounds good. I've added a few sentences to this effect to the KIP.
> > > > >
> > > > > On Thu, Nov 16, 2017 at 5:02 PM, Randall Hauch 
> > > wrote:
> > > > >
> > > > >> Nice job updating the KIP. The PR (
> > > > >> https://github.com/apache/kafka/pull/2755/files) for the proposed
> > > > >> implementation does prevent names from being empty, and it trims
> > > > >> whitespace
> > > > >> from the name only when creating a new connector. However, the
> KIP's
> > > > >> "Proposed Change" section should probably be very clear about
> this,
> > > and
> > > > >> the
> > > > >> migration section should address how a connector that was created
> > with
> > > > >> leading and/or trailing whitespace characters will still be able
> to
> > be
> > > > >> updated and deleted. I think that decreases the likelihood of this
> > > > change
> > > > >> negatively impacting existing users. Basically, going forward, the
> > > names
> > > > >> of
> > > > >> new connectors will be trimmed.
> > > > >>
> > > > >> WDYT?
> > > > >>
> > > > >> On Thu, Nov 16, 2017 at 9:32 AM, Sönke Liebau <
> > > > >> soenke.lie...@opencore.com.invalid> wrote:
> > > > >>
> > > > >> > I've added some more detail to the 

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

2018-01-08 Thread Apache Jenkins Server
See 

--
[...truncated 3.87 MB...]
org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigRegexRouter PASSED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigSetSchemaMetadata STARTED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigSetSchemaMetadata PASSED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigTimestampConverter STARTED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigTimestampConverter PASSED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigHoistField STARTED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigHoistField PASSED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigMaskField STARTED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigMaskField PASSED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigInsertField STARTED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigInsertField PASSED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigFlatten STARTED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigFlatten PASSED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigReplaceField STARTED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigReplaceField PASSED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigTimestampRouter STARTED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigTimestampRouter PASSED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigValueToKey STARTED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigValueToKey PASSED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigExtractField STARTED

org.apache.kafka.connect.runtime.TransformationConfigTest > 
testEmbeddedConfigExtractField PASSED

org.apache.kafka.connect.runtime.ConnectorConfigTest > unconfiguredTransform 
STARTED

org.apache.kafka.connect.runtime.ConnectorConfigTest > unconfiguredTransform 
PASSED

org.apache.kafka.connect.runtime.ConnectorConfigTest > 
multipleTransformsOneDangling STARTED

org.apache.kafka.connect.runtime.ConnectorConfigTest > 
multipleTransformsOneDangling PASSED

org.apache.kafka.connect.runtime.ConnectorConfigTest > misconfiguredTransform 
STARTED

org.apache.kafka.connect.runtime.ConnectorConfigTest > misconfiguredTransform 
PASSED

org.apache.kafka.connect.runtime.ConnectorConfigTest > noTransforms STARTED

org.apache.kafka.connect.runtime.ConnectorConfigTest > noTransforms PASSED

org.apache.kafka.connect.runtime.ConnectorConfigTest > danglingTransformAlias 
STARTED

org.apache.kafka.connect.runtime.ConnectorConfigTest > danglingTransformAlias 
PASSED

org.apache.kafka.connect.runtime.ConnectorConfigTest > multipleTransforms 
STARTED

org.apache.kafka.connect.runtime.ConnectorConfigTest > multipleTransforms PASSED

org.apache.kafka.connect.runtime.ConnectorConfigTest > singleTransform STARTED

org.apache.kafka.connect.runtime.ConnectorConfigTest > singleTransform PASSED

org.apache.kafka.connect.runtime.ConnectorConfigTest > wrongTransformationType 
STARTED

org.apache.kafka.connect.runtime.ConnectorConfigTest > wrongTransformationType 
PASSED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testPutConnectorConfig STARTED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testPutConnectorConfig PASSED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testPutTaskConfigs STARTED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testPutTaskConfigs PASSED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testCorruptConfig STARTED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testCorruptConfig PASSED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testCreateConnectorFailedBasicValidation STARTED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testCreateConnectorFailedBasicValidation PASSED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testCreateConnectorFailedCustomValidation STARTED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testCreateConnectorFailedCustomValidation PASSED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testCreateConnectorAlreadyExists STARTED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testCreateConnectorAlreadyExists PASSED

org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest > 
testDestroyConnector STARTED

org.apac

Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig constructors public

2018-01-08 Thread Matthias J. Sax
We cannot simply change to  without breaking a lot of code and
making it a nightmare to fix it up... :(

I will leave the VOTE open for some more time if people still want to
comment. Otherwise, we would have 3 bindings vote now.


-Matthias

On 1/8/18 10:58 AM, Ewen Cheslack-Postava wrote:
> +1 (binding), though I still think the Map should be  instead of
> .
> 
> I also think its better to just expose the defaults as constants on the
> class. Apparently there was discussion of this before and the concern is
> that people write code that rely on the default configs and we might break
> their code if we change it. I don't really buy this as using the constant
> allows you to to symbolically reference the value rather than making your
> own copy of it. Usually if we change a default like that there is an
> important reason why and having the old copied value might be worse than
> having the value change out from under you. Having the defaults explicitly
> exposed can also be helpful when writing tests sometimes.
> 
> -Ewen
> 
> On Wed, Jan 3, 2018 at 9:30 AM, Colin McCabe  wrote:
> 
>> On Thu, Dec 21, 2017, at 10:28, Jason Gustafson wrote:
>>> Hey Matthias,
>>>
>>> Let me suggest an alternative. As you have mentioned, these config
>> classes
>>> do not give users much benefit currently. Maybe we change that? I think
>>> many users would appreciate having a builder for configuration since it
>>> provides type safety and is generally a much friendlier pattern to work
>>> with programmatically. Users could then do something like this:
>>>
>>> ConsumerConfig config = ConsumerConfig.newBuilder()
>>> .setBootstrapServers("localhost:9092")
>>> .setGroupId("group")
>>> .setRequestTimeout(15, TimeUnit.SECONDS)
>>> .build();
>>>
>>> Consumer consumer = new KafkaConsumer(config);
>>>
>>> An additional benefit of this is that it gives us a better way to expose
>>> config deprecations. In any case, it would make it less odd to expose the
>>> public constructor without giving users anything useful to do with the
>>> class.
>>
>> Yeah, that would be good.  The builder idea would definitely make it a lot
>> easier to configure clients programmatically.
>>
>> I do wonder if there are some cross-version compatibility issues here.  If
>> there's any configuration that needs to be set by the client, but then
>> propagated to the broker to be applied, the validation of that
>> configuration really needs to be done by the broker itself.  The client
>> code doesn't know the broker version, so it can't validate these configs.
>> One example is topic configurations (although those are not set by
>> ProducerConfig).  I'm not sure how big of an issue this is with our current
>> configurations.
>>
>> Another problem here is that all these builder functions become API, and
>> cannot easily be changed.  So if we want to change a configuration key that
>> formerly accepted an int to accept a long, it will be difficult to do
>> that.  We would have to add a new function with a separate name.
>>
>> best,
>> Colin
>>
>>
>>>
>>> What do you think?
>>>
>>> -Jason
>>>
>>> On Wed, Dec 20, 2017 at 5:59 PM, Matthias J. Sax 
>>> wrote:
>>>
 It's tailored for internal usage. I think client constructors don't
 benefit from accepting those config objects. We just want to be able to
 access the default values for certain parameters.

 From a user point of view, it's actually boiler plate code if you pass
 in a config object instead of a plain Properties object because the
 config object itself is immutable.

 I actually create a JIRA to remove the constructors from KafkaStreams
 that do accept StreamsConfig for exact this reason:
 https://issues.apache.org/jira/browse/KAFKA-6386


 -Matthias


 On 12/20/17 3:33 PM, Jason Gustafson wrote:
> Hi Matthias,
>
> Isn't it a little weird to make these constructors public but not
>> also
> expose the corresponding client constructors that use them?
>
> -Jason
>
> On Tue, Dec 19, 2017 at 9:30 AM, Bill Bejeck 
>> wrote:
>
>> +1
>>
>> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang >>
>> wrote:
>>
>>> +1
>>>
>>> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <
>> t.j.bent...@gmail.com>
>>> wrote:
>>>
 +1

 On 18 December 2017 at 23:28, Vahid S Hashemian <
>>> vahidhashem...@us.ibm.com
>
 wrote:

> +1
>
> Thanks for the KIP.
>
> --Vahid
>
>
>
> From:   Ted Yu 
> To: dev@kafka.apache.org
> Date:   12/18/2017 02:45 PM
> Subject:Re: [VOTE] KIP-243: Make ProducerConfig and
 ConsumerConfig
> constructors public
>
>
>
> +1
>
> nit: via "copy and past" an 'e' is missing at the end.
>
> On Mon, Dec 18, 2017 at 2:38 PM, Matthias J. Sax <
>>> ma

Re: [VOTE] KIP-226 - Dynamic Broker Configuration

2018-01-08 Thread Rajini Sivaram
Hi Jun,

No,  password.encoder.secret cannot be updated dynamically at the moment.
Dynamic configs are stored in ZooKeeper and since ZK is not secure, all
password configs in ZK are encrypted using password.encoder.secret. We
cannot make password.encoder.secret dynamic since it would need another
secret to encrypt it for storing in ZK and that secret would need to be
static and cannot be rotated.

On Mon, Jan 8, 2018 at 6:33 PM, Jun Rao  wrote:

> Hi, Rajini,
>
> Could password.encoder.secret be updated dynamically? If so, each broker
> will still have access to the old secret when password.encoder.secret is
> updated. Perhaps that's a simpler way to handle changing secret than
> introducing an extra config.
>
> Thanks,
>
> Jun
>
> On Fri, Jan 5, 2018 at 3:09 AM, Rajini Sivaram 
> wrote:
>
> > Hi Jun,
> >
> > We are using 2-way encryption. The password configs encoded are
> > keystore/truststore passwords and JAAS configuration. We need to be able
> to
> > extract the actual values for these, so we cannot use 1-way hash. So if
> we
> > have the old secret, we can decrypt and get the original values.
> >
> > Thank you,
> >
> > Rajini
> >
> > On Fri, Jan 5, 2018 at 12:11 AM, Jun Rao  wrote:
> >
> > > Hi, Rajin,
> > >
> > > Does providing the old-secret help? My understanding is that the
> encoded
> > > passwd is the result of a 1-way hash with the secret. So, one can't
> > decode
> > > the passwd with old-secret. If that's the case, one still needs to
> > provide
> > > the unencrypted paaswd to re-encode with the new secret?
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > > On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram <
> rajinisiva...@gmail.com>
> > > wrote:
> > >
> > > > Hi Jun/Jason,
> > > >
> > > > I was wondering whether it is worth adding a new property (static
> > config
> > > in
> > > > server.properties) to pass in the previous encoder password as well
> > when
> > > > changing encoder password. So you would set:
> > > >
> > > >- password.encoder.secret=new-password
> > > >- password.encoder.old.secret=old-password
> > > >
> > > > When the broker starts up and loads passwords from ZK, it would check
> > if
> > > > old-password is being used. If so, it would re-encode all passwords
> in
> > ZK
> > > > using new-password and store them back in ZK. If the new-password is
> > > > already in use in ZK, the old one will be ignored. This needs an
> extra
> > > > property, but makes it simpler for the user since all other passwords
> > can
> > > > be used from ZK.
> > > >
> > > > What do you think?
> > > >
> > > >
> > > >
> > > > On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <
> > rajinisiva...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Jason,
> > > > >
> > > > > Thank you for reviewing and voting.
> > > > >
> > > > > Thanks, I had missed the rename. Have updated the KIP.
> > > > >
> > > > > The configs can be defined in the static server.properties or in
> > > > > ZooKeeper. If a ZK config cannot be decoded (or is not valid), we
> log
> > > an
> > > > > error and revert to the static config or default. When updating the
> > > > secret
> > > > > used by the encode, we expect all password values to be specified
> in
> > > > > server.properties. And the decoding or sanity check of the password
> > in
> > > ZK
> > > > > would fail with the new secret, so we would use the password values
> > > from
> > > > > server.properties. Once the broker starts up, the values can be
> reset
> > > in
> > > > ZK
> > > > > using AdminClient and they will be encoded using the new secret.
> > > > >
> > > > >
> > > > > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <
> ja...@confluent.io>
> > > > > wrote:
> > > > >
> > > > >> +1 Thanks for the KIP. One minor nit: I think we changed
> > > > >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in
> > the
> > > > PR.
> > > > >>
> > > > >> As far as updating secrets, I wasn't sure I understand how that
> will
> > > > work.
> > > > >> Do the password configs accept multiple values?
> > > > >>
> > > > >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <
> > > rajinisiva...@gmail.com
> > > > >
> > > > >> wrote:
> > > > >>
> > > > >> > Hi Jun,
> > > > >> >
> > > > >> > Thank you for reviewing and voting.
> > > > >> >
> > > > >> > 50. I have updated the KIP to describe how the secret may be
> > > changed.
> > > > >> All
> > > > >> > dynamically configurable passwords and per-broker configs. So
> the
> > > > secret
> > > > >> > can be different across brokers and updated using rolling
> restart.
> > > In
> > > > >> order
> > > > >> > to update the secret, each broker needs to be restarted with an
> > > > updated
> > > > >> > server.properties which contains the new secret as well as the
> > > current
> > > > >> > values of all the password configs. Admin client can then be
> used
> > to
> > > > >> update
> > > > >> > the passwords in ZooKeeper that are encrypted using the new
> > secret.
> > > > >> >
> > > > >> > 51. leader.replication.throttled.replicas and
> > 

[jira] [Resolved] (KAFKA-6428) Fail builds on findbugs warnings

2018-01-08 Thread Ewen Cheslack-Postava (JIRA)

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

Ewen Cheslack-Postava resolved KAFKA-6428.
--
Resolution: Invalid

Seems it was already setup to report & fail, the reporting on Jenkins was just 
missing.

> Fail builds on findbugs warnings
> 
>
> Key: KAFKA-6428
> URL: https://issues.apache.org/jira/browse/KAFKA-6428
> Project: Kafka
>  Issue Type: Improvement
>  Components: build
>Reporter: Ewen Cheslack-Postava
>Assignee: Ewen Cheslack-Postava
>
> Findbugs spots likely bugs, and especially for warnings at the High level, it 
> actually has pretty good signal for real bugs (or just things that might be 
> risky). We should be failing builds, especially PRs, if any sufficiently high 
> warnings are listed. We should get this enabled for that level and then 
> decide if we want to adjust the level of warnings we want to address.
> This likely relates to KAFKA-5887 since findbugs may not be sufficiently 
> maintained for JDK9 support. In any case, the intent is to fail the build 
> based on whichever tool is used.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


Re: [DISCUSS] KIP-232: Detect outdated metadata by adding ControllerMetadataEpoch field

2018-01-08 Thread Jason Gustafson
>
> I am not sure I understand the benefit of incrementing this epoch after
> topic deletion. At a high level, client can not differentiate between topic
> deletion and topic creation when the global epoch is incremented. Can you
> provide more specific use-case?


Say you send two metadata requests to two separate brokers. In the
responses, one of them says a certain topic exists and one says it does
not. Who is right? My suggestion is to bump the topic epoch on deletion and
include it in the metadata response when returning
UNKNOWN_TOPIC_OR_PARTITION. Then the client always knows which metadata is
more current (if not necessarily up to date). Because of this ambiguity,
Kafka clients currently have no choice but retry on unknown topic errors.
Yes, you can say it is already handled, but this gives us some better
options in the future. In the consumer, users are often asking to be
notified when they attempt to fetch from unknown topics, for example,
because it could indicate a configuration problem. We have difficulty
supporting this at the moment.

Currently when broker returns UNKNOWN_TOPIC_OR_PARTITION, it means that the
> topic is not follower or leader of this partition. Note that
> UNKNOWN_TOPIC_OR_PARTITION does not necessarily tell client whether this
> partition exists on other broker or not. UNKNOWN_TOPIC_OR_PARTITION can be
> caused either when the broker has not yet processed the latest
> LeaderAndIsrRequest, or the client is using outdated metadata.


I don't think this is right. Metadata is propagated through the
UpdateMetadata request which the controller sends to all brokers. Brokers
will return UNKNOWN_TOPIC_OR_PARTITION in a metadata response if they don't
have metadata cached for the requested topic.

There is one problem though which I think might be what you're getting at.
After a topic is deleted, the controller will leave it out of future
UpdateMetadata requests, which means the deleted epoch would not be
propagated to all brokers and we'd be stuck in the current state. Suppose
instead that when a topic is deleted, we 1) bump the topic epoch, and 2)
set an expiration time (say several hours). When the expiration time is
reached, we delete the topic metadata in zookeeper; until then, the
controller continues to propagate it as usual with a flag indicating it no
longer exists. The point of the epoch is solving edge cases around topic
deletion and recreation, so the expiration timer gives clients a window to
observe the deletion before the metadata is removed. It also ensures that
topic metadata is eventually cleaned up following deletion.

What do you think?

In comparison to byte[], String has the benefit of being more readable and
> it is also the same type of the existing metadata field, which is used for
> a similar purpose by user. Do you think this is reasonable?


I don't have too strong of a feeling about it. I'm not sure how important
readability is since it's intended to be opaque to the user. To clarify a
little bit, I think we should continue to send the topic and leader epochs
in the protocol directly as separate fields. It's only when we surface it
through the consumer API that we add some obscurity since we don't want
users to depend on the fields directly and we don't want to make API
changes in the future if we need to add something else which is also
internal. In fact, rather than using byte[] or String directly, perhaps we
could just expose it as an object and give it a readable toString()?


Thanks,
Jason


On Fri, Jan 5, 2018 at 5:12 PM, Dong Lin  wrote:

> Hey Jason,
>
> Thanks a lot for the comments. I will comment inline. And I have updated
> the KIP accordingly. Could you take another look?
>
> On Fri, Jan 5, 2018 at 11:15 AM, Jason Gustafson 
> wrote:
>
> > Hi Dong,
> >
> > Sorry for the late reply. I think the latest revision is looking good. I
> > have a few minor suggestions:
> >
> > 1. The name "partition_epoch" makes me think it changes independently at
> > the partition level, but all partitions for a topic should have the same
> > epoch. Maybe "topic_epoch" is nearer the mark?
> >
>
> Actually, in the current proposal, partitions of the same topic will have
> different epoch. Every time a new partition is created, either due to topic
> creation or partition expansion, the global epoch is incremented by 1 and
> is assigned to that partition. This is probably why we currently call it
> partition_epoch.
>
> Thinking about your idea more, one alternative approach following your idea
> is to use a topic_epoch is that incremented by 1 whenever we create a
> topic. We should store a single topic_epoch in
> znode /brokers/topics/[topic] without storing the list of partition_epoch
> for all partitions. This same epoch will be used for the new partitions
> after partition expansion of the existing topic. This approach has more
> simpler znode format than the existing KIP and it still allows us to detect
> topic created after topic deletion. I think this is better.

Re: [DISCUSS] KIP-232: Detect outdated metadata by adding ControllerMetadataEpoch field

2018-01-08 Thread Ismael Juma
On Fri, Jan 5, 2018 at 7:15 PM, Jason Gustafson  wrote:
>
> class OffsetAndMetadata {
>   long offset;
>   byte[] offsetMetadata;
>   String metadata;
> }


> Admittedly, the naming is a bit annoying, but we can probably come up with
> something better. Internally the byte array would have a version. If in the
> future we have anything else we need to add, we can update the version and
> we wouldn't need any new APIs.
>

We can also add fields to a class in a compatible way. So, it seems to me
that the main advantage of the byte array is that it's opaque to the user.
Is that correct? If so, we could also add any opaque metadata in a subclass
so that users don't even see it (unless they cast it, but then they're on
their own).

Ismael

The corresponding seek() and position() APIs might look something like this:
>
> void seek(TopicPartition partition, long offset, byte[] offsetMetadata);
> byte[] positionMetadata(TopicPartition partition);
>
> What do you think?
>
> Thanks,
> Jason
>
> On Thu, Jan 4, 2018 at 7:04 PM, Dong Lin  wrote:
>
> > Hey Jun, Jason,
> >
> > Thanks much for all the feedback. I have updated the KIP based on the
> > latest discussion. Can you help check whether it looks good?
> >
> > Thanks,
> > Dong
> >
> > On Thu, Jan 4, 2018 at 5:36 PM, Dong Lin  wrote:
> >
> > > Hey Jun,
> > >
> > > Hmm... thinking about this more, I am not sure that the proposed API is
> > > sufficient. For users that store offset externally, we probably need
> > extra
> > > API to return the leader_epoch and partition_epoch for all partitions
> > that
> > > consumers are consuming. I suppose these users currently use position()
> > to
> > > get the offset. Thus we probably need a new method
> positionWithEpoch(..)
> > to
> > > return . Does this sound
> > reasonable?
> > >
> > > Thanks,
> > > Dong
> > >
> > >
> > > On Thu, Jan 4, 2018 at 5:26 PM, Jun Rao  wrote:
> > >
> > >> Hi, Dong,
> > >>
> > >> Yes, that's what I am thinking. OffsetEpoch will be composed of
> > >> (partition_epoch,
> > >> leader_epoch).
> > >>
> > >> Thanks,
> > >>
> > >> Jun
> > >>
> > >>
> > >> On Thu, Jan 4, 2018 at 4:22 PM, Dong Lin  wrote:
> > >>
> > >> > Hey Jun,
> > >> >
> > >> > Thanks much. I like the the new API that you proposed. I am not sure
> > >> what
> > >> > you exactly mean by offset_epoch. I suppose that we can use the pair
> > of
> > >> > (partition_epoch, leader_epoch) as the offset_epoch, right?
> > >> >
> > >> > Thanks,
> > >> > Dong
> > >> >
> > >> > On Thu, Jan 4, 2018 at 4:02 PM, Jun Rao  wrote:
> > >> >
> > >> > > Hi, Dong,
> > >> > >
> > >> > > Got it. The api that you proposed works. The question is whether
> > >> that's
> > >> > the
> > >> > > api that we want to have in the long term. My concern is that
> while
> > >> the
> > >> > api
> > >> > > change is simple, the new api seems harder to explain and use. For
> > >> > example,
> > >> > > a consumer storing offsets externally now needs to call
> > >> > > waitForMetadataUpdate() after calling seek().
> > >> > >
> > >> > > An alternative approach is to make the following compatible api
> > >> changes
> > >> > in
> > >> > > Consumer.
> > >> > > * Add an additional OffsetEpoch field in OffsetAndMetadata. (no
> need
> > >> to
> > >> > > change the CommitSync() api)
> > >> > > * Add a new api seek(TopicPartition partition, long offset,
> > >> OffsetEpoch
> > >> > > offsetEpoch). We can potentially deprecate the old api
> > >> > seek(TopicPartition
> > >> > > partition, long offset) in the future.
> > >> > >
> > >> > > The alternative approach has similar amount of api changes as
> yours
> > >> but
> > >> > has
> > >> > > the following benefits.
> > >> > > 1. The api works in a similar way as how offset management works
> now
> > >> and
> > >> > is
> > >> > > probably what we want in the long term.
> > >> > > 2. It can reset offsets better when there is data loss due to
> > unclean
> > >> > > leader election or correlated replica failure.
> > >> > > 3. It can reset offsets better when topic is recreated.
> > >> > >
> > >> > > Thanks,
> > >> > >
> > >> > > Jun
> > >> > >
> > >> > >
> > >> > > On Thu, Jan 4, 2018 at 2:05 PM, Dong Lin 
> > wrote:
> > >> > >
> > >> > > > Hey Jun,
> > >> > > >
> > >> > > > Yeah I agree that ideally we don't want an ever growing global
> > >> metadata
> > >> > > > version. I just think it may be more desirable to keep the
> > consumer
> > >> API
> > >> > > > simple.
> > >> > > >
> > >> > > > In my current proposal, metadata version returned in the fetch
> > >> response
> > >> > > > will be stored with the offset together. More specifically, the
> > >> > > > metadata_epoch in the new offset topic schema will be the
> largest
> > >> > > > metadata_epoch from all the MetadataResponse and FetchResponse
> > ever
> > >> > > > received by this consumer.
> > >> > > >
> > >> > > > We probably don't have to change the consumer API for
> > >> > > > commitSync(Map). If user
> calls
> > >> > > > commitSync(...) to commit offset 10 for a given partition, fo

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

2018-01-08 Thread Apache Jenkins Server
See 


Changes:

[wangguoz] MINOR: fix image 404s in streams doc

--
[...truncated 189.35 KB...]

kafka.admin.ReassignPartitionsCommandTest > 
shouldFindMovingReplicasMultipleTopics STARTED

kafka.admin.ReassignPartitionsCommandTest > 
shouldFindMovingReplicasMultipleTopics PASSED

kafka.admin.ReassignPartitionsCommandTest > 
shouldNotOverwriteExistingPropertiesWhenLimitIsAdded STARTED

kafka.admin.ReassignPartitionsCommandTest > 
shouldNotOverwriteExistingPropertiesWhenLimitIsAdded PASSED

kafka.admin.ReassignPartitionsCommandTest > 
shouldFindMovingReplicasMultipleTopicsAndPartitions STARTED

kafka.admin.ReassignPartitionsCommandTest > 
shouldFindMovingReplicasMultipleTopicsAndPartitions PASSED

kafka.admin.ReassignPartitionsCommandTest > 
shouldRemoveThrottleLimitFromAllBrokers STARTED

kafka.admin.ReassignPartitionsCommandTest > 
shouldRemoveThrottleLimitFromAllBrokers PASSED

kafka.admin.ReassignPartitionsCommandTest > shouldFindMovingReplicas STARTED

kafka.admin.ReassignPartitionsCommandTest > shouldFindMovingReplicas PASSED

kafka.admin.ReassignPartitionsCommandTest > 
shouldFindMovingReplicasMultiplePartitions STARTED

kafka.admin.ReassignPartitionsCommandTest > 
shouldFindMovingReplicasMultiplePartitions PASSED

kafka.admin.ReassignPartitionsCommandTest > shouldSetQuotaLimit STARTED

kafka.admin.ReassignPartitionsCommandTest > shouldSetQuotaLimit PASSED

kafka.admin.ReassignPartitionsCommandTest > 
shouldFindMovingReplicasWhenProposedIsSubsetOfExisting STARTED

kafka.admin.ReassignPartitionsCommandTest > 
shouldFindMovingReplicasWhenProposedIsSubsetOfExisting PASSED

kafka.admin.ReassignPartitionsCommandTest > shouldUpdateQuotaLimit STARTED

kafka.admin.ReassignPartitionsCommandTest > shouldUpdateQuotaLimit PASSED

kafka.admin.ReassignPartitionsCommandTest > 
shouldFindTwoMovingReplicasInSamePartition STARTED

kafka.admin.ReassignPartitionsCommandTest > 
shouldFindTwoMovingReplicasInSamePartition PASSED

kafka.admin.ReassignPartitionsCommandTest > 
shouldNotOverwriteEntityConfigsWhenUpdatingThrottledReplicas STARTED

kafka.admin.ReassignPartitionsCommandTest > 
shouldNotOverwriteEntityConfigsWhenUpdatingThrottledReplicas PASSED

kafka.admin.DeleteConsumerGroupTest > 
testGroupWideDeleteInZKDoesNothingForActiveConsumerGroup STARTED

kafka.admin.DeleteConsumerGroupTest > 
testGroupWideDeleteInZKDoesNothingForActiveConsumerGroup PASSED

kafka.admin.DeleteConsumerGroupTest > 
testGroupTopicWideDeleteInZKDoesNothingForActiveGroupConsumingMultipleTopics 
STARTED

kafka.admin.DeleteConsumerGroupTest > 
testGroupTopicWideDeleteInZKDoesNothingForActiveGroupConsumingMultipleTopics 
PASSED

kafka.admin.DeleteConsumerGroupTest > 
testConsumptionOnRecreatedTopicAfterTopicWideDeleteInZK STARTED

kafka.admin.DeleteConsumerGroupTest > 
testConsumptionOnRecreatedTopicAfterTopicWideDeleteInZK PASSED

kafka.admin.DeleteConsumerGroupTest > testTopicWideDeleteInZK STARTED

kafka.admin.DeleteConsumerGroupTest > testTopicWideDeleteInZK PASSED

kafka.admin.DeleteConsumerGroupTest > 
testGroupTopicWideDeleteInZKForGroupConsumingOneTopic STARTED

kafka.admin.DeleteConsumerGroupTest > 
testGroupTopicWideDeleteInZKForGroupConsumingOneTopic PASSED

kafka.admin.DeleteConsumerGroupTest > 
testGroupTopicWideDeleteInZKForGroupConsumingMultipleTopics STARTED

kafka.admin.DeleteConsumerGroupTest > 
testGroupTopicWideDeleteInZKForGroupConsumingMultipleTopics PASSED

kafka.admin.DeleteConsumerGroupTest > testGroupWideDeleteInZK STARTED

kafka.admin.DeleteConsumerGroupTest > testGroupWideDeleteInZK PASSED

kafka.admin.AdminZkClientTest > testGetBrokerMetadatas STARTED

kafka.admin.AdminZkClientTest > testGetBrokerMetadatas PASSED

kafka.admin.AdminZkClientTest > testBootstrapClientIdConfig STARTED

kafka.admin.AdminZkClientTest > testBootstrapClientIdConfig PASSED

kafka.admin.AdminZkClientTest > testTopicConfigChange STARTED

kafka.admin.AdminZkClientTest > testTopicConfigChange PASSED

kafka.admin.AdminZkClientTest > testManualReplicaAssignment STARTED

kafka.admin.AdminZkClientTest > testManualReplicaAssignment PASSED

kafka.admin.AdminZkClientTest > testConcurrentTopicCreation STARTED

kafka.admin.AdminZkClientTest > testConcurrentTopicCreation PASSED

kafka.admin.AdminZkClientTest > shouldPropagateDynamicBrokerConfigs STARTED

kafka.admin.AdminZkClientTest > shouldPropagateDynamicBrokerConfigs PASSED

kafka.admin.AdminZkClientTest > testTopicCreationWithCollision STARTED

kafka.admin.AdminZkClientTest > testTopicCreationWithCollision PASSED

kafka.admin.AdminZkClientTest > testTopicCreationInZK STARTED

kafka.admin.AdminZkClientTest > testTopicCreationInZK PASSED

kafka.admin.TopicCommandTest > testCreateIfNotExists STARTED

kafka.admin.TopicCommandTest > testCreateIfNotExists PASSED

kafka.admin.TopicCommandTest > testCreateAlterTopicWithRackAware STARTED

kafka.admin.TopicCommandTest > testCreateAlterTopic

Re: [VOTE] KIP-226 - Dynamic Broker Configuration

2018-01-08 Thread Jun Rao
Hi, Rajini,

Thanks for the explanation. Then your suggestion sounds good to me.

Jun

On Mon, Jan 8, 2018 at 1:32 PM, Rajini Sivaram 
wrote:

> Hi Jun,
>
> No,  password.encoder.secret cannot be updated dynamically at the moment.
> Dynamic configs are stored in ZooKeeper and since ZK is not secure, all
> password configs in ZK are encrypted using password.encoder.secret. We
> cannot make password.encoder.secret dynamic since it would need another
> secret to encrypt it for storing in ZK and that secret would need to be
> static and cannot be rotated.
>
> On Mon, Jan 8, 2018 at 6:33 PM, Jun Rao  wrote:
>
> > Hi, Rajini,
> >
> > Could password.encoder.secret be updated dynamically? If so, each broker
> > will still have access to the old secret when password.encoder.secret is
> > updated. Perhaps that's a simpler way to handle changing secret than
> > introducing an extra config.
> >
> > Thanks,
> >
> > Jun
> >
> > On Fri, Jan 5, 2018 at 3:09 AM, Rajini Sivaram 
> > wrote:
> >
> > > Hi Jun,
> > >
> > > We are using 2-way encryption. The password configs encoded are
> > > keystore/truststore passwords and JAAS configuration. We need to be
> able
> > to
> > > extract the actual values for these, so we cannot use 1-way hash. So if
> > we
> > > have the old secret, we can decrypt and get the original values.
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> > > On Fri, Jan 5, 2018 at 12:11 AM, Jun Rao  wrote:
> > >
> > > > Hi, Rajin,
> > > >
> > > > Does providing the old-secret help? My understanding is that the
> > encoded
> > > > passwd is the result of a 1-way hash with the secret. So, one can't
> > > decode
> > > > the passwd with old-secret. If that's the case, one still needs to
> > > provide
> > > > the unencrypted paaswd to re-encode with the new secret?
> > > >
> > > > Thanks,
> > > >
> > > > Jun
> > > >
> > > > On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram <
> > rajinisiva...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Jun/Jason,
> > > > >
> > > > > I was wondering whether it is worth adding a new property (static
> > > config
> > > > in
> > > > > server.properties) to pass in the previous encoder password as well
> > > when
> > > > > changing encoder password. So you would set:
> > > > >
> > > > >- password.encoder.secret=new-password
> > > > >- password.encoder.old.secret=old-password
> > > > >
> > > > > When the broker starts up and loads passwords from ZK, it would
> check
> > > if
> > > > > old-password is being used. If so, it would re-encode all passwords
> > in
> > > ZK
> > > > > using new-password and store them back in ZK. If the new-password
> is
> > > > > already in use in ZK, the old one will be ignored. This needs an
> > extra
> > > > > property, but makes it simpler for the user since all other
> passwords
> > > can
> > > > > be used from ZK.
> > > > >
> > > > > What do you think?
> > > > >
> > > > >
> > > > >
> > > > > On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <
> > > rajinisiva...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Jason,
> > > > > >
> > > > > > Thank you for reviewing and voting.
> > > > > >
> > > > > > Thanks, I had missed the rename. Have updated the KIP.
> > > > > >
> > > > > > The configs can be defined in the static server.properties or in
> > > > > > ZooKeeper. If a ZK config cannot be decoded (or is not valid), we
> > log
> > > > an
> > > > > > error and revert to the static config or default. When updating
> the
> > > > > secret
> > > > > > used by the encode, we expect all password values to be specified
> > in
> > > > > > server.properties. And the decoding or sanity check of the
> password
> > > in
> > > > ZK
> > > > > > would fail with the new secret, so we would use the password
> values
> > > > from
> > > > > > server.properties. Once the broker starts up, the values can be
> > reset
> > > > in
> > > > > ZK
> > > > > > using AdminClient and they will be encoded using the new secret.
> > > > > >
> > > > > >
> > > > > > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <
> > ja...@confluent.io>
> > > > > > wrote:
> > > > > >
> > > > > >> +1 Thanks for the KIP. One minor nit: I think we changed
> > > > > >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG
> in
> > > the
> > > > > PR.
> > > > > >>
> > > > > >> As far as updating secrets, I wasn't sure I understand how that
> > will
> > > > > work.
> > > > > >> Do the password configs accept multiple values?
> > > > > >>
> > > > > >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <
> > > > rajinisiva...@gmail.com
> > > > > >
> > > > > >> wrote:
> > > > > >>
> > > > > >> > Hi Jun,
> > > > > >> >
> > > > > >> > Thank you for reviewing and voting.
> > > > > >> >
> > > > > >> > 50. I have updated the KIP to describe how the secret may be
> > > > changed.
> > > > > >> All
> > > > > >> > dynamically configurable passwords and per-broker configs. So
> > the
> > > > > secret
> > > > > >> > can be different across brokers and updated using rolling
> > restart.
> > > > In
> > > > > >> or

Re: [DISCUSS] KIP-232: Detect outdated metadata by adding ControllerMetadataEpoch field

2018-01-08 Thread Dong Lin
Hey Jason,

Thanks for the comments.

On Mon, Jan 8, 2018 at 2:27 PM, Jason Gustafson  wrote:

> >
> > I am not sure I understand the benefit of incrementing this epoch after
> > topic deletion. At a high level, client can not differentiate between
> topic
> > deletion and topic creation when the global epoch is incremented. Can you
> > provide more specific use-case?
>
>
> Say you send two metadata requests to two separate brokers. In the
> responses, one of them says a certain topic exists and one says it does
> not. Who is right? My suggestion is to bump the topic epoch on deletion and
> include it in the metadata response when returning
> UNKNOWN_TOPIC_OR_PARTITION. Then the client always knows which metadata is
> more current (if not necessarily up to date). Because of this ambiguity,
> Kafka clients currently have no choice but retry on unknown topic errors.
> Yes, you can say it is already handled, but this gives us some better
> options in the future. In the consumer, users are often asking to be
> notified when they attempt to fetch from unknown topics, for example,
> because it could indicate a configuration problem. We have difficulty
> supporting this at the moment.
>
> Currently when broker returns UNKNOWN_TOPIC_OR_PARTITION, it means that the
> > topic is not follower or leader of this partition. Note that
> > UNKNOWN_TOPIC_OR_PARTITION does not necessarily tell client whether this
> > partition exists on other broker or not. UNKNOWN_TOPIC_OR_PARTITION can
> be
> > caused either when the broker has not yet processed the latest
> > LeaderAndIsrRequest, or the client is using outdated metadata.
>

My bad. I was thinking about the UNKNOWN_TOPIC_OR_PARTITION from
ProduceResponse or FetchReponse but we are actually discussing the error
from MetadataResponse.



>
>
> I don't think this is right. Metadata is propagated through the
> UpdateMetadata request which the controller sends to all brokers. Brokers
> will return UNKNOWN_TOPIC_OR_PARTITION in a metadata response if they don't
> have metadata cached for the requested topic.
>
> There is one problem though which I think might be what you're getting at.
> After a topic is deleted, the controller will leave it out of future
> UpdateMetadata requests, which means the deleted epoch would not be
> propagated to all brokers and we'd be stuck in the current state. Suppose
> instead that when a topic is deleted, we 1) bump the topic epoch, and 2)
> set an expiration time (say several hours). When the expiration time is
> reached, we delete the topic metadata in zookeeper; until then, the
> controller continues to propagate it as usual with a flag indicating it no
> longer exists. The point of the epoch is solving edge cases around topic
> deletion and recreation, so the expiration timer gives clients a window to
> observe the deletion before the metadata is removed. It also ensures that
> topic metadata is eventually cleaned up following deletion.
>
> What do you think?
>

Yeah that is what I am thinking. As of current proposal, when a topic is
deleted, all topic-specific information will be immediately removed and
client will not see this topic in the latest MetadataResponse. This makes
it hard to distinguish between the metadata before and after the topic
deletion even if we bump up the global topic_epoch in the zookeeper.

I think your approach will allow user to distinguish between the metadata
before and after the topic deletion. I also agree that this can be
potentially be useful to user. I am just not very sure whether we already
have a good use-case to make the additional complexity worthwhile. It seems
that this feature is kind of independent of the main problem of this KIP.
Could we add this as a future work?



> In comparison to byte[], String has the benefit of being more readable and
> > it is also the same type of the existing metadata field, which is used
> for
> > a similar purpose by user. Do you think this is reasonable?
>
>
> I don't have too strong of a feeling about it. I'm not sure how important
> readability is since it's intended to be opaque to the user. To clarify a
> little bit, I think we should continue to send the topic and leader epochs
> in the protocol directly as separate fields. It's only when we surface it
> through the consumer API that we add some obscurity since we don't want
> users to depend on the fields directly and we don't want to make API
> changes in the future if we need to add something else which is also
> internal. In fact, rather than using byte[] or String directly, perhaps we
> could just expose it as an object and give it a readable toString()?
>

I am also not very strong about this. String just seems like a nature
choice given that we already have similar json formatted strong in
zookeeper and we use String for the existing metadata in OffsetAndMetadata.
I just feel it is probably better to use some Java class than using raw
byte array in our Consumer API. But I don't have a strong reason.

If w

Re: [DISCUSS] KIP-232: Detect outdated metadata by adding ControllerMetadataEpoch field

2018-01-08 Thread Dong Lin
Hey Ismael,

I guess we actually need user to see this field so that user can store this
value in the external store together with the offset. We just prefer the
value to be opaque to discourage most users from interpreting this value.
One more advantage of using such an opaque field is to be able to evolve
the information (or schema) of this value without changing consumer API in
the future.

I also thinking it is probably OK for user to be able to interpret this
value, particularly for those advanced users.

Thanks,
Dong

On Mon, Jan 8, 2018 at 2:34 PM, Ismael Juma  wrote:

> On Fri, Jan 5, 2018 at 7:15 PM, Jason Gustafson 
> wrote:
> >
> > class OffsetAndMetadata {
> >   long offset;
> >   byte[] offsetMetadata;
> >   String metadata;
> > }
>
>
> > Admittedly, the naming is a bit annoying, but we can probably come up
> with
> > something better. Internally the byte array would have a version. If in
> the
> > future we have anything else we need to add, we can update the version
> and
> > we wouldn't need any new APIs.
> >
>
> We can also add fields to a class in a compatible way. So, it seems to me
> that the main advantage of the byte array is that it's opaque to the user.
> Is that correct? If so, we could also add any opaque metadata in a subclass
> so that users don't even see it (unless they cast it, but then they're on
> their own).
>
> Ismael
>
> The corresponding seek() and position() APIs might look something like
> this:
> >
> > void seek(TopicPartition partition, long offset, byte[] offsetMetadata);
> > byte[] positionMetadata(TopicPartition partition);
> >
> > What do you think?
> >
> > Thanks,
> > Jason
> >
> > On Thu, Jan 4, 2018 at 7:04 PM, Dong Lin  wrote:
> >
> > > Hey Jun, Jason,
> > >
> > > Thanks much for all the feedback. I have updated the KIP based on the
> > > latest discussion. Can you help check whether it looks good?
> > >
> > > Thanks,
> > > Dong
> > >
> > > On Thu, Jan 4, 2018 at 5:36 PM, Dong Lin  wrote:
> > >
> > > > Hey Jun,
> > > >
> > > > Hmm... thinking about this more, I am not sure that the proposed API
> is
> > > > sufficient. For users that store offset externally, we probably need
> > > extra
> > > > API to return the leader_epoch and partition_epoch for all partitions
> > > that
> > > > consumers are consuming. I suppose these users currently use
> position()
> > > to
> > > > get the offset. Thus we probably need a new method
> > positionWithEpoch(..)
> > > to
> > > > return . Does this sound
> > > reasonable?
> > > >
> > > > Thanks,
> > > > Dong
> > > >
> > > >
> > > > On Thu, Jan 4, 2018 at 5:26 PM, Jun Rao  wrote:
> > > >
> > > >> Hi, Dong,
> > > >>
> > > >> Yes, that's what I am thinking. OffsetEpoch will be composed of
> > > >> (partition_epoch,
> > > >> leader_epoch).
> > > >>
> > > >> Thanks,
> > > >>
> > > >> Jun
> > > >>
> > > >>
> > > >> On Thu, Jan 4, 2018 at 4:22 PM, Dong Lin 
> wrote:
> > > >>
> > > >> > Hey Jun,
> > > >> >
> > > >> > Thanks much. I like the the new API that you proposed. I am not
> sure
> > > >> what
> > > >> > you exactly mean by offset_epoch. I suppose that we can use the
> pair
> > > of
> > > >> > (partition_epoch, leader_epoch) as the offset_epoch, right?
> > > >> >
> > > >> > Thanks,
> > > >> > Dong
> > > >> >
> > > >> > On Thu, Jan 4, 2018 at 4:02 PM, Jun Rao  wrote:
> > > >> >
> > > >> > > Hi, Dong,
> > > >> > >
> > > >> > > Got it. The api that you proposed works. The question is whether
> > > >> that's
> > > >> > the
> > > >> > > api that we want to have in the long term. My concern is that
> > while
> > > >> the
> > > >> > api
> > > >> > > change is simple, the new api seems harder to explain and use.
> For
> > > >> > example,
> > > >> > > a consumer storing offsets externally now needs to call
> > > >> > > waitForMetadataUpdate() after calling seek().
> > > >> > >
> > > >> > > An alternative approach is to make the following compatible api
> > > >> changes
> > > >> > in
> > > >> > > Consumer.
> > > >> > > * Add an additional OffsetEpoch field in OffsetAndMetadata. (no
> > need
> > > >> to
> > > >> > > change the CommitSync() api)
> > > >> > > * Add a new api seek(TopicPartition partition, long offset,
> > > >> OffsetEpoch
> > > >> > > offsetEpoch). We can potentially deprecate the old api
> > > >> > seek(TopicPartition
> > > >> > > partition, long offset) in the future.
> > > >> > >
> > > >> > > The alternative approach has similar amount of api changes as
> > yours
> > > >> but
> > > >> > has
> > > >> > > the following benefits.
> > > >> > > 1. The api works in a similar way as how offset management works
> > now
> > > >> and
> > > >> > is
> > > >> > > probably what we want in the long term.
> > > >> > > 2. It can reset offsets better when there is data loss due to
> > > unclean
> > > >> > > leader election or correlated replica failure.
> > > >> > > 3. It can reset offsets better when topic is recreated.
> > > >> > >
> > > >> > > Thanks,
> > > >> > >
> > > >> > > Jun
> > > >> > >
> > > >> > >
> > > >> > > On Thu, Jan 4,

[jira] [Created] (KAFKA-6433) Connect distributed workers should fail if their config is "incompatible" with leader's

2018-01-08 Thread Randall Hauch (JIRA)
Randall Hauch created KAFKA-6433:


 Summary: Connect distributed workers should fail if their config 
is "incompatible" with leader's
 Key: KAFKA-6433
 URL: https://issues.apache.org/jira/browse/KAFKA-6433
 Project: Kafka
  Issue Type: Improvement
  Components: KafkaConnect
Affects Versions: 1.0.0
Reporter: Randall Hauch


Currently, each distributed worker config must have the same `worker.id` and 
must use the same internal topics for configs, offsets, and status. 
Additionally, each worker must be configured to have the same connectors, SMTs, 
and converters; confusing error messages will result when some workers are able 
to deploy connector tasks with SMTs while others fail when they are missing 
plugins the other workers do have.

Ideally, a Connect workers would only be allowed to join the cluster if it were 
"compatible" with the the existing cluster, where "compatible" perhaps includes 
using the same internal topics and having the same set of plugins.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


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

2018-01-08 Thread Apache Jenkins Server
See 




Re: [DISCUSS] KIP-232: Detect outdated metadata by adding ControllerMetadataEpoch field

2018-01-08 Thread Jason Gustafson
Hi Dong,


I think your approach will allow user to distinguish between the metadata
> before and after the topic deletion. I also agree that this can be
> potentially be useful to user. I am just not very sure whether we already
> have a good use-case to make the additional complexity worthwhile. It seems
> that this feature is kind of independent of the main problem of this KIP.
> Could we add this as a future work?


Do you think it's fair if we bump the topic epoch on deletion and leave
propagation of the epoch for deleted topics for future work? I don't think
this adds much complexity and it makes the behavior consistent: every topic
mutation results in an epoch bump.

Thanks,
Jason

On Mon, Jan 8, 2018 at 3:14 PM, Dong Lin  wrote:

> Hey Ismael,
>
> I guess we actually need user to see this field so that user can store this
> value in the external store together with the offset. We just prefer the
> value to be opaque to discourage most users from interpreting this value.
> One more advantage of using such an opaque field is to be able to evolve
> the information (or schema) of this value without changing consumer API in
> the future.
>
> I also thinking it is probably OK for user to be able to interpret this
> value, particularly for those advanced users.
>
> Thanks,
> Dong
>
> On Mon, Jan 8, 2018 at 2:34 PM, Ismael Juma  wrote:
>
> > On Fri, Jan 5, 2018 at 7:15 PM, Jason Gustafson 
> > wrote:
> > >
> > > class OffsetAndMetadata {
> > >   long offset;
> > >   byte[] offsetMetadata;
> > >   String metadata;
> > > }
> >
> >
> > > Admittedly, the naming is a bit annoying, but we can probably come up
> > with
> > > something better. Internally the byte array would have a version. If in
> > the
> > > future we have anything else we need to add, we can update the version
> > and
> > > we wouldn't need any new APIs.
> > >
> >
> > We can also add fields to a class in a compatible way. So, it seems to me
> > that the main advantage of the byte array is that it's opaque to the
> user.
> > Is that correct? If so, we could also add any opaque metadata in a
> subclass
> > so that users don't even see it (unless they cast it, but then they're on
> > their own).
> >
> > Ismael
> >
> > The corresponding seek() and position() APIs might look something like
> > this:
> > >
> > > void seek(TopicPartition partition, long offset, byte[]
> offsetMetadata);
> > > byte[] positionMetadata(TopicPartition partition);
> > >
> > > What do you think?
> > >
> > > Thanks,
> > > Jason
> > >
> > > On Thu, Jan 4, 2018 at 7:04 PM, Dong Lin  wrote:
> > >
> > > > Hey Jun, Jason,
> > > >
> > > > Thanks much for all the feedback. I have updated the KIP based on the
> > > > latest discussion. Can you help check whether it looks good?
> > > >
> > > > Thanks,
> > > > Dong
> > > >
> > > > On Thu, Jan 4, 2018 at 5:36 PM, Dong Lin 
> wrote:
> > > >
> > > > > Hey Jun,
> > > > >
> > > > > Hmm... thinking about this more, I am not sure that the proposed
> API
> > is
> > > > > sufficient. For users that store offset externally, we probably
> need
> > > > extra
> > > > > API to return the leader_epoch and partition_epoch for all
> partitions
> > > > that
> > > > > consumers are consuming. I suppose these users currently use
> > position()
> > > > to
> > > > > get the offset. Thus we probably need a new method
> > > positionWithEpoch(..)
> > > > to
> > > > > return . Does this sound
> > > > reasonable?
> > > > >
> > > > > Thanks,
> > > > > Dong
> > > > >
> > > > >
> > > > > On Thu, Jan 4, 2018 at 5:26 PM, Jun Rao  wrote:
> > > > >
> > > > >> Hi, Dong,
> > > > >>
> > > > >> Yes, that's what I am thinking. OffsetEpoch will be composed of
> > > > >> (partition_epoch,
> > > > >> leader_epoch).
> > > > >>
> > > > >> Thanks,
> > > > >>
> > > > >> Jun
> > > > >>
> > > > >>
> > > > >> On Thu, Jan 4, 2018 at 4:22 PM, Dong Lin 
> > wrote:
> > > > >>
> > > > >> > Hey Jun,
> > > > >> >
> > > > >> > Thanks much. I like the the new API that you proposed. I am not
> > sure
> > > > >> what
> > > > >> > you exactly mean by offset_epoch. I suppose that we can use the
> > pair
> > > > of
> > > > >> > (partition_epoch, leader_epoch) as the offset_epoch, right?
> > > > >> >
> > > > >> > Thanks,
> > > > >> > Dong
> > > > >> >
> > > > >> > On Thu, Jan 4, 2018 at 4:02 PM, Jun Rao 
> wrote:
> > > > >> >
> > > > >> > > Hi, Dong,
> > > > >> > >
> > > > >> > > Got it. The api that you proposed works. The question is
> whether
> > > > >> that's
> > > > >> > the
> > > > >> > > api that we want to have in the long term. My concern is that
> > > while
> > > > >> the
> > > > >> > api
> > > > >> > > change is simple, the new api seems harder to explain and use.
> > For
> > > > >> > example,
> > > > >> > > a consumer storing offsets externally now needs to call
> > > > >> > > waitForMetadataUpdate() after calling seek().
> > > > >> > >
> > > > >> > > An alternative approach is to make the following compatible
> api
> > > > >> changes
> > > > >> > in
> > > > >> > > Consumer.
> > > > >>

Re: [DISCUSS] KIP-208: Add SSL support to Kafka Connect REST interface

2018-01-08 Thread Ewen Cheslack-Postava
On Mon, Jan 8, 2018 at 11:39 AM, Randall Hauch  wrote:

> Nice feedback, Ewen. Thanks!
>
> On Thu, Jan 4, 2018 at 5:11 PM, Ewen Cheslack-Postava 
> wrote:
>
> > Hey Jakub,
> >
> > Sorry for not getting to this sooner. Overall the proposal looks good to
> > me, I just had a couple of questions.
> >
> > 1. For the configs/overrides, does this happen on a per-setting basis or
> if
> > one override is included do we not use any of the original settings? I
> > suspect that if you need to override one setting, it probably means
> you're
> > using an entirely different config and so the latter behavior seems
> better
> > to me. We've talked a bit about doing something similar for the
> > producer/consumer security settings as well so you don't have to specify
> > security configs in 3 places in your worker config.
> >
>
> Not sure if you were referring to
> https://issues.apache.org/jira/browse/KAFKA-6387, but I just withdrew that
> proposal (and the corresponding KIP-246) because behavior with existing
> configurations was not backward compatible, so existing configs might have
> very different behavior after the "inheritance" was implemented.
>
> But regardless, I do think that in this case if you have to override one of
> the settings you probably need to override multiple. So I'd be in favor of
> requiring all configs to be specified in the overridden `listeners.*`
> properties.
>

Yeah, a related case i was thinking of is how key.converter and
value.converter overrides work in Connectors. It's not exactly the same,
but in that case, if you include the key.converter setting in the connector
config, then nothing with key.converter prefix from the worker is passed
along. Just might be worth clarifying the all-or-nothing behavior. Also how
we apply it in this case (e.g. is there one key setting we can use that, if
it appears, then we do not inherit any security configs from the worker?)


>
>
> >
> > 2. For using default values from the worker config, I am wondering how
> > convinced we are that it will be common for them to be the same? I really
> > don't have enough experience w/ these setups to know, so just a question
> > here. I think the other thing to take into account here is that even
> though
> > we're not dealing with authorization in this KIP, we will eventually want
> > it for these APIs. Would we expect to be using the same principal for
> Kafka
> > and the Connect REST API? In a case where a company has a Connect cluster
> > that, e.g., an ops team manages and they are the only ones that are
> > supposed to make changes, that would make sense to me. But for a setup
> > where some dev team is allowed to use the REST API to create new
> connectors
> > but the cluster is managed by an ops team, I would think the Kafka
> > credentials would be different. I'm not sure how frequent each case would
> > be, so I'm a bit unsure about the default of using the worker security
> > configs by default. Thoughts?
> >
> > 3. We should probably specify the default in the table for
> > rest.advertised.security.protocol because in ConfigDef if you don't
> > specify
> > a default value it becomes a required config. The HTTP default will
> > probably need to be in there anyway.
> >
> > 4. Do we want to list the existing settings as deprecated and just move
> to
> > using listeners for consistency? We don't need to remove them anytime
> soon,
> > but given that the broker is doing the same, maybe we should just do that
> > in this KIP?
> >
>
> Marking them as deprecated in this KIP sounds good to me.
>
> >
> > I think these are mostly small details, overall it looks like a good
> plan!
> >
>
> +1
>
> Randall
>
>
> >
> > Thanks,
> > Ewen
> >
> > On Tue, Oct 24, 2017 at 5:19 AM, Jakub Scholz  wrote:
> >
> > > There has been no discussion since my last update week ago. Unless
> > someone
> > > has some further comments in the next 48 hours, I will start the voting
> > for
> > > this KIP.
> > >
> > > Thanks & Regards
> > > Jakub
> > >
> > > On Tue, Oct 17, 2017 at 5:54 PM, Jakub Scholz  wrote:
> > >
> > > > Ok, so I updated the KIP according to what we discussed. Please have
> a
> > > > look at the updates. Two points I'm not 100% sure about:
> > > >
> > > > 1) Should we mark the rest.host.name and rest.port options as
> > > deprecated?
> > > >
> > > > 2) I needed to also address the advertised hostname / port. With
> > multiple
> > > > listeners it is not clear anymore which one should be used. I saw as
> > one
> > > > option to add advertised.listeners option and some modified version
> of
> > > > inter.broker.listener.name option to follow what is done in Kafka
> > > > brokers. But for the Connect REST interface, we do not advertise the
> > > > address to the clients like in Kafka broker. So we only need to tell
> > > other
> > > > workers how to connect - and for that we need only one advertised
> > > address.
> > > > So I decided to reuse the existing rest.advertised.host.name and
> > > > rest.advertised.port options 

Re: [DISCUSS] KIP-232: Detect outdated metadata by adding ControllerMetadataEpoch field

2018-01-08 Thread Dong Lin
Hey Jason,

Certainly. This sounds good. I have updated the KIP to clarity that the
global epoch will be incremented by 1 each time a topic is deleted.

Thanks,
Dong

On Mon, Jan 8, 2018 at 4:09 PM, Jason Gustafson  wrote:

> Hi Dong,
>
>
> I think your approach will allow user to distinguish between the metadata
> > before and after the topic deletion. I also agree that this can be
> > potentially be useful to user. I am just not very sure whether we already
> > have a good use-case to make the additional complexity worthwhile. It
> seems
> > that this feature is kind of independent of the main problem of this KIP.
> > Could we add this as a future work?
>
>
> Do you think it's fair if we bump the topic epoch on deletion and leave
> propagation of the epoch for deleted topics for future work? I don't think
> this adds much complexity and it makes the behavior consistent: every topic
> mutation results in an epoch bump.
>
> Thanks,
> Jason
>
> On Mon, Jan 8, 2018 at 3:14 PM, Dong Lin  wrote:
>
> > Hey Ismael,
> >
> > I guess we actually need user to see this field so that user can store
> this
> > value in the external store together with the offset. We just prefer the
> > value to be opaque to discourage most users from interpreting this value.
> > One more advantage of using such an opaque field is to be able to evolve
> > the information (or schema) of this value without changing consumer API
> in
> > the future.
> >
> > I also thinking it is probably OK for user to be able to interpret this
> > value, particularly for those advanced users.
> >
> > Thanks,
> > Dong
> >
> > On Mon, Jan 8, 2018 at 2:34 PM, Ismael Juma  wrote:
> >
> > > On Fri, Jan 5, 2018 at 7:15 PM, Jason Gustafson 
> > > wrote:
> > > >
> > > > class OffsetAndMetadata {
> > > >   long offset;
> > > >   byte[] offsetMetadata;
> > > >   String metadata;
> > > > }
> > >
> > >
> > > > Admittedly, the naming is a bit annoying, but we can probably come up
> > > with
> > > > something better. Internally the byte array would have a version. If
> in
> > > the
> > > > future we have anything else we need to add, we can update the
> version
> > > and
> > > > we wouldn't need any new APIs.
> > > >
> > >
> > > We can also add fields to a class in a compatible way. So, it seems to
> me
> > > that the main advantage of the byte array is that it's opaque to the
> > user.
> > > Is that correct? If so, we could also add any opaque metadata in a
> > subclass
> > > so that users don't even see it (unless they cast it, but then they're
> on
> > > their own).
> > >
> > > Ismael
> > >
> > > The corresponding seek() and position() APIs might look something like
> > > this:
> > > >
> > > > void seek(TopicPartition partition, long offset, byte[]
> > offsetMetadata);
> > > > byte[] positionMetadata(TopicPartition partition);
> > > >
> > > > What do you think?
> > > >
> > > > Thanks,
> > > > Jason
> > > >
> > > > On Thu, Jan 4, 2018 at 7:04 PM, Dong Lin 
> wrote:
> > > >
> > > > > Hey Jun, Jason,
> > > > >
> > > > > Thanks much for all the feedback. I have updated the KIP based on
> the
> > > > > latest discussion. Can you help check whether it looks good?
> > > > >
> > > > > Thanks,
> > > > > Dong
> > > > >
> > > > > On Thu, Jan 4, 2018 at 5:36 PM, Dong Lin 
> > wrote:
> > > > >
> > > > > > Hey Jun,
> > > > > >
> > > > > > Hmm... thinking about this more, I am not sure that the proposed
> > API
> > > is
> > > > > > sufficient. For users that store offset externally, we probably
> > need
> > > > > extra
> > > > > > API to return the leader_epoch and partition_epoch for all
> > partitions
> > > > > that
> > > > > > consumers are consuming. I suppose these users currently use
> > > position()
> > > > > to
> > > > > > get the offset. Thus we probably need a new method
> > > > positionWithEpoch(..)
> > > > > to
> > > > > > return . Does this sound
> > > > > reasonable?
> > > > > >
> > > > > > Thanks,
> > > > > > Dong
> > > > > >
> > > > > >
> > > > > > On Thu, Jan 4, 2018 at 5:26 PM, Jun Rao 
> wrote:
> > > > > >
> > > > > >> Hi, Dong,
> > > > > >>
> > > > > >> Yes, that's what I am thinking. OffsetEpoch will be composed of
> > > > > >> (partition_epoch,
> > > > > >> leader_epoch).
> > > > > >>
> > > > > >> Thanks,
> > > > > >>
> > > > > >> Jun
> > > > > >>
> > > > > >>
> > > > > >> On Thu, Jan 4, 2018 at 4:22 PM, Dong Lin 
> > > wrote:
> > > > > >>
> > > > > >> > Hey Jun,
> > > > > >> >
> > > > > >> > Thanks much. I like the the new API that you proposed. I am
> not
> > > sure
> > > > > >> what
> > > > > >> > you exactly mean by offset_epoch. I suppose that we can use
> the
> > > pair
> > > > > of
> > > > > >> > (partition_epoch, leader_epoch) as the offset_epoch, right?
> > > > > >> >
> > > > > >> > Thanks,
> > > > > >> > Dong
> > > > > >> >
> > > > > >> > On Thu, Jan 4, 2018 at 4:02 PM, Jun Rao 
> > wrote:
> > > > > >> >
> > > > > >> > > Hi, Dong,
> > > > > >> > >
> > > > > >> > > Got it. The api that you proposed works. The question is
> > whether
> > > > > >> that

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

2018-01-08 Thread Apache Jenkins Server
See 




Re: [VOTE] KIP-174 Deprecate and remove internal converter configs in WorkerConfig

2018-01-08 Thread Ewen Cheslack-Postava
+1 binding. Thanks for the KIP!

-Ewen

On Mon, Jan 8, 2018 at 8:34 AM, Ted Yu  wrote:

> +1
>
> On Mon, Jan 8, 2018 at 4:27 AM, UMESH CHAUDHARY 
> wrote:
>
> > Hello All,
> > Since there are no outstanding comments on this, so I'd like to start a
> > vote.
> >
> > Please find the KIP here
> >  > 174+-+Deprecate+and+remove+internal+converter+configs+in+WorkerConfig>
> > and
> > the related JIRA here  >.
> >
> > The KIP suggests to deprecate and remove the configs:
> > internal.key.converter, internal.value.converter
> >
> > Appreciate your comments.
> >
> > Regards,
> > Umesh
> >
>


Re: [VOTE] KIP-174 Deprecate and remove internal converter configs in WorkerConfig

2018-01-08 Thread Gwen Shapira
+1 binding

On Mon, Jan 8, 2018 at 4:59 PM Ewen Cheslack-Postava 
wrote:

> +1 binding. Thanks for the KIP!
>
> -Ewen
>
> On Mon, Jan 8, 2018 at 8:34 AM, Ted Yu  wrote:
>
> > +1
> >
> > On Mon, Jan 8, 2018 at 4:27 AM, UMESH CHAUDHARY 
> > wrote:
> >
> > > Hello All,
> > > Since there are no outstanding comments on this, so I'd like to start a
> > > vote.
> > >
> > > Please find the KIP here
> > >  > > 174+-+Deprecate+and+remove+internal+converter+configs+in+WorkerConfig>
> > > and
> > > the related JIRA here <
> https://issues.apache.org/jira/browse/KAFKA-5540
> > >.
> > >
> > > The KIP suggests to deprecate and remove the configs:
> > > internal.key.converter, internal.value.converter
> > >
> > > Appreciate your comments.
> > >
> > > Regards,
> > > Umesh
> > >
> >
>


Re: [DISCUSS] KIP-232: Detect outdated metadata by adding ControllerMetadataEpoch field

2018-01-08 Thread Jun Rao
Hi, Dong,

Thanks for the updated KIP. A few more comments.

60. Perhaps having a partition epoch is more flexible since in the future,
we may support deleting a partition as well.

61. It seems that the leader epoch returned in the position() call should
the the leader epoch returned in the fetch response, not the one in the
metadata cache of the client.

62. I am wondering if we should return the partition epoch in the fetch
response as well. In the current proposal, if a topic is recreated and the
new leader is on the same broker as the old one, there is nothing to force
the metadata refresh in the client. So, the client may still associate the
offset with the old partition epoch.

63. There is some subtle coordination between the LeaderAndIsrRequest and
UpdateMetadataRequest. Currently, when a leader changes, the controller
first sends the LeaderAndIsrRequest to the assigned replicas and the
UpdateMetadataRequest to every broker. So, there could be a small window
when the leader already receives the new partition epoch in the
LeaderAndIsrRequest, but the metadata cache in the broker hasn't been
updated with the latest partition epoch. Not sure what's the best way to
address this issue. Perhaps we can update the metadata cache on the broker
with both LeaderAndIsrRequest and UpdateMetadataRequest. The challenge is
that the two have slightly different data. For example, only the latter has
all endpoints.

64. The enforcement of leader epoch in Offset commit: We allow a consumer
to set an arbitrary offset. So it's possible for offsets or leader epoch to
go backwards. I am not sure if we could always enforce that the leader
epoch only goes up on the broker.

65. Good point on handling missing partition epoch due to topic deletion.
Another potential way to address this is to additionally propagate the
global partition epoch to brokers and the clients. This way, when a
partition epoch is missing, we can use the global partition epoch to reason
about which metadata is more recent.

66. A client may also get an offset by time using the offsetForTimes() api.
So, we probably want to include offsetInternalMetadata in OffsetAndTimestamp
as well.

67. InteralMetadata can be a bit confusing with the metadata field already
there. Perhaps we can just call it OffsetEpoch. It might be useful to make
OffsetEpoch printable at least for debugging purpose. Once you do that, we
are already exposing the internal fields. So, not sure if it's worth hiding
them. If we do want to hide them, perhaps we can have sth like the
following. The binary encoding is probably more efficient than JSON for
external storage.

OffsetEpoch {
 static OffsetEpoch decode(byte[]);

  public byte[] encode();

  public String toString();
}

Jun

On Mon, Jan 8, 2018 at 4:22 PM, Dong Lin  wrote:

> Hey Jason,
>
> Certainly. This sounds good. I have updated the KIP to clarity that the
> global epoch will be incremented by 1 each time a topic is deleted.
>
> Thanks,
> Dong
>
> On Mon, Jan 8, 2018 at 4:09 PM, Jason Gustafson 
> wrote:
>
> > Hi Dong,
> >
> >
> > I think your approach will allow user to distinguish between the metadata
> > > before and after the topic deletion. I also agree that this can be
> > > potentially be useful to user. I am just not very sure whether we
> already
> > > have a good use-case to make the additional complexity worthwhile. It
> > seems
> > > that this feature is kind of independent of the main problem of this
> KIP.
> > > Could we add this as a future work?
> >
> >
> > Do you think it's fair if we bump the topic epoch on deletion and leave
> > propagation of the epoch for deleted topics for future work? I don't
> think
> > this adds much complexity and it makes the behavior consistent: every
> topic
> > mutation results in an epoch bump.
> >
> > Thanks,
> > Jason
> >
> > On Mon, Jan 8, 2018 at 3:14 PM, Dong Lin  wrote:
> >
> > > Hey Ismael,
> > >
> > > I guess we actually need user to see this field so that user can store
> > this
> > > value in the external store together with the offset. We just prefer
> the
> > > value to be opaque to discourage most users from interpreting this
> value.
> > > One more advantage of using such an opaque field is to be able to
> evolve
> > > the information (or schema) of this value without changing consumer API
> > in
> > > the future.
> > >
> > > I also thinking it is probably OK for user to be able to interpret this
> > > value, particularly for those advanced users.
> > >
> > > Thanks,
> > > Dong
> > >
> > > On Mon, Jan 8, 2018 at 2:34 PM, Ismael Juma  wrote:
> > >
> > > > On Fri, Jan 5, 2018 at 7:15 PM, Jason Gustafson 
> > > > wrote:
> > > > >
> > > > > class OffsetAndMetadata {
> > > > >   long offset;
> > > > >   byte[] offsetMetadata;
> > > > >   String metadata;
> > > > > }
> > > >
> > > >
> > > > > Admittedly, the naming is a bit annoying, but we can probably come
> up
> > > > with
> > > > > something better. Internally the byte array would have a version.
> If
> > in
> > > > t

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

2018-01-08 Thread Apache Jenkins Server
See 


Changes:

[jason] KAFKA-6096: Add multi-threaded tests for group coordinator, txn manager

--
[...truncated 400.63 KB...]

kafka.tools.ConsoleProducerTest > testParseKeyProp STARTED

kafka.tools.ConsoleProducerTest > testParseKeyProp PASSED

kafka.tools.ConsoleProducerTest > testValidConfigsOldProducer STARTED

kafka.tools.ConsoleProducerTest > testValidConfigsOldProducer PASSED

kafka.tools.ConsoleProducerTest > testInvalidConfigs STARTED

kafka.tools.ConsoleProducerTest > testInvalidConfigs PASSED

kafka.tools.ConsoleProducerTest > testValidConfigsNewProducer STARTED

kafka.tools.ConsoleProducerTest > testValidConfigsNewProducer PASSED

kafka.tools.MirrorMakerTest > 
testDefaultMirrorMakerMessageHandlerWithNoTimestampInSourceMessage STARTED

kafka.tools.MirrorMakerTest > 
testDefaultMirrorMakerMessageHandlerWithNoTimestampInSourceMessage PASSED

kafka.tools.MirrorMakerTest > testDefaultMirrorMakerMessageHandler STARTED

kafka.tools.MirrorMakerTest > testDefaultMirrorMakerMessageHandler PASSED

kafka.tools.MirrorMakerTest > testDefaultMirrorMakerMessageHandlerWithHeaders 
STARTED

kafka.tools.MirrorMakerTest > testDefaultMirrorMakerMessageHandlerWithHeaders 
PASSED

kafka.tools.ConsumerPerformanceTest > testDetailedHeaderMatchBody STARTED

kafka.tools.ConsumerPerformanceTest > testDetailedHeaderMatchBody PASSED

kafka.tools.ConsumerPerformanceTest > testNonDetailedHeaderMatchBody STARTED

kafka.tools.ConsumerPerformanceTest > testNonDetailedHeaderMatchBody PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidOldConsumerConfigWithAutoOffsetResetSmallest STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidOldConsumerConfigWithAutoOffsetResetSmallest PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithNoOffsetReset STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithNoOffsetReset PASSED

kafka.tools.ConsoleConsumerTest > shouldLimitReadsToMaxMessageLimit STARTED

kafka.tools.ConsoleConsumerTest > shouldLimitReadsToMaxMessageLimit PASSED

kafka.tools.ConsoleConsumerTest > shouldParseValidNewConsumerValidConfig STARTED

kafka.tools.ConsoleConsumerTest > shouldParseValidNewConsumerValidConfig PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetAndMatchingFromBeginning 
STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetAndMatchingFromBeginning 
PASSED

kafka.tools.ConsoleConsumerTest > shouldStopWhenOutputCheckErrorFails STARTED

kafka.tools.ConsoleConsumerTest > shouldStopWhenOutputCheckErrorFails PASSED

kafka.tools.ConsoleConsumerTest > 
shouldExitOnInvalidConfigWithAutoOffsetResetAndConflictingFromBeginningNewConsumer
 STARTED

kafka.tools.ConsoleConsumerTest > 
shouldExitOnInvalidConfigWithAutoOffsetResetAndConflictingFromBeginningNewConsumer
 PASSED

kafka.tools.ConsoleConsumerTest > 
shouldSetAutoResetToSmallestWhenFromBeginningConfigured STARTED

kafka.tools.ConsoleConsumerTest > 
shouldSetAutoResetToSmallestWhenFromBeginningConfigured PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetEarliest STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetEarliest PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewSimpleConsumerValidConfigWithStringOffset STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewSimpleConsumerValidConfigWithStringOffset PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidOldConsumerConfigWithAutoOffsetResetLargest STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidOldConsumerConfigWithAutoOffsetResetLargest PASSED

kafka.tools.ConsoleConsumerTest > shouldParseConfigsFromFile STARTED

kafka.tools.ConsoleConsumerTest > shouldParseConfigsFromFile PASSED

kafka.tools.ConsoleConsumerTest > groupIdsProvidedInDifferentPlacesMustMatch 
STARTED

kafka.tools.ConsoleConsumerTest > groupIdsProvidedInDifferentPlacesMustMatch 
PASSED

kafka.tools.ConsoleConsumerTest > 
shouldExitOnInvalidConfigWithAutoOffsetResetAndConflictingFromBeginningOldConsumer
 STARTED

kafka.tools.ConsoleConsumerTest > 
shouldExitOnInvalidConfigWithAutoOffsetResetAndConflictingFromBeginningOldConsumer
 PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewSimpleConsumerValidConfigWithNumericOffset STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewSimpleConsumerValidConfigWithNumericOffset PASSED

kafka.tools.ConsoleConsumerTest > testDefaultConsumer STARTED

kafka.tools.ConsoleConsumerTest > testDefaultConsumer PASSED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetLatest STARTED

kafka.tools.ConsoleConsumerTest > 
shouldParseValidNewConsumerConfigWithAutoOffsetResetLatest PASSED

kafka.tools.ConsoleConsumerTest > shouldParseValidOldConsumerValidConfig START

Re: [VOTE] KIP-219 - Improve Quota Communication

2018-01-08 Thread Jun Rao
Hi, Jiangjie,

Sorry for the late response. The proposal sounds good overall. A couple of
minor comments below.

1. For throttling a fetch request, we could potentially just send an empty
response. We can return a throttle time calculated from a full response,
but only mute the channel on the server based on a throttle time calculated
based on the empty response. This has the benefit that the server will mute
the channel much shorter, which will prevent the consumer from rebalancing
when throttled.

2. The wiki says "connections.max.idle.ms should be ignored during the
throttle time X." This has the potential issue that a server may not detect
that a client connection is already gone until after an arbitrary amount of
time. Perhaps we could still just close a connection if the server has
muted it for longer than connections.max.idle.ms. This will at least bound
the time for a server to detect closed client connections.

Thanks,

Jun


On Mon, Nov 20, 2017 at 5:30 PM, Becket Qin  wrote:

> Hi,
>
> We would like to start the voting thread for KIP-219. The KIP proposes to
> improve the quota communication between the brokers and clients, especially
> for cases of long throttling time.
>
> The KIP wiki is following:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-219+-+Improve+quota+
> communication
>
> The discussion thread is here:
> http://markmail.org/search/?q=kafka+KIP-219#query:kafka%
> 20KIP-219+page:1+mid:ooxabguy7nz7l7zy+state:results
>
> Thanks,
>
> Jiangjie (Becket) Qin
>


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

2018-01-08 Thread Apache Jenkins Server
See 


Changes:

[jason] KAFKA-6422 Mirror maker will throw null pointer exception when the

--
[...truncated 3.38 MB...]

kafka.utils.CoreUtilsTest > testReadInt STARTED

kafka.utils.CoreUtilsTest > testReadInt PASSED

kafka.utils.CoreUtilsTest > testAtomicGetOrUpdate STARTED

kafka.utils.CoreUtilsTest > testAtomicGetOrUpdate PASSED

kafka.utils.CoreUtilsTest > testUrlSafeBase64EncodeUUID STARTED

kafka.utils.CoreUtilsTest > testUrlSafeBase64EncodeUUID PASSED

kafka.utils.CoreUtilsTest > testCsvMap STARTED

kafka.utils.CoreUtilsTest > testCsvMap PASSED

kafka.utils.CoreUtilsTest > testInLock STARTED

kafka.utils.CoreUtilsTest > testInLock PASSED

kafka.utils.CoreUtilsTest > testTryAll STARTED

kafka.utils.CoreUtilsTest > testTryAll PASSED

kafka.utils.CoreUtilsTest > testSwallow STARTED

kafka.utils.CoreUtilsTest > testSwallow PASSED

kafka.utils.IteratorTemplateTest > testIterator STARTED

kafka.utils.IteratorTemplateTest > testIterator PASSED

kafka.utils.json.JsonValueTest > testJsonObjectIterator STARTED

kafka.utils.json.JsonValueTest > testJsonObjectIterator PASSED

kafka.utils.json.JsonValueTest > testDecodeLong STARTED

kafka.utils.json.JsonValueTest > testDecodeLong PASSED

kafka.utils.json.JsonValueTest > testAsJsonObject STARTED

kafka.utils.json.JsonValueTest > testAsJsonObject PASSED

kafka.utils.json.JsonValueTest > testDecodeDouble STARTED

kafka.utils.json.JsonValueTest > testDecodeDouble PASSED

kafka.utils.json.JsonValueTest > testDecodeOption STARTED

kafka.utils.json.JsonValueTest > testDecodeOption PASSED

kafka.utils.json.JsonValueTest > testDecodeString STARTED

kafka.utils.json.JsonValueTest > testDecodeString PASSED

kafka.utils.json.JsonValueTest > testJsonValueToString STARTED

kafka.utils.json.JsonValueTest > testJsonValueToString PASSED

kafka.utils.json.JsonValueTest > testAsJsonObjectOption STARTED

kafka.utils.json.JsonValueTest > testAsJsonObjectOption PASSED

kafka.utils.json.JsonValueTest > testAsJsonArrayOption STARTED

kafka.utils.json.JsonValueTest > testAsJsonArrayOption PASSED

kafka.utils.json.JsonValueTest > testAsJsonArray STARTED

kafka.utils.json.JsonValueTest > testAsJsonArray PASSED

kafka.utils.json.JsonValueTest > testJsonValueHashCode STARTED

kafka.utils.json.JsonValueTest > testJsonValueHashCode PASSED

kafka.utils.json.JsonValueTest > testDecodeInt STARTED

kafka.utils.json.JsonValueTest > testDecodeInt PASSED

kafka.utils.json.JsonValueTest > testDecodeMap STARTED

kafka.utils.json.JsonValueTest > testDecodeMap PASSED

kafka.utils.json.JsonValueTest > testDecodeSeq STARTED

kafka.utils.json.JsonValueTest > testDecodeSeq PASSED

kafka.utils.json.JsonValueTest > testJsonObjectGet STARTED

kafka.utils.json.JsonValueTest > testJsonObjectGet PASSED

kafka.utils.json.JsonValueTest > testJsonValueEquals STARTED

kafka.utils.json.JsonValueTest > testJsonValueEquals PASSED

kafka.utils.json.JsonValueTest > testJsonArrayIterator STARTED

kafka.utils.json.JsonValueTest > testJsonArrayIterator PASSED

kafka.utils.json.JsonValueTest > testJsonObjectApply STARTED

kafka.utils.json.JsonValueTest > testJsonObjectApply PASSED

kafka.utils.json.JsonValueTest > testDecodeBoolean STARTED

kafka.utils.json.JsonValueTest > testDecodeBoolean PASSED

kafka.producer.AsyncProducerTest > testFailedSendRetryLogic STARTED

kafka.producer.AsyncProducerTest > testFailedSendRetryLogic PASSED

kafka.producer.AsyncProducerTest > testQueueTimeExpired STARTED

kafka.producer.AsyncProducerTest > testQueueTimeExpired PASSED

kafka.producer.AsyncProducerTest > testPartitionAndCollateEvents STARTED

kafka.producer.AsyncProducerTest > testPartitionAndCollateEvents PASSED

kafka.producer.AsyncProducerTest > testBatchSize STARTED

kafka.producer.AsyncProducerTest > testBatchSize PASSED

kafka.producer.AsyncProducerTest > testSerializeEvents STARTED

kafka.producer.AsyncProducerTest > testSerializeEvents PASSED

kafka.producer.AsyncProducerTest > testProducerQueueSize STARTED

kafka.producer.AsyncProducerTest > testProducerQueueSize PASSED

kafka.producer.AsyncProducerTest > testRandomPartitioner STARTED

kafka.producer.AsyncProducerTest > testRandomPartitioner PASSED

kafka.producer.AsyncProducerTest > testInvalidConfiguration STARTED

kafka.producer.AsyncProducerTest > testInvalidConfiguration PASSED

kafka.producer.AsyncProducerTest > testInvalidPartition STARTED

kafka.producer.AsyncProducerTest > testInvalidPartition PASSED

kafka.producer.AsyncProducerTest > testNoBroker STARTED

kafka.producer.AsyncProducerTest > testNoBroker PASSED

kafka.producer.AsyncProducerTest > testProduceAfterClosed STARTED

kafka.producer.AsyncProducerTest > testProduceAfterClosed PASSED

kafka.producer.AsyncProducerTest > testJavaProducer STARTED

kafka.producer.AsyncProducerTest > testJavaProducer PASSED

kafka.producer.AsyncProducerTest > testIncompatibleEncoder STARTED

kafka.producer.Asy

Re: [VOTE] KIP-219 - Improve Quota Communication

2018-01-08 Thread Becket Qin
Thanks for the comments, Jun.

1. Good point.
2. Also makes sense. Usually the connection.max.idle.ms is high enough so
the throttling is impacted.

I have updated the KIP to reflect the changes.

Thanks,

Jiangjie (Becket) Qin


On Mon, Jan 8, 2018 at 6:30 PM, Jun Rao  wrote:

> Hi, Jiangjie,
>
> Sorry for the late response. The proposal sounds good overall. A couple of
> minor comments below.
>
> 1. For throttling a fetch request, we could potentially just send an empty
> response. We can return a throttle time calculated from a full response,
> but only mute the channel on the server based on a throttle time calculated
> based on the empty response. This has the benefit that the server will mute
> the channel much shorter, which will prevent the consumer from rebalancing
> when throttled.
>
> 2. The wiki says "connections.max.idle.ms should be ignored during the
> throttle time X." This has the potential issue that a server may not detect
> that a client connection is already gone until after an arbitrary amount of
> time. Perhaps we could still just close a connection if the server has
> muted it for longer than connections.max.idle.ms. This will at least bound
> the time for a server to detect closed client connections.
>
> Thanks,
>
> Jun
>
>
> On Mon, Nov 20, 2017 at 5:30 PM, Becket Qin  wrote:
>
> > Hi,
> >
> > We would like to start the voting thread for KIP-219. The KIP proposes to
> > improve the quota communication between the brokers and clients,
> especially
> > for cases of long throttling time.
> >
> > The KIP wiki is following:
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 219+-+Improve+quota+
> > communication
> >
> > The discussion thread is here:
> > http://markmail.org/search/?q=kafka+KIP-219#query:kafka%
> > 20KIP-219+page:1+mid:ooxabguy7nz7l7zy+state:results
> >
> > Thanks,
> >
> > Jiangjie (Becket) Qin
> >
>


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

2018-01-08 Thread Apache Jenkins Server
See 


Changes:

[jason] KAFKA-6096: Add multi-threaded tests for group coordinator, txn manager

--
[...truncated 407.45 KB...]

kafka.utils.CoreUtilsTest > testReadInt STARTED

kafka.utils.CoreUtilsTest > testReadInt PASSED

kafka.utils.CoreUtilsTest > testAtomicGetOrUpdate STARTED

kafka.utils.CoreUtilsTest > testAtomicGetOrUpdate PASSED

kafka.utils.CoreUtilsTest > testUrlSafeBase64EncodeUUID STARTED

kafka.utils.CoreUtilsTest > testUrlSafeBase64EncodeUUID PASSED

kafka.utils.CoreUtilsTest > testCsvMap STARTED

kafka.utils.CoreUtilsTest > testCsvMap PASSED

kafka.utils.CoreUtilsTest > testInLock STARTED

kafka.utils.CoreUtilsTest > testInLock PASSED

kafka.utils.CoreUtilsTest > testTryAll STARTED

kafka.utils.CoreUtilsTest > testTryAll PASSED

kafka.utils.CoreUtilsTest > testSwallow STARTED

kafka.utils.CoreUtilsTest > testSwallow PASSED

kafka.utils.IteratorTemplateTest > testIterator STARTED

kafka.utils.IteratorTemplateTest > testIterator PASSED

kafka.utils.json.JsonValueTest > testJsonObjectIterator STARTED

kafka.utils.json.JsonValueTest > testJsonObjectIterator PASSED

kafka.utils.json.JsonValueTest > testDecodeLong STARTED

kafka.utils.json.JsonValueTest > testDecodeLong PASSED

kafka.utils.json.JsonValueTest > testAsJsonObject STARTED

kafka.utils.json.JsonValueTest > testAsJsonObject PASSED

kafka.utils.json.JsonValueTest > testDecodeDouble STARTED

kafka.utils.json.JsonValueTest > testDecodeDouble PASSED

kafka.utils.json.JsonValueTest > testDecodeOption STARTED

kafka.utils.json.JsonValueTest > testDecodeOption PASSED

kafka.utils.json.JsonValueTest > testDecodeString STARTED

kafka.utils.json.JsonValueTest > testDecodeString PASSED

kafka.utils.json.JsonValueTest > testJsonValueToString STARTED

kafka.utils.json.JsonValueTest > testJsonValueToString PASSED

kafka.utils.json.JsonValueTest > testAsJsonObjectOption STARTED

kafka.utils.json.JsonValueTest > testAsJsonObjectOption PASSED

kafka.utils.json.JsonValueTest > testAsJsonArrayOption STARTED

kafka.utils.json.JsonValueTest > testAsJsonArrayOption PASSED

kafka.utils.json.JsonValueTest > testAsJsonArray STARTED

kafka.utils.json.JsonValueTest > testAsJsonArray PASSED

kafka.utils.json.JsonValueTest > testJsonValueHashCode STARTED

kafka.utils.json.JsonValueTest > testJsonValueHashCode PASSED

kafka.utils.json.JsonValueTest > testDecodeInt STARTED

kafka.utils.json.JsonValueTest > testDecodeInt PASSED

kafka.utils.json.JsonValueTest > testDecodeMap STARTED

kafka.utils.json.JsonValueTest > testDecodeMap PASSED

kafka.utils.json.JsonValueTest > testDecodeSeq STARTED

kafka.utils.json.JsonValueTest > testDecodeSeq PASSED

kafka.utils.json.JsonValueTest > testJsonObjectGet STARTED

kafka.utils.json.JsonValueTest > testJsonObjectGet PASSED

kafka.utils.json.JsonValueTest > testJsonValueEquals STARTED

kafka.utils.json.JsonValueTest > testJsonValueEquals PASSED

kafka.utils.json.JsonValueTest > testJsonArrayIterator STARTED

kafka.utils.json.JsonValueTest > testJsonArrayIterator PASSED

kafka.utils.json.JsonValueTest > testJsonObjectApply STARTED

kafka.utils.json.JsonValueTest > testJsonObjectApply PASSED

kafka.utils.json.JsonValueTest > testDecodeBoolean STARTED

kafka.utils.json.JsonValueTest > testDecodeBoolean PASSED

kafka.producer.AsyncProducerTest > testFailedSendRetryLogic STARTED

kafka.producer.AsyncProducerTest > testFailedSendRetryLogic PASSED

kafka.producer.AsyncProducerTest > testQueueTimeExpired STARTED

kafka.producer.AsyncProducerTest > testQueueTimeExpired PASSED

kafka.producer.AsyncProducerTest > testPartitionAndCollateEvents STARTED

kafka.producer.AsyncProducerTest > testPartitionAndCollateEvents PASSED

kafka.producer.AsyncProducerTest > testBatchSize STARTED

kafka.producer.AsyncProducerTest > testBatchSize PASSED

kafka.producer.AsyncProducerTest > testSerializeEvents STARTED

kafka.producer.AsyncProducerTest > testSerializeEvents PASSED

kafka.producer.AsyncProducerTest > testProducerQueueSize STARTED

kafka.producer.AsyncProducerTest > testProducerQueueSize PASSED

kafka.producer.AsyncProducerTest > testRandomPartitioner STARTED

kafka.producer.AsyncProducerTest > testRandomPartitioner PASSED

kafka.producer.AsyncProducerTest > testInvalidConfiguration STARTED

kafka.producer.AsyncProducerTest > testInvalidConfiguration PASSED

kafka.producer.AsyncProducerTest > testInvalidPartition STARTED

kafka.producer.AsyncProducerTest > testInvalidPartition PASSED

kafka.producer.AsyncProducerTest > testNoBroker STARTED

kafka.producer.AsyncProducerTest > testNoBroker PASSED

kafka.producer.AsyncProducerTest > testProduceAfterClosed STARTED

kafka.producer.AsyncProducerTest > testProduceAfterClosed PASSED

kafka.producer.AsyncProducerTest > testJavaProducer STARTED

kafka.producer.AsyncProducerTest > testJavaProducer PASSED

kafka.producer.AsyncProducerTest > testIncompatibleEncoder STARTED

kafka.produ

Build failed in Jenkins: kafka-1.0-jdk7 #120

2018-01-08 Thread Apache Jenkins Server
See 


Changes:

[jason] KAFKA-6422 Mirror maker will throw null pointer exception when the

--
[...truncated 206.47 KB...]

kafka.api.SslEndToEndAuthorizationTest > 
testNoConsumeWithDescribeAclViaSubscribe PASSED

kafka.api.SslEndToEndAuthorizationTest > 
testNoConsumeWithoutDescribeAclViaAssign STARTED

kafka.api.SslEndToEndAuthorizationTest > 
testNoConsumeWithoutDescribeAclViaAssign PASSED

kafka.api.SslEndToEndAuthorizationTest > testNoGroupAcl STARTED

kafka.api.SslEndToEndAuthorizationTest > testNoGroupAcl PASSED

kafka.api.SslEndToEndAuthorizationTest > testNoProduceWithDescribeAcl STARTED

kafka.api.SslEndToEndAuthorizationTest > testNoProduceWithDescribeAcl PASSED

kafka.api.SslEndToEndAuthorizationTest > testProduceConsumeViaSubscribe STARTED

kafka.api.SslEndToEndAuthorizationTest > testProduceConsumeViaSubscribe PASSED

kafka.api.SslEndToEndAuthorizationTest > testNoProduceWithoutDescribeAcl STARTED

kafka.api.SslEndToEndAuthorizationTest > testNoProduceWithoutDescribeAcl PASSED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testTransactionalProducerWithAuthenticationFailure STARTED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testTransactionalProducerWithAuthenticationFailure PASSED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testManualAssignmentConsumerWithAuthenticationFailure STARTED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testManualAssignmentConsumerWithAuthenticationFailure PASSED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testProducerWithAuthenticationFailure STARTED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testProducerWithAuthenticationFailure PASSED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testConsumerGroupServiceWithAuthenticationFailure STARTED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testConsumerGroupServiceWithAuthenticationFailure PASSED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testManualAssignmentConsumerWithAutoCommitDisabledWithAuthenticationFailure 
STARTED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testManualAssignmentConsumerWithAutoCommitDisabledWithAuthenticationFailure 
PASSED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testKafkaAdminClientWithAuthenticationFailure STARTED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testKafkaAdminClientWithAuthenticationFailure PASSED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testConsumerWithAuthenticationFailure STARTED

kafka.api.SaslClientsWithInvalidCredentialsTest > 
testConsumerWithAuthenticationFailure PASSED

kafka.api.PlaintextProducerSendTest > 
testSendCompressedMessageWithLogAppendTime STARTED

kafka.api.PlaintextProducerSendTest > 
testSendCompressedMessageWithLogAppendTime PASSED

kafka.api.PlaintextProducerSendTest > testAutoCreateTopic STARTED

kafka.api.PlaintextProducerSendTest > testAutoCreateTopic PASSED

kafka.api.PlaintextProducerSendTest > testSendWithInvalidCreateTime STARTED

kafka.api.PlaintextProducerSendTest > testSendWithInvalidCreateTime PASSED

kafka.api.PlaintextProducerSendTest > testBatchSizeZero STARTED

kafka.api.PlaintextProducerSendTest > testBatchSizeZero PASSED

kafka.api.PlaintextProducerSendTest > testWrongSerializer STARTED

kafka.api.PlaintextProducerSendTest > testWrongSerializer PASSED

kafka.api.PlaintextProducerSendTest > 
testSendNonCompressedMessageWithLogAppendTime STARTED

kafka.api.PlaintextProducerSendTest > 
testSendNonCompressedMessageWithLogAppendTime PASSED

kafka.api.PlaintextProducerSendTest > 
testSendNonCompressedMessageWithCreateTime STARTED

kafka.api.PlaintextProducerSendTest > 
testSendNonCompressedMessageWithCreateTime PASSED

kafka.api.PlaintextProducerSendTest > testClose STARTED

kafka.api.PlaintextProducerSendTest > testClose PASSED

kafka.api.PlaintextProducerSendTest > testFlush STARTED

kafka.api.PlaintextProducerSendTest > testFlush PASSED

kafka.api.PlaintextProducerSendTest > testSendToPartition STARTED

kafka.api.PlaintextProducerSendTest > testSendToPartition PASSED

kafka.api.PlaintextProducerSendTest > testSendOffset STARTED

kafka.api.PlaintextProducerSendTest > testSendOffset PASSED

kafka.api.PlaintextProducerSendTest > testSendCompressedMessageWithCreateTime 
STARTED

kafka.api.PlaintextProducerSendTest > testSendCompressedMessageWithCreateTime 
PASSED

kafka.api.PlaintextProducerSendTest > testCloseWithZeroTimeoutFromCallerThread 
STARTED

kafka.api.PlaintextProducerSendTest > testCloseWithZeroTimeoutFromCallerThread 
PASSED

kafka.api.PlaintextProducerSendTest > testCloseWithZeroTimeoutFromSenderThread 
STARTED

kafka.api.PlaintextProducerSendTest > testCloseWithZeroTimeoutFromSenderThread 
PASSED

kafka.api.PlaintextProducerSendTest > testSendBeforeAndAfterPartitionExpansion 
STARTED

kafka.api.PlaintextProducerSendTest > testSendBeforeAndAfterPartitionExpansion 
PASSED

kafka.api.MetricsTest > testMetrics STARTED

Re: [DISCUSS] KIP-227: Introduce Incremental FetchRequests to Increase Partition Scalability

2018-01-08 Thread Andrey Falko
On Sat, Jan 6, 2018 at 5:57 PM, Colin McCabe  wrote:
> On Thu, Jan 4, 2018, at 10:37, Jun Rao wrote:
>>
>> 4. The process to mark partition as dirty requires updating every fetch
>> session having the partition. This may add some overhead. An alternative
>> approach is to check the difference btw cached fetch offset and HW (or LEO)
>> when serving the fetch request.
>
> That's a good point.  The caching approach avoids needing to update every 
> fetch session when one of those numbers changes.  I think an even more 
> important advantage is that it's simpler to implement -- we don't have to 
> worry about forgetting to update a fetch session when one of those numbers 
> changes.  The disadvantage is some extra memory consumption per partition per 
> fetch session.
>
> I think the advantage, especially in terms of simplicity, might override the 
> memory concern.  My initial implementation uses the caching approach.  I will 
> update the KIP once I have this working.
>

We're very interested in this KIP because it might improve one of our
topic-heavy clusters. I have a stress-test generating topics across a
number of kafka brokers; if you'd like early and quick feedback on
your implementation let me know!

The discussion thread is very long, so hopefully I'm not asking
something that was asked before: does Kafka already expose
`FetchRequest` size for monitoring purposes? It might improve the KIP
to track the before-and-after behavior.

-Andrey