Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-03 Thread David Jacot
Hi Colin,

Thanks for your input. Please, find my comments below:

>> Currently, we don't parse the contents of ApiVersionsRequest at all,
since it's an empty message.  KIP-511 proposes adding some fields here,
which will clearly change that situation.  In the future, any changes to
ApiVersionsRequest will have to only add stuff at the end, rather than
changing fields that already exist.  This will significantly limit our
ability to add new things over time.
>> Therefore, I think we should make sure that KIP-511 is implemented after
KIP-482, so that the version we freeze into place can be one that includes
the ability to add tagged fields, and includes the more efficient string
and array serialization specified in KIP-482.  It's probably worth spelling
that out here.

I agree with you. It makes sense to bump the version once for the two
cases.

>> On another topic, when the client sends an unsupported version N of
ApiVersionsRequest, and the broker only supports versions up to M, the
broker currently falls back to sending a version 0 response.  Why can't the
broker fall back to version M instead?  Previously, we only had versions 1
and 0 of ApiVersionsRequest, so the two behaviors were identical.  But
going forward, once we have version 2 and later of ApiVersoinsRequest, it
doesn't make sense to fall back all the way to 0 if the broker supports 1
(for example).
>> If you agree, it would be good to spell this out in the KIP, so that if
we want to add more things to the response, we can, without losing them
each time the client's version of ApiVersionsRequest exceeds the broker's.

I fully agree with you and I have already outlined this in the proposal,
see "ApiVersions Request/Response Handling". The idea is to use the version
M in the broker so it can leverage the provided information it knows about.
The broker can then send back response with version M as well. On the
client side, it is a bit more tricky. As there is no version in the
response, the client will use the version used for the request to parse the
response. If fields have been added to the schema of the response version
N, it won't work to parse M. As the client doesn't know the version used,
we have two options: 1) use version 0 which is a prefix of all others but
it means losing information; or 2) try versions in descending order from N
to 0. I guess that N-1 would the one in most of the cases.

What is your opinion regarding the client side?

Best,
David

On Wed, Sep 4, 2019 at 12:13 AM Colin McCabe  wrote:

> Hi David,
>
> Thanks again for the KIP.
>
> Currently, we don't parse the contents of ApiVersionsRequest at all, since
> it's an empty message.  KIP-511 proposes adding some fields here, which
> will clearly change that situation.  In the future, any changes to
> ApiVersionsRequest will have to only add stuff at the end, rather than
> changing fields that already exist.  This will significantly limit our
> ability to add new things over time.
>
> Therefore, I think we should make sure that KIP-511 is implemented after
> KIP-482, so that the version we freeze into place can be one that includes
> the ability to add tagged fields, and includes the more efficient string
> and array serialization specified in KIP-482.  It's probably worth spelling
> that out here.
>
> On another topic, when the client sends an unsupported version N of
> ApiVersionsRequest, and the broker only supports versions up to M, the
> broker currently falls back to sending a version 0 response.  Why can't the
> broker fall back to version M instead?  Previously, we only had versions 1
> and 0 of ApiVersionsRequest, so the two behaviors were identical.  But
> going forward, once we have version 2 and later of ApiVersoinsRequest, it
> doesn't make sense to fall back all the way to 0 if the broker supports 1
> (for example).
>
> If you agree, it would be good to spell this out in the KIP, so that if we
> want to add more things to the response, we can, without losing them each
> time the client's version of ApiVersionsRequest exceeds the broker's.
>
> best,
> Colin
>
>
> On Tue, Sep 3, 2019, at 01:26, David Jacot wrote:
> > Hi all,
> >
> > I have updated the KIP to address the various comments. I have also
> added a
> > section about the handling of the ApiVersionsRequest/Response in the
> broker.
> >
> > Please, let me know what you think. I would like to make it for the next
> > release if possible.
> >
> > Best,
> > David
> >
> > On Fri, Aug 30, 2019 at 10:31 AM David Jacot 
> wrote:
> >
> > > Hi Magnus,
> > >
> > > Thank you for your feedback. Please, find my comments below.
> > >
> > > 1. I thought that the clientId was meant for this purpose (providing
> > > information about the application). Is there a gap I don't see?
> > >
> > > 2. I have put two fields to avoid requiring deep validation and
> parsing on
> > > the broker. I believe that it will be easier to use the metadata
> downstream
> > > like this.
> > >
> > > 3. Good point. I think that the broker shou

Re: [VOTE] KIP-512:Adding headers to RecordMetaData

2019-09-03 Thread Maulin Vasavada
+1 (non-binding)

On Tue, Sep 3, 2019 at 3:38 PM Renuka M  wrote:

> Hi All,
>
> After good discussion for KIP-512
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-512%3AAdding+headers+to+RecordMetaData
> ,
> am starting thread for voting.
>
> Thanks
> Renuka M
>


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

2019-09-03 Thread Apache Jenkins Server
See 


Changes:

[rhauch] Changed for updatedTasks, avoids stopping and starting of unnecessary

[rhauch] MINOR: Add unit test for KAFKA-8676 to guard against unrequired task

--
[...truncated 1.98 MB...]

org.apache.kafka.streams.integration.MetricsIntegrationTest > 
shouldAddMetricsForWindowStore PASSED

org.apache.kafka.streams.integration.MetricsIntegrationTest > 
shouldAddMetricsForSessionStore STARTED

org.apache.kafka.streams.integration.MetricsIntegrationTest > 
shouldAddMetricsForSessionStore PASSED

org.apache.kafka.streams.integration.MetricsIntegrationTest > 
shouldNotAddRocksDBMetricsIfRecordingLevelIsInfo STARTED

org.apache.kafka.streams.integration.MetricsIntegrationTest > 
shouldNotAddRocksDBMetricsIfRecordingLevelIsInfo PASSED

org.apache.kafka.streams.integration.RepartitionOptimizingIntegrationTest > 
shouldSendCorrectRecords_OPTIMIZED STARTED

org.apache.kafka.streams.integration.RepartitionOptimizingIntegrationTest > 
shouldSendCorrectRecords_OPTIMIZED PASSED

org.apache.kafka.streams.integration.RepartitionOptimizingIntegrationTest > 
shouldSendCorrectResults_NO_OPTIMIZATION STARTED

org.apache.kafka.streams.integration.RepartitionOptimizingIntegrationTest > 
shouldSendCorrectResults_NO_OPTIMIZATION PASSED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldBeAbleToQueryMapValuesState STARTED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldBeAbleToQueryMapValuesState PASSED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldNotMakeStoreAvailableUntilAllStoresAvailable STARTED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldNotMakeStoreAvailableUntilAllStoresAvailable PASSED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
queryOnRebalance STARTED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
queryOnRebalance PASSED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
concurrentAccesses STARTED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
concurrentAccesses PASSED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldAllowToQueryAfterThreadDied STARTED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldAllowToQueryAfterThreadDied PASSED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldBeAbleToQueryStateWithZeroSizedCache STARTED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldBeAbleToQueryStateWithZeroSizedCache PASSED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldBeAbleToQueryMapValuesAfterFilterState STARTED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldBeAbleToQueryMapValuesAfterFilterState PASSED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldBeAbleToQueryFilterState STARTED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldBeAbleToQueryFilterState PASSED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldBeAbleToQueryStateWithNonZeroSizedCache STARTED

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > 
shouldBeAbleToQueryStateWithNonZeroSizedCache PASSED

org.apache.kafka.streams.integration.GlobalThreadShutDownOrderTest > 
shouldFinishGlobalStoreOperationOnShutDown STARTED

org.apache.kafka.streams.integration.GlobalThreadShutDownOrderTest > 
shouldFinishGlobalStoreOperationOnShutDown PASSED

org.apache.kafka.streams.integration.InternalTopicIntegrationTest > 
shouldCompactTopicsForKeyValueStoreChangelogs STARTED

org.apache.kafka.streams.integration.InternalTopicIntegrationTest > 
shouldCompactTopicsForKeyValueStoreChangelogs PASSED

org.apache.kafka.streams.integration.InternalTopicIntegrationTest > 
shouldCompactAndDeleteTopicsForWindowStoreChangelogs STARTED

org.apache.kafka.streams.integration.InternalTopicIntegrationTest > 
shouldCompactAndDeleteTopicsForWindowStoreChangelogs PASSED

org.apache.kafka.streams.integration.KStreamAggregationDedupIntegrationTest > 
shouldReduce STARTED

org.apache.kafka.streams.integration.KStreamAggregationDedupIntegrationTest > 
shouldReduce PASSED

org.apache.kafka.streams.integration.KStreamAggregationDedupIntegrationTest > 
shouldGroupByKey STARTED

org.apache.kafka.streams.integration.KStreamAggregationDedupIntegrationTest > 
shouldGroupByKey PASSED

org.apache.kafka.streams.integration.KStreamAggregationDedupIntegrationTest > 
shouldReduceWindowed STARTED

org.apache.kafka.streams.integration.KStreamAggregationDedupIntegrationTest > 
shouldReduceWindowed PASSED

org.apache.kafka.streams.integration.StateRestorationIntegrationTest > 
shouldRestoreNullRecord STARTED
ERROR: Could not install GRADLE_4_10_3_HOME
java.lang.NullPointerException

org.apache.kafk

Re: [DISCUSS] KIP-486 Support for pluggable KeyStore and TrustStore

2019-09-03 Thread Maulin Vasavada
Hi all

Please check
https://github.com/maulin-vasavada/kafka/commit/44f86395b1ba3fe4bd87de89029d72da77995ff8


This is just the first cut obviously. There are few call outs I would like
to make,

1. So far I kept the old SslEngineBuilder hence I had to name the interface
with "I" (that can change later)

2. I did not yet add the creation of SslEngineBuilder via loading the
configuration like 'ssl.engine.builder.class'. Hence you see direct
creation of DefaultSslEngineBuilder class

3. Due to validation logic in the current SslFactory I had to add more
methods in ISslEngineBuilder interface (like keystore(), truststore() etc).
Due to other classes like EchoServer depending upon needing SSLContext, I
had to add getSSLContext() also in the interface.

4. With these changes and with existing old SslEngineBuilder, the
clients/core projects builds with tests successfully but I didn't add any
additional tests yet

5. I wanted to have DefaultSslEngineBuilder in such a way that if somebody
wants to implement custom SslEngineBuilder they can extend and override
only key required methods without repeating any logic.

6. For reconfigurable interface I kept the way suggested by Rajini -
meaning SslFactory really is reconfigurable BUT it relies on the
ISslEngineBuilder to define the reconfigurable options. This means that
ISslEngineBuilder dictates based on which reconfigurable params the
SslFactory should try to reconfigure the SSLEngine.

With this - open to all the suggestions and further improvements.

Thanks
Maulin


On Tue, Sep 3, 2019 at 10:07 AM Colin McCabe  wrote:

> On Mon, Sep 2, 2019, at 03:33, Rajini Sivaram wrote:
> > I would expect SslEngineBuilder interface to look something like this,
> > perhaps with some tweaking:
> >
> > public interface SslEngineBuilder extends Configurable, Closeable {
> >
> > Set reconfigurableConfigs();
> >
> > boolean shouldBeRebuilt(Map nextConfigs);
> >
> > SSLEngine createSslEngine(Mode mode, String peerHost, int
> > peerPort, String endpointIdentification);
> >
> > }
> >
> > The existing SslEngineBuilder class would be renamed and will implement
> > this interface. Loading of keystore/truststore will be in
> SslEngineBuilder
> > as it is now.  The method `shouldBeRebuilt()` will validate configs
> during
> > reconfiguration and decide if reconfiguration is required because
> keystore
> > or truststore changed. SslFactory.reconfigurableConfigs() will return
> > SslEngineBuilder.reconfigurableConfigs() as well including any custom
> > configs of SslEngineBuilder, so no other changes will be required when we
> > eventually support custom SSL configs.
> >
> > We don't want to make SslFactory the pluggable class since that contains
> > validation logic for SSL engines. Everything that we want to customise is
> > contained in SslEngineBuilder. Basically custom SslEngineBuilder will
> > validate custom configs during reconfiguration and create SSLEngine.
> > SslFactory will continue to perform validation of SSLEngines and this
> will
> > not be customizable. SslEngineBuilder will not be reconfigurable, instead
> > we create a new builder as we do now to avoid having to deal with
> > thread-safety and atomicity of updates. We could consider using a public
> > Reconfigurable interface as the pluggable interface for consistency, but
> I
> > think we would still want to create a new Builder on reconfiguration and
> > retain non-pluggable SSL engine validation in SslFactory.
>
> +1
>
> C.
>
> >
> >
> > On Fri, Aug 30, 2019 at 10:21 PM Maulin Vasavada <
> maulin.vasav...@gmail.com>
> > wrote:
> >
> > > Looking at SslFactory and SslEngineBuilder I feel the responsibilities
> are
> > > not clear. Both has public method for createSSLEngine for example. I
> feel
> > > the SslEngineBuilder was created to move out lot of code but it is not
> > > necessarily a public class (e.g. I don't think anybody calling
> > > SslEngineBuilder separately without SslFactory in between). I am
> currently
> > > inclined toward what Celement is suggesting - having pluggable
> SslFactory.
> > >
> > > Let me do this - let me refactor SslFactory and SslEngineBuilder and
> review
> > > what I can come up with you guys. Let us see if we can address all the
> > > objections raised previously for KIP-383's iterations. I'll need
> sometime
> > > though. Let me try to do it by next of next week.
> > >
> > > Thanks
> > > Maulin
> > >
> > > On Fri, Aug 30, 2019 at 12:25 PM Pellerin, Clement <
> > > clement_pelle...@ibi.com>
> > > wrote:
> > >
> > > > What is your solution to the objection that killed the second
> iteration
> > > of
> > > > KIP-383?
> > > > Mainly, how do you support validation of reconfiguration requests
> that
> > > > involve new custom properties implemented by the pluggable factory?
> > > > Custom properties do not exist yet, but that is very legitimate
> thing to
> > > > design for the future.
> > > >
> > > > That's why I favor a pluggable SslFactory instead of an
> SslEngineBuilder
> > 

Build failed in Jenkins: kafka-2.3-jdk8 #99

2019-09-03 Thread Apache Jenkins Server
See 


Changes:

[rhauch] Changed for updatedTasks, avoids stopping and starting of unnecessary

[rhauch] MINOR: Add unit test for KAFKA-8676 to guard against unrequired task

--
[...truncated 2.92 MB...]
kafka.api.GroupAuthorizerIntegrationTest > testCommitWithTopicDescribe STARTED

kafka.api.GroupAuthorizerIntegrationTest > testCommitWithTopicDescribe PASSED

kafka.api.GroupAuthorizerIntegrationTest > testAuthorizationWithTopicExisting 
STARTED

kafka.api.GroupAuthorizerIntegrationTest > testAuthorizationWithTopicExisting 
PASSED

kafka.api.GroupAuthorizerIntegrationTest > 
testUnauthorizedDeleteRecordsWithoutDescribe STARTED

kafka.api.GroupAuthorizerIntegrationTest > 
testUnauthorizedDeleteRecordsWithoutDescribe PASSED

kafka.api.GroupAuthorizerIntegrationTest > testMetadataWithTopicDescribe STARTED

kafka.api.GroupAuthorizerIntegrationTest > testMetadataWithTopicDescribe PASSED

kafka.api.GroupAuthorizerIntegrationTest > testProduceWithTopicDescribe STARTED

kafka.api.GroupAuthorizerIntegrationTest > testProduceWithTopicDescribe PASSED

kafka.api.GroupAuthorizerIntegrationTest > testDescribeGroupApiWithNoGroupAcl 
STARTED

kafka.api.GroupAuthorizerIntegrationTest > testDescribeGroupApiWithNoGroupAcl 
PASSED

kafka.api.GroupAuthorizerIntegrationTest > 
testPatternSubscriptionMatchingInternalTopic STARTED

kafka.api.GroupAuthorizerIntegrationTest > 
testPatternSubscriptionMatchingInternalTopic PASSED

kafka.api.GroupAuthorizerIntegrationTest > 
testSendOffsetsWithNoConsumerGroupDescribeAccess STARTED

kafka.api.GroupAuthorizerIntegrationTest > 
testSendOffsetsWithNoConsumerGroupDescribeAccess PASSED

kafka.api.GroupAuthorizerIntegrationTest > testOffsetFetchTopicDescribe STARTED

kafka.api.GroupAuthorizerIntegrationTest > testOffsetFetchTopicDescribe PASSED

kafka.api.GroupAuthorizerIntegrationTest > testCommitWithTopicAndGroupRead 
STARTED

kafka.api.GroupAuthorizerIntegrationTest > testCommitWithTopicAndGroupRead 
PASSED

kafka.api.GroupAuthorizerIntegrationTest > 
testIdempotentProducerNoIdempotentWriteAclInInitProducerId STARTED

kafka.api.GroupAuthorizerIntegrationTest > 
testIdempotentProducerNoIdempotentWriteAclInInitProducerId PASSED

kafka.api.GroupAuthorizerIntegrationTest > 
testSimpleConsumeWithExplicitSeekAndNoGroupAccess STARTED

kafka.api.GroupAuthorizerIntegrationTest > 
testSimpleConsumeWithExplicitSeekAndNoGroupAccess PASSED

kafka.api.SaslPlaintextConsumerTest > testCoordinatorFailover STARTED

kafka.api.SaslPlaintextConsumerTest > testCoordinatorFailover PASSED

kafka.api.SaslPlaintextConsumerTest > testSimpleConsumption STARTED

kafka.api.SaslPlaintextConsumerTest > testSimpleConsumption PASSED

kafka.api.TransactionsTest > testBasicTransactions STARTED

kafka.api.TransactionsTest > testBasicTransactions PASSED

kafka.api.TransactionsTest > testFencingOnSendOffsets STARTED

kafka.api.TransactionsTest > testFencingOnSendOffsets PASSED

kafka.api.TransactionsTest > testFencingOnAddPartitions STARTED

kafka.api.TransactionsTest > testFencingOnAddPartitions PASSED

kafka.api.TransactionsTest > testFencingOnTransactionExpiration STARTED

kafka.api.TransactionsTest > testFencingOnTransactionExpiration PASSED

kafka.api.TransactionsTest > testDelayedFetchIncludesAbortedTransaction STARTED

kafka.api.TransactionsTest > testDelayedFetchIncludesAbortedTransaction PASSED

kafka.api.TransactionsTest > testOffsetMetadataInSendOffsetsToTransaction 
STARTED

kafka.api.TransactionsTest > testOffsetMetadataInSendOffsetsToTransaction PASSED

kafka.api.TransactionsTest > testConsecutivelyRunInitTransactions STARTED

kafka.api.TransactionsTest > testConsecutivelyRunInitTransactions PASSED

kafka.api.TransactionsTest > testReadCommittedConsumerShouldNotSeeUndecidedData 
STARTED

kafka.api.TransactionsTest > testReadCommittedConsumerShouldNotSeeUndecidedData 
PASSED

kafka.api.TransactionsTest > testFencingOnSend STARTED

kafka.api.TransactionsTest > testFencingOnSend PASSED

kafka.api.TransactionsTest > testFencingOnCommit STARTED

kafka.api.TransactionsTest > testFencingOnCommit PASSED

kafka.api.TransactionsTest > testMultipleMarkersOneLeader STARTED

kafka.api.TransactionsTest > testMultipleMarkersOneLeader PASSED

kafka.api.TransactionsTest > testCommitTransactionTimeout STARTED

kafka.api.TransactionsTest > testCommitTransactionTimeout PASSED

kafka.api.TransactionsTest > testSendOffsets STARTED

kafka.api.TransactionsTest > testSendOffsets PASSED

kafka.api.DescribeAuthorizedOperationsTest > testClusterAuthorizedOperations 
STARTED

kafka.api.DescribeAuthorizedOperationsTest > testClusterAuthorizedOperations 
PASSED

kafka.api.DescribeAuthorizedOperationsTest > testTopicAuthorizedOperations 
STARTED

kafka.api.DescribeAuthorizedOperationsTest > testTopicAuthorizedOperations 
PASSED

kafka.api.DescribeAuthorizedOperationsTest > 
testConsumerGroupAuthorizedOperations STARTED

kafka.

Re: [VOTE] KIP-401: TransformerSupplier/ProcessorSupplier StateStore connecting

2019-09-03 Thread Guozhang Wang
Hi Paul, thanks for the confirmation!

Since we have three binding votes now I think you can proceed and mark it
as accepted.

On Tue, Sep 3, 2019 at 3:17 PM Paul Whalen  wrote:

> Yeah, agreed on it being the same reference. That’s the way I have it in
> the working PR and I’ll update the KIP for clarity.
>
> > On Sep 3, 2019, at 5:04 PM, Matthias J. Sax 
> wrote:
> >
> > I am strongly in favor of "must be the same reference".
> >
> >
> > -Matthias
> >
> >> On 9/3/19 2:09 PM, Guozhang Wang wrote:
> >> Hi Paul,
> >>
> >> Thanks for the KIP! +1 (binding).
> >>
> >> One minor comment about the following:
> >>
> >> "In order to solve the problem of addStateStore potentially being called
> >> twice for the same store (because more than one Supplier specifies it),
> the
> >> check for duplicate stores in addStateStores will be relaxed to *allow
> for
> >> duplicates if the same StoreBuilder instance for the same store name*."
> >>
> >> It worth clarifying how should we check if the StoreBuilder instances
> are
> >> the same: either 1) equality by reference or 2) equality based on e.g.
> >> #equals override function so that two different instances may still be
> >> considered "equal". I think you meant 1), just wanted to confirm :)
> >>
> >>
> >> Guozhang
> >>
> >>> On Thu, Aug 29, 2019 at 3:14 PM Paul Whalen 
> wrote:
> >>>
> >>> Thanks for the votes all! With two binding votes we’re in need of one
> more
> >>> for the KIP to be accepted. With the 2.4 release coming in September,
> it
> >>> would be great to get another committer to take a look soon so I could
> set
> >>> aside some time to get implementation/documentation done to make it
> into
> >>> the release.
> >>>
> >>> Thanks,
> >>> Paul
> >>>
>  On Aug 20, 2019, at 5:47 PM, Bill Bejeck  wrote:
> 
>  Thanks for the KIP.
> 
>  +1 (binding)
> 
>  On Tue, Aug 20, 2019 at 6:28 PM Matthias J. Sax <
> matth...@confluent.io>
>  wrote:
> 
> > +1 (binding)
> >
> >
> >> On 6/17/19 2:32 PM, John Roesler wrote:
> >> I'm +1 (nonbinding) on the current iteration of the proposal.
> >>
> >>> On Mon, May 27, 2019 at 1:58 PM Paul Whalen 
> >>> wrote:
> >>>
> >>> I spoke too early a month ago, but I believe the proposal is
> finalized
> > now
> >>> and ready for voting.
> >>>
> >>> KIP:
> >>>
> >
> >>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97553756
> >>>
> >>> Discussion:
> >>>
> >
> >>>
> https://lists.apache.org/thread.html/600996d83d485f2b8daf45037de64a60cebdfac9b234bf3449b6b753@%3Cdev.kafka.apache.org%3E
> >>>
> >>> Pull request (still a WIP, obviously):
> >>> https://github.com/apache/kafka/pull/6824
> >>>
> >>> Thanks,
> >>> Paul
> >>>
>  On Wed, Apr 24, 2019 at 8:00 PM Paul Whalen 
> >>> wrote:
> 
>  Hi all,
> 
>  After some good discussion on and adjustments to KIP-401 (which I
> > renamed
>  slightly for clarity), chatter has died down so I figured I may as
> >>> well
>  start a vote.
> 
>  KIP:
>  TransformerSupplier/ProcessorSupplier StateStore connecting
>  <
> >
> >>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97553756>
>  Discussion:
> 
> 
> >
> >>>
> https://lists.apache.org/thread.html/600996d83d485f2b8daf45037de64a60cebdfac9b234bf3449b6b753@%3Cdev.kafka.apache.org%3E
> 
>  Thanks!
>  Paul
> 
> >
> >
> >>>
> >>
> >>
> >
>


-- 
-- Guozhang


[jira] [Created] (KAFKA-8864) Kafka Producer deadlocked on flush call

2019-09-03 Thread Shaan Appel (Jira)
Shaan Appel created KAFKA-8864:
--

 Summary: Kafka Producer deadlocked on flush call
 Key: KAFKA-8864
 URL: https://issues.apache.org/jira/browse/KAFKA-8864
 Project: Kafka
  Issue Type: Bug
  Components: clients, producer 
Affects Versions: 2.1.0
Reporter: Shaan Appel


Some times the {{producer.flush}} call will be blocked by some lock. This may 
have been caused during a brief network outage.
"controlPort-19" #159 prio=6 os_prio=-1 tid=0x7f8db0022800 nid=0xac waiting 
on condition [0x7f8cb67e9000]
   java.lang.Thread.State: WAITING (parking)
at sun.misc.Unsafe.park(Native Method)
- parking to wait for  <0x7f9f01812880> (a 
java.util.concurrent.CountDownLatch$Sync)
at java.util.concurrent.locks.LockSupport.park(LockSupport.java:175)
at 
java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:836)
at 
java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireSharedInterruptibly(AbstractQueuedSynchronizer.java:997)
at 
java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1304)
at java.util.concurrent.CountDownLatch.await(CountDownLatch.java:231)
at 
org.apache.kafka.clients.producer.internals.ProduceRequestResult.await(ProduceRequestResult.java:76)
at 
org.apache.kafka.clients.producer.internals.RecordAccumulator.awaitFlushCompletion(RecordAccumulator.java:693)
at 
org.apache.kafka.clients.producer.KafkaProducer.flush(KafkaProducer.java:1062)



--
This message was sent by Atlassian Jira
(v8.3.2#803003)


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

2019-09-03 Thread Apache Jenkins Server
See 


Changes:

[bill] MINOR: Use new `Admin` interface instead of `KafkaAdminClient` class

--
[...truncated 5.99 MB...]
org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testRestartConnectorLeaderRedirect STARTED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testRestartConnectorLeaderRedirect PASSED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testRestartConnectorOwnerRedirect STARTED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testRestartConnectorOwnerRedirect PASSED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testRestartTaskNotFound STARTED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testRestartTaskNotFound PASSED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testRestartTaskLeaderRedirect STARTED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testRestartTaskLeaderRedirect PASSED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testRestartTaskOwnerRedirect STARTED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testRestartTaskOwnerRedirect PASSED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testListConnectors STARTED

org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest > 
testListConnectors PASSED

org.apache.kafka.connect.runtime.rest.resources.RootResourceTest > testRootGet 
STARTED

org.apache.kafka.connect.runtime.rest.resources.RootResourceTest > testRootGet 
PASSED

org.apache.kafka.connect.runtime.isolation.DelegatingClassLoaderTest > 
testWhiteListedManifestResources STARTED

org.apache.kafka.connect.runtime.isolation.DelegatingClassLoaderTest > 
testWhiteListedManifestResources PASSED

org.apache.kafka.connect.runtime.isolation.DelegatingClassLoaderTest > 
testOtherResources STARTED

org.apache.kafka.connect.runtime.isolation.DelegatingClassLoaderTest > 
testOtherResources PASSED

org.apache.kafka.connect.runtime.isolation.PluginDescTest > 
testPluginDescWithNullVersion STARTED

org.apache.kafka.connect.runtime.isolation.PluginDescTest > 
testPluginDescWithNullVersion PASSED

org.apache.kafka.connect.runtime.isolation.PluginDescTest > 
testPluginDescComparison STARTED

org.apache.kafka.connect.runtime.isolation.PluginDescTest > 
testPluginDescComparison PASSED

org.apache.kafka.connect.runtime.isolation.PluginDescTest > 
testRegularPluginDesc STARTED

org.apache.kafka.connect.runtime.isolation.PluginDescTest > 
testRegularPluginDesc PASSED

org.apache.kafka.connect.runtime.isolation.PluginDescTest > 
testPluginDescEquality STARTED

org.apache.kafka.connect.runtime.isolation.PluginDescTest > 
testPluginDescEquality PASSED

org.apache.kafka.connect.runtime.isolation.PluginDescTest > 
testPluginDescWithSystemClassLoader STARTED

org.apache.kafka.connect.runtime.isolation.PluginDescTest > 
testPluginDescWithSystemClassLoader PASSED

org.apache.kafka.connect.runtime.isolation.PluginsTest > 
shouldInstantiateAndConfigureConnectRestExtension STARTED

org.apache.kafka.connect.runtime.isolation.PluginsTest > 
shouldInstantiateAndConfigureConnectRestExtension PASSED

org.apache.kafka.connect.runtime.isolation.PluginsTest > 
shouldInstantiateAndConfigureConverters STARTED

org.apache.kafka.connect.runtime.isolation.PluginsTest > 
shouldInstantiateAndConfigureConverters PASSED

org.apache.kafka.connect.runtime.isolation.PluginsTest > 
shouldInstantiateAndConfigureInternalConverters STARTED

org.apache.kafka.connect.runtime.isolation.PluginsTest > 
shouldInstantiateAndConfigureInternalConverters PASSED

org.apache.kafka.connect.runtime.isolation.PluginsTest > 
shouldInstantiateAndConfigureDefaultHeaderConverter STARTED

org.apache.kafka.connect.runtime.isolation.PluginsTest > 
shouldInstantiateAndConfigureDefaultHeaderConverter PASSED

org.apache.kafka.connect.runtime.isolation.PluginsTest > 
shouldInstantiateAndConfigureExplicitlySetHeaderConverterWithCurrentClassLoader 
STARTED

org.apache.kafka.connect.runtime.isolation.PluginsTest > 
shouldInstantiateAndConfigureExplicitlySetHeaderConverterWithCurrentClassLoader 
PASSED

org.apache.kafka.connect.runtime.isolation.PluginUtilsTest > 
testPluginUrlsWithRelativeSymlinkForwards STARTED

org.apache.kafka.connect.runtime.isolation.PluginUtilsTest > 
testPluginUrlsWithRelativeSymlinkForwards PASSED

org.apache.kafka.connect.runtime.isolation.PluginUtilsTest > 
testOrderOfPluginUrlsWithJars STARTED

org.apache.kafka.connect.runtime.isolation.PluginUtilsTest > 
testOrderOfPluginUrlsWithJars PASSED

org.apache.kafka.connect.runtime.isolation.PluginUtilsTest > 
testPluginUrlsWithClasses STARTED

org.apache.kafka.connect.runtime.isolation.PluginUtilsTest > 
testPluginUrlsWithClasses PASSED

org.apache.kafka.connect.

Re: Request for permission to create KIP

2019-09-03 Thread Bill Bejeck
Lucas,

You're all set now.

-Bill

On Tue, Sep 3, 2019 at 7:00 PM Lucas Bradstreet  wrote:

> Hi,
>
> Could I please be given permission to add a KIP to
>
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals
> ?
> My username is lucasbradstreet.
>
> Thanks
>


Request for permission to create KIP

2019-09-03 Thread Lucas Bradstreet
Hi,

Could I please be given permission to add a KIP to
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals?
My username is lucasbradstreet.

Thanks


Re: [DISCUSS] KIP-504 - Add new Java Authorizer Interface

2019-09-03 Thread Don Bosco Durai
Hi Rajini

Help me understand this a bit more.

1. For all practical purpose, without authorization you can't go to the next 
step. The calling code needs to block anyway. So should we just let the 
implementation code do the async part?
2. If you feel management calls need to be async, then we should consider 
another set of async APIs. I don't feel we should complicate it further ( 
3. Another concern I have is wrt performance. Kafka has been built to handle 
1000s per second requests. Not sure whether making it async will add any 
unnecessary overhead.
4. How much complication would this add on the calling side? And is it worth it?

Thanks

Bosco


On 9/3/19, 8:50 AM, "Rajini Sivaram"  wrote:

Hi all,

Ismael brought up a point that it will be good to make the Authorizer
interface asynchronous to avoid blocking request threads during remote
operations.

1) Since we want to support different backends for authorization metadata,
making createAcls() and deleteAcls() asynchronous makes sense since these
always involve remote operations. When KIP-500 removes ZooKeeper, we would
want to move ACLs to Kafka and async updates will avoid unnecessary
blocking.
2) For authorize() method, we currently use cached ACLs in the built-in
authorizers, so synchronous authorise operations work well now. But async
authorize() would support this scenario as well as authorizers in large
organisations where an LRU cache would enable a smaller cache even when the
backend holds a large amount of ACLs for infrequently used resources or
users who don't use the system frequently.

For both cases, the built-in authorizer will continue to be synchronous,
using CompletableFuture.completedFuture() to return the actual result. But
async API will make custom authorizer implementations more flexible. I
would like to know if there are any concerns with these changes before
updating the KIP.

*Proposed API:*
public interface Authorizer extends Configurable, Closeable {

Map> start(AuthorizerServerInfo 
serverInfo);
List>
authorize(AuthorizableRequestContext requestContext, List
actions);
List>
createAcls(AuthorizableRequestContext requestContext, List
aclBindings);
List>
deleteAcls(AuthorizableRequestContext requestContext,
List aclBindingFilters);
CompletionStage> acls(AclBindingFilter filter);
}


Thank you,

Rajini

On Sun, Aug 18, 2019 at 6:25 PM Don Bosco Durai  wrote:

> Hi Rajini
>
> Thanks for clarifying. I am good for now.
>
> Regards
>
> Bosco
>
>
> On 8/16/19, 11:30 AM, "Rajini Sivaram"  wrote:
>
> Hi Don,
>
> That should be fine. I guess Ranger loads policies from the database
> synchronously when the authorizer is configured, similar to
> SimpleAclAuthorizer loading from ZooKeeper. Ranger can continue to 
load
> synchronously from `configure()` or `start()` and return an empty map
> from
> `start()`. That would retain the existing behaviour.. When the same
> database stores policies for all listeners and the policies are not
> stored
> in Kafka, there is no value in making the load asynchronous.
>
> Regards,
>
> Rajini
>
>
> On Fri, Aug 16, 2019 at 6:43 PM Don Bosco Durai 
> wrote:
>
> > Hi Rajini
> >
> > Assuming this doesn't affect custom plugins, I don't have any
> concerns
> > regarding this.
> >
> > I do have one question regarding:
> >
> > "For authorizers that don’t store metadata in ZooKeeper, ensure that
> > authorizer metadata for each listener is available before starting
> up the
> > listener. This enables different authorization metadata stores for
> > different listeners."
> >
> > Since Ranger uses its own database for storing policies, do you
> anticipate
> > any issues?
> >
> > Thanks
> >
> > Bosco
> >
> >
> > On 8/16/19, 6:49 AM, "Rajini Sivaram" 
> wrote:
> >
> > Hi all,
> >
> > I made another change to the KIP. The KIP was originally
> proposing to
> > extend SimpleAclAuthorizer to also implement the new API (in
> addition
> > to
> > the existing API). But since we use the new API when available,
> this
> > can
> > break custom authorizers that extend this class and override
> specific
> > methods of the old API. To avoid breaking any existing custom
> > implementations that extend this class, particularly because it
> is in
> > the
> > public package kafka.security.auth, the KIP now proposes to
> ret

Jenkins build is back to normal : kafka-trunk-jdk11 #789

2019-09-03 Thread Apache Jenkins Server
See 




[VOTE] KIP-512:Adding headers to RecordMetaData

2019-09-03 Thread Renuka M
Hi All,

After good discussion for KIP-512
https://cwiki.apache.org/confluence/display/KAFKA/KIP-512%3AAdding+headers+to+RecordMetaData,
am starting thread for voting.

Thanks
Renuka M


Re: [VOTE] KIP-401: TransformerSupplier/ProcessorSupplier StateStore connecting

2019-09-03 Thread Paul Whalen
Yeah, agreed on it being the same reference. That’s the way I have it in the 
working PR and I’ll update the KIP for clarity. 

> On Sep 3, 2019, at 5:04 PM, Matthias J. Sax  wrote:
> 
> I am strongly in favor of "must be the same reference".
> 
> 
> -Matthias
> 
>> On 9/3/19 2:09 PM, Guozhang Wang wrote:
>> Hi Paul,
>> 
>> Thanks for the KIP! +1 (binding).
>> 
>> One minor comment about the following:
>> 
>> "In order to solve the problem of addStateStore potentially being called
>> twice for the same store (because more than one Supplier specifies it), the
>> check for duplicate stores in addStateStores will be relaxed to *allow for
>> duplicates if the same StoreBuilder instance for the same store name*."
>> 
>> It worth clarifying how should we check if the StoreBuilder instances are
>> the same: either 1) equality by reference or 2) equality based on e.g.
>> #equals override function so that two different instances may still be
>> considered "equal". I think you meant 1), just wanted to confirm :)
>> 
>> 
>> Guozhang
>> 
>>> On Thu, Aug 29, 2019 at 3:14 PM Paul Whalen  wrote:
>>> 
>>> Thanks for the votes all! With two binding votes we’re in need of one more
>>> for the KIP to be accepted. With the 2.4 release coming in September, it
>>> would be great to get another committer to take a look soon so I could set
>>> aside some time to get implementation/documentation done to make it into
>>> the release.
>>> 
>>> Thanks,
>>> Paul
>>> 
 On Aug 20, 2019, at 5:47 PM, Bill Bejeck  wrote:
 
 Thanks for the KIP.
 
 +1 (binding)
 
 On Tue, Aug 20, 2019 at 6:28 PM Matthias J. Sax 
 wrote:
 
> +1 (binding)
> 
> 
>> On 6/17/19 2:32 PM, John Roesler wrote:
>> I'm +1 (nonbinding) on the current iteration of the proposal.
>> 
>>> On Mon, May 27, 2019 at 1:58 PM Paul Whalen 
>>> wrote:
>>> 
>>> I spoke too early a month ago, but I believe the proposal is finalized
> now
>>> and ready for voting.
>>> 
>>> KIP:
>>> 
> 
>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97553756
>>> 
>>> Discussion:
>>> 
> 
>>> https://lists.apache.org/thread.html/600996d83d485f2b8daf45037de64a60cebdfac9b234bf3449b6b753@%3Cdev.kafka.apache.org%3E
>>> 
>>> Pull request (still a WIP, obviously):
>>> https://github.com/apache/kafka/pull/6824
>>> 
>>> Thanks,
>>> Paul
>>> 
 On Wed, Apr 24, 2019 at 8:00 PM Paul Whalen 
>>> wrote:
 
 Hi all,
 
 After some good discussion on and adjustments to KIP-401 (which I
> renamed
 slightly for clarity), chatter has died down so I figured I may as
>>> well
 start a vote.
 
 KIP:
 TransformerSupplier/ProcessorSupplier StateStore connecting
 <
> 
>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97553756>
 Discussion:
 
 
> 
>>> https://lists.apache.org/thread.html/600996d83d485f2b8daf45037de64a60cebdfac9b234bf3449b6b753@%3Cdev.kafka.apache.org%3E
 
 Thanks!
 Paul
 
> 
> 
>>> 
>> 
>> 
> 


Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-03 Thread Colin McCabe
Hi David,

Thanks again for the KIP.

Currently, we don't parse the contents of ApiVersionsRequest at all, since it's 
an empty message.  KIP-511 proposes adding some fields here, which will clearly 
change that situation.  In the future, any changes to ApiVersionsRequest will 
have to only add stuff at the end, rather than changing fields that already 
exist.  This will significantly limit our ability to add new things over time.  

Therefore, I think we should make sure that KIP-511 is implemented after 
KIP-482, so that the version we freeze into place can be one that includes the 
ability to add tagged fields, and includes the more efficient string and array 
serialization specified in KIP-482.  It's probably worth spelling that out here.

On another topic, when the client sends an unsupported version N of 
ApiVersionsRequest, and the broker only supports versions up to M, the broker 
currently falls back to sending a version 0 response.  Why can't the broker 
fall back to version M instead?  Previously, we only had versions 1 and 0 of 
ApiVersionsRequest, so the two behaviors were identical.  But going forward, 
once we have version 2 and later of ApiVersoinsRequest, it doesn't make sense 
to fall back all the way to 0 if the broker supports 1 (for example).

If you agree, it would be good to spell this out in the KIP, so that if we want 
to add more things to the response, we can, without losing them each time the 
client's version of ApiVersionsRequest exceeds the broker's.

best,
Colin


On Tue, Sep 3, 2019, at 01:26, David Jacot wrote:
> Hi all,
> 
> I have updated the KIP to address the various comments. I have also added a
> section about the handling of the ApiVersionsRequest/Response in the broker.
> 
> Please, let me know what you think. I would like to make it for the next
> release if possible.
> 
> Best,
> David
> 
> On Fri, Aug 30, 2019 at 10:31 AM David Jacot  wrote:
> 
> > Hi Magnus,
> >
> > Thank you for your feedback. Please, find my comments below.
> >
> > 1. I thought that the clientId was meant for this purpose (providing
> > information about the application). Is there a gap I don't see?
> >
> > 2. I have put two fields to avoid requiring deep validation and parsing on
> > the broker. I believe that it will be easier to use the metadata downstream
> > like this.
> >
> > 3. Good point. I think that the broker should have some sort of validation
> > and fail the ApiVersionRequest and/or the Connection if the validation
> > fails. To avoid backward compatibility issues, I think we should come up
> > with a minimal validation and ensure it won't become more restrictive in
> > the future.
> >
> > 4. I don't have strong opinion regarding this one but as the focus of the
> > KIP is to gather the client's information, I suggest to discuss/address
> > this later on.
> >
> > Best,
> > David
> >
> > On Thu, Aug 29, 2019 at 6:56 PM Colin McCabe  wrote:
> >
> >> On Fri, Aug 23, 2019, at 00:07, Magnus Edenhill wrote:
> >> > Great proposal, this feature is well overdue!
> >> >
> >> > 1)
> >> > From an operator's perspective I don't think the kafka client
> >> > implementation name and version are sufficient,
> >> > I also believe the application name and version are of interest.
> >> > You could have all applications in your cluster run the same kafka
> >> client
> >> > and version, but only one type or
> >> > version of an application misbehave and needing to be tracked down.
> >>
> >> Hi Magnus,
> >>
> >> I think it might be better to leave this out of scope for now, and think
> >> about it in the context of more generalized request tracing.  This is a
> >> very deep rabbit hole, and I could easily see it delaying this KIP for a
> >> long time.  For example, if you have multiple Spark jobs producing to
> >> Kafka, just knowing that a client is being used by Spark may not be that
> >> helpful.  So maybe you want a third set of fields to describe the spark
> >> application ID and version, etc?  And then maybe that, itself, was created
> >> by some framework... etc. Probably better to defer this discussion for now
> >> and see how version tracking works out.
> >>
> >> >
> >> > While the application and client name and version could be combined in
> >> the
> >> > ClientName/ClientVersion fields by
> >> > the user (e.g. like User-Agent), it would not be in a generalized format
> >> > and hard for generic monitoring tools to parse correctly.
> >> >
> >> > So I'd suggest keeping ClientName and ClientVersion as the client
> >> > implementation name ("java" or "org.apache.kafka...") and version,
> >> > which can't be changed by the user/app developer, and providing two
> >> > optional fields for the application counterpart:
> >> > ApplicationName and ApplicationVersion, which are backed by
> >> corresponding
> >> > configuration properties (application.name, application.version).
> >> >
> >> > 2)
> >> > Do ..Name and ..Version need to be two separate fields, seeing how the
> >> two
> >> > fields are am

Re: [VOTE] KIP-482: The Kafka Protocol should Support Optional Tagged Fields

2019-09-03 Thread Jose Armando Garcia Sancio
+1 (non-binding)

Looking forward to this improvement.

On Tue, Sep 3, 2019 at 12:49 PM David Jacot  wrote:

> +1 (non-binding)
>
> Thank for the KIP. Great addition to the Kafka protocol!
>
> Best,
> David
>
> Le mar. 3 sept. 2019 à 19:17, Colin McCabe  a écrit :
>
> > Hi all,
> >
> > I'd like to start the vote for KIP-482: The Kafka Protocol should Support
> > Optional Tagged Fields.
> >
> > KIP:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-482%3A+The+Kafka+Protocol+should+Support+Optional+Tagged+Fields
> >
> > Discussion thread here:
> >
> https://lists.apache.org/thread.html/cdc801ae886491b73ef7efecac7ef81b24382f8b6b025899ee343f7a@%3Cdev.kafka.apache.org%3E
> >
> > best,
> > Colin
> >
>


-- 
-Jose


Re: request for permission to create KIP

2019-09-03 Thread Matthias J. Sax
What is your wiki ID (ie, account name)

-Matthias

On 8/26/19 11:43 AM, KUN DU wrote:
> Hey,
> 
> I want to create a KIP  for
> https://issues.apache.org/jira/browse/KAFKA-7711
> to initiate discussion.
> 
> Can someone grant me permission?
> 
> Thanks,
> Kun
> 



signature.asc
Description: OpenPGP digital signature


Re: [VOTE] KIP-401: TransformerSupplier/ProcessorSupplier StateStore connecting

2019-09-03 Thread Matthias J. Sax
I am strongly in favor of "must be the same reference".


-Matthias

On 9/3/19 2:09 PM, Guozhang Wang wrote:
> Hi Paul,
> 
> Thanks for the KIP! +1 (binding).
> 
> One minor comment about the following:
> 
> "In order to solve the problem of addStateStore potentially being called
> twice for the same store (because more than one Supplier specifies it), the
> check for duplicate stores in addStateStores will be relaxed to *allow for
> duplicates if the same StoreBuilder instance for the same store name*."
> 
> It worth clarifying how should we check if the StoreBuilder instances are
> the same: either 1) equality by reference or 2) equality based on e.g.
> #equals override function so that two different instances may still be
> considered "equal". I think you meant 1), just wanted to confirm :)
> 
> 
> Guozhang
> 
> On Thu, Aug 29, 2019 at 3:14 PM Paul Whalen  wrote:
> 
>> Thanks for the votes all! With two binding votes we’re in need of one more
>> for the KIP to be accepted. With the 2.4 release coming in September, it
>> would be great to get another committer to take a look soon so I could set
>> aside some time to get implementation/documentation done to make it into
>> the release.
>>
>> Thanks,
>> Paul
>>
>>> On Aug 20, 2019, at 5:47 PM, Bill Bejeck  wrote:
>>>
>>> Thanks for the KIP.
>>>
>>> +1 (binding)
>>>
>>> On Tue, Aug 20, 2019 at 6:28 PM Matthias J. Sax 
>>> wrote:
>>>
 +1 (binding)


> On 6/17/19 2:32 PM, John Roesler wrote:
> I'm +1 (nonbinding) on the current iteration of the proposal.
>
>> On Mon, May 27, 2019 at 1:58 PM Paul Whalen 
>> wrote:
>>
>> I spoke too early a month ago, but I believe the proposal is finalized
 now
>> and ready for voting.
>>
>> KIP:
>>

>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97553756
>>
>> Discussion:
>>

>> https://lists.apache.org/thread.html/600996d83d485f2b8daf45037de64a60cebdfac9b234bf3449b6b753@%3Cdev.kafka.apache.org%3E
>>
>> Pull request (still a WIP, obviously):
>> https://github.com/apache/kafka/pull/6824
>>
>> Thanks,
>> Paul
>>
>>> On Wed, Apr 24, 2019 at 8:00 PM Paul Whalen 
>> wrote:
>>>
>>> Hi all,
>>>
>>> After some good discussion on and adjustments to KIP-401 (which I
 renamed
>>> slightly for clarity), chatter has died down so I figured I may as
>> well
>>> start a vote.
>>>
>>> KIP:
>>> TransformerSupplier/ProcessorSupplier StateStore connecting
>>> <

>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97553756>
>>> Discussion:
>>>
>>>

>> https://lists.apache.org/thread.html/600996d83d485f2b8daf45037de64a60cebdfac9b234bf3449b6b753@%3Cdev.kafka.apache.org%3E
>>>
>>> Thanks!
>>> Paul
>>>


>>
> 
> 



signature.asc
Description: OpenPGP digital signature


Re: [VOTE] KIP-401: TransformerSupplier/ProcessorSupplier StateStore connecting

2019-09-03 Thread Guozhang Wang
Hi Paul,

Thanks for the KIP! +1 (binding).

One minor comment about the following:

"In order to solve the problem of addStateStore potentially being called
twice for the same store (because more than one Supplier specifies it), the
check for duplicate stores in addStateStores will be relaxed to *allow for
duplicates if the same StoreBuilder instance for the same store name*."

It worth clarifying how should we check if the StoreBuilder instances are
the same: either 1) equality by reference or 2) equality based on e.g.
#equals override function so that two different instances may still be
considered "equal". I think you meant 1), just wanted to confirm :)


Guozhang

On Thu, Aug 29, 2019 at 3:14 PM Paul Whalen  wrote:

> Thanks for the votes all! With two binding votes we’re in need of one more
> for the KIP to be accepted. With the 2.4 release coming in September, it
> would be great to get another committer to take a look soon so I could set
> aside some time to get implementation/documentation done to make it into
> the release.
>
> Thanks,
> Paul
>
> > On Aug 20, 2019, at 5:47 PM, Bill Bejeck  wrote:
> >
> > Thanks for the KIP.
> >
> > +1 (binding)
> >
> > On Tue, Aug 20, 2019 at 6:28 PM Matthias J. Sax 
> > wrote:
> >
> >> +1 (binding)
> >>
> >>
> >>> On 6/17/19 2:32 PM, John Roesler wrote:
> >>> I'm +1 (nonbinding) on the current iteration of the proposal.
> >>>
>  On Mon, May 27, 2019 at 1:58 PM Paul Whalen 
> wrote:
> 
>  I spoke too early a month ago, but I believe the proposal is finalized
> >> now
>  and ready for voting.
> 
>  KIP:
> 
> >>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97553756
> 
>  Discussion:
> 
> >>
> https://lists.apache.org/thread.html/600996d83d485f2b8daf45037de64a60cebdfac9b234bf3449b6b753@%3Cdev.kafka.apache.org%3E
> 
>  Pull request (still a WIP, obviously):
>  https://github.com/apache/kafka/pull/6824
> 
>  Thanks,
>  Paul
> 
> > On Wed, Apr 24, 2019 at 8:00 PM Paul Whalen 
> wrote:
> >
> > Hi all,
> >
> > After some good discussion on and adjustments to KIP-401 (which I
> >> renamed
> > slightly for clarity), chatter has died down so I figured I may as
> well
> > start a vote.
> >
> > KIP:
> > TransformerSupplier/ProcessorSupplier StateStore connecting
> > <
> >>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97553756>
> > Discussion:
> >
> >
> >>
> https://lists.apache.org/thread.html/600996d83d485f2b8daf45037de64a60cebdfac9b234bf3449b6b753@%3Cdev.kafka.apache.org%3E
> >
> > Thanks!
> > Paul
> >
> >>
> >>
>


-- 
-- Guozhang


Re: [DISCUSS] KIP-500: Replace ZooKeeper with a Self-Managed Metadata Quorum

2019-09-03 Thread Ron Dagostino
Thanks, Colin.  That all makes sense, especially the part about the onerous
testing requirements associated with supporting both Zookeeper and the new
metadata quorum simultaneously.  Given that, I now buy into the idea that
the transition to the new metadata quorum becomes the main path forward
once the final bridge release is cut.  There may be back-ports of bugs and
features based on demand as you suggest at the end of your reply, but there
is no guarantee of that happening.  I'm good with that now.

Ron

On Tue, Sep 3, 2019 at 1:05 PM Colin McCabe  wrote:

> On Mon, Sep 2, 2019, at 07:51, Ron Dagostino wrote:
> > Hi Colin.  It is not unusual for customers to wait before upgrading —
> > to avoid so-called “point-zero” releases — to avoid as many of the
> > inevitable bugs that ride along with new functionality as possible.
> > Removal of Zookeeper is going feel especially risky to these customers,
> > and arguably it is going to feel risky even to customers who might
> > otherwise be less sensitive to upgrade risk.
> >
> > This leads me to believe it is reasonable to expect that the uptake of
> > the new ZK-less consensus quorum could be delayed in many installations
> > — that such customers might wait longer than usual to adopt the feature
> > and abandon their Zookeeper servers.
> >
> > Will it be possible to use releases beyond the bridge release and not
> > abandon Zookeeper?  For example, what would happen if post-bridge the
> > new consensus quorum servers are never started?  Would Kafka still work
> > fine?  At what point MUST Zookeeper be abandoned?  Taking the
> > perspective of the above customers, I think they would prefer to have
> > others adopt the new ZK-less consensus quorum for several months and
> > encounter many of the inevitable bugs before adopting it themselves.
> > But at the same time they will not want to be stuck on the bridge
> > release that whole time because there are going to be both bug fixes
> > and new features that they will want to take advantage of.
> >
> > If the bridge release is the last one that supports Zookeeper, and if
> > some customers stay on that release for a while, then I could see those
> > customers wanting back-ports of bug fixes and features to occur for a
> > period of time that extends beyond what is normally done.
> >
> > Basically, to sum all of the above up, I think there is a reasonable
> > probability that a single bridge release only could become a potential
> > barrier that causes angst for the project and the community.
> >
> > I wonder if it would be in the interest of the project and the
> > community to mitigate the risk of there being a bridge release barrier
> > by extending the time when ZK would still be supported — perhaps for up
> > to a year — and the new co send us quorum could remain optional.
> >
> > Ron
> >
>
> Hi Ron,
>
> Changing things always involves risk.  This is why we are trying to do as
> much as we can incrementally.  For example, removing ZK dependencies from
> tools, and from brokers.  However, there are things that can't really be
> done incrementally, and one of these is switching over to a new metadata
> store.
>
> It might seem like supporting multiple options for where to store metadata
> would be safer somehow, but actually this is not the case.  Having to
> support totally different code paths involves a lot more work and a lot
> more testing.  We already have configurations that aren't tested enough.
> Doubling (at least) the number of configurations we have to test is a
> non-starter.
>
> This also ties in with the discussion in the KIP about why we don't plan
> on supporting pluggable consensus or pluggable metadata storage.  Doing
> this would force us to use only the least-common denominator features of
> every metadata storage.  We would not be able to take advantage of metadata
> as a stream of events, or any of the features that ZK doesn't have.
> Configuration would also be quite complex.
>
> As the KIP states, the upgrade from a bridge release (there may be several
> bridge releases) to ZK will have no impact on clients.  It also won't have
> any impact on cluster sizing (ZK nodes will simply become controller
> nodes).  And it will be possible to do with a rolling upgrade.  I agree
> that some people may be nervous about running the new software, and we may
> want to have more point releases of the older branches.
>
> This is something that we'll discuss when people propose release
> schedules.  In general, this isn't fundamentally different than someone
> wanting a new release of 1.x because they don't want to upgrade to 2.x.  If
> there's enough interest, we'll do it.
>
> best,
> Colin
>
> >
> > > On Aug 26, 2019, at 6:55 PM, Colin McCabe  wrote:
> > >
> > > Hi Ryanne,
> > >
> > > Good point.  I added a section titled "future work" with information
> about the follow-on KIPs that we discussed here.
> > >
> > > best,
> > > Colin
> > >
> > >
> > >> On Fri, Aug 23, 2019, at 13:15, Ryanne Dolan wr

Re: [VOTE] KIP-482: The Kafka Protocol should Support Optional Tagged Fields

2019-09-03 Thread David Jacot
+1 (non-binding)

Thank for the KIP. Great addition to the Kafka protocol!

Best,
David

Le mar. 3 sept. 2019 à 19:17, Colin McCabe  a écrit :

> Hi all,
>
> I'd like to start the vote for KIP-482: The Kafka Protocol should Support
> Optional Tagged Fields.
>
> KIP:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-482%3A+The+Kafka+Protocol+should+Support+Optional+Tagged+Fields
>
> Discussion thread here:
> https://lists.apache.org/thread.html/cdc801ae886491b73ef7efecac7ef81b24382f8b6b025899ee343f7a@%3Cdev.kafka.apache.org%3E
>
> best,
> Colin
>


Re: [DISCUSS] KIP-504 - Add new Java Authorizer Interface

2019-09-03 Thread Colin McCabe
Hi Rajini,

That's an interesting point.  I guess I assumed that we would always cache 
metadata locally, so that a synchronous operation would be OK here.  But, I 
suppose as time goes on, we will eventually want paged authorization metadata.

If the operations are done asynchronously, then that implies background 
thread(s) at work somewhere.  We should specify when that these thread(s) are 
created, and when they are stopped.  Probably they should be created in start, 
and stopped in close?  We should update the JavaDoc.

best,
Colin

On Tue, Sep 3, 2019, at 05:50, Rajini Sivaram wrote:
> Hi all,
> 
> Ismael brought up a point that it will be good to make the Authorizer
> interface asynchronous to avoid blocking request threads during remote
> operations.
> 
> 1) Since we want to support different backends for authorization metadata,
> making createAcls() and deleteAcls() asynchronous makes sense since these
> always involve remote operations. When KIP-500 removes ZooKeeper, we would
> want to move ACLs to Kafka and async updates will avoid unnecessary
> blocking.
> 2) For authorize() method, we currently use cached ACLs in the built-in
> authorizers, so synchronous authorise operations work well now. But async
> authorize() would support this scenario as well as authorizers in large
> organisations where an LRU cache would enable a smaller cache even when the
> backend holds a large amount of ACLs for infrequently used resources or
> users who don't use the system frequently.
> 
> For both cases, the built-in authorizer will continue to be synchronous,
> using CompletableFuture.completedFuture() to return the actual result. But
> async API will make custom authorizer implementations more flexible. I
> would like to know if there are any concerns with these changes before
> updating the KIP.
> 
> *Proposed API:*
> public interface Authorizer extends Configurable, Closeable {
> 
> Map> start(AuthorizerServerInfo 
> serverInfo);
> List>
> authorize(AuthorizableRequestContext requestContext, List
> actions);
> List>
> createAcls(AuthorizableRequestContext requestContext, List
> aclBindings);
> List>
> deleteAcls(AuthorizableRequestContext requestContext,
> List aclBindingFilters);
> CompletionStage> acls(AclBindingFilter filter);
> }
> 
> 
> Thank you,
> 
> Rajini
> 
> On Sun, Aug 18, 2019 at 6:25 PM Don Bosco Durai  wrote:
> 
> > Hi Rajini
> >
> > Thanks for clarifying. I am good for now.
> >
> > Regards
> >
> > Bosco
> >
> >
> > On 8/16/19, 11:30 AM, "Rajini Sivaram"  wrote:
> >
> > Hi Don,
> >
> > That should be fine. I guess Ranger loads policies from the database
> > synchronously when the authorizer is configured, similar to
> > SimpleAclAuthorizer loading from ZooKeeper. Ranger can continue to load
> > synchronously from `configure()` or `start()` and return an empty map
> > from
> > `start()`. That would retain the existing behaviour.. When the same
> > database stores policies for all listeners and the policies are not
> > stored
> > in Kafka, there is no value in making the load asynchronous.
> >
> > Regards,
> >
> > Rajini
> >
> >
> > On Fri, Aug 16, 2019 at 6:43 PM Don Bosco Durai 
> > wrote:
> >
> > > Hi Rajini
> > >
> > > Assuming this doesn't affect custom plugins, I don't have any
> > concerns
> > > regarding this.
> > >
> > > I do have one question regarding:
> > >
> > > "For authorizers that don’t store metadata in ZooKeeper, ensure that
> > > authorizer metadata for each listener is available before starting
> > up the
> > > listener. This enables different authorization metadata stores for
> > > different listeners."
> > >
> > > Since Ranger uses its own database for storing policies, do you
> > anticipate
> > > any issues?
> > >
> > > Thanks
> > >
> > > Bosco
> > >
> > >
> > > On 8/16/19, 6:49 AM, "Rajini Sivaram" 
> > wrote:
> > >
> > > Hi all,
> > >
> > > I made another change to the KIP. The KIP was originally
> > proposing to
> > > extend SimpleAclAuthorizer to also implement the new API (in
> > addition
> > > to
> > > the existing API). But since we use the new API when available,
> > this
> > > can
> > > break custom authorizers that extend this class and override
> > specific
> > > methods of the old API. To avoid breaking any existing custom
> > > implementations that extend this class, particularly because it
> > is in
> > > the
> > > public package kafka.security.auth, the KIP now proposes to
> > retain the
> > > old
> > > authorizer as-is.  SimpleAclAuthorizer will continue to
> > implement the
> > > old
> > > API, but will be deprecated. A new authorizer implementation
> > > kafka.security.authorizer.AclAuthorizer will be added for the
> > new API
> > > (this
> > > will not be in the public package).

[VOTE] KIP-482: The Kafka Protocol should Support Optional Tagged Fields

2019-09-03 Thread Colin McCabe
Hi all,

I'd like to start the vote for KIP-482: The Kafka Protocol should Support 
Optional Tagged Fields.

KIP: 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-482%3A+The+Kafka+Protocol+should+Support+Optional+Tagged+Fields

Discussion thread here: 
https://lists.apache.org/thread.html/cdc801ae886491b73ef7efecac7ef81b24382f8b6b025899ee343f7a@%3Cdev.kafka.apache.org%3E

best,
Colin


Re: [DISCUSS] KIP-486 Support for pluggable KeyStore and TrustStore

2019-09-03 Thread Colin McCabe
On Mon, Sep 2, 2019, at 03:33, Rajini Sivaram wrote:
> I would expect SslEngineBuilder interface to look something like this,
> perhaps with some tweaking:
> 
> public interface SslEngineBuilder extends Configurable, Closeable {
> 
> Set reconfigurableConfigs();
> 
> boolean shouldBeRebuilt(Map nextConfigs);
> 
> SSLEngine createSslEngine(Mode mode, String peerHost, int
> peerPort, String endpointIdentification);
> 
> }
> 
> The existing SslEngineBuilder class would be renamed and will implement
> this interface. Loading of keystore/truststore will be in SslEngineBuilder
> as it is now.  The method `shouldBeRebuilt()` will validate configs during
> reconfiguration and decide if reconfiguration is required because keystore
> or truststore changed. SslFactory.reconfigurableConfigs() will return
> SslEngineBuilder.reconfigurableConfigs() as well including any custom
> configs of SslEngineBuilder, so no other changes will be required when we
> eventually support custom SSL configs.
> 
> We don't want to make SslFactory the pluggable class since that contains
> validation logic for SSL engines. Everything that we want to customise is
> contained in SslEngineBuilder. Basically custom SslEngineBuilder will
> validate custom configs during reconfiguration and create SSLEngine.
> SslFactory will continue to perform validation of SSLEngines and this will
> not be customizable. SslEngineBuilder will not be reconfigurable, instead
> we create a new builder as we do now to avoid having to deal with
> thread-safety and atomicity of updates. We could consider using a public
> Reconfigurable interface as the pluggable interface for consistency, but I
> think we would still want to create a new Builder on reconfiguration and
> retain non-pluggable SSL engine validation in SslFactory.

+1

C.

> 
> 
> On Fri, Aug 30, 2019 at 10:21 PM Maulin Vasavada 
> wrote:
> 
> > Looking at SslFactory and SslEngineBuilder I feel the responsibilities are
> > not clear. Both has public method for createSSLEngine for example. I feel
> > the SslEngineBuilder was created to move out lot of code but it is not
> > necessarily a public class (e.g. I don't think anybody calling
> > SslEngineBuilder separately without SslFactory in between). I am currently
> > inclined toward what Celement is suggesting - having pluggable SslFactory.
> >
> > Let me do this - let me refactor SslFactory and SslEngineBuilder and review
> > what I can come up with you guys. Let us see if we can address all the
> > objections raised previously for KIP-383's iterations. I'll need sometime
> > though. Let me try to do it by next of next week.
> >
> > Thanks
> > Maulin
> >
> > On Fri, Aug 30, 2019 at 12:25 PM Pellerin, Clement <
> > clement_pelle...@ibi.com>
> > wrote:
> >
> > > What is your solution to the objection that killed the second iteration
> > of
> > > KIP-383?
> > > Mainly, how do you support validation of reconfiguration requests that
> > > involve new custom properties implemented by the pluggable factory?
> > > Custom properties do not exist yet, but that is very legitimate thing to
> > > design for the future.
> > >
> > > That's why I favor a pluggable SslFactory instead of an SslEngineBuilder
> > > factory.
> > >
> > > -Original Message-
> > > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com]
> > > Sent: Friday, August 30, 2019 3:07 PM
> > > To: dev@kafka.apache.org
> > > Subject: Re: [DISCUSS] KIP-486 Support for pluggable KeyStore and
> > > TrustStore
> > >
> > > +1 for making SslEngineBuilder configurable upon more thoughts.
> > >
> > > However, in the abstraction and default implementation we should make
> > sure
> > > when we do have a requirement to plugin custom key/trust store people
> > don't
> > > have to write lot more code which may not be related to it.
> > >
> > > Having said that, does this mean, we resurrect KIP-383 and update it with
> > > latest context and go from there?
> > >
> > > We are willing to take up that work for making it configurable.
> > >
> > > Thanks
> > > Maulin
> > >
> > >
> > >
> > >
> > >
> > > On Fri, Aug 30, 2019 at 10:34 AM Maulin Vasavada <
> > > maulin.vasav...@gmail.com>
> > > wrote:
> > >
> > > > Why don't we make SSLEngineBuilder code delegate the whole Key/Trust
> > > store
> > > > initialization to the interfaces we are proposing? Default
> > implementation
> > > > for those key/trust store loader interfaces will be "file based"
> > loading
> > > vs
> > > > if somebody wants to customize any of it they can.
> > > >
> > > > Would that make sense?
> > > >
> > > > On Fri, Aug 30, 2019 at 10:03 AM Colin McCabe 
> > > wrote:
> > > >
> > > >> +1 for making SslEngineBuilder configurable.  This would give
> > > >> implementers a lot more flexibility-- to use key distribution methods
> > > that
> > > >> were not files, for example.
> > > >>
> > > >> best,
> > > >> Colin
> > > >>
> > > >>
> > > >> On Fri, Aug 30, 2019, at 02:03, Rajini Sivaram wrote:
> > > >> > Just to make sure we are on the 

Re: [DISCUSS] KIP-500: Replace ZooKeeper with a Self-Managed Metadata Quorum

2019-09-03 Thread Colin McCabe
On Mon, Sep 2, 2019, at 07:51, Ron Dagostino wrote:
> Hi Colin.  It is not unusual for customers to wait before upgrading — 
> to avoid so-called “point-zero” releases — to avoid as many of the 
> inevitable bugs that ride along with new functionality as possible.  
> Removal of Zookeeper is going feel especially risky to these customers, 
> and arguably it is going to feel risky even to customers who might 
> otherwise be less sensitive to upgrade risk.
> 
> This leads me to believe it is reasonable to expect that the uptake of 
> the new ZK-less consensus quorum could be delayed in many installations 
> — that such customers might wait longer than usual to adopt the feature 
> and abandon their Zookeeper servers.
> 
> Will it be possible to use releases beyond the bridge release and not 
> abandon Zookeeper?  For example, what would happen if post-bridge the 
> new consensus quorum servers are never started?  Would Kafka still work 
> fine?  At what point MUST Zookeeper be abandoned?  Taking the 
> perspective of the above customers, I think they would prefer to have 
> others adopt the new ZK-less consensus quorum for several months and 
> encounter many of the inevitable bugs before adopting it themselves.  
> But at the same time they will not want to be stuck on the bridge 
> release that whole time because there are going to be both bug fixes 
> and new features that they will want to take advantage of.
> 
> If the bridge release is the last one that supports Zookeeper, and if 
> some customers stay on that release for a while, then I could see those 
> customers wanting back-ports of bug fixes and features to occur for a 
> period of time that extends beyond what is normally done.
> 
> Basically, to sum all of the above up, I think there is a reasonable 
> probability that a single bridge release only could become a potential 
> barrier that causes angst for the project and the community.
> 
> I wonder if it would be in the interest of the project and the 
> community to mitigate the risk of there being a bridge release barrier 
> by extending the time when ZK would still be supported — perhaps for up 
> to a year — and the new co send us quorum could remain optional.
> 
> Ron
> 

Hi Ron,

Changing things always involves risk.  This is why we are trying to do as much 
as we can incrementally.  For example, removing ZK dependencies from tools, and 
from brokers.  However, there are things that can't really be done 
incrementally, and one of these is switching over to a new metadata store.

It might seem like supporting multiple options for where to store metadata 
would be safer somehow, but actually this is not the case.  Having to support 
totally different code paths involves a lot more work and a lot more testing.  
We already have configurations that aren't tested enough.  Doubling (at least) 
the number of configurations we have to test is a non-starter.

This also ties in with the discussion in the KIP about why we don't plan on 
supporting pluggable consensus or pluggable metadata storage.  Doing this would 
force us to use only the least-common denominator features of every metadata 
storage.  We would not be able to take advantage of metadata as a stream of 
events, or any of the features that ZK doesn't have.  Configuration would also 
be quite complex.

As the KIP states, the upgrade from a bridge release (there may be several 
bridge releases) to ZK will have no impact on clients.  It also won't have any 
impact on cluster sizing (ZK nodes will simply become controller nodes).  And 
it will be possible to do with a rolling upgrade.  I agree that some people may 
be nervous about running the new software, and we may want to have more point 
releases of the older branches.

This is something that we'll discuss when people propose release schedules.  In 
general, this isn't fundamentally different than someone wanting a new release 
of 1.x because they don't want to upgrade to 2.x.  If there's enough interest, 
we'll do it.

best,
Colin

> 
> > On Aug 26, 2019, at 6:55 PM, Colin McCabe  wrote:
> > 
> > Hi Ryanne,
> > 
> > Good point.  I added a section titled "future work" with information about 
> > the follow-on KIPs that we discussed here.
> > 
> > best,
> > Colin
> > 
> > 
> >> On Fri, Aug 23, 2019, at 13:15, Ryanne Dolan wrote:
> >> Thanks Colin, sgtm. Please make this clear in the KIP -- otherwise it is
> >> hard to nail down what we are voting for.
> >> 
> >> Ryanne
> >> 
> >> 
> >>> On Fri, Aug 23, 2019, 12:58 PM Colin McCabe  wrote:
> >>> 
>  On Fri, Aug 23, 2019, at 06:24, Ryanne Dolan wrote:
>  Colin, can you outline what specifically would be in scope for this KIP
> >>> vs
>  deferred to the follow-on KIPs you've mentioned? Maybe a Future Work
>  section? Is the idea to get to the bridge release with this KIP, and then
>  go from there?
>  
>  Ryanne
>  
> >>> 
> >>> Hi Ryanne,
> >>> 
> >>> The goal for KIP-500 is to set out an overall vision for

[DISCUSS] KIP-514: Add a bounded flush() API to Kafka Producer

2019-09-03 Thread KUN DU
Hi,

I would like to start discussion on KIP-514 that proposes we add a
bounded flush() API to producer.

Link to the KIP:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-514%3A+Add+a+bounded+flush%28%29+API+to+Kafka+Producer

Suggestions and feedback are welcome!

Thanks,
Kun


[jira] [Created] (KAFKA-8863) Add InsertHeader and DropHeaders connect transforms KIP-145

2019-09-03 Thread Albert Lozano (Jira)
Albert Lozano created KAFKA-8863:


 Summary: Add InsertHeader and DropHeaders connect transforms 
KIP-145
 Key: KAFKA-8863
 URL: https://issues.apache.org/jira/browse/KAFKA-8863
 Project: Kafka
  Issue Type: New Feature
  Components: clients, KafkaConnect
Reporter: Albert Lozano


[https://cwiki.apache.org/confluence/display/KAFKA/KIP-145+-+Expose+Record+Headers+in+Kafka+Connect]

Continuing the work done in the 
[PR4319|[https://github.com/apache/kafka/pull/4319],] implementing the 
transforms to work with headers would be awesome.



--
This message was sent by Atlassian Jira
(v8.3.2#803003)


[jira] [Created] (KAFKA-8862) Misleading exception message for non-existant partition

2019-09-03 Thread Tom Bentley (Jira)
Tom Bentley created KAFKA-8862:
--

 Summary: Misleading exception message for non-existant partition
 Key: KAFKA-8862
 URL: https://issues.apache.org/jira/browse/KAFKA-8862
 Project: Kafka
  Issue Type: Bug
  Components: producer 
Affects Versions: 2.3.0
Reporter: Tom Bentley
Assignee: Tom Bentley


https://issues.apache.org/jira/browse/KAFKA-6833 changed the logic of the 
{{KafkaProducer.waitOnMetadata}} so that if a partition did not exist it would 
wait for it to exist.
It means that if called with an incorrect partition the method will eventually 
throw a {{TimeoutException}}, which covers both topic and partition 
non-existence cases.

However, the exception message was not changed for the case where 
{{metadata.awaitUpdate(version, remainingWaitMs)}} throws a 
{{TimeoutException}}.

This results in a confusing exception message. For example, if a producer tries 
to send to a non-existent partition of an existing topic the message is 
"Topic %s not present in metadata after %d ms.", when timeout via the other 
code path would come with message
"Partition %d of topic %s with partition count %d is not present in metadata 
after %d ms."





--
This message was sent by Atlassian Jira
(v8.3.2#803003)


Re: [DISCUSS] KIP-515: Enable ZK client to use the new TLS supported authentication

2019-09-03 Thread Ismael Juma
Hi Pere,

Thanks for the KIP. With regards to the CLI tools, most of them support
direct access to ZK for compatibility reasons and we encourage usage of the
Kafka protocol instead. I am not sure we should be extending them as
described in the KIP. What are your thoughts on that?

Ismael

On Thu, Aug 29, 2019 at 11:11 AM Pere Urbón Bayes 
wrote:

> Hi,
>  this is my first KIP for a change in Apache Kafka, so I'm really need to
> the process. Looking forward to hearing from you and learn the best ropes
> here.
>
> I would like to propose this KIP-515 to enable the ZookeeperClients to take
> full advantage of the TLS communication in the new Zookeeper 3.5.5.
> Specially interesting it the Zookeeper Security Migration, that without
> this change will not work with TLS, disabling users to use ACLs when the
> Zookeeper cluster use TLS.
>
> link:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-515%3A+Enable+ZK+client+to+use+the+new+TLS+supported+authentication
>
> Looking forward to hearing from you on this,
>
> /cheers
>
> --
> Pere Urbon-Bayes
> Software Architect
> http://www.purbon.com
> https://twitter.com/purbon
> https://www.linkedin.com/in/purbon/
>


Re: [DISCUSS] KIP-504 - Add new Java Authorizer Interface

2019-09-03 Thread Rajini Sivaram
Hi all,

Ismael brought up a point that it will be good to make the Authorizer
interface asynchronous to avoid blocking request threads during remote
operations.

1) Since we want to support different backends for authorization metadata,
making createAcls() and deleteAcls() asynchronous makes sense since these
always involve remote operations. When KIP-500 removes ZooKeeper, we would
want to move ACLs to Kafka and async updates will avoid unnecessary
blocking.
2) For authorize() method, we currently use cached ACLs in the built-in
authorizers, so synchronous authorise operations work well now. But async
authorize() would support this scenario as well as authorizers in large
organisations where an LRU cache would enable a smaller cache even when the
backend holds a large amount of ACLs for infrequently used resources or
users who don't use the system frequently.

For both cases, the built-in authorizer will continue to be synchronous,
using CompletableFuture.completedFuture() to return the actual result. But
async API will make custom authorizer implementations more flexible. I
would like to know if there are any concerns with these changes before
updating the KIP.

*Proposed API:*
public interface Authorizer extends Configurable, Closeable {

Map> start(AuthorizerServerInfo serverInfo);
List>
authorize(AuthorizableRequestContext requestContext, List
actions);
List>
createAcls(AuthorizableRequestContext requestContext, List
aclBindings);
List>
deleteAcls(AuthorizableRequestContext requestContext,
List aclBindingFilters);
CompletionStage> acls(AclBindingFilter filter);
}


Thank you,

Rajini

On Sun, Aug 18, 2019 at 6:25 PM Don Bosco Durai  wrote:

> Hi Rajini
>
> Thanks for clarifying. I am good for now.
>
> Regards
>
> Bosco
>
>
> On 8/16/19, 11:30 AM, "Rajini Sivaram"  wrote:
>
> Hi Don,
>
> That should be fine. I guess Ranger loads policies from the database
> synchronously when the authorizer is configured, similar to
> SimpleAclAuthorizer loading from ZooKeeper. Ranger can continue to load
> synchronously from `configure()` or `start()` and return an empty map
> from
> `start()`. That would retain the existing behaviour.. When the same
> database stores policies for all listeners and the policies are not
> stored
> in Kafka, there is no value in making the load asynchronous.
>
> Regards,
>
> Rajini
>
>
> On Fri, Aug 16, 2019 at 6:43 PM Don Bosco Durai 
> wrote:
>
> > Hi Rajini
> >
> > Assuming this doesn't affect custom plugins, I don't have any
> concerns
> > regarding this.
> >
> > I do have one question regarding:
> >
> > "For authorizers that don’t store metadata in ZooKeeper, ensure that
> > authorizer metadata for each listener is available before starting
> up the
> > listener. This enables different authorization metadata stores for
> > different listeners."
> >
> > Since Ranger uses its own database for storing policies, do you
> anticipate
> > any issues?
> >
> > Thanks
> >
> > Bosco
> >
> >
> > On 8/16/19, 6:49 AM, "Rajini Sivaram" 
> wrote:
> >
> > Hi all,
> >
> > I made another change to the KIP. The KIP was originally
> proposing to
> > extend SimpleAclAuthorizer to also implement the new API (in
> addition
> > to
> > the existing API). But since we use the new API when available,
> this
> > can
> > break custom authorizers that extend this class and override
> specific
> > methods of the old API. To avoid breaking any existing custom
> > implementations that extend this class, particularly because it
> is in
> > the
> > public package kafka.security.auth, the KIP now proposes to
> retain the
> > old
> > authorizer as-is.  SimpleAclAuthorizer will continue to
> implement the
> > old
> > API, but will be deprecated. A new authorizer implementation
> > kafka.security.authorizer.AclAuthorizer will be added for the
> new API
> > (this
> > will not be in the public package).
> >
> > Please let me know if you have any concerns.
> >
> > Regards,
> >
> > Rajini
> >
> >
> > On Fri, Aug 16, 2019 at 8:48 AM Rajini Sivaram <
> > rajinisiva...@gmail.com>
> > wrote:
> >
> > > Thanks Colin.
> > >
> > > If there are no other concerns, I will start vote later today.
> Many
> > thanks
> > > to every one for the feedback.
> > >
> > > Regards,
> > >
> > > Rajini
> > >
> > >
> > > On Thu, Aug 15, 2019 at 11:57 PM Colin McCabe <
> cmcc...@apache.org>
> > wrote:
> > >
> > >> Thanks, Rajini.  It looks good to me.
> > >>
> > >> best,
> > >> Colin
> > >>
> > >>
> > >> On Thu, Aug 15, 2019, at 11:37, Rajini Sivaram wro

Re: [DISCUSS] KIP-373: Allow users to create delegation tokens for other users

2019-09-03 Thread Gabor Somogyi
+1 (non-binding) I've had a deeper look and this would be a good addition
to Spark.


On Thu, Aug 15, 2019 at 6:19 PM Viktor Somogyi-Vass 
wrote:

> Started to implement my proposition and thought about it a little bit more
> and it seems like I overthought the problem and we'd actually be better off
> by having only the User resource type only and not CreateUsers. The problem
> with CreateUsers is that a resource apparently is created only in addAcls
> (at least in SimpleAclAuthorizer). Therefore we'd need to check before
> creating the token that the owner user has been created and the token
> creator has the permissions. Then add the user resource and the token. This
> means that we'd only use CreateUsers when creating tokens iff the token
> requester principal already has CreateTokens permissions with that user
> (the owner) so it's kinda duplicate.
> It would work though if we require the resources to be added beforehand but
> it's not the case and is the detail of the Authorizer implementation.
>
> I'll update the KIP accordingly and apologies for the extra round :).
>
> Thanks,
> Viktor
>
> On Wed, Aug 14, 2019 at 2:40 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> wrote:
>
> > Sorry, reading my email the second time I probably wasn't clear.
> > So basically the concept is that there is a user who can add other users
> > as resources (such as userB and userC) prior to creating the "userA can
> > create delegation token for userB and userC" association with
> CreateTokens.
> > To limit who can add new users as resources I thought we can introduce a
> > CreateUser operation. It's true though that we could also say that a
> Create
> > operation permission on the Cluster resource would be enough to create
> new
> > users but I think from a generic security perspective it's better if we
> > don't extend already existing operations.
> > So a classic flow would be that prior to creating the delegation token
> for
> > userB, userB itself has to be added by another user who has CreateUser
> > permissions. After this a CreateToken permission has to be created that
> > says "userA can create delegation tokens for userB" and after this userA
> > can actually create the token.
> > Let me know what you think.
> >
> > Viktor
> >
> > On Wed, Aug 14, 2019 at 1:30 PM Manikumar 
> > wrote:
> >
> >> Hi,
> >>
> >> Why do we need  new ACL operation  "CreateUsers"?
> >> I think,  "CreateTokens" Operation is sufficient to create "UserA can
> >> create tokens for UserB, UserC" association.
> >>
> >> Thanks,
> >>
> >> On Tue, Aug 13, 2019 at 3:37 PM Viktor Somogyi-Vass <
> >> viktorsomo...@gmail.com>
> >> wrote:
> >>
> >> > Hi Manikumar,
> >> >
> >> > Yea, I just brought up superuser for the sake of simplicity :).
> >> > Anyway, your proposition makes sense to me, I'll modify the KIP for
> >> this.
> >> >
> >> > The changes summarized:
> >> > 1. We'll need a new ACL operation as well (say "CreateUsers") to
> create
> >> the
> >> > "UserA can create tokens for UserB, UserC" association. This can be
> used
> >> > via the createAcls API of the AdminClient.
> >> > 2. CreateToken will be a User level operation (instead of a Cluster
> >> level
> >> > as in previous drafts). So that means any user who wants to create a
> >> > delegation token for other users will have to have an ACL set up by a
> >> > higher level user to authorize this.
> >> > 3. DescribeToken will also be a User level operation. In this case
> >> tokenT
> >> > owned by userB will be described if userA has a Describe ACL on tokenT
> >> or
> >> > has a DescribeToken ACL on userB. Note that in the latter case userA
> >> will
> >> > be able to describe all other tokens belonging to userB.
> >> >
> >> > Would this work for you?
> >> >
> >> > Viktor
> >> >
> >> > On Mon, Aug 12, 2019 at 5:45 PM Colin McCabe 
> >> wrote:
> >> >
> >> > > +1 for better access control here. In general, impersonating another
> >> user
> >> > > seems like it’s equivalent to super user access.
> >> > >
> >> > > Colin
> >> > >
> >> > > On Mon, Aug 12, 2019, at 05:43, Manikumar wrote:
> >> > > > Hi Viktor,
> >> > > >
> >> > > > As per the KIP, It's not only superuser, any user with required
> >> > > permissions
> >> > > > (CreateTokens on Cluster Resource), can create the tokens for
> other
> >> > > users.
> >> > > > Current proposed permissions defined like, "UserA can create
> tokens
> >> for
> >> > > any
> >> > > > user".
> >> > > > I am thinking, can we change the permissions like "UserA can
> create
> >> > > tokens
> >> > > > for UserB, UserC"?
> >> > > >
> >> > > > Thanks,
> >> > > >
> >> > > >
> >> > > >
> >> > > >
> >> > > >
> >> > > >
> >> > > > On Fri, Aug 9, 2019 at 6:39 PM Viktor Somogyi-Vass <
> >> > > viktorsomo...@gmail.com>
> >> > > > wrote:
> >> > > >
> >> > > > > Hey Manikumar,
> >> > > > >
> >> > > > > Thanks for the feedback.
> >> > > > > I'm not sure I fully grasp the use-case. Would this be a quota?
> >> Do we
> >> > > say
> >> > > > > something like "there can be 1

Re: [DISCUSS] KIP-515: Enable ZK client to use the new TLS supported authentication

2019-09-03 Thread Jorge Esteban Quilcate Otoya
Hi Pere,

Have you add your KIP to the list here
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals?

I found the KIP number assigned to another.



On Mon, Sep 2, 2019 at 2:23 PM Pere Urbón Bayes 
wrote:

> Thanks for your time Harsha,
>anyone else with comments? looking forward to hearing from you.
>
> Stupid question: when do you move from discussion to vote?
>
> Missatge de Harsha Chintalapani  del dia dv., 30 d’ag.
> 2019 a les 21:59:
>
> > Thanks Pere. KIP looks good to me.
> > -Harsha
> >
> >
> > On Fri, Aug 30, 2019 at 10:05 AM, Pere Urbón Bayes  >
> > wrote:
> >
> >> Not really,
> >>   my idea is to keep the JAAS parameter, so people don't see major
> >> changes. But if you pass a properties file, then this takes precedence
> over
> >> the other, with the idea that you can do sasl as well with the
> properties
> >> files.
> >>
> >> Makes sense?
> >>
> >> -- Pere
> >>
> >> Missatge de Harsha Chintalapani  del dia dv., 30 d’ag.
> >> 2019 a les 19:00:
> >>
> >>> Hi Pere,
> >>>   Thanks for the KIP. Enabling SSL for zookeeper for Kafka
> makes
> >>> sense.
> >>> "The changes are planned to be introduced in a compatible way, by
> >>> keeping the current JAAS variable precedence."
> >>> Can you elaborate a bit here. If the user configures a JAAS file with
> >>> Client section it will take precedence over zookeeper SSL configs?
> >>>
> >>> Thanks,
> >>> Harsha
> >>>
> >>>
> >>>
> >>> On Fri, Aug 30, 2019 at 7:50 AM, Pere Urbón Bayes <
> pere.ur...@gmail.com>
> >>> wrote:
> >>>
>  Hi,
>  quick question, I saw in another mail that 2.4 release is planned for
>  September. I think it would be really awesome to have this for this
>  release, do you think we can make it?
> 
>  -- Pere
> 
>  Missatge de Pere Urbón Bayes  del dia dj., 29
>  d’ag. 2019 a les 20:10:
> 
>  Hi,
>  this is my first KIP for a change in Apache Kafka, so I'm really need
>  to the process. Looking forward to hearing from you and learn the best
>  ropes here.
> 
>  I would like to propose this KIP-515 to enable the ZookeeperClients to
>  take full advantage of the TLS communication in the new Zookeeper
> 3.5.5.
>  Specially interesting it the Zookeeper Security Migration, that
> without
>  this change will not work with TLS, disabling users to use ACLs when
> the
>  Zookeeper cluster use TLS.
> 
>  link:
> 
> 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-515%3A+Enable+ZK+client+to+use+the+new+TLS+supported+authentication
> 
>  Looking forward to hearing from you on this,
> 
>  /cheers
> 
>  --
>  Pere Urbon-Bayes
>  Software Architect
>  http://www.purbon.com
>  https://twitter.com/purbon
>  https://www.linkedin.com/in/purbon/
> 
>  --
>  Pere Urbon-Bayes
>  Software Architect
>  http://www.purbon.com
>  https://twitter.com/purbon
>  https://www.linkedin.com/in/purbon/
> 
> >>>
> >>>
> >>
> >> --
> >> Pere Urbon-Bayes
> >> Software Architect
> >> http://www.purbon.com
> >> https://twitter.com/purbon
> >> https://www.linkedin.com/in/purbon/
> >>
> >
> >
>
> --
> Pere Urbon-Bayes
> Software Architect
> http://www.purbon.com
> https://twitter.com/purbon
> https://www.linkedin.com/in/purbon/
>


[jira] [Created] (KAFKA-8861) Fix flaky RegexSourceIntegrationTest.testMultipleConsumersCanReadFromPartitionedTopic

2019-09-03 Thread Chia-Ping Tsai (Jira)
Chia-Ping Tsai created KAFKA-8861:
-

 Summary: Fix flaky 
RegexSourceIntegrationTest.testMultipleConsumersCanReadFromPartitionedTopic
 Key: KAFKA-8861
 URL: https://issues.apache.org/jira/browse/KAFKA-8861
 Project: Kafka
  Issue Type: Bug
Reporter: Chia-Ping Tsai
Assignee: Chia-Ping Tsai


This is similar to KAFKA-8011. The error stack is shown below.

{code:java}
java.util.ConcurrentModificationException
at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:909)
at java.util.ArrayList$Itr.next(ArrayList.java:859)
at java.util.AbstractList.equals(AbstractList.java:521)
at 
org.apache.kafka.streams.integration.RegexSourceIntegrationTest.lambda$testMultipleConsumersCanReadFromPartitionedTopic$5(RegexSourceIntegrationTest.java:351)
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:366)
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:336)
at 
org.apache.kafka.streams.integration.RegexSourceIntegrationTest.testMultipleConsumersCanReadFromPartitionedTopic(RegexSourceIntegrationTest.java:351)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at 
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at 
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at 
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at 
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at 
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at 
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:305)
at 
org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:365)
at 
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at 
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:330)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:78)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:328)
{code}




--
This message was sent by Atlassian Jira
(v8.3.2#803003)


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

2019-09-03 Thread Apache Jenkins Server
See 


Changes:

[manikumar] KAFKA-8860: Let SslPrincipalMapper split SSL principal mapping rules

--
[...truncated 2.57 MB...]
org.apache.kafka.streams.integration.TableTableJoinIntegrationTest > 
testOuter[caching enabled = true] FAILED
java.lang.AssertionError: Condition not met within timeout 15000. Never 
received expected final result.
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:376)
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:336)
at 
org.apache.kafka.streams.integration.AbstractJoinIntegrationTest.runTest(AbstractJoinIntegrationTest.java:255)
at 
org.apache.kafka.streams.integration.TableTableJoinIntegrationTest.testOuter(TableTableJoinIntegrationTest.java:162)

org.apache.kafka.streams.integration.TableTableJoinIntegrationTest > 
testLeft[caching enabled = true] STARTED
org.apache.kafka.streams.integration.TableTableJoinIntegrationTest.testLeft[caching
 enabled = true] failed, log available in 

 enabled = true].test.stdout

org.apache.kafka.streams.integration.TableTableJoinIntegrationTest > 
testLeft[caching enabled = true] FAILED
java.lang.AssertionError: Condition not met within timeout 15000. Never 
received expected final result.
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:376)
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:336)
at 
org.apache.kafka.streams.integration.AbstractJoinIntegrationTest.runTest(AbstractJoinIntegrationTest.java:255)
at 
org.apache.kafka.streams.integration.TableTableJoinIntegrationTest.testLeft(TableTableJoinIntegrationTest.java:131)

org.apache.kafka.streams.integration.TableTableJoinIntegrationTest > 
testInnerInner[caching enabled = true] STARTED
org.apache.kafka.streams.integration.TableTableJoinIntegrationTest.testInnerInner[caching
 enabled = true] failed, log available in 

 enabled = true].test.stdout

org.apache.kafka.streams.integration.TableTableJoinIntegrationTest > 
testInnerInner[caching enabled = true] FAILED
java.lang.AssertionError: Condition not met within timeout 15000. Never 
received expected final result.
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:376)
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:336)
at 
org.apache.kafka.streams.integration.AbstractJoinIntegrationTest.runTest(AbstractJoinIntegrationTest.java:255)
at 
org.apache.kafka.streams.integration.TableTableJoinIntegrationTest.testInnerInner(TableTableJoinIntegrationTest.java:197)

org.apache.kafka.streams.integration.TableTableJoinIntegrationTest > 
testInnerOuter[caching enabled = true] STARTED
org.apache.kafka.streams.integration.TableTableJoinIntegrationTest.testInnerOuter[caching
 enabled = true] failed, log available in 

 enabled = true].test.stdout

org.apache.kafka.streams.integration.TableTableJoinIntegrationTest > 
testInnerOuter[caching enabled = true] FAILED
java.lang.AssertionError: Condition not met within timeout 15000. Never 
received expected final result.
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:376)
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:336)
at 
org.apache.kafka.streams.integration.AbstractJoinIntegrationTest.runTest(AbstractJoinIntegrationTest.java:255)
at 
org.apache.kafka.streams.integration.TableTableJoinIntegrationTest.testInnerOuter(TableTableJoinIntegrationTest.java:289)

org.apache.kafka.streams.integration.TableTableJoinIntegrationTest > 
testInnerLeft[caching enabled = true] STARTED
org.apache.kafka.streams.integration.TableTableJoinIntegrationTest.testInnerLeft[caching
 enabled = true] failed, log available in 

 enabled = true].test.stdout

org.apache.kafka.streams.integration.TableTableJoinIntegrationTest > 
testInnerLeft[caching enabled = true] FAILED
java.lang.AssertionError: Condition not met within timeout 15000. Never 
received expected final result.
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:376)
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:336)
at 
org

Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-03 Thread David Jacot
Hi all,

I have updated the KIP to address the various comments. I have also added a
section about the handling of the ApiVersionsRequest/Response in the broker.

Please, let me know what you think. I would like to make it for the next
release if possible.

Best,
David

On Fri, Aug 30, 2019 at 10:31 AM David Jacot  wrote:

> Hi Magnus,
>
> Thank you for your feedback. Please, find my comments below.
>
> 1. I thought that the clientId was meant for this purpose (providing
> information about the application). Is there a gap I don't see?
>
> 2. I have put two fields to avoid requiring deep validation and parsing on
> the broker. I believe that it will be easier to use the metadata downstream
> like this.
>
> 3. Good point. I think that the broker should have some sort of validation
> and fail the ApiVersionRequest and/or the Connection if the validation
> fails. To avoid backward compatibility issues, I think we should come up
> with a minimal validation and ensure it won't become more restrictive in
> the future.
>
> 4. I don't have strong opinion regarding this one but as the focus of the
> KIP is to gather the client's information, I suggest to discuss/address
> this later on.
>
> Best,
> David
>
> On Thu, Aug 29, 2019 at 6:56 PM Colin McCabe  wrote:
>
>> On Fri, Aug 23, 2019, at 00:07, Magnus Edenhill wrote:
>> > Great proposal, this feature is well overdue!
>> >
>> > 1)
>> > From an operator's perspective I don't think the kafka client
>> > implementation name and version are sufficient,
>> > I also believe the application name and version are of interest.
>> > You could have all applications in your cluster run the same kafka
>> client
>> > and version, but only one type or
>> > version of an application misbehave and needing to be tracked down.
>>
>> Hi Magnus,
>>
>> I think it might be better to leave this out of scope for now, and think
>> about it in the context of more generalized request tracing.  This is a
>> very deep rabbit hole, and I could easily see it delaying this KIP for a
>> long time.  For example, if you have multiple Spark jobs producing to
>> Kafka, just knowing that a client is being used by Spark may not be that
>> helpful.  So maybe you want a third set of fields to describe the spark
>> application ID and version, etc?  And then maybe that, itself, was created
>> by some framework... etc. Probably better to defer this discussion for now
>> and see how version tracking works out.
>>
>> >
>> > While the application and client name and version could be combined in
>> the
>> > ClientName/ClientVersion fields by
>> > the user (e.g. like User-Agent), it would not be in a generalized format
>> > and hard for generic monitoring tools to parse correctly.
>> >
>> > So I'd suggest keeping ClientName and ClientVersion as the client
>> > implementation name ("java" or "org.apache.kafka...") and version,
>> > which can't be changed by the user/app developer, and providing two
>> > optional fields for the application counterpart:
>> > ApplicationName and ApplicationVersion, which are backed by
>> corresponding
>> > configuration properties (application.name, application.version).
>> >
>> > 2)
>> > Do ..Name and ..Version need to be two separate fields, seeing how the
>> two
>> > fields are ambigious when separated?
>> > If we're looking to identify unique versions, combining the two fields
>> > would be sufficient (e.g., "java-2.3.1", "librdkafka/1.2.0",
>> "sarama@1.2.3")
>> > and perhaps easier to work with.
>> > The actual format or content of the name-version string is irrelevant as
>> > long as it identifies a unique name+version.
>> >
>>
>> Hmm.  Wouldn't the same arguments you made above about a combined
>> named+version field being "hard for generic monitoring tools to parse
>> correctly" apply here?  In any case, there seems to be no reason not to
>> just have two fields.  It avoids string parsing.
>>
>> >
>> > 3)
>> > As for allowed characters, will the broker fail the ApiVersionResponse
>> if
>> > any of these fields contain invalid characters,
>> > or will the broker sanitize the strings?
>> > For future backwards compatibility (when the broker constraints change
>> but
>> > clients are not updated) I suggest the latter.
>> >
>>
>> I would argue that we should be strict about the characters that we
>> accept, and just close the connection if the string is bad.  There's no
>> reason to let clients troll us with "librdkafka  " (two spaces at the end)
>> or a version string with slashes or control characters in it.  In fact, I
>> would argue we should just allow something like ([\.][a-z][A-Z][0-9])+
>> This ensures that JMX will work well.  We can always loosen the
>> restrictions later if there is a real need.
>>
>> > 4)
>> > And while we're at it, can we add the broker name and version to the
>> > ApiVersionResponse?
>> > While an application must not use this information to detect features
>> (Hi
>> > Jay!), it is good for troubleshooting
>> > and providing more meaningful logs to