Re: [DISCUSS] KIP-347: Enable batching in FindCoordinatorRequest

[Yishun Guan]

It would be great if I could get some feedbacks on this KIP, thanks!

On Thu, Aug 9, 2018, 10:35 AM Yishun Guan  wrote:

> To add more context for KIP-347: https://github.com/apache/kafka/pull/5353
>
> On Wed, Aug 8, 2018 at 3:55 PM Yishun Guan  wrote:
>
>> Hi all,
>>
>> I would like to start a discussion on:
>>
>> KIP-347: Enable batching in FindCoordinatorRequest
>> https://cwiki.apache.org/confluence/x/CgZPBQ
>>
>> Thanks @Guozhang Wang  for his help and patience!
>>
>> Thanks,
>> Yishun
>>
>

[jira] [Resolved] (KAFKA-6966) Extend `TopologyDescription.Sink` to return `TopicNameExtractor`

[Matthias J. Sax (JIRA)]

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

Matthias J. Sax resolved KAFKA-6966.

Resolution: Fixed

> Extend `TopologyDescription.Sink` to return `TopicNameExtractor`
> 
>
> Key: KAFKA-6966
> URL: https://issues.apache.org/jira/browse/KAFKA-6966
> Project: Kafka
>  Issue Type: Bug
>  Components: streams
>Affects Versions: 2.0.0
>Reporter: Matthias J. Sax
>Assignee: Nishanth Pradeep
>Priority: Major
>  Labels: beginner, kip, newbie
> Fix For: 2.1.0
>
>
> With KIP-303, a dynamic routing feature was added and 
> `TopologyDescription.Sink#topic()` returns `null` if this feature is used.
> It would be useful to get the actually used `TopicNameExtractor` class from 
> the `TopologyDescription`.
> We suggest to add `Class 
> TopologyDescription.Sink#topicNameExtractor()` and let it return `null` if 
> dynamic routing feature is not used.
> KIP-321: 
> [https://cwiki.apache.org/confluence/display/KAFKA/KIP-321%3A+Update+TopologyDescription+to+better+represent+Source+and+Sink+Nodes]



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

Re: [DISCUSS] KIP-342 Add Customizable SASL extensions to OAuthBearer authentication

[Rajini Sivaram]

Hi Stanislav,

For the point that Ron made above for:

public OAuthBearerExtensionsValidatorCallback(OAuthBearerToken token,
SaslExtensions
extensions)


I don't think we should ever invoke extensions callback without the token.
We can first validate the token and invoke extensions callback only if
token is non-null. Can we clarify that in the javadoc?

   - public SaslExtensions extensions() : Extensions should be non-null
   - public OAuthBearerToken token() : Token should be non-null


Also agree with Ron that we should have the ability to return errors for
all invalid extensions, even if a callback handler may choose to stop on
first failure.

I think we also need another method to return the extensions that were
validated and will be made available as negotiated properties. As per the
RFC, server should ignore unknown extensions. So callback handlers need to
be able to validate the ones they know of and return those. Other
extensions should not be added to the SaslServer's negotiated properties.

   - public SaslExtensions validatedExtensions()



On Fri, Aug 10, 2018 at 3:26 PM, Ron Dagostino  wrote:

> Hi Stanislav.  Here are a few KIP comments.
>
> << values to ensure they conform to the OAuth standard
> It is the SASL/OAUTHBEARER standard that defines the regular expressions
> (specifically, https://tools.ietf.org/html/rfc7628#section-3.1) rather
> than
> any of the OAuth specifications.  It would be good to make this
> clarification.
>
> << SaslExtensions extensions)
> This constructor lacks Javadoc in the KIP.  Could you add it, and also
> indicate which of the two parameters are required vs. optional?  The
> Javadoc for the token() method indicates that the return value could be
> null, but that would only be true if the constructor accepted a null value
> for the token.  I'm okay with the constructor accepting a null token
> (Rajini, you may differ in opinion, in which case I defer to your
> preference).  But please do clarify this issue.
>
> I also am not sure if exposing just one invalid extension name and error
> message in the OAuthBearerExtensionsValidatorCallback class is good
> enough.  An alternative to invalidExtensionName() and errorMessage()
> methods would be to return an always non-null but potentially empty
> Map so that potentially all of the provided extensions
> could be validated and the list of invalid extension names could be
> returned (along with the error message for each of them).  If we adopted
> this alternative then the error(String invalidExtensionName, String
> errorMessage) method might need to be renamed addError(String
> invalidExtensionName, String errorMessage).  I suspect it would be better
> to go with the map approach to support returning multiple error messages
> even if the default unsecured token validator implementation only adds the
> first invalid extension name -- at least it would allow others to be more
> complete if they wish.  It might also be worth discussing whether a has
> Error() method would be appropriate to add (returning true if the map is
> non-empty).  I don't have a strong preference on the issue of supporting 1
> vs. multiple errors (though I lean slightly towards supporting multiple
> errors).  I defer to the preference of others in this regard.
>
> Finally, now that we are actually validating extensions, the comment that
> "An attempt to use [auth] will result in an exception" might cause
> confusion and perhaps needs to be clarified to state that the exception
> occurs on the client side before the extensions are sent to the server
> rather than during extension validation on the server side (e.g. "An
> attempt to send [auth] will result in an exception on the client").
>
> Ron
>
>
> On Fri, Aug 10, 2018 at 7:22 AM Stanislav Kozlovski <
> stanis...@confluent.io>
> wrote:
>
> > Hi Rajini, Ron
> >
> > I've updated the KIP with the latest changes following our discussion.
> > Please do give it a read. If you feel it is alright, I will follow up
> with
> > a PR later.
> >
> > Best,
> > Stanislav
> >
> > On Thu, Aug 9, 2018 at 10:09 AM Rajini Sivaram 
> > wrote:
> >
> > > Hi Ron/Stansilav,
> > >
> > > OK, let's just go with 2. I think it would be better to add a
> > > OAuth-specific extensions handler OAuthBearerExtensionsValidator
> Callback
> > > that
> > > provides OAuthBearerToken.
> > >
> > > To summarise, we chose option 2 out of these four options:
> > >
> > >1. {OAuthBearerValidatorCallback, SaslExtensionsValidatorCallback}
> :
> > We
> > >don't want to use multiple ordered callbacks since we don't want the
> > >context of one callback to come from another.callback,
> > >2. OAuthBearerExtensionsValidatorCallback(OAuthBearerToken token,
> > >SaslExtensions ext): This allows extensions to be validated using
> > >context from the token, we are ok with this.
> > >3. SaslExtensionsValidatorCallback(Map context,
> > >SaslExtensions ext): This doesn't really offer any real advantage
> over
> > > 2.
> > 

[jira] [Created] (KAFKA-7277) Migrate Streams API to Duration instead of longMs times

[John Roesler (JIRA)]

John Roesler created KAFKA-7277:
---

 Summary: Migrate Streams API to Duration instead of longMs times
 Key: KAFKA-7277
 URL: https://issues.apache.org/jira/browse/KAFKA-7277
 Project: Kafka
  Issue Type: Improvement
Reporter: John Roesler


Right now Streams API unversally represents time as ms-since-unix-epoch.

There's nothing wrong, per se, with this, but Duration is more ergonomic for an 
API.

What we don't want is to present a heterogeneous API, so we need to make sure 
the whole Streams API is in terms of Duration.

 

Implementation note: Durations potentially worsen memory pressure and gc 
performance, so internally, we will still use longMs as the representation. 



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

Re: [EXTERNAL] [VOTE] KIP-310: Add a Kafka Source Connector to Kafka Connect

[McCaig, Rhys]

Thanks Stephane!

If there is a desire for further discussion I am certainly open to reverting 
this to a discussion thread. For now I’ll keep this vote open until we get 
either 3 binding votes or further request for discussion from the community.

Do you have any additional thoughts on the KIP you’d like to add?

Cheers,
Rhys

> On Aug 10, 2018, at 2:14 AM, Stephane Maarek  
> wrote:
> 
> Hi Rhys,
> 
> Overall I'm +1 (non binding), but you're going to need 3 binding votes for
> this KIP to pass.
> I don't feel there has been enough discussion on this from the community.
> Can we get some input from other people?
> 
> Thanks for starting the vote nonetheless :)
> Stephane
> 
> On 8 August 2018 at 20:28, McCaig, Rhys  wrote:
> 
>> Hi
>> 
>> Could we get a couple of votes on this KIP - voting closes in 24 hours.
>> 
>> Thanks,
>> 
>> Rhys
>> 
>>> On Aug 6, 2018, at 11:51 AM, McCaig, Rhys 
>> wrote:
>>> 
>>> Hi All,
>>> 
>>> I’m starting a vote on KIP-310: Add a Kafka Source Connector to Kafka
>> Connect
>>> 
>>> KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> 310%3A+Add+a+Kafka+Source+Connector+to+Kafka+Connect> ps://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> 310:+Add+a+Kafka+Source+Connector+to+Kafka+Connect>
>>> Discussion Thread: http://mail-archives.apache.
>> org/mod_mbox/kafka-dev/201808.mbox/%3c17E8D696-E51C-4BEB-
>> bd70-9324d4b53...@comcast.com%3e> apache.org/mod_mbox/kafka-dev/201808.mbox/<17E8D696-E51C-
>> 4beb-bd70-9324d4b53...@comcast.com>>
>>> 
>>> Cheers,
>>> Rhys
>> 
>> 

[jira] [Resolved] (KAFKA-7147) Allow kafka-reassign-partitions.sh and kafka-log-dirs.sh to take admin client property file

[Dong Lin (JIRA)]

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

Dong Lin resolved KAFKA-7147.
-
Resolution: Fixed

> Allow kafka-reassign-partitions.sh and kafka-log-dirs.sh to take admin client 
> property file
> ---
>
> Key: KAFKA-7147
> URL: https://issues.apache.org/jira/browse/KAFKA-7147
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Dong Lin
>Assignee: Dong Lin
>Priority: Major
> Fix For: 2.1.0
>
>
> Currently both ReassignPartitionsCommand and LogDirsCommand instantiates 
> AdminClient using bootstrap.servers and client.id provided by the user. Since 
> it does not provide other ssl-related properties, these tools will not be 
> able to talk to broker over SSL.
> In order to solve this problem, these tools should allow users to provide 
> property file containing configs to be passed to AdminClient.



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

Re: [DISCUSS] KIP-347: Enable batching in FindCoordinatorRequest

[Ted Yu]

bq. this is the foundation of some later possible optimizations(enable
batching in *describeConsumerGroups ...*

*Can you say more why this change lays the foundation for the future
optimizations ?*

*You mentioned **FIND_COORDINATOR_REQUEST_V3 in the wiki but I don't see it
in PR.*
*I assume you would add that later.*

*Please read your wiki and fix grammatical error such as the following:*

bq. that need to be make

Thanks

On Wed, Aug 8, 2018 at 3:55 PM Yishun Guan  wrote:

> Hi all,
>
> I would like to start a discussion on:
>
> KIP-347: Enable batching in FindCoordinatorRequest
> https://cwiki.apache.org/confluence/x/CgZPBQ
>
> Thanks @Guozhang Wang  for his help and patience!
>
> Thanks,
> Yishun
>

Re: [VOTE] KIP-280: Enhanced log compaction

[Jason Gustafson]

Hi Luis,

It's still not very clear to me why we need the header-based strategy. Can
you elaborate why having the timestamp-based approach alone is not
sufficient? The use case in the motivation just describes a "most recent
snapshot" use case.

Thanks,
Jason

On Thu, Aug 9, 2018 at 4:36 AM, Luís Cabral 
wrote:

> Hi,
>
>
> So, after a "short" break, I've finally managed to find time to resume
> this KIP. Sorry to all for the delay.
>
> Continuing the conversation of the configurations being global vs  topic,
> I've checked this and it seems that they are only available globally.
>
> This configuration is passed to the log cleaner via "CleanerConfig.scala",
> which only accepts global configurations. This seems intentional, as the
> log cleaner is not mutable and doesn't get instantiated that often. I think
> that changing this to accept per-topic configuration would be very nice,
> but perhaps as a part of a different KIP.
>
>
> Following the Kafka documentation, these are the settings I'm referring to:
>
> -- --
>
> Updating Log Cleaner Configs
>
> Log cleaner configs may be updated dynamically at cluster-default
> level used by all brokers. The changes take effect on the next iteration of
> log cleaning. One or more of these configs may be updated:
>
> * log.cleaner.threads
>
> * log.cleaner.io.max.bytes.per.second
>
> * log.cleaner.dedupe.buffer.size
>
> * log.cleaner.io.buffer.size
>
> * log.cleaner.io.buffer.load.factor
>
> * log.cleaner.backoff.ms
>
> -- --
>
>
>
> Please feel free to confirm, otherwise I will update the KIP to reflect
> these configuration nuances in the next few days.
>
>
> Best Regards,
>
> Luis
>
>
>
> On Monday, July 9, 2018, 1:57:38 PM GMT+2, Andras Beni <
> andrasb...@cloudera.com.INVALID> wrote:
>
>
>
>
>
> Hi Luís,
>
> Can you please clarify how the header value has to be encoded in case log
> compaction strategy is 'header'. As I see current PR reads varLong in
> CleanerCache.extractVersion and read String and uses toLong in
> Cleaner.extractVersion while the KIP says no more than 'the header value
> (which must be of type "long")'.
>
> Otherwise +1 for the KIP
>
> As for current implementation: it seems in Cleaner class header key
> "version" is hardwired.
>
> Andras
>
>
>
> On Fri, Jul 6, 2018 at 10:36 PM Jun Rao  wrote:
>
> > Hi, Guozhang,
> >
> > For #4, what you suggested could make sense for timestamp based de-dup,
> but
> > not sure how general it is since the KIP also supports de-dup based on
> > header.
> >
> > Thanks,
> >
> > Jun
> >
> > On Fri, Jul 6, 2018 at 1:12 PM, Guozhang Wang 
> wrote:
> >
> > > Hello Jun,
> > > Thanks for your feedbacks. I'd agree on #3 that it's worth adding a
> > special
> > > check to not delete the last message, since although unlikely, it is
> > still
> > > possible that a new active segment gets rolled out but contains no data
> > > yet, and hence the actual last message in this case would be in a
> > > "compact-able" segment.
> > >
> > > For the second part of #4 you raised, maybe we could educate users to
> > set "
> > > message.timestamp.difference.max.ms" to be no larger than "
> > > log.cleaner.delete.retention.ms" (its default value is
> Long.MAX_VALUE)?
> > A
> > > more aggressive approach would be changing the default value of the
> > former
> > > to be the value of the latter if:
> > >
> > > 1. cleanup.policy = compact OR compact,delete
> > > 2. log.cleaner.compaction.strategy != offset
> > >
> > > Because in this case I think it makes sense to really allow users send
> > any
> > > data longer than "log.cleaner.delete.retention.ms", WDYT?
> > >
> > >
> > > Guozhang
> > >
> > >
> > > On Fri, Jul 6, 2018 at 11:51 AM, Jun Rao  wrote:
> > >
> > > > Hi, Luis,
> > > >
> > > > 1. The cleaning policy is configurable at both global and topic
> level.
> > > The
> > > > global one has the name log.cleanup.policy and the topic level has
> the
> > > name
> > > > cleanup.policy by just stripping the log prefix. We can probably do
> the
> > > > same for the new configs.
> > > >
> > > > 2. Since this KIP may require an admin to configure a larger dedup
> > buffer
> > > > size, it would be useful to document this impact in the wiki and the
> > > > release notes.
> > > >
> > > > 3. Yes, it's unlikely for the last message to be removed in the
> current
> > > > implementation since we never clean the active segment. However, in
> > > theory,
> > > > this can happen. So it would be useful to guard this explicitly.
> > > >
> > > > 4. Just thought about another issue. We probably want to be a bit
> > careful
> > > > with key deletion. Currently, one can delete a key by sending a
> message
> > > > with a delete tombstone (a null payload). To prevent a reader from
> > > missing
> > > > a deletion if it's removed too quickly, we depend on a configuration
> > > > log.cleaner.delete.retention.ms (defaults to 1 day). The delete
> > > tombstone
> > > > will only be physically removed from the log 

Re: [DISCUSS] KIP-342 Add Customizable SASL extensions to OAuthBearer authentication

[Ron Dagostino]

Hi Stanislav.  Here are a few KIP comments.

< Hi Rajini, Ron
>
> I've updated the KIP with the latest changes following our discussion.
> Please do give it a read. If you feel it is alright, I will follow up with
> a PR later.
>
> Best,
> Stanislav
>
> On Thu, Aug 9, 2018 at 10:09 AM Rajini Sivaram 
> wrote:
>
> > Hi Ron/Stansilav,
> >
> > OK, let's just go with 2. I think it would be better to add a
> > OAuth-specific extensions handler OAuthBearerExtensionsValidatorCallback
> > that
> > provides OAuthBearerToken.
> >
> > To summarise, we chose option 2 out of these four options:
> >
> >1. {OAuthBearerValidatorCallback, SaslExtensionsValidatorCallback} :
> We
> >don't want to use multiple ordered callbacks since we don't want the
> >context of one callback to come from another.callback,
> >2. OAuthBearerExtensionsValidatorCallback(OAuthBearerToken token,
> >SaslExtensions ext): This allows extensions to be validated using
> >context from the token, we are ok with this.
> >3. SaslExtensionsValidatorCallback(Map context,
> >SaslExtensions ext): This doesn't really offer any real advantage over
> > 2.
> >4. OAuthBearerValidatorCallback(String token, SaslExtensions ext): We
> >don't want token validator to see extensions since these are insecure
> > but
> >token validation needs to be secure. So we prefer to use a second
> > callback
> >handler to validate extensions after securely validating token.
> >
> >
> >
> > On Wed, Aug 8, 2018 at 8:52 PM, Ron Dagostino  wrote:
> >
> > > Hi Rajini.  I think we are considering the following two options.  Let
> me
> > > try to describe them along with their relative
> advantages/disadvantages.
> > >
> > > Option #1: Send two callbacks in a single array to the callback
> handler:
> > > ch.handle(new Callback[] {tokenCallback, extensionsCallback});
> > >
> > > Option #2: Send two callbacks separately, in two separate arrays, to
> the
> > > callback handler:
> > > ch.handle(new Callback[] {tokenCallback});
> > > ch.handle(new Callback[] {extensionsCallback});
> > >
> > > I actually don't see any objective disadvantage with #1.  If we don't
> get
> > > an exception then we know we have the information we need; if we do get
> > an
> > > exception then we can tell if the first callback was handled because
> > either
> > > its token() method or its errorStatus() method will return non-null; if
> > > both return null then we just send the token callback by itself and we
> > > don't publish any extension as negotiated properties.  There is no
> > > possibility of partial results, and I don't think there is a
> performance
> > > penalty due to potential re-validation here, either.
> > >
> > > I  see a subjective disadvantage with #1.  It feels awkward to me to
> > > provide the token as context for extension validation via the first
> > > callback.
> > >
> > > Actually, it just occurred to me why it feels awkward, and I think this
> > is
> > > an objective disadvantage of this approach.  It would be impossible to
> do
> > > extension validation in such a scenario without also doing token
> > validation
> > > first.  We are using the first callback as a way to provide context,
> but
> > we
> > > are also using that first callback to request token validation.  We are
> > > complecting two separate things -- context and a request for validation
> 

[jira] [Created] (KAFKA-7275) Prototype lock-free metrics

[John Roesler (JIRA)]

John Roesler created KAFKA-7275:
---

 Summary: Prototype lock-free metrics
 Key: KAFKA-7275
 URL: https://issues.apache.org/jira/browse/KAFKA-7275
 Project: Kafka
  Issue Type: Improvement
  Components: metrics, streams
Reporter: John Roesler
Assignee: John Roesler


Currently, we have to be a little conservative in how granularly we measure 
things to avoid heavy synchronization costs in the metrics.

It should be possible to refactor the thread-safe implementation to use 
volatile and java.util.concurrent.atomic instead and realize a pretty large 
performance improvement.

However, before investing too much time in it, we should run some benchmarks to 
gauge how much improvement we can expect.

I'd propose to run the benchmarks on trunk with debug turned on, and then to 
just remove all synchronization and run again to get an upper-bound performance 
improvement.

If the results are promising, we can start prototyping a lock-free 
implementation.



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

[jira] [Created] (KAFKA-7276) Consider using re2j to speed up regex operations

[Ted Yu (JIRA)]

Ted Yu created KAFKA-7276:
-

 Summary: Consider using re2j to speed up regex operations
 Key: KAFKA-7276
 URL: https://issues.apache.org/jira/browse/KAFKA-7276
 Project: Kafka
  Issue Type: Task
Reporter: Ted Yu


https://github.com/google/re2j

re2j claims to do linear time regular expression matching in Java.

Its benefit is most obvious for deeply nested regex (such as a | b | c | d).

We should consider using re2j to speed up regex operations.



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

Jenkins build is back to normal : kafka-trunk-jdk10 #393

[Apache Jenkins Server]

See 

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

[Apache Jenkins Server]

See 


Changes:

[lindong28] KAFKA-7147; ReassignPartitionsCommand should be able to connect to

--
[...truncated 1.54 MB...]

kafka.zk.KafkaZkClientTest > testCreateAndGetTopicPartitionStatesRaw PASSED

kafka.zk.KafkaZkClientTest > testLogDirGetters STARTED

kafka.zk.KafkaZkClientTest > testLogDirGetters PASSED

kafka.zk.KafkaZkClientTest > testSetGetAndDeletePartitionReassignment STARTED

kafka.zk.KafkaZkClientTest > testSetGetAndDeletePartitionReassignment PASSED

kafka.zk.KafkaZkClientTest > testIsrChangeNotificationsDeletion STARTED

kafka.zk.KafkaZkClientTest > testIsrChangeNotificationsDeletion PASSED

kafka.zk.KafkaZkClientTest > testGetDataAndVersion STARTED

kafka.zk.KafkaZkClientTest > testGetDataAndVersion PASSED

kafka.zk.KafkaZkClientTest > testGetChildren STARTED

kafka.zk.KafkaZkClientTest > testGetChildren PASSED

kafka.zk.KafkaZkClientTest > testSetAndGetConsumerOffset STARTED

kafka.zk.KafkaZkClientTest > testSetAndGetConsumerOffset PASSED

kafka.zk.KafkaZkClientTest > testClusterIdMethods STARTED

kafka.zk.KafkaZkClientTest > testClusterIdMethods PASSED

kafka.zk.KafkaZkClientTest > testEntityConfigManagementMethods STARTED

kafka.zk.KafkaZkClientTest > testEntityConfigManagementMethods PASSED

kafka.zk.KafkaZkClientTest > testUpdateLeaderAndIsr STARTED

kafka.zk.KafkaZkClientTest > testUpdateLeaderAndIsr PASSED

kafka.zk.KafkaZkClientTest > testUpdateBrokerInfo STARTED

kafka.zk.KafkaZkClientTest > testUpdateBrokerInfo PASSED

kafka.zk.KafkaZkClientTest > testCreateRecursive STARTED

kafka.zk.KafkaZkClientTest > testCreateRecursive PASSED

kafka.zk.KafkaZkClientTest > testGetConsumerOffsetNoData STARTED

kafka.zk.KafkaZkClientTest > testGetConsumerOffsetNoData PASSED

kafka.zk.KafkaZkClientTest > testDeleteTopicPathMethods STARTED

kafka.zk.KafkaZkClientTest > testDeleteTopicPathMethods PASSED

kafka.zk.KafkaZkClientTest > testSetTopicPartitionStatesRaw STARTED

kafka.zk.KafkaZkClientTest > testSetTopicPartitionStatesRaw PASSED

kafka.zk.KafkaZkClientTest > testAclManagementMethods STARTED

kafka.zk.KafkaZkClientTest > testAclManagementMethods PASSED

kafka.zk.KafkaZkClientTest > testPreferredReplicaElectionMethods STARTED

kafka.zk.KafkaZkClientTest > testPreferredReplicaElectionMethods PASSED

kafka.zk.KafkaZkClientTest > testPropagateLogDir STARTED

kafka.zk.KafkaZkClientTest > testPropagateLogDir PASSED

kafka.zk.KafkaZkClientTest > testGetDataAndStat STARTED

kafka.zk.KafkaZkClientTest > testGetDataAndStat PASSED

kafka.zk.KafkaZkClientTest > testReassignPartitionsInProgress STARTED

kafka.zk.KafkaZkClientTest > testReassignPartitionsInProgress PASSED

kafka.zk.KafkaZkClientTest > testCreateTopLevelPaths STARTED

kafka.zk.KafkaZkClientTest > testCreateTopLevelPaths PASSED

kafka.zk.KafkaZkClientTest > testIsrChangeNotificationGetters STARTED

kafka.zk.KafkaZkClientTest > testIsrChangeNotificationGetters PASSED

kafka.zk.KafkaZkClientTest > testLogDirEventNotificationsDeletion STARTED

kafka.zk.KafkaZkClientTest > testLogDirEventNotificationsDeletion PASSED

kafka.zk.KafkaZkClientTest > testGetLogConfigs STARTED

kafka.zk.KafkaZkClientTest > testGetLogConfigs PASSED

kafka.zk.KafkaZkClientTest > testBrokerSequenceIdMethods STARTED

kafka.zk.KafkaZkClientTest > testBrokerSequenceIdMethods PASSED

kafka.zk.KafkaZkClientTest > testCreateSequentialPersistentPath STARTED

kafka.zk.KafkaZkClientTest > testCreateSequentialPersistentPath PASSED

kafka.zk.KafkaZkClientTest > testConditionalUpdatePath STARTED

kafka.zk.KafkaZkClientTest > testConditionalUpdatePath PASSED

kafka.zk.KafkaZkClientTest > testDeleteTopicZNode STARTED

kafka.zk.KafkaZkClientTest > testDeleteTopicZNode PASSED

kafka.zk.KafkaZkClientTest > testDeletePath STARTED

kafka.zk.KafkaZkClientTest > testDeletePath PASSED

kafka.zk.KafkaZkClientTest > testGetBrokerMethods STARTED

kafka.zk.KafkaZkClientTest > testGetBrokerMethods PASSED

kafka.zk.KafkaZkClientTest > testCreateTokenChangeNotification STARTED

kafka.zk.KafkaZkClientTest > testCreateTokenChangeNotification PASSED

kafka.zk.KafkaZkClientTest > testGetTopicsAndPartitions STARTED

kafka.zk.KafkaZkClientTest > testGetTopicsAndPartitions PASSED

kafka.zk.KafkaZkClientTest > testRegisterBrokerInfo STARTED

kafka.zk.KafkaZkClientTest > testRegisterBrokerInfo PASSED

kafka.zk.KafkaZkClientTest > testConsumerOffsetPath STARTED

kafka.zk.KafkaZkClientTest > testConsumerOffsetPath PASSED

kafka.zk.KafkaZkClientTest > testControllerManagementMethods STARTED

kafka.zk.KafkaZkClientTest > testControllerManagementMethods PASSED

kafka.zk.KafkaZkClientTest > testTopicAssignmentMethods STARTED

kafka.zk.KafkaZkClientTest > testTopicAssignmentMethods PASSED

kafka.zk.KafkaZkClientTest > testPropagateIsrChanges STARTED

kafka.zk.KafkaZkClientTest > testPropagateIsrChanges PASSED

kafka.zk.KafkaZkClientTest > testControllerEpochMethods STARTED

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

[Apache Jenkins Server]

See 


Changes:

[lindong28] KAFKA-7147; ReassignPartitionsCommand should be able to connect to

--
[...truncated 877.49 KB...]

kafka.zookeeper.ZooKeeperClientTest > testConnectionTimeout STARTED

kafka.zookeeper.ZooKeeperClientTest > testConnectionTimeout PASSED

kafka.zookeeper.ZooKeeperClientTest > 
testBlockOnRequestCompletionFromStateChangeHandler STARTED

kafka.zookeeper.ZooKeeperClientTest > 
testBlockOnRequestCompletionFromStateChangeHandler PASSED

kafka.zookeeper.ZooKeeperClientTest > testUnresolvableConnectString STARTED

kafka.zookeeper.ZooKeeperClientTest > testUnresolvableConnectString PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetChildrenNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetChildrenNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testPipelinedGetData STARTED

kafka.zookeeper.ZooKeeperClientTest > testPipelinedGetData PASSED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChildChangeHandlerForChildChange 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChildChangeHandlerForChildChange 
PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetChildrenExistingZNodeWithChildren 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetChildrenExistingZNodeWithChildren 
PASSED

kafka.zookeeper.ZooKeeperClientTest > testSetDataExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testSetDataExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testMixedPipeline STARTED

kafka.zookeeper.ZooKeeperClientTest > testMixedPipeline PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetDataExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetDataExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testDeleteExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testDeleteExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testSessionExpiry STARTED

kafka.zookeeper.ZooKeeperClientTest > testSessionExpiry PASSED

kafka.zookeeper.ZooKeeperClientTest > testSetDataNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testSetDataNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testDeleteNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testDeleteNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testExistsExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testExistsExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testZooKeeperStateChangeRateMetrics 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testZooKeeperStateChangeRateMetrics PASSED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChangeHandlerForDeletion STARTED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChangeHandlerForDeletion PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetAclNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetAclNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testStateChangeHandlerForAuthFailure 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testStateChangeHandlerForAuthFailure 
PASSED

kafka.network.SocketServerTest > testGracefulClose STARTED

kafka.network.SocketServerTest > testGracefulClose PASSED

kafka.network.SocketServerTest > 
testSendActionResponseWithThrottledChannelWhereThrottlingAlreadyDone STARTED

kafka.network.SocketServerTest > 
testSendActionResponseWithThrottledChannelWhereThrottlingAlreadyDone PASSED

kafka.network.SocketServerTest > controlThrowable STARTED

kafka.network.SocketServerTest > controlThrowable PASSED

kafka.network.SocketServerTest > testRequestMetricsAfterStop STARTED

kafka.network.SocketServerTest > testRequestMetricsAfterStop PASSED

kafka.network.SocketServerTest > testConnectionIdReuse STARTED

kafka.network.SocketServerTest > testConnectionIdReuse PASSED

kafka.network.SocketServerTest > testClientDisconnectionUpdatesRequestMetrics 
STARTED

kafka.network.SocketServerTest > testClientDisconnectionUpdatesRequestMetrics 
PASSED

kafka.network.SocketServerTest > testProcessorMetricsTags STARTED

kafka.network.SocketServerTest > testProcessorMetricsTags PASSED

kafka.network.SocketServerTest > testMaxConnectionsPerIp STARTED

kafka.network.SocketServerTest > testMaxConnectionsPerIp PASSED

kafka.network.SocketServerTest > testConnectionId STARTED

kafka.network.SocketServerTest > testConnectionId PASSED

kafka.network.SocketServerTest > 
testBrokerSendAfterChannelClosedUpdatesRequestMetrics STARTED

kafka.network.SocketServerTest > 
testBrokerSendAfterChannelClosedUpdatesRequestMetrics PASSED

kafka.network.SocketServerTest > testNoOpAction STARTED

kafka.network.SocketServerTest > testNoOpAction PASSED

kafka.network.SocketServerTest > simpleRequest STARTED

kafka.network.SocketServerTest > simpleRequest PASSED

kafka.network.SocketServerTest > closingChannelException STARTED

kafka.network.SocketServerTest > closingChannelException PASSED

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

[Apache Jenkins Server]

See 


Changes:

[mjsax] KAFKA-6966: Extend TopologyDescription to better represent Source and

--
[...truncated 428.43 KB...]
kafka.zookeeper.ZooKeeperClientTest > testGetDataNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testConnectionTimeout STARTED

kafka.zookeeper.ZooKeeperClientTest > testConnectionTimeout PASSED

kafka.zookeeper.ZooKeeperClientTest > 
testBlockOnRequestCompletionFromStateChangeHandler STARTED

kafka.zookeeper.ZooKeeperClientTest > 
testBlockOnRequestCompletionFromStateChangeHandler PASSED

kafka.zookeeper.ZooKeeperClientTest > testUnresolvableConnectString STARTED

kafka.zookeeper.ZooKeeperClientTest > testUnresolvableConnectString PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetChildrenNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetChildrenNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testPipelinedGetData STARTED

kafka.zookeeper.ZooKeeperClientTest > testPipelinedGetData PASSED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChildChangeHandlerForChildChange 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChildChangeHandlerForChildChange 
PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetChildrenExistingZNodeWithChildren 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetChildrenExistingZNodeWithChildren 
PASSED

kafka.zookeeper.ZooKeeperClientTest > testSetDataExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testSetDataExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testMixedPipeline STARTED

kafka.zookeeper.ZooKeeperClientTest > testMixedPipeline PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetDataExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetDataExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testDeleteExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testDeleteExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testSessionExpiry STARTED

kafka.zookeeper.ZooKeeperClientTest > testSessionExpiry PASSED

kafka.zookeeper.ZooKeeperClientTest > testSetDataNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testSetDataNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testDeleteNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testDeleteNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testExistsExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testExistsExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testZooKeeperStateChangeRateMetrics 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testZooKeeperStateChangeRateMetrics PASSED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChangeHandlerForDeletion STARTED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChangeHandlerForDeletion PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetAclNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetAclNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testStateChangeHandlerForAuthFailure 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testStateChangeHandlerForAuthFailure 
PASSED

kafka.network.SocketServerTest > testGracefulClose STARTED

kafka.network.SocketServerTest > testGracefulClose PASSED

kafka.network.SocketServerTest > 
testSendActionResponseWithThrottledChannelWhereThrottlingAlreadyDone STARTED

kafka.network.SocketServerTest > 
testSendActionResponseWithThrottledChannelWhereThrottlingAlreadyDone PASSED

kafka.network.SocketServerTest > controlThrowable STARTED

kafka.network.SocketServerTest > controlThrowable PASSED

kafka.network.SocketServerTest > testRequestMetricsAfterStop STARTED

kafka.network.SocketServerTest > testRequestMetricsAfterStop PASSED

kafka.network.SocketServerTest > testConnectionIdReuse STARTED

kafka.network.SocketServerTest > testConnectionIdReuse PASSED

kafka.network.SocketServerTest > testClientDisconnectionUpdatesRequestMetrics 
STARTED

kafka.network.SocketServerTest > testClientDisconnectionUpdatesRequestMetrics 
PASSED

kafka.network.SocketServerTest > testProcessorMetricsTags STARTED

kafka.network.SocketServerTest > testProcessorMetricsTags PASSED

kafka.network.SocketServerTest > testMaxConnectionsPerIp STARTED

kafka.network.SocketServerTest > testMaxConnectionsPerIp PASSED

kafka.network.SocketServerTest > testConnectionId STARTED

kafka.network.SocketServerTest > testConnectionId PASSED

kafka.network.SocketServerTest > 
testBrokerSendAfterChannelClosedUpdatesRequestMetrics STARTED

kafka.network.SocketServerTest > 
testBrokerSendAfterChannelClosedUpdatesRequestMetrics PASSED

kafka.network.SocketServerTest > testNoOpAction STARTED

kafka.network.SocketServerTest > testNoOpAction PASSED

kafka.network.SocketServerTest > simpleRequest STARTED

kafka.network.SocketServerTest > simpleRequest PASSED

kafka.network.SocketServerTest > closingChannelException STARTED

[jira] [Resolved] (KAFKA-7140) Remove deprecated poll usages

[Jason Gustafson (JIRA)]

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

Jason Gustafson resolved KAFKA-7140.

Resolution: Fixed

> Remove deprecated poll usages
> -
>
> Key: KAFKA-7140
> URL: https://issues.apache.org/jira/browse/KAFKA-7140
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Viktor Somogyi
>Assignee: Viktor Somogyi
>Priority: Minor
>
> There are a couple of poll(long) usages of the consumer in test and non-test 
> code. This jira would aim to remove the non-test usages of the method.



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

Re: [DISCUSS] KIP-346 - Limit blast radius of log compaction failure

[Stanislav Kozlovski]

Hey Jason,

My initial suggestion was to *track *the uncleanable disk space.
I can see why marking a log directory as offline after a certain threshold
of uncleanable disk space is more useful.
I'm not sure if we can set that threshold to be of certain size (e.g 100GB)
as log directories might have different sizes.  Maybe a percentage would be
better then (e.g 30% of whole log dir size), WDYT?

I feel it still makes sense to have a metric tracking how many uncleanable
partitions there are and the total amount of uncleanable disk space (per
log dir, via a JMX tag).
But now, rather than fail the log directory after a certain count of
uncleanable partitions, we could fail it after a certain percentage (or
size) of its storage is uncleanable.

I'd like to hear other people's thoughts on this. Sound good?

Best,
Stanislav




On Fri, Aug 10, 2018 at 12:40 AM Jason Gustafson  wrote:

> Hey Stanislav,
>
> Sorry, I was probably looking at an older version (I had the tab open for
> so long!).
>
> I have been thinking about `max.uncleanable.partitions` and wondering if
> it's what we really want. The main risk if the cleaner cannot clean a
> partition is eventually running out of disk space. This is the most common
> problem we have seen with cleaner failures and it can happen even if there
> is just one uncleanable partition. We've actually seen cases in which a
> single __consumer_offsets grew large enough to fill a significant portion
> of the disk. The difficulty with allowing a system to run out of disk space
> before failing is that it makes recovery difficult and time consuming.
> Clean shutdown, for example, requires writing some state to disk. Without
> clean shutdown, it can take the broker significantly longer to startup
> because it has do more segment recovery.
>
> For this problem, `max.uncleanable.partitions` does not really help. You
> can set it to 1 and fail fast, but that is not much better than the
> existing state. You had a suggestion previously in the KIP to use the size
> of uncleanable disk space instead. What was the reason for rejecting that?
> Intuitively, it seems like a better fit for a cleaner failure. It would
> provide users some time to react to failures while still protecting them
> from exhausting the disk.
>
> Thanks,
> Jason
>
>
>
>
> On Thu, Aug 9, 2018 at 9:46 AM, Stanislav Kozlovski <
> stanis...@confluent.io>
> wrote:
>
> > Hey Jason,
> >
> > 1. *10* is the default value, it says so in the KIP
> > 2. This is a good catch. As the current implementation stands, it's not a
> > useful metric since the thread continues to run even if all log
> directories
> > are offline (although I'm not sure what the broker's behavior is in that
> > scenario). I'll make sure the thread stops if all log directories are
> > online.
> >
> > I don't know which "Needs Discussion" item you're referencing, there
> hasn't
> > been any in the KIP since August 1 and that was for the metric only. KIP
> > History
> >  > pageId=89064875>
> >
> > I've updated the KIP to mention the "time-since-last-run" metric.
> >
> > Thanks,
> > Stanislav
> >
> > On Wed, Aug 8, 2018 at 12:12 AM Jason Gustafson 
> > wrote:
> >
> > > Hi Stanislav,
> > >
> > > Just a couple quick questions:
> > >
> > > 1. I may have missed it, but what will be the default value for
> > > `max.uncleanable.partitions`?
> > > 2. It seems there will be some impact for users that monitoring
> > > "time-since-last-run-ms" in order to detect cleaner failures. Not sure
> > it's
> > > a major concern, but probably worth mentioning in the compatibility
> > > section. Also, is this still a useful metric after this KIP?
> > >
> > > Also, maybe the "Needs Discussion" item can be moved to rejected
> > > alternatives since you've moved to a vote? I think leaving this for
> > > potential future work is reasonable.
> > >
> > > Thanks,
> > > Jason
> > >
> > >
> > > On Mon, Aug 6, 2018 at 12:29 PM, Ray Chiang 
> wrote:
> > >
> > > > I'm okay with that.
> > > >
> > > > -Ray
> > > >
> > > > On 8/6/18 10:59 AM, Colin McCabe wrote:
> > > >
> > > >> Perhaps we could start with max.uncleanable.partitions and then
> > > implement
> > > >> max.uncleanable.partitions.per.logdir in a follow-up change if it
> > seemed
> > > >> to be necessary?  What do you think?
> > > >>
> > > >> regards,
> > > >> Colin
> > > >>
> > > >>
> > > >> On Sat, Aug 4, 2018, at 10:53, Stanislav Kozlovski wrote:
> > > >>
> > > >>> Hey Ray,
> > > >>>
> > > >>> Thanks for the explanation. In regards to the configuration
> property
> > -
> > > >>> I'm
> > > >>> not sure. As long as it has sufficient documentation, I find
> > > >>> "max.uncleanable.partitions" to be okay. If we were to add the
> > > >>> distinction
> > > >>> explicitly, maybe it should be `max.uncleanable.partitions.
> > per.logdir`
> > > ?
> > > >>>
> > > >>> On Thu, Aug 2, 2018 at 7:32 PM Ray Chiang 
> > wrote:
> > > >>>
> > > >>> One more thing occurred to 

Re: [EXTERNAL] [VOTE] KIP-310: Add a Kafka Source Connector to Kafka Connect

[Stephane Maarek]

Hi Rhys,

Overall I'm +1 (non binding), but you're going to need 3 binding votes for
this KIP to pass.
I don't feel there has been enough discussion on this from the community.
Can we get some input from other people?

Thanks for starting the vote nonetheless :)
Stephane

On 8 August 2018 at 20:28, McCaig, Rhys  wrote:

> Hi
>
> Could we get a couple of votes on this KIP - voting closes in 24 hours.
>
> Thanks,
>
> Rhys
>
> > On Aug 6, 2018, at 11:51 AM, McCaig, Rhys 
> wrote:
> >
> > Hi All,
> >
> > I’m starting a vote on KIP-310: Add a Kafka Source Connector to Kafka
> Connect
> >
> > KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 310%3A+Add+a+Kafka+Source+Connector+to+Kafka+Connect ps://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 310:+Add+a+Kafka+Source+Connector+to+Kafka+Connect>
> > Discussion Thread: http://mail-archives.apache.
> org/mod_mbox/kafka-dev/201808.mbox/%3c17E8D696-E51C-4BEB-
> bd70-9324d4b53...@comcast.com%3e apache.org/mod_mbox/kafka-dev/201808.mbox/<17E8D696-E51C-
> 4beb-bd70-9324d4b53...@comcast.com>>
> >
> > Cheers,
> > Rhys
>
>

[jira] [Resolved] (KAFKA-6701) synchronize Log modification between delete cleanup and async delete

[Dong Lin (JIRA)]

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

Dong Lin resolved KAFKA-6701.
-
Resolution: Fixed

The issue appears to have been fixed in 
https://issues.apache.org/jira/browse/KAFKA-5163. More specifically, 
https://issues.apache.org/jira/browse/KAFKA-5163 added method `Log.renameDir()` 
and this method will grab the per-log lock before making modification to the 
log's directory etc.

> synchronize Log modification between delete cleanup and async delete
> 
>
> Key: KAFKA-6701
> URL: https://issues.apache.org/jira/browse/KAFKA-6701
> Project: Kafka
>  Issue Type: Bug
>Reporter: Sumant Tambe
>Assignee: Sumant Tambe
>Priority: Major
>
> Kafka broker crashes without any evident disk failures 
> From [~becket_qin]: This looks a bug in kafka when topic deletion and log 
> retention cleanup happen at the same time, the log retention cleanup may see 
> ClosedChannelException after the log has been renamed for async deletion.
> The root cause is that the topic deletion should have set the isClosed flag 
> of the partition log to true and the retention should not bother to do the 
> old log segments deletion when the log is closed.



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

Re: [DISCUSS] KIP-317: Transparent Data Encryption

[Sönke Liebau]

Hi Viktor,

thanks for your input! We could accommodate magic headers by removing any
known fixed bytes pre-encryption, sticking them in a header field and
prepending them after decryption. However, I am not sure whether this is
actually necessary, as most modern (AES for sure) algorithms are considered
to be resistant to known-plaintext types of attack. Even if the entire
plaintext is known to the attacker he still needs to brute-force the key -
which may take a while.

Something different to consider in this context are compression sidechannel
attacks like CRIME or BREACH, which may be relevant depending on what type
of data is being sent through Kafka. Both these attacks depend on the
encrypted record containing a combination of secret and user controlled
data.
For example if Kafka was used to forward data that the user entered on a
website along with a secret API key that the website adds to a back-end
server and the user can obtain the Kafka messages, these attacks would
become relevant. Not much we can do about that except disallow encryption
when compression is enabled (TLS chose this approach in version 1.3)

I agree with you, that we definitely need to clearly document any risks and
how much security can reasonably be expected in any given scenario. We
might even consider logging a warning message when sending data that is
compressed and encrypted.

On a different note, I've started amending the KIP to make key management
and distribution pluggable, should hopefully be able to publish sometime
Monday.

Best regards,
Sönke


On Thu, Jun 21, 2018 at 12:26 PM, Viktor Somogyi 
wrote:

> Hi Sönke,
>
> Compressing before encrypting has its dangers as well. Suppose you have a
> known compression format which adds a magic header and you're using a block
> cipher with a small enough block, then it becomes much easier to figure out
> the encryption key. For instance you can look at Snappy's stream
> identifier: https://github.com/google/snappy/blob/master/framing_
> format.txt
> . Based on this you should only use block ciphers where block sizes are
> much larger then 6 bytes. AES for instance should be good with its 128 bits
> = 16 bytes but even this isn't entirely secure as the first 6 bytes already
> leaked some information - and it depends on the cypher that how much it is.
> Also if we suppose that an adversary accesses a broker and takes all the
> data, they'll have much easier job to decrypt it as they'll have much more
> examples.
> So overall we should make sure to define and document the compatible
> encryptions with the supported compression methods and the level of
> security they provide to make sure the users are fully aware of the
> security implications.
>
> Cheers,
> Viktor
>
> On Tue, Jun 19, 2018 at 11:55 AM Sönke Liebau
>  wrote:
>
> > Hi Stephane,
> >
> > thanks for pointing out the broken pictures, I fixed those.
> >
> > Regarding encrypting before or after batching the messages, you are
> > correct, I had not thought of compression and how this changes things.
> > Encrypted data does not really encrypt well. My reasoning at the time
> > of writing was that if we encrypt the entire batch we'd have to wait
> > for the batch to be full before starting to encrypt. Whereas with per
> > message encryption we can encrypt them as they come in and more or
> > less have them ready for sending when the batch is complete.
> > However I think the difference will probably not be that large (will
> > do some testing) and offset by just encrypting once instead of many
> > times, which has a certain overhead every time. Also, from a security
> > perspective encrypting longer chunks of data is preferable - another
> > benefit.
> >
> > This does however take away the ability of the broker to see the
> > individual records inside the encrypted batch, so this would need to
> > be stored and retrieved as a single record - just like is done for
> > compressed batches. I am not 100% sure that this won't create issues,
> > especially when considering transactions, I will need to look at the
> > compression code some more. In essence though, since it works for
> > compression I see no reason why it can't be made to work here.
> >
> > On a different note, going down this route might make us reconsider
> > storing the key with the data, as this might significantly reduce
> > storage overhead - still much higher than just storing them once
> > though.
> >
> > Best regards,
> > Sönke
> >
> > On Tue, Jun 19, 2018 at 5:59 AM, Stephane Maarek
> >  wrote:
> > > Hi Sonke
> > >
> > > Very much needed feature and discussion. FYI the image links seem
> broken.
> > >
> > > My 2 cents (if I understood correctly): you say "This process will be
> > > implemented after Serializer and Interceptors are done with the message
> > > right before it is added to the batch to be sent, in order to ensure
> that
> > > existing serializers and interceptors keep working with encryption just
> > > like without it."
> > >
> > > I think 

Re: [DISCUSS] KIP-342 Add Customizable SASL extensions to OAuthBearer authentication

[Stanislav Kozlovski]

Hi Rajini, Ron

I've updated the KIP with the latest changes following our discussion.
Please do give it a read. If you feel it is alright, I will follow up with
a PR later.

Best,
Stanislav

On Thu, Aug 9, 2018 at 10:09 AM Rajini Sivaram 
wrote:

> Hi Ron/Stansilav,
>
> OK, let's just go with 2. I think it would be better to add a
> OAuth-specific extensions handler OAuthBearerExtensionsValidatorCallback
> that
> provides OAuthBearerToken.
>
> To summarise, we chose option 2 out of these four options:
>
>1. {OAuthBearerValidatorCallback, SaslExtensionsValidatorCallback} : We
>don't want to use multiple ordered callbacks since we don't want the
>context of one callback to come from another.callback,
>2. OAuthBearerExtensionsValidatorCallback(OAuthBearerToken token,
>SaslExtensions ext): This allows extensions to be validated using
>context from the token, we are ok with this.
>3. SaslExtensionsValidatorCallback(Map context,
>SaslExtensions ext): This doesn't really offer any real advantage over
> 2.
>4. OAuthBearerValidatorCallback(String token, SaslExtensions ext): We
>don't want token validator to see extensions since these are insecure
> but
>token validation needs to be secure. So we prefer to use a second
> callback
>handler to validate extensions after securely validating token.
>
>
>
> On Wed, Aug 8, 2018 at 8:52 PM, Ron Dagostino  wrote:
>
> > Hi Rajini.  I think we are considering the following two options.  Let me
> > try to describe them along with their relative advantages/disadvantages.
> >
> > Option #1: Send two callbacks in a single array to the callback handler:
> > ch.handle(new Callback[] {tokenCallback, extensionsCallback});
> >
> > Option #2: Send two callbacks separately, in two separate arrays, to the
> > callback handler:
> > ch.handle(new Callback[] {tokenCallback});
> > ch.handle(new Callback[] {extensionsCallback});
> >
> > I actually don't see any objective disadvantage with #1.  If we don't get
> > an exception then we know we have the information we need; if we do get
> an
> > exception then we can tell if the first callback was handled because
> either
> > its token() method or its errorStatus() method will return non-null; if
> > both return null then we just send the token callback by itself and we
> > don't publish any extension as negotiated properties.  There is no
> > possibility of partial results, and I don't think there is a performance
> > penalty due to potential re-validation here, either.
> >
> > I  see a subjective disadvantage with #1.  It feels awkward to me to
> > provide the token as context for extension validation via the first
> > callback.
> >
> > Actually, it just occurred to me why it feels awkward, and I think this
> is
> > an objective disadvantage of this approach.  It would be impossible to do
> > extension validation in such a scenario without also doing token
> validation
> > first.  We are using the first callback as a way to provide context, but
> we
> > are also using that first callback to request token validation.  We are
> > complecting two separate things -- context and a request for validation
> --
> > into one thing, so this approach has an element of complexity to it.
> >
> > The second option has no such complexity.  If we want to provide context
> to
> > the extension validation then we do that by adding a token to the
> > extensionsCallback instance before we provide it to the callback handler.
> > How we do that -- whether by Map or via a typed getter --
> > feels like a subjective decision, and assuming you agree with the
> > complexity argument and choose option #2, I would defer to your
> preference
> > as to how to implement it.
> >
> > Ron
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > On Wed, Aug 8, 2018 at 3:10 PM Rajini Sivaram 
> > wrote:
> >
> > > Hi Ron,
> > >
> > > Yes, I was thinking of a SaslExtensionsValidatorCallback with
> additional
> > > context as well initially, but I didn't like the idea of name-value
> pairs
> > > and I didn't want generic  objects passed around through the callback
> So
> > > providing context through other callbacks felt like a neater fit. There
> > > are pros and cons for both approaches, so we could go with either.
> > >
> > > Callbacks are provided to callback handlers in an array and there is
> > > implicit ordering in the callbacks provided to the callback handler.
> > > In the typical example of {NameCallback, PasswordCallback}, we expect
> > that
> > > ordering so that password callback knows what the user name is. Kafka
> > > guarantees ordering of server callbacks in each of its SASL mechanisms
> > and
> > > this is explicitly stated in
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 86%3A+Configurable+SASL+callback+handlers
> > > .
> > > Until now, we didn't need to worry about ordering for OAuth.
> > >
> > > We currently do not have any optional callbacks - configured callback
> > > handlers