Jenkins build is back to normal : kafka-2.4-jdk8 #236

2020-07-28 Thread Apache Jenkins Server
See 




Re: Build failed in Jenkins: kafka-trunk-jdk8 #4749

2020-07-28 Thread Benny Lee



From: Apache Jenkins Server 
Sent: Wednesday, 29 July 2020 2:12 PM
To: dev@kafka.apache.org 
Subject: Build failed in Jenkins: kafka-trunk-jdk8 #4749

See 


Changes:

[github] KAFKA-10224: Update jersey license from CDDL to EPLv2 (#9089)


--
[...truncated 2.76 MB...]
org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> joinShouldProduceNullsWhenValueHasNonMatchingForeignKey[leftJoin=false, 
optimization=none, materialized=false, rejoin=true] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> joinShouldProduceNullsWhenValueHasNonMatchingForeignKey[leftJoin=false, 
optimization=none, materialized=false, rejoin=true] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromLeftThenDeleteLeftEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=true] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromLeftThenDeleteLeftEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=true] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromRightThenDeleteRightEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=true] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromRightThenDeleteRightEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=true] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldUnsubscribeOldForeignKeyIfLeftSideIsUpdated[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldUnsubscribeOldForeignKeyIfLeftSideIsUpdated[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldNotEmitTombstonesWhenDeletingNonExistingRecords[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldNotEmitTombstonesWhenDeletingNonExistingRecords[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldEmitTombstoneWhenDeletingNonJoiningRecords[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldEmitTombstoneWhenDeletingNonJoiningRecords[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> joinShouldProduceNullsWhenValueHasNonMatchingForeignKey[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> joinShouldProduceNullsWhenValueHasNonMatchingForeignKey[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromLeftThenDeleteLeftEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromLeftThenDeleteLeftEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromRightThenDeleteRightEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromRightThenDeleteRightEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[at_least_once] STARTED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[at_least_once] PASSED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[exactly_once] STARTED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[exactly_once] PASSED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[exactly_once_beta] STARTED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[exactly_once_beta] PASSED


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

2020-07-28 Thread Apache Jenkins Server
See 


Changes:

[github] KAFKA-10224: Update jersey license from CDDL to EPLv2 (#9089)


--
[...truncated 2.76 MB...]
org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> joinShouldProduceNullsWhenValueHasNonMatchingForeignKey[leftJoin=false, 
optimization=none, materialized=false, rejoin=true] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> joinShouldProduceNullsWhenValueHasNonMatchingForeignKey[leftJoin=false, 
optimization=none, materialized=false, rejoin=true] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromLeftThenDeleteLeftEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=true] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromLeftThenDeleteLeftEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=true] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromRightThenDeleteRightEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=true] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromRightThenDeleteRightEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=true] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldUnsubscribeOldForeignKeyIfLeftSideIsUpdated[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldUnsubscribeOldForeignKeyIfLeftSideIsUpdated[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldNotEmitTombstonesWhenDeletingNonExistingRecords[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldNotEmitTombstonesWhenDeletingNonExistingRecords[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldEmitTombstoneWhenDeletingNonJoiningRecords[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> shouldEmitTombstoneWhenDeletingNonJoiningRecords[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> joinShouldProduceNullsWhenValueHasNonMatchingForeignKey[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> joinShouldProduceNullsWhenValueHasNonMatchingForeignKey[leftJoin=false, 
optimization=none, materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromLeftThenDeleteLeftEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromLeftThenDeleteLeftEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromRightThenDeleteRightEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=false] STARTED

org.apache.kafka.streams.integration.KTableKTableForeignKeyJoinIntegrationTest 
> doJoinFromRightThenDeleteRightEntity[leftJoin=false, optimization=none, 
materialized=false, rejoin=false] PASSED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[at_least_once] STARTED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[at_least_once] PASSED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[exactly_once] STARTED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[exactly_once] PASSED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[exactly_once_beta] STARTED

org.apache.kafka.streams.integration.SuppressionDurabilityIntegrationTest > 
shouldRecoverBufferAfterShutdown[exactly_once_beta] PASSED

org.apache.kafka.streams.integration.RocksDBMetricsIntegrationTest > 
shouldExposeRocksDBMetricsBeforeAndAfterFailureWithEmptyStateDir[at_least_once] 
STARTED

org.apache.kafka.streams.integration.RocksDBMetricsIntegrationTest > 

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-28 Thread Boyang Chen
Thanks for the feedback Colin!

On Tue, Jul 28, 2020 at 2:11 PM Colin McCabe  wrote:

> Hi Boyang,
>
> Thanks for updating this.  A few comments below:
>
> In the "Routing Request Security" section, there is a reference to "older
> requests that need redirection."  But after these new revisions, both new
> and old requests need redirection.  So we should rephrase this.
>
> > In addition, to avoid exposing this forwarding power to the admin
> clients,
> > the routing request shall be forwarded towards the controller broker
> internal
> > endpoint which should be only visible to other brokers inside the
> cluster
> > in the KIP-500 controller. Any admin configuration request with broker
> > principal should not be going through the public endpoint and will be
> > rejected for security purpose.
>
> We should also describe how this will work in the pre-KIP-500 case.  In
> that case, CLUSTER_ACTION gets the extra permissions described here only
> when the message comes in on the inter-broker listener.  We should state
> that here.
>
> (I can see that you have this information later on in the "Security Access
> Changes" section, but it would be good to have it here as well, to avoid
> confusion.)
>
> > To be more strict of protecting controller information, the
> "ControllerId"
> > field in new MetadataResponse shall be set to -1 when the original
> request
> > comes from a non-broker client and it is already on v10. We shall use the
> > request listener name to distinguish whether a given request is
> inter-broker,
> > or from the client.
>
> I'm not sure why we would need to distinguish between broker clients and
> non-broker clients.  Brokers don't generally send MetadataRequests to other
> brokers, do they?  Brokers learn about metadata from UpdateMetadataRequest
> and LeaderAndIsrRequest, not by sending MetadataRequests to other brokers.
>
> We do have one use case where the MetadataRequest gets sent between the
brokers, which is the InterBrokerSendThread. Currently we don't rely on it
to get the controller id, so I guess your suggestion should be good to
enforce. We could use some meta comment on the NetworkClient that it should
not be used to get the controller location.

Probably what we want here is: v0-v9: return a random broker in the cluster
> as the controller ID.  v10: no controllerID present in the
> MetadataResponse.  We should also deprecate the adminClient method which
> gets the controllerId.
>
> > BROKER_AUTHORIZATION_FAILURE(92, "Authorization failed for the
> > request during forwarding, this indicates an internal error on the broker
> > cluster security setup.", BrokerAuthorizationFailureException::new);
>
> Grammar nitpick: It would be good to have a period between "forwarding"
> and "this" to avoid a run-on sentence :)
>
> best,
> Colin
>
>
> On Mon, Jul 27, 2020, at 21:47, Boyang Chen wrote:
> > Hey there,
> >
> > I'm re-opening this thread because after some initial implementations
> > started, we spotted some gaps in the approved KIP as well as some
> > inconsistencies with KIP-631 controller. There are a couple of addendums
> to
> > the existing KIP, specifically:
> >
> > 1. As the controller is foreseen to be only accessible to the brokers,
> the
> > new admin client would not have direct access to the controller. It is
> > guaranteed on the MetadataResponse level which no longer provides
> > `ControllerId` to client side requests.
> >
> > 2. The broker would forward any direct ZK path mutation requests,
> including
> > topic creation/deletion, reassignment, etc since we deprecate the direct
> > controller access on the client side. No more protocol version bump is
> > necessary for the configuration requests.
> >
> > 3. To make sure forwarding requests pass the authorization, broker
> > principal CLUSTER_ACTION would be allowed to be used as an alternative
> > authentication method for a variety of principal operations, including
> > ALTER, ALTER_CONFIG, DELETE, etc. It is because the forwarding request
> > needs to use the proxy broker's own principal, which is currently not
> > supported to be used for many configuration change authentication listed
> > above. The full list could be found in the KIP.
> >
> > 4. Add a new BROKER_AUTHORIZATION_FAILURE error code to indicate any
> > internal security configuration failure, when the forwarded request
> failed
> > authentication on the controller side.
> >
> > Let me know what you think. With such a major refinement of the KIP, I'm
> > open for re-vote after discussions converge.
> >
> > Boyang
> >
> > On Wed, Jul 1, 2020 at 2:17 PM Boyang Chen 
> > wrote:
> >
> > > Hey folks,
> > >
> > > I have also synced on the KIP-578 which was doing the partition limit,
> to
> > > make sure the partition limit error code would be properly propagated
> once
> > > it is done on top of KIP-590. Let me know if you have further
> questions or
> > > concerns.
> > >
> > > Boyang
> > >
> > > On Tue, Jun 23, 2020 at 5:08 PM Boyang Chen <
> 

Re: [DISCUSS] KIP-630: Kafka Raft Snapshot

2020-07-28 Thread Jose Garcia Sancio
Thanks Ron. Your comments and suggestions were helpful. You can see my
changes to the KIP here:
https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158864763=15=14

My comments are below...

On Mon, Jul 27, 2020 at 11:29 AM Ron Dagostino  wrote:
>
> Hi Jose.  Thanks for the KIP.  Here are some questions and some nit 
> corrections.
>
> <<< In KIP-500 the Kafka Controller, which is the quorum leader from
> KIP-595, will materialize the entries in the metadata log into memory.
> Technically I think the quorum leader is referred to as the Active
> Controller in KIP-500.  Maybe replace "Kafka Controller" with "Active
> Controller"?  I think the term "Kafka Controller" is fine as used
> throughout the rest of the KIP to refer to the entire thing, but when
> referring specifically to the leader I think "Active Controller" is
> the term that is defined in KIP-500.
>

Made those changes.

>
> <<< Each broker in KIP-500, which will be a replica of the metadata
> log, will materialize the entries in the log into a metadata cache
> This wording confused me because I assumed that "replica" was a formal
> term and only (non-Active) Controllers are formally "replicas" of the
> metadata log -- Kafka brokers would be clients that read the log and
> then use the data for their own purpose as opposed to formally being
> replicas with this understanding of the term "replica".  Is that
> correct, and if so, maybe replace "replica" with "client"?
>

In KIP-595 we have two types of replicas: voters and observers. Voter
replicas are Kafka Controllers and one one of them will become the
Active controller. Observer replicas fetch from the log and attempt to
keep up with the LEO of the Active Controller. I think you can
consider all of them as "client" of the replicated log.

>
> <<< The type of in-memory state machines what we plan to implement
> >>> The type of in-memory state machines that we plan to implement
> nit
>

Done.

>
> <<< doesn't map very well to an key and offset based clean up policy.
> >>> doesn't map very well to a key and offset based clean up policy.
> nit
>

Done.

>
> <<< When starting a broker either because it is a new broker, a broker
> was upgraded or a failed broker is restarting. Loading the state
> represented by the __cluster_metadata topic partition is required
> before the broker is available
> >>> When starting a broker either because it is a new broker or it is 
> >>> restarting, loading the state represented by the __cluster_metadata topic 
> >>> partition is required before the broker is available.
> Reword for simplicity and clarity?
>

Done.

>
> <<< With snapshot based of the in-memory state Kafka can be much more 
> aggressive
> >>> By taking and transmitting a snapshot of the in-memory state as described 
> >>> below Kafka can be much more aggressive
> Tough to refer to the concept of snapshot here without having
> described what it is, so refer to "as described below" to help orient
> the reader?
>

Made some changes to these sentences. I agree that fully understanding
parts of the motivated section requires reading the rest of the
document. I wanted to make sure that we had this in the motivation
section.

>
> <<< In the future this mechanism will also be useful for
> high-throughput topic partitions like the Group Coordinator and
> Transaction Coordinator.
> >>> In the future this mechanism may also be useful for high-throughput topic 
> >>> partitions like the Group Coordinator and Transaction Coordinator.
> Tough to say "will" when that is an assumption that would depend on a KIP?
>

Yeah. Changed it.

>
> << __cluster_metadata topic partition then the ... Kafka Controller will
> need to replicate 3.81 MB to each broker in the cluster (10) or 38.14
> MB.
> It might be good to append a sentence that explicitly states how much
> data is replicated for the delta/event -- right now it is implied to
> be very small, but that's kind of like leaving the punch line to a
> joke implied :-)
>

Thanks. I updated the example and added numbers for the events/deltas case.


>
> <<< Follower and observer replicas fetch the snapshots from the leader
> they attempt to fetch an offset from the leader and the leader doesn’t
> have that offset in the log
> >>> Follower and observer replicas fetch a snapshot from the leader when they 
> >>> attempt to fetch an offset from the leader and the leader doesn’t have 
> >>> that offset in the log
> nit
>

Done.

>
> >>> Generating and loading the snapshot will be delegated to the Kafka 
> >>> Controller.
> >>> The Kafka Controller will notify the Kafka Raft client when it has 
> >>> generated a snapshot and up to which offset is included in the snapshot.
> >>> The Kafka Raft client will notify the Kafka Controller when a new 
> >>> snapshot has been fetched from the leader.
> This paragraph confuses me.  What is the "Kafka Raft client" -- is
> this the broker? Or is it some other subsystem (or all other
> subsystems aside from log 

Re: Re: [Discuss] KIP-581: Value of optional null field which has default value

2020-07-28 Thread 379377...@qq.com
Hi Chris,

Thanks for your good suggestion, the KIP document and draft PR has been 
updated, please review again.

And I found due to my misoperation, the mail thread has been broken, no idea 
how to fix it.




Thanks
Cheng Pan
 
From: Christopher Egerton
Date: 2020-05-04 10:53
To: dev
Subject: Re: [Discuss] KIP-581: Value of optional null field which has default 
value
Hi Cheng,
 
I think refactoring that method should be fine (if maybe a little painful);
the method itself is private and all places that it's invoked directly are
either package-private or non-static, so it shouldn't affect any of the
public methods of the JSON converter to change "convertToConnect" to be
non-static. Even if it did, the only parts of the JSON converter that are
public API (and therefore officially subject to concerns about
compatibility) are the methods it implements that satisfy the "Converter"
and "HeaderConverter" interfaces.
 
Would you mind explicitly specifying in the KIP that the new property will
be added for the JSON converter only, and that it will affect both
serialization and deserialization?
 
Cheers,
 
Chris
 
On Tue, Apr 28, 2020 at 10:52 AM 379377944 <379377...@qq.com> wrote:
 
> Hi Chris,
>
>
> Thanks for your reminder, the original implement is deprecated, I just
> update the JIRA with the new
> PR link:  https://github.com/apache/kafka/pull/8575
>
>
> As question 2), I agree with you that we should consider both
> serialization and deserialization, and as you said, I only implement the
> serialization now. This is  because the original serde implement is not
> symmetrical, the convertToConnect is a static method and can’t access the
> field in JsonConverter
> instance, maybe I should do some refactoring to implement the
> deserialization.
>
>
> Thanks,
> Cheng Pan
>  Original Message
> Sender: Christopher Egerton
> Recipient: dev
> Date: Wednesday, Apr 15, 2020 02:28
> Subject: Re: [Discuss] KIP-581: Value of optional null field which has
> default value
>
>
> Hi Cheng, Thanks for the KIP! I really appreciate the care that was taken
> to ensure backwards compatibility for existing users, and the minimal
> changes to public interface that are suggested to address this. I have two
> quick requests for clarification: 1) Where is the proposed
> "accept.optional.null" property going to apply? It's hinted that it'll take
> effect on the JSON converter but not actually called out anywhere. 2)
> Assuming this takes effect on the JSON converter, is the intent to alter
> the semantics for both serialization and deserialization? The code snippet
> from the JSON converter that's included in the KIP comes from the
> "convertToJson" method, which is used for serialization. However, based on
> https://github.com/apache/kafka/blob/ea47a885b1fe47dfb87c1dc86db1b0e7eb8a273c/connect/json/src/main/java/org/apache/kafka/connect/json/JsonConverter.java#L712-L713
> it looks like the converter also inserts the default value for
> optional-but-null data during deserialization. Thanks again for the KIP!
> Cheers, Chris On Wed, Mar 18, 2020 at 12:00 AM Cheng Pan <379377...@qq.com>
> wrote: > Hi all, > > I'd like to use this thread to discuss KIP-581: Value
> of optional null > field which has default value, please see detail at: >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-581%3A+Value+of+optional+null+field+which+has+default+value
> > > > There are some previous discussion at: >
> https://github.com/apache/kafka/pull/7112 > > > I'm a beginner for apache
> project, please let me know if I did any thing > wrong. > > > Best regards,
> > Cheng Pan


Jenkins build is back to normal : kafka-2.5-jdk8 #173

2020-07-28 Thread Apache Jenkins Server
See 




Build failed in Jenkins: kafka-2.6-jdk8 #102

2020-07-28 Thread Apache Jenkins Server
See 


Changes:

[rhauch] KAFKA-10224: Update jersey license from CDDL to EPLv2 (#9089)


--
[...truncated 6.30 MB...]
org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldRequireCustomTopicNameIfNotDefaultFactoryTopicName STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldRequireCustomTopicNameIfNotDefaultFactoryTopicName PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldRequireCustomTopicNameIfNotDefaultFactoryTopicNameWithNullKey STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldRequireCustomTopicNameIfNotDefaultFactoryTopicNameWithNullKey PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateConsumerRecord STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateConsumerRecord PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateNullKeyConsumerRecord STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateNullKeyConsumerRecord PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateConsumerRecordWithOtherTopicName STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateConsumerRecordWithOtherTopicName PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > shouldAdvanceTime 
STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > shouldAdvanceTime 
PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldNotAllowToCreateTopicWithNullTopicNameWithKeyValuePairs STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldNotAllowToCreateTopicWithNullTopicNameWithKeyValuePairs PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldRequireCustomTopicNameIfNotDefaultFactoryTopicNameWithKeyValuePairsAndCustomTimestamps
 STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldRequireCustomTopicNameIfNotDefaultFactoryTopicNameWithKeyValuePairsAndCustomTimestamps
 PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldRequireCustomTopicNameIfNotDefaultFactoryTopicNameWithKeyValuePairs 
STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldRequireCustomTopicNameIfNotDefaultFactoryTopicNameWithKeyValuePairs PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateConsumerRecordWithOtherTopicNameAndTimestamp STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateConsumerRecordWithOtherTopicNameAndTimestamp PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldNotAllowToCreateTopicWithNullTopicNameWithKeyValuePairsAndCustomTimestamps
 STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldNotAllowToCreateTopicWithNullTopicNameWithKeyValuePairsAndCustomTimestamps
 PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldNotAllowToCreateTopicWithNullTopicName STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldNotAllowToCreateTopicWithNullTopicName PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateConsumerRecordsFromKeyValuePairsWithCustomTimestampAndIncrementsAndNotAdvanceTime
 STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateConsumerRecordsFromKeyValuePairsWithCustomTimestampAndIncrementsAndNotAdvanceTime
 PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateNullKeyConsumerRecordWithTimestampWithTimestamp STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateNullKeyConsumerRecordWithTimestampWithTimestamp PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldRequireCustomTopicNameIfNotDefaultFactoryTopicNameWithNullKeyAndDefaultTimestamp
 STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldRequireCustomTopicNameIfNotDefaultFactoryTopicNameWithNullKeyAndDefaultTimestamp
 PASSED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateConsumerRecordsFromKeyValuePairsWithTimestampAndIncrements STARTED

org.apache.kafka.streams.test.ConsumerRecordFactoryTest > 
shouldCreateConsumerRecordsFromKeyValuePairsWithTimestampAndIncrements PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldReturnIsOpen 
STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldReturnIsOpen 
PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldReturnName 
STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldReturnName 
PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > 
shouldPutWithUnknownTimestamp STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > 
shouldPutWithUnknownTimestamp PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > 

Re: [VOTE] KIP-450: Sliding Window Aggregations in the DSL

2020-07-28 Thread Guozhang Wang
+1 (binding)

On Tue, Jul 28, 2020 at 4:44 PM Matthias J. Sax  wrote:

> +1 (binding)
>
> On 7/28/20 4:35 PM, Sophie Blee-Goldman wrote:
> > Thanks for the KIP! It's been an enlightening discussion
> >
> > +1 (non-binding)
> >
> > Sophie
> >
> > On Tue, Jul 28, 2020 at 8:03 AM Leah Thomas 
> wrote:
> >
> >> Hi all,
> >>
> >> I'd like to kick-off the vote for KIP-450
> >> <
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-450%3A+Sliding+Window+Aggregations+in+the+DSL
> >>> ,
> >> adding sliding window aggregations to the DSL. The discussion thread is
> >> here
> >> <
> >>
> http://mail-archives.apache.org/mod_mbox/kafka-dev/202007.mbox/%3ccabug4nfjkrroe_rf4ht2p1wu1pt7o-qd74h_0l7a4bnsmmg...@mail.gmail.com%3e
> >>>
> >> .
> >>
> >> Cheers,
> >> Leah
> >>
> >
>
>

-- 
-- Guozhang


Re: [DISCUSS] KIP-450: Sliding Windows

2020-07-28 Thread Matthias J. Sax
Thanks Leah. Overall LGTM.

A few nits:

- the first figure shows window [9,19] but the window is not aligned
properly (it should be 1ms to the right; right now, it's aligned to
window [8,18])

- in the second figure, a hopping window would create more windows, ie,
the first window would be [-6,14) and the last window would be [19,29),
thus it's not just 10 windows but 26 windows (if I did not miss count)

- "Two new windows will be created by the late record"

late -> out-of-order


-Matthias



On 7/28/20 4:34 PM, Sophie Blee-Goldman wrote:
> Thanks for the update Leah -- I think that all makes sense.
> 
> Cheers,
> Sophie
> 
> On Tue, Jul 28, 2020 at 3:55 PM Leah Thomas  wrote:
> 
>> Another minor tweak, instead of defining the window by the *size*, it will
>> be defined by *timeDifference*, which is the maximum time difference
>> between two events. This is a more precise way to define a window due to
>> its inclusive ends, while allowing the user to create the window they
>> expect. This definition fits with the current examples, where a record at
>> *10* would fall into a window of time difference *5* from *[5,10]*. This
>> window contains any records at 5, 6, 7, 8, 9, and 10, which is 6 instances
>> instead of 5. This semantic difference is why I've shifted *size* to
>> *timeDifference*.
>>
>> The new builder will be *withTimeDifferenceAndGrace*, keeping with other
>> conventions.
>>
>> Let me know if there are any concerns! The vote thread is open as well
>> here:
>> http://mail-archives.apache.org/mod_mbox/kafka-dev/202007.mbox/browser
>>
>> Best,
>> Leah
>>
>> On Mon, Jul 27, 2020 at 3:58 PM Leah Thomas  wrote:
>>
>>> A small tweak - to make it more clear to users that grace is required, as
>>> well as cleaning up some of the confusing grammar semantics of windows,
>> the
>>> main builder method for *slidingWindows* will be *withSizeAndGrace*
>> instead
>>> of *of*.  This looks like it'll be the last change (for now) on the
>>> public API. If anyone has any comments or thoughts let me know.
>> Otherwise,
>>> I'll take this to vote shortly.
>>>
>>> Best,
>>> Leah
>>>
>>> On Fri, Jul 24, 2020 at 3:45 PM Leah Thomas 
>> wrote:
>>>
 To accommodate the change to a final class, I've added another
 *windowedBy()* function in *CogroupedKStream.java *to handle
 SlidingWindows.

 As far as the discussion goes, I think this is the last change we've
 talked about. If anyone has other comments or concerns, please let me
>> know!

 Cheers,
 Leah

 On Thu, Jul 23, 2020 at 7:34 PM Leah Thomas 
>> wrote:

> Thanks for the discussion about extending TimeWindows. I agree that
> making it future proof is important, and the implementation of
> SlidingWindows is unique enough that it seems logical to make it its
>> own
> final class.
>
> On that note, I've updated the KIP to make SlidingWindows a stand alone
> final class, and added the *windowedBy() *API in *KGroupedStream *to
> handle SlidingWindows. It seems that SlidingWindows would still be
>> able to
> leverage *TimeWindowedKStream by* creating a SlidingWindows version of
> *TimeWindowedKStreamImpl* that implements *TimeWindowedKStream. *If
> anyone sees issues with this implementation, please let me know.
>
> Best,
> Leah
>
> On Wed, Jul 22, 2020 at 10:47 PM John Roesler 
> wrote:
>
>> Thanks for the reply, Sophie.
>>
>> That all sounds about right to me.
>>
>> The Windows “interface”/algorithm is quite flexible, so it makes sense
>> for it to be extensible. Different implementations can (and do)
>> enumerate
>> different windows to suit different use cases.
>>
>> On the other hand, I can’t think of any way to extend SessionWindows
>> to
>> do something different using the same algorithm, so it makes sense
>> for it
>> to stay final.
>>
>> If we think SlidingWindows is similarly not usefully extensible, then
>> we can make it final. It’s easy to remove final later, but adding it
>> is not
>> possible. Or we could go the other route and just make it an
>> interface, on
>> general principle. Both of these choices are safe API design.
>>
>> Thanks again,
>> John
>>
>> On Wed, Jul 22, 2020, at 21:54, Sophie Blee-Goldman wrote:

 Users could pass in a custom `SessionWindows` as
 long as the session algorithm works correctly for it.
>>>
>>>
>>> Well not really, SessionWindows is a final class. TimeWindows is
>> also
>> a
>>> final class, so neither of these can be extended/customized. For a
>> given
>>> Windows then there would only be three (non-overlapping)
>> possibilities:
>>> either it's TimeWindows, SlidingWindows, or a custom  Windows. I
>> don't
>>> think there's any problem with determining what the user wants in
>> this case
>>> --
>>> we would just check if it's a SlidingWindows 

Re: [VOTE] KIP-450: Sliding Window Aggregations in the DSL

2020-07-28 Thread Matthias J. Sax
+1 (binding)

On 7/28/20 4:35 PM, Sophie Blee-Goldman wrote:
> Thanks for the KIP! It's been an enlightening discussion
> 
> +1 (non-binding)
> 
> Sophie
> 
> On Tue, Jul 28, 2020 at 8:03 AM Leah Thomas  wrote:
> 
>> Hi all,
>>
>> I'd like to kick-off the vote for KIP-450
>> <
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-450%3A+Sliding+Window+Aggregations+in+the+DSL
>>> ,
>> adding sliding window aggregations to the DSL. The discussion thread is
>> here
>> <
>> http://mail-archives.apache.org/mod_mbox/kafka-dev/202007.mbox/%3ccabug4nfjkrroe_rf4ht2p1wu1pt7o-qd74h_0l7a4bnsmmg...@mail.gmail.com%3e
>>>
>> .
>>
>> Cheers,
>> Leah
>>
> 



signature.asc
Description: OpenPGP digital signature


Re: [VOTE] KIP-450: Sliding Window Aggregations in the DSL

2020-07-28 Thread Sophie Blee-Goldman
Thanks for the KIP! It's been an enlightening discussion

+1 (non-binding)

Sophie

On Tue, Jul 28, 2020 at 8:03 AM Leah Thomas  wrote:

> Hi all,
>
> I'd like to kick-off the vote for KIP-450
> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-450%3A+Sliding+Window+Aggregations+in+the+DSL
> >,
> adding sliding window aggregations to the DSL. The discussion thread is
> here
> <
> http://mail-archives.apache.org/mod_mbox/kafka-dev/202007.mbox/%3ccabug4nfjkrroe_rf4ht2p1wu1pt7o-qd74h_0l7a4bnsmmg...@mail.gmail.com%3e
> >
> .
>
> Cheers,
> Leah
>


Re: [DISCUSS] KIP-450: Sliding Windows

2020-07-28 Thread Sophie Blee-Goldman
Thanks for the update Leah -- I think that all makes sense.

Cheers,
Sophie

On Tue, Jul 28, 2020 at 3:55 PM Leah Thomas  wrote:

> Another minor tweak, instead of defining the window by the *size*, it will
> be defined by *timeDifference*, which is the maximum time difference
> between two events. This is a more precise way to define a window due to
> its inclusive ends, while allowing the user to create the window they
> expect. This definition fits with the current examples, where a record at
> *10* would fall into a window of time difference *5* from *[5,10]*. This
> window contains any records at 5, 6, 7, 8, 9, and 10, which is 6 instances
> instead of 5. This semantic difference is why I've shifted *size* to
> *timeDifference*.
>
> The new builder will be *withTimeDifferenceAndGrace*, keeping with other
> conventions.
>
> Let me know if there are any concerns! The vote thread is open as well
> here:
> http://mail-archives.apache.org/mod_mbox/kafka-dev/202007.mbox/browser
>
> Best,
> Leah
>
> On Mon, Jul 27, 2020 at 3:58 PM Leah Thomas  wrote:
>
> > A small tweak - to make it more clear to users that grace is required, as
> > well as cleaning up some of the confusing grammar semantics of windows,
> the
> > main builder method for *slidingWindows* will be *withSizeAndGrace*
> instead
> > of *of*.  This looks like it'll be the last change (for now) on the
> > public API. If anyone has any comments or thoughts let me know.
> Otherwise,
> > I'll take this to vote shortly.
> >
> > Best,
> > Leah
> >
> > On Fri, Jul 24, 2020 at 3:45 PM Leah Thomas 
> wrote:
> >
> >> To accommodate the change to a final class, I've added another
> >> *windowedBy()* function in *CogroupedKStream.java *to handle
> >> SlidingWindows.
> >>
> >> As far as the discussion goes, I think this is the last change we've
> >> talked about. If anyone has other comments or concerns, please let me
> know!
> >>
> >> Cheers,
> >> Leah
> >>
> >> On Thu, Jul 23, 2020 at 7:34 PM Leah Thomas 
> wrote:
> >>
> >>> Thanks for the discussion about extending TimeWindows. I agree that
> >>> making it future proof is important, and the implementation of
> >>> SlidingWindows is unique enough that it seems logical to make it its
> own
> >>> final class.
> >>>
> >>> On that note, I've updated the KIP to make SlidingWindows a stand alone
> >>> final class, and added the *windowedBy() *API in *KGroupedStream *to
> >>> handle SlidingWindows. It seems that SlidingWindows would still be
> able to
> >>> leverage *TimeWindowedKStream by* creating a SlidingWindows version of
> >>> *TimeWindowedKStreamImpl* that implements *TimeWindowedKStream. *If
> >>> anyone sees issues with this implementation, please let me know.
> >>>
> >>> Best,
> >>> Leah
> >>>
> >>> On Wed, Jul 22, 2020 at 10:47 PM John Roesler 
> >>> wrote:
> >>>
>  Thanks for the reply, Sophie.
> 
>  That all sounds about right to me.
> 
>  The Windows “interface”/algorithm is quite flexible, so it makes sense
>  for it to be extensible. Different implementations can (and do)
> enumerate
>  different windows to suit different use cases.
> 
>  On the other hand, I can’t think of any way to extend SessionWindows
> to
>  do something different using the same algorithm, so it makes sense
> for it
>  to stay final.
> 
>  If we think SlidingWindows is similarly not usefully extensible, then
>  we can make it final. It’s easy to remove final later, but adding it
> is not
>  possible. Or we could go the other route and just make it an
> interface, on
>  general principle. Both of these choices are safe API design.
> 
>  Thanks again,
>  John
> 
>  On Wed, Jul 22, 2020, at 21:54, Sophie Blee-Goldman wrote:
>  > >
>  > > Users could pass in a custom `SessionWindows` as
>  > > long as the session algorithm works correctly for it.
>  >
>  >
>  > Well not really, SessionWindows is a final class. TimeWindows is
> also
>  a
>  > final class, so neither of these can be extended/customized. For a
>  given
>  > Windows then there would only be three (non-overlapping)
>  possibilities:
>  > either it's TimeWindows, SlidingWindows, or a custom  Windows. I
> don't
>  > think there's any problem with determining what the user wants in
>  this case
>  > --
>  > we would just check if it's a SlidingWindows and connect the new
>  processor,
>  > or else connect the existing hopping/tumbling window processor.
>  >
>  > I'll admit that last sentence does leave a bad taste in my mouth.
>  Part of
>  > that
>  > is probably the "leaking" API Matthias pointed out; we just assume
> the
>  > hopping/tumbling window implementation fits all custom windows. But
> I
>  guess
>  > if you really needed to customize the algorithm any further you may
>  as well
>  > stick in a transformer and do it all yourself.
>  >
>  > Anyways, given what 

[VOTE] KIP-648: Renaming getter method for Interactive Queries

2020-07-28 Thread John Thomas
Hello everyone,

I'd like to kick-off a vote for KIP-648 : Renaming getter method for 
Interactive Queries
https://cwiki.apache.org/confluence/display/KAFKA/KIP-648%3A+Renaming+getter+method+for+Interactive+Queries

It's a straight forward change to include new getters and deprecate the 
existing ones for the class KeyQueryMetadata.

Thanks,
John



Re: [VOTE] 2.6.0 RC1

2020-07-28 Thread Randall Hauch
I've announced RC2 on a different thread entitled "[VOTE] 2.6.0 RC2" (see
https://lists.apache.org/thread.html/rc8a3aa6986204adbb9ff326b8de849b3c8bac5b6b2b436e8143afea9%40%3Cdev.kafka.apache.org%3E).
Please use that thread to highlight any blockers with that release
candidate.

Best regards,

Randall

On Mon, Jul 27, 2020 at 12:55 PM Randall Hauch  wrote:

> Thanks, John. Looks like we're still trying to get a green build for
> https://github.com/apache/kafka/pull/9066.
>
> On Fri, Jul 24, 2020 at 3:46 PM John Roesler  wrote:
>
>> Hi Randall,
>>
>> I'm sorry to say we have also identified that this flaky test
>> failure turned out to be a real blocker bug:
>> https://issues.apache.org/jira/browse/KAFKA-10287
>>
>> There is a PR in progress.
>>
>> Thanks,
>> -John
>>
>> On Fri, Jul 24, 2020, at 12:26, Matthias J. Sax wrote:
>> > We found a regression bug that seems to be a blocker:
>> > https://issues.apache.org/jira/browse/KAFKA-10306
>> >
>> > Will work on a PR today.
>> >
>> >
>> > -Matthias
>> >
>> > On 7/22/20 9:40 AM, Randall Hauch wrote:
>> > > Any thoughts, Rajini?
>> > >
>> > > On Mon, Jul 20, 2020 at 9:55 PM Randall Hauch 
>> wrote:
>> > >
>> > >>
>> > >> When I was checking the documentation for RC1 after the tag was
>> pushed, I
>> > >> noticed that the fix Rajini mentioned in the RC0 vote thread (
>> > >> https://github.com/apache/kafka/pull/8979
>> > >> <
>> https://github.com/apache/kafka/pull/8979/files#diff-369f0debebfcda6709beeaf11612b34bR20-R21
>> >)
>> > >> and merged to the `2.6` branch includes the following comment about
>> being
>> > >> deprecated in 2.7:
>> > >>
>> https://github.com/apache/kafka/pull/8979/files#diff-369f0debebfcda6709beeaf11612b34bR20-R21
>> > >> .
>> > >>
>> > >> Rajini, can you please check the commits merged to the `2.6` do not
>> have
>> > >> the reference to 2.7? Since these are JavaDocs, I'm assuming that
>> we'll
>> > >> need to cut RC2.
>> > >>
>> > >> But it'd be good for everyone else to double check this release.
>> > >>
>> > >> Best regards,
>> > >>
>> > >> Randall Hauch
>> > >>
>> > >> On Mon, Jul 20, 2020 at 9:50 PM Randall Hauch 
>> wrote:
>> > >>
>> > >>> Hello Kafka users, developers and client-developers,
>> > >>>
>> > >>> This is the second candidate for release of Apache Kafka 2.6.0.
>> This is a
>> > >>> major release that includes many new features, including:
>> > >>>
>> > >>> * TLSv1.3 has been enabled by default for Java 11 or newer.
>> > >>> * Smooth scaling out of Kafka Streams applications
>> > >>> * Kafka Streams support for emit on change
>> > >>> * New metrics for better operational insight
>> > >>> * Kafka Connect can automatically create topics for source
>> connectors
>> > >>> * Improved error reporting options for sink connectors in Kafka
>> Connect
>> > >>> * New Filter and conditional SMTs in Kafka Connect
>> > >>> * The default value for the `client.dns.lookup` configuration is
>> > >>> now `use_all_dns_ips`
>> > >>> * Upgrade Zookeeper to 3.5.8
>> > >>>
>> > >>> This release also includes a few other features, 76 improvements,
>> and 165
>> > >>> bug fixes.
>> > >>>
>> > >>> Release notes for the 2.6.0 release:
>> > >>> https://home.apache.org/~rhauch/kafka-2.6.0-rc1/RELEASE_NOTES.html
>> > >>>
>> > >>> *** Please download, test and vote by Monday, July 20, 9am PT
>> > >>>
>> > >>> Kafka's KEYS file containing PGP keys we use to sign the release:
>> > >>> https://kafka.apache.org/KEYS
>> > >>>
>> > >>> * Release artifacts to be voted upon (source and binary):
>> > >>> https://home.apache.org/~rhauch/kafka-2.6.0-rc1/
>> > >>>
>> > >>> * Maven artifacts to be voted upon:
>> > >>>
>> https://repository.apache.org/content/groups/staging/org/apache/kafka/
>> > >>>
>> > >>> * Javadoc:
>> > >>> https://home.apache.org/~rhauch/kafka-2.6.0-rc1/javadoc/
>> > >>>
>> > >>> * Tag to be voted upon (off 2.6 branch) is the 2.6.0 tag:
>> > >>> https://github.com/apache/kafka/releases/tag/2.6.0-rc1
>> > >>>
>> > >>> * Documentation:
>> > >>> https://kafka.apache.org/26/documentation.html
>> > >>>
>> > >>> * Protocol:
>> > >>> https://kafka.apache.org/26/protocol.html
>> > >>>
>> > >>> * Successful Jenkins builds for the 2.6 branch:
>> > >>> Unit/integration tests:
>> https://builds.apache.org/job/kafka-2.6-jdk8/91/ (one
>> > >>> flaky test)
>> > >>> System tests: (link to follow)
>> > >>>
>> > >>> Thanks,
>> > >>> Randall Hauch
>> > >>>
>> > >>
>> > >
>> >
>> >
>> > Attachments:
>> > * signature.asc
>>
>


Re: [DISCUSS] KIP-450: Sliding Windows

2020-07-28 Thread Leah Thomas
Another minor tweak, instead of defining the window by the *size*, it will
be defined by *timeDifference*, which is the maximum time difference
between two events. This is a more precise way to define a window due to
its inclusive ends, while allowing the user to create the window they
expect. This definition fits with the current examples, where a record at
*10* would fall into a window of time difference *5* from *[5,10]*. This
window contains any records at 5, 6, 7, 8, 9, and 10, which is 6 instances
instead of 5. This semantic difference is why I've shifted *size* to
*timeDifference*.

The new builder will be *withTimeDifferenceAndGrace*, keeping with other
conventions.

Let me know if there are any concerns! The vote thread is open as well
here: http://mail-archives.apache.org/mod_mbox/kafka-dev/202007.mbox/browser

Best,
Leah

On Mon, Jul 27, 2020 at 3:58 PM Leah Thomas  wrote:

> A small tweak - to make it more clear to users that grace is required, as
> well as cleaning up some of the confusing grammar semantics of windows, the
> main builder method for *slidingWindows* will be *withSizeAndGrace* instead
> of *of*.  This looks like it'll be the last change (for now) on the
> public API. If anyone has any comments or thoughts let me know. Otherwise,
> I'll take this to vote shortly.
>
> Best,
> Leah
>
> On Fri, Jul 24, 2020 at 3:45 PM Leah Thomas  wrote:
>
>> To accommodate the change to a final class, I've added another
>> *windowedBy()* function in *CogroupedKStream.java *to handle
>> SlidingWindows.
>>
>> As far as the discussion goes, I think this is the last change we've
>> talked about. If anyone has other comments or concerns, please let me know!
>>
>> Cheers,
>> Leah
>>
>> On Thu, Jul 23, 2020 at 7:34 PM Leah Thomas  wrote:
>>
>>> Thanks for the discussion about extending TimeWindows. I agree that
>>> making it future proof is important, and the implementation of
>>> SlidingWindows is unique enough that it seems logical to make it its own
>>> final class.
>>>
>>> On that note, I've updated the KIP to make SlidingWindows a stand alone
>>> final class, and added the *windowedBy() *API in *KGroupedStream *to
>>> handle SlidingWindows. It seems that SlidingWindows would still be able to
>>> leverage *TimeWindowedKStream by* creating a SlidingWindows version of
>>> *TimeWindowedKStreamImpl* that implements *TimeWindowedKStream. *If
>>> anyone sees issues with this implementation, please let me know.
>>>
>>> Best,
>>> Leah
>>>
>>> On Wed, Jul 22, 2020 at 10:47 PM John Roesler 
>>> wrote:
>>>
 Thanks for the reply, Sophie.

 That all sounds about right to me.

 The Windows “interface”/algorithm is quite flexible, so it makes sense
 for it to be extensible. Different implementations can (and do) enumerate
 different windows to suit different use cases.

 On the other hand, I can’t think of any way to extend SessionWindows to
 do something different using the same algorithm, so it makes sense for it
 to stay final.

 If we think SlidingWindows is similarly not usefully extensible, then
 we can make it final. It’s easy to remove final later, but adding it is not
 possible. Or we could go the other route and just make it an interface, on
 general principle. Both of these choices are safe API design.

 Thanks again,
 John

 On Wed, Jul 22, 2020, at 21:54, Sophie Blee-Goldman wrote:
 > >
 > > Users could pass in a custom `SessionWindows` as
 > > long as the session algorithm works correctly for it.
 >
 >
 > Well not really, SessionWindows is a final class. TimeWindows is also
 a
 > final class, so neither of these can be extended/customized. For a
 given
 > Windows then there would only be three (non-overlapping)
 possibilities:
 > either it's TimeWindows, SlidingWindows, or a custom  Windows. I don't
 > think there's any problem with determining what the user wants in
 this case
 > --
 > we would just check if it's a SlidingWindows and connect the new
 processor,
 > or else connect the existing hopping/tumbling window processor.
 >
 > I'll admit that last sentence does leave a bad taste in my mouth.
 Part of
 > that
 > is probably the "leaking" API Matthias pointed out; we just assume the
 > hopping/tumbling window implementation fits all custom windows. But I
 guess
 > if you really needed to customize the algorithm any further you may
 as well
 > stick in a transformer and do it all yourself.
 >
 > Anyways, given what we have, it does seem weird to apply one algorithm
 > for most Windows types and then swap in a different one for one
 specific
 > extension of Windows. So adding a new #windowedBy(SlidingWindows)
 > sounds reasonable to me.
 >
 > I'm still not convinced that we need a whole new TimeWindowedKStream
 > equivalent class for sliding windows though. It seems like 

Jenkins build is back to normal : kafka-2.3-jdk8 #221

2020-07-28 Thread Apache Jenkins Server
See 




Re: [VOTE] KIP-554: Add Broker-side SCRAM Config API

2020-07-28 Thread Ron Dagostino
Hi everyone.  I just wanted to notify that while implementing this I
discovered that we had declared the ScramMechanism enum to have the values
HMAC_SHA_{256,512} instead of SCRAM_SHA_{256,512}.  I believe Rajini had
indicated that this should be changed back on May 7th, and the change makes
sense to me given that these are the formal SASL SCRAM mechanism names
(with dash replaced by underscore so they are valid Java identifiers).  I
have updated the KIP.  Let me know if you have any questions/concerns,
otherwise we can assume this change is acceptable.

Ron

On Tue, Jul 21, 2020 at 1:57 PM Colin McCabe  wrote:

> Hi all,
>
> With binding +1s from Rajini Sivaram, David Arthur, and Boyang Chen, and
> non-binding +1s from Tom Bentley, the vote passes.
>
> Thanks to everyone who commented and helped to improve the proposal,
> especially Ron Dagostino, David, and Boyang.
>
> best,
> Colin
>
>
> On Thu, Jul 16, 2020, at 16:02, Ron Dagostino wrote:
> > Hi Colin.  I updated the KIP with various renames.  I've also created a
> > draft PR at https://github.com/apache/kafka/pull/9032 that still needs a
> > bunch of real implementation but nonetheless represents the renames in
> code.
> >
> > The biggest changes are that there are now derived classes public class
> > UserScramCredentialAdditionUpsertion and public class
> > UserScramCredentialDeletion.  I don't know what the reaction to the use
> of
> > the term "upsertion" will be, but that's the best thing I could come up
> > with to reflect that these requests are "upserts" (update if there,
> > otherwise insert).  It was referred to as an "Addition" before, which I
> > felt was not technically correct.  If you diff the most recent two
> versions
> > of the KIP it diffs pretty cleanly and makes the changes pretty apparent.
> >
> > Ron
> >
> >
> > On Thu, Jul 16, 2020 at 11:38 AM Colin McCabe 
> wrote:
> >
> > > On Thu, Jul 16, 2020, at 08:06, Ron Dagostino wrote:
> > > > Thanks, Colin.  The standard "about" message for ThrottleTimeMs seems
> > > > to be "The duration in milliseconds for which the request was
> throttled
> > > > due to a quota violation, or zero if the request did not violate any
> > > quota."
> > > > as opposed to "The time spent waiting for quota." Should we adjust to
> > > > match the typical definition?
> > > >
> > >
> > > Hi Ron,
> > >
> > > Good point.  Let's keep the "about" text consistent.  I changed it.
> > >
> > > >
> > > > I'm wondering if describing Scram credentials should require READ
> > > privilege
> > > > rather than ALTER on the cluster?   Altering SCRAM credentials of
> course
> > > > requires ALTER privilege, and I can see the argument for requiring
> ALTER
> > > > privilege to describe them as well, but it did catch my eye as
> something
> > > > worth questioning/confirming.
> > > >
> > >
> > > Also a good point.  I spoke with Rajini about this offline, and she
> > > pointed out that we can already see user names in ACLs if we have
> DESCRIBE
> > > on CLUSTER.  So it should be fine to have describeScramUsers require
> > > DESCRIBE on CLUSTER as well.
> > >
> > > >
> > > > I'm also now thinking that "UNKNOWN" shouldn't be listed in the
> > > > ScramMechanism enum.  I thought maybe it was a catch-all so we will
> > > always
> > > > be able to deserialize something regardless of what key actually
> appears,
> > > > but I just realized that SCRAM credentials and Client Quotas are
> mixed
> > > > together in the same JSON, so it will be up to the corresponding API
> to
> > > > ignore what it doesn't recognize -- i.e. if both client quotas and
> SCRAM
> > > > credentials are defined for a user, then invoking
> DescribeClientQuotas
> > > must
> > > > only describe the quota configs and invoking DescribeScramUsers must
> only
> > > > describe the SCRAM configs.
> > > >
> > >
> > > The reason to have the UNKNOWN enum is so that we can add new SCRAM
> > > mechanisms in the future.  If we don't have it, then we're basically
> saying
> > > we can never add new mechanisms.
> > >
> > > I agree that the decision to put SCRAM users under the same ZK path as
> > > client quotas makes this more complex than we'd like it to be, but all
> is
> > > not lost.  For one thing, we could always just add a new ZK path for
> SCRAM
> > > users in the future if we really need to.  With a new path we wouldn't
> have
> > > to worry about namespace collisions.  For another thing, in the
> > > post-KIP-500 world this won't be an issue.
> > >
> > > In the short term, a simpler solution might work here.  For example,
> can
> > > we just assume that any key that starts with "SCRAM-" is not a quota,
> but a
> > > scram user?  (Or look at some other aspect of the key).
> > >
> > > >
> > > >  Also, note that invoking kafka-configs.sh
> > > > --bootstrap-server ... --entity-type user --describe will require the
> > > > invocation of two separate APIs -- one to describe quotas and one to
> > > > describe SCRAM credentials; I don't think this is a problem, but I
> did

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

2020-07-28 Thread Apache Jenkins Server
See 




[VOTE] 2.6.0 RC2

2020-07-28 Thread Randall Hauch
Hello Kafka users, developers and client-developers,

This is the third candidate for release of Apache Kafka 2.6.0. This is a
major release that includes many new features, including:

* TLSv1.3 has been enabled by default for Java 11 or newer.
* Smooth scaling out of Kafka Streams applications
* Kafka Streams support for emit on change
* New metrics for better operational insight
* Kafka Connect can automatically create topics for source connectors
* Improved error reporting options for sink connectors in Kafka Connect
* New Filter and conditional SMTs in Kafka Connect
* The default value for the `client.dns.lookup` configuration is
now `use_all_dns_ips`
* Upgrade Zookeeper to 3.5.8

This release also includes a few other features, 74 improvements, 175 bug
fixes, plus other fixes.

Release notes for the 2.6.0 release:
https://home.apache.org/~rhauch/kafka-2.6.0-rc2/RELEASE_NOTES.html

*** Please download, test and vote by Monday, August 3, 9am PT

Kafka's KEYS file containing PGP keys we use to sign the release:
https://kafka.apache.org/KEYS

* Release artifacts to be voted upon (source and binary):
https://home.apache.org/~rhauch/kafka-2.6.0-rc2/

* Maven artifacts to be voted upon:
https://repository.apache.org/content/groups/staging/org/apache/kafka/

* Javadoc:
https://home.apache.org/~rhauch/kafka-2.6.0-rc2/javadoc/

* Tag to be voted upon (off 2.6 branch) is the 2.6.0 tag:
https://github.com/apache/kafka/releases/tag/2.6.0-rc2

* Documentation:
https://kafka.apache.org/26/documentation.html

* Protocol:
https://kafka.apache.org/26/protocol.html

* Successful Jenkins builds for the 2.6 branch:
Unit/integration tests: https://builds.apache.org/job/kafka-2.6-jdk8/101/
System tests: (link to follow)


Thanks,
Randall Hauch


Build failed in Jenkins: kafka-trunk-jdk14 #324

2020-07-28 Thread Apache Jenkins Server
See 


Changes:

[github] KAFKA-10224: Update jersey license from CDDL to EPLv2 (#9089)


--
[...truncated 3.20 MB...]

org.apache.kafka.streams.TopologyTestDriverTest > shouldThrowForMissingTime[Eos 
enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldCaptureInternalTopicNamesIfWrittenInto[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldCaptureInternalTopicNamesIfWrittenInto[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldPunctuateOnWallClockTimeDeprecated[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldPunctuateOnWallClockTimeDeprecated[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldProcessRecordForTopic[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldProcessRecordForTopic[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldForwardRecordsFromSubtopologyToSubtopology[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldForwardRecordsFromSubtopologyToSubtopology[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldNotUpdateStoreForSmallerValue[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldNotUpdateStoreForSmallerValue[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldCreateStateDirectoryForStatefulTopology[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldCreateStateDirectoryForStatefulTopology[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldPunctuateIfWallClockTimeAdvances[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldPunctuateIfWallClockTimeAdvances[Eos enabled = false] PASSED

org.apache.kafka.streams.test.TestRecordTest > testConsumerRecord STARTED

org.apache.kafka.streams.test.TestRecordTest > testConsumerRecord PASSED

org.apache.kafka.streams.test.TestRecordTest > testToString STARTED

org.apache.kafka.streams.test.TestRecordTest > testToString PASSED

org.apache.kafka.streams.test.TestRecordTest > testInvalidRecords STARTED

org.apache.kafka.streams.test.TestRecordTest > testInvalidRecords PASSED

org.apache.kafka.streams.test.TestRecordTest > testPartialConstructorEquals 
STARTED

org.apache.kafka.streams.test.TestRecordTest > testPartialConstructorEquals 
PASSED

org.apache.kafka.streams.test.TestRecordTest > testMultiFieldMatcher STARTED

org.apache.kafka.streams.test.TestRecordTest > testMultiFieldMatcher PASSED

org.apache.kafka.streams.test.TestRecordTest > testFields STARTED

org.apache.kafka.streams.test.TestRecordTest > testFields PASSED

org.apache.kafka.streams.test.TestRecordTest > testProducerRecord STARTED

org.apache.kafka.streams.test.TestRecordTest > testProducerRecord PASSED

org.apache.kafka.streams.test.TestRecordTest > testEqualsAndHashCode STARTED

org.apache.kafka.streams.test.TestRecordTest > testEqualsAndHashCode PASSED

org.apache.kafka.streams.test.wordcount.WindowedWordCountProcessorTest > 
shouldWorkWithInMemoryStore STARTED

org.apache.kafka.streams.test.wordcount.WindowedWordCountProcessorTest > 
shouldWorkWithInMemoryStore PASSED

org.apache.kafka.streams.test.wordcount.WindowedWordCountProcessorTest > 
shouldFailWithLogging STARTED

org.apache.kafka.streams.test.wordcount.WindowedWordCountProcessorTest > 
shouldFailWithLogging PASSED

org.apache.kafka.streams.test.wordcount.WindowedWordCountProcessorTest > 
shouldFailWithCaching STARTED

org.apache.kafka.streams.test.wordcount.WindowedWordCountProcessorTest > 
shouldFailWithCaching PASSED

org.apache.kafka.streams.test.wordcount.WindowedWordCountProcessorTest > 
shouldWorkWithPersistentStore STARTED

org.apache.kafka.streams.test.wordcount.WindowedWordCountProcessorTest > 
shouldWorkWithPersistentStore PASSED

org.apache.kafka.streams.internals.KeyValueStoreFacadeTest > shouldReturnIsOpen 
STARTED

org.apache.kafka.streams.internals.KeyValueStoreFacadeTest > shouldReturnIsOpen 
PASSED

org.apache.kafka.streams.internals.KeyValueStoreFacadeTest > 
shouldDeleteAndReturnPlainValue STARTED

org.apache.kafka.streams.internals.KeyValueStoreFacadeTest > 
shouldDeleteAndReturnPlainValue PASSED

org.apache.kafka.streams.internals.KeyValueStoreFacadeTest > shouldReturnName 
STARTED

org.apache.kafka.streams.internals.KeyValueStoreFacadeTest > shouldReturnName 
PASSED

org.apache.kafka.streams.internals.KeyValueStoreFacadeTest > 
shouldPutWithUnknownTimestamp STARTED

org.apache.kafka.streams.internals.KeyValueStoreFacadeTest > 
shouldPutWithUnknownTimestamp PASSED

org.apache.kafka.streams.internals.KeyValueStoreFacadeTest > 
shouldPutAllWithUnknownTimestamp STARTED


Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-28 Thread Colin McCabe
Hi Boyang,

Thanks for updating this.  A few comments below:

In the "Routing Request Security" section, there is a reference to "older 
requests that need redirection."  But after these new revisions, both new and 
old requests need redirection.  So we should rephrase this.

> In addition, to avoid exposing this forwarding power to the admin clients,
> the routing request shall be forwarded towards the controller broker internal
> endpoint which should be only visible to other brokers inside the cluster 
> in the KIP-500 controller. Any admin configuration request with broker 
> principal should not be going through the public endpoint and will be 
> rejected for security purpose.

We should also describe how this will work in the pre-KIP-500 case.  In that 
case, CLUSTER_ACTION gets the extra permissions described here only when the 
message comes in on the inter-broker listener.  We should state that here.

(I can see that you have this information later on in the "Security Access 
Changes" section, but it would be good to have it here as well, to avoid 
confusion.)

> To be more strict of protecting controller information, the "ControllerId"
> field in new MetadataResponse shall be set to -1 when the original request
> comes from a non-broker client and it is already on v10. We shall use the
> request listener name to distinguish whether a given request is inter-broker,
> or from the client.

I'm not sure why we would need to distinguish between broker clients and 
non-broker clients.  Brokers don't generally send MetadataRequests to other 
brokers, do they?  Brokers learn about metadata from UpdateMetadataRequest and 
LeaderAndIsrRequest, not by sending MetadataRequests to other brokers.

Probably what we want here is: v0-v9: return a random broker in the cluster as 
the controller ID.  v10: no controllerID present in the MetadataResponse.  We 
should also deprecate the adminClient method which gets the controllerId.

> BROKER_AUTHORIZATION_FAILURE(92, "Authorization failed for the
> request during forwarding, this indicates an internal error on the broker
> cluster security setup.", BrokerAuthorizationFailureException::new);

Grammar nitpick: It would be good to have a period between "forwarding" and 
"this" to avoid a run-on sentence :)

best,
Colin


On Mon, Jul 27, 2020, at 21:47, Boyang Chen wrote:
> Hey there,
> 
> I'm re-opening this thread because after some initial implementations
> started, we spotted some gaps in the approved KIP as well as some
> inconsistencies with KIP-631 controller. There are a couple of addendums to
> the existing KIP, specifically:
> 
> 1. As the controller is foreseen to be only accessible to the brokers, the
> new admin client would not have direct access to the controller. It is
> guaranteed on the MetadataResponse level which no longer provides
> `ControllerId` to client side requests.
> 
> 2. The broker would forward any direct ZK path mutation requests, including
> topic creation/deletion, reassignment, etc since we deprecate the direct
> controller access on the client side. No more protocol version bump is
> necessary for the configuration requests.
> 
> 3. To make sure forwarding requests pass the authorization, broker
> principal CLUSTER_ACTION would be allowed to be used as an alternative
> authentication method for a variety of principal operations, including
> ALTER, ALTER_CONFIG, DELETE, etc. It is because the forwarding request
> needs to use the proxy broker's own principal, which is currently not
> supported to be used for many configuration change authentication listed
> above. The full list could be found in the KIP.
> 
> 4. Add a new BROKER_AUTHORIZATION_FAILURE error code to indicate any
> internal security configuration failure, when the forwarded request failed
> authentication on the controller side.
> 
> Let me know what you think. With such a major refinement of the KIP, I'm
> open for re-vote after discussions converge.
> 
> Boyang
> 
> On Wed, Jul 1, 2020 at 2:17 PM Boyang Chen 
> wrote:
> 
> > Hey folks,
> >
> > I have also synced on the KIP-578 which was doing the partition limit, to
> > make sure the partition limit error code would be properly propagated once
> > it is done on top of KIP-590. Let me know if you have further questions or
> > concerns.
> >
> > Boyang
> >
> > On Tue, Jun 23, 2020 at 5:08 PM Boyang Chen 
> > wrote:
> >
> >> Thanks for the clarification, Colin and Ismael. Personally I also feel
> >> Option A is better to prioritize fixing the gap. Just to be clear, the
> >> proposed solution would be:
> >>
> >> 1. Bump the Metadata RPC version to return POLICY_VIOLATION. In the
> >> application level, we should swap the error message with the actual failure
> >> reason such as "violation of topic creation policy when attempting to auto
> >> create internal topic through MetadataRequest."
> >>
> >> 2. For older Metadata RPC, return AUTHORIZATION_FAILED to fail fast.
> >>
> >> Will address our other discussed points as well 

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-28 Thread Colin McCabe
Hi David,

Thanks for bringing this up.  This is indeed something that we overlooked, that 
we'll have to figure out.

The active controller may not be co-located with a broker in the post-KIP-500 
world.  So it does not make sense to have the client communicate directly with 
the controller.  Just to reiterate, the controller network is not accessible to 
clients, much like the ZK network today.

It seems like we can still do what we need to do for KIP-599, but we just can't 
mute the channel.  This just means we need to do the backpressure at a slightly 
higher level.

best,
Colin


On Tue, Jul 28, 2020, at 00:51, David Jacot wrote:
> Hi Boyang,
> 
> Thanks for the update.
> 
> 1./2. In KIP-599 (accepted and already in trunk), we throttle the
> CreateTopicsRequest,
> CreatePartitionsRequest, and DeleteTopicsRequests by muting the channel used
> by the Admin client and setting the throttleTimeMs in the response. The
> change that
> you propose breaks this. If we distribute these requests to all brokers,
> muting channels
> does not make sense anymore.
> 
> Have we considered continuing sending controller requests to the broker
> that hosts the
> controller? We could continue to send requests to the broker listener and
> redirect them
> to the controller internally. This would still keep the controller
> isolated. Another advantage
> of doing so is that the forwarding on the same machine does not require to
> go over the
> network.
> 
> Could you elaborate a bit more on the motivation and the reasoning behind
> this change?
> Is there a requirement or a strong advantage that I have missed?
> 
> Best,
> David
> 
> On Tue, Jul 28, 2020 at 6:48 AM Boyang Chen 
> wrote:
> 
> > Hey there,
> >
> > I'm re-opening this thread because after some initial implementations
> > started, we spotted some gaps in the approved KIP as well as some
> > inconsistencies with KIP-631 controller. There are a couple of addendums to
> > the existing KIP, specifically:
> >
> > 1. As the controller is foreseen to be only accessible to the brokers, the
> > new admin client would not have direct access to the controller. It is
> > guaranteed on the MetadataResponse level which no longer provides
> > `ControllerId` to client side requests.
> >
> > 2. The broker would forward any direct ZK path mutation requests, including
> > topic creation/deletion, reassignment, etc since we deprecate the direct
> > controller access on the client side. No more protocol version bump is
> > necessary for the configuration requests.
> >
> > 3. To make sure forwarding requests pass the authorization, broker
> > principal CLUSTER_ACTION would be allowed to be used as an alternative
> > authentication method for a variety of principal operations, including
> > ALTER, ALTER_CONFIG, DELETE, etc. It is because the forwarding request
> > needs to use the proxy broker's own principal, which is currently not
> > supported to be used for many configuration change authentication listed
> > above. The full list could be found in the KIP.
> >
> > 4. Add a new BROKER_AUTHORIZATION_FAILURE error code to indicate any
> > internal security configuration failure, when the forwarded request failed
> > authentication on the controller side.
> >
> > Let me know what you think. With such a major refinement of the KIP, I'm
> > open for re-vote after discussions converge.
> >
> > Boyang
> >
> > On Wed, Jul 1, 2020 at 2:17 PM Boyang Chen 
> > wrote:
> >
> > > Hey folks,
> > >
> > > I have also synced on the KIP-578 which was doing the partition limit, to
> > > make sure the partition limit error code would be properly propagated
> > once
> > > it is done on top of KIP-590. Let me know if you have further questions
> > or
> > > concerns.
> > >
> > > Boyang
> > >
> > > On Tue, Jun 23, 2020 at 5:08 PM Boyang Chen 
> > > wrote:
> > >
> > >> Thanks for the clarification, Colin and Ismael. Personally I also feel
> > >> Option A is better to prioritize fixing the gap. Just to be clear, the
> > >> proposed solution would be:
> > >>
> > >> 1. Bump the Metadata RPC version to return POLICY_VIOLATION. In the
> > >> application level, we should swap the error message with the actual
> > failure
> > >> reason such as "violation of topic creation policy when attempting to
> > auto
> > >> create internal topic through MetadataRequest."
> > >>
> > >> 2. For older Metadata RPC, return AUTHORIZATION_FAILED to fail fast.
> > >>
> > >> Will address our other discussed points as well in the KIP, let me know
> > >> if you have further questions.
> > >>
> > >> Thanks,
> > >> Boyang
> > >>
> > >> On Tue, Jun 23, 2020 at 10:41 AM Ismael Juma  wrote:
> > >>
> > >>> Option A is basically what I was thinking. But with a slight
> > adjustment:
> > >>>
> > >>> New versions of MetadataResponse return POLICY_VIOLATION, old versions
> > >>> return AUTHORIZATION_FAILED. The latter works correctly with old Java
> > >>> clients (i.e. the client fails fast and propagates the error), I've
> > >>> tested
> > >>> 

Re: [DISCUSS] KIP-645: Replace abstract class Windows with a proper interface

2020-07-28 Thread Sophie Blee-Goldman
Awesome, thanks for looking into the Window improvements as well!
(And I'm sorry I brought this upon you). I hope it's not too painful to get
everything in the Windows ecosystem looking good and reasonable,
and the benefits are pretty clear.

Haven't had a careful look through the POC yet but the proposal and
public changes you described sound great. Thanks for the KIP!

On Tue, Jul 28, 2020 at 10:27 AM John Roesler  wrote:

> Hi Sophie,
>
> A quick update: I've pushed a commit to the POC PR
> that includes the migration of Window to become a
> data class instead of an abstract class. It's quite a bit
> of code, but it does look like there is a clean
> deprecation/migration path.
>
> The basic idea is that we drop the "abstract" modifier
> from Window an deprecate its constructor. We add
> a public static `Window.withBounds(start,end)` to
> replace the constructor.
>
> Because the constructor is deprecated, existing
> subclasses will continue to compile, but receive
> a deprecation warning.
>
> We'd also slightly modify EnumerableWindowDefinition
> to _not_ be a parameterized class and instead
> update windowsFor like this:
>  Map windowsFor(final long timestamp)
>
> Then, any existing caller that expects to get back
> a subclass of windows during the deprecation period
> would still get a valid return. For example, if you
> had a custom window definition, and you
> invoke this method in your tests, assigning it to a
> subclass of Window, all your code would still compile,
> but you would get deprecation warnings.
>
> After the deprecation period, we could migrate Window
> to be a final class with a private constructor safely.
>
> If that sounds reasonable to you, I can update the
> KIP accordingly.
>
> Thanks,
> -John
>
> On Mon, Jul 27, 2020, at 22:11, John Roesler wrote:
> > Thanks for the reply, Sophie.
> >
> > Yes, I'd neglected to specify that Windows will implement maxSize()
> > by delegating to size(). It's updated now. I'd also neglected to say that
> > I plan to alter both windowBy methods to use the new interface now.
> > Because Windows will implement the new interface, all implementations
> > will continue to work with windowBy.
> > So, yes, there are public methods changed, but no compatibility issues
> > arise. Existing implementations will only get a deprecation warning.
> >
> > The Window type is interesting. It actually seems to serve as just a data
> > container. It almost doesn't need to be subclassed at all, since all
> > implementations would just need to store the start and end bounds.
> > As far as I can tell, the only purpose to subclass it is to implement
> > "overlap(Window other)", to tell if the window is both the same type
> > as and overlaps with the other window. But weirdly, this is unused
> > in the codebase.
> >
> > So one way we could go is to try and migrate it to just a final class,
> > effectively a tuple of `(start, end)`.
> >
> > However, another opportunity is to let it be a witness of the actual type
> > of the window, so that you wouldn't be able to join a TimeWindow
> > with a SessionWindow, for example.
> >
> > However, because of covariance, it's more painful to change Window
> > than Windows, so it might not be worth it right now. If anything, it
> > would be more feasible to migrate toward the "simple data container"
> > approach. What are your thoughts?
> >
> > Thanks,
> > -John
> >
> >
> > On Mon, Jul 27, 2020, at 18:19, Sophie Blee-Goldman wrote:
> > > Thanks for taking the time to really fill in the background details for
> > > this KIP.
> > > The Motivation section is very informative. Hopefully this will also
> serve
> > > as a
> > > warning against making similar such mistakes in the future :P
> > >
> > > I notice that the `Window` class that
> > > parametrizes EnumerableWindowDefinition
> > > is also abstract. Did you consider migrating that to an interface as
> well?
> > >
> > > Also, pretty awesome that we can solve the issue with varying fixed
> sized
> > > windows
> > > (eg calendar months) on the side. For users who may have already
> extended
> > > Windows,
> > > do you plan to just have Windows implement the new #maxSize method and
> > > return the existing
> > > size until Windows gets removed?
> > >
> > > One final note: this seems to be implicitly implied throughout the KIP
> but
> > > just to be clear,
> > > you will be replacing any DSL methods that accept Windows with
> identical
> > > DSL methods
> > > that take the new EnumerableWindowDefinition as argument. So there is
> > > nothing deprecated
> > > and nothing added, but there are public methods changed. Is that right?
> > >
> > > On Sun, Jul 26, 2020 at 1:23 PM John Roesler 
> wrote:
> > >
> > > > Thanks Sophie and Boyang for asking for specifics.
> > > >
> > > > As far as I can tell, if we were to _remove_ all the
> non-public-method
> > > > members from Windows, including any constructors, we are left with
> > > > effectively an interface. I think this was my plan before. I don't

Re: [DISCUSS] KIP-405: Kafka Tiered Storage

2020-07-28 Thread Ying Zheng
1001.

We did consider this approach. The concerns are
1)  This makes unclean-leader-election rely on remote storage. In case the
remote storage
 is unavailable, Kafka will not be able to finish the
unclean-leader-election.
2) Since the user set local retention time (or local retention bytes), I
think we are expected to
keep that much local data when possible (avoid truncating all the local
data). But, as you said,
unclean leader elections are very rare, this may not be a big problem.

The current design uses the leader broker as source-of-truth. This is
consistent with the
existing Kafka behavior.

By using remote storage as the source-of-truth, the follower logic can be a
little simpler,
but the leader logic is going to be more complex. Overall, I don't see
there many benefits
of using remote storage as the source-of-truth.



On Tue, Jul 28, 2020 at 10:25 AM Jun Rao  wrote:

> Hi, Satish,
>
> Thanks for the reply.
>
> 1001. In your example, I was thinking that you could just download the
> latest leader epoch from the object store. After that you know the leader
> should end with offset 1100. The leader will delete all its local data
> before offset 1000 and start accepting new messages at offset 1100.
> Consumer requests for messages before offset 1100 will be served from the
> object store. The benefit with this approach is that it's simpler to reason
> about who is the source of truth. The downside is slightly  increased
> unavailability window during unclean leader election. Since unclean leader
> elections are rare, I am not sure if this is a big concern.
>
> 1008. Yes, I think introducing sth like local.retention.ms seems more
> consistent.
>
> Jun
>
> On Tue, Jul 28, 2020 at 2:30 AM Satish Duggana 
> wrote:
>
> > HI Jun,
> > Thanks for your comments. We put our inline replies below.
> >
> > 1001. I was thinking that you could just use the tiered metadata to do
> the
> > reconciliation. The tiered metadata contains offset ranges and epoch
> > history. Those should be enough for reconciliation purposes.
> >
> > If we use remote storage as the source-of-truth during
> > unclean-leader-election, it's possible that after reconciliation the
> > remote storage will have more recent data than the new leader's local
> > storage. For example, the new leader's latest message is offset 1000,
> > while the remote storage has message 1100. In such a case, the new
> > leader will have to download the messages from 1001 to 1100, before
> > accepting new messages from producers. Otherwise, there would be a gap
> > in the local data between 1000 and 1101.
> >
> > Moreover, with the current design, leader epoch history is stored in
> > remote storage, rather than the metadata topic. We did consider saving
> > epoch history in remote segment metadata. But the concern is that
> > there is currently no limit for the epoch history size. Theoretically,
> > if a user has a very long remote retention time and there are very
> > frequent leadership changes, the leader epoch history can become too
> > long to fit into a regular Kafka message.
> >
> >
> > 1003.3 Having just a serverEndpoint string is probably not enough.
> > Connecting to a Kafka cluster may need various security credentials. We
> can
> > make RLMM configurable and pass in the properties through the configure()
> > method. Ditto for RSM.
> >
> > RLMM and  RSM are already configurable and they take properties which
> > start with "remote.log.metadata." and "remote.log.storage."
> > respectively and a few others. We have listener-name as the config for
> > RLMM and other properties(like security) can be sent as you suggested.
> > We will update the KIP with the details.
> >
> >
> > 1008.1 We started with log.retention.hours and log.retention.minutes, and
> > added log.retention.ms later. If we are adding a new configuration, ms
> > level config alone is enough and is simpler. We can build tools to make
> the
> > configuration at different granularities easier. The definition of
> > log.retention.ms is "The number of milliseconds to keep a log file
> before
> > deleting it". The deletion is independent of whether tiering is enabled
> or
> > not. If this changes to just the local portion of the data, we are
> changing
> > the meaning of an existing configuration.
> >
> > We are fine with either way. We can go with log.retention. as the
> > effective log retention instead of local log retention. With this
> > convention, we need to introduce  local.log.retention instead of
> > remote.log.retention.ms that we proposed. If log.retention.ms as -1
> > then remote retention is also considered as unlimited but user should
> > be able to set the local.retention.ms.
> > So, we need to introduce local.log.retention.ms and
> > local.log.retention.bytes which should  always  be <=
> > log.retention.ms/bytes respectively.
> >
> >
> >
> > On Fri, Jul 24, 2020 at 3:37 AM Jun Rao  wrote:
> > >
> > > Hi, Satish,
> > >
> > > Thanks for the reply. A few quick comments below.

how one can configure that consumer must consume topic messages by 150 mins (exactly 2.5 hours)

2020-07-28 Thread Rao, Vasudevan P
Hi

I am working on Kafka in our environment.  We need to restrict or delay the 
consumer to consume topic by 150 mins.  I did some tweak in server. properties 
at the broker level (we have 5 brokers in one env) but that does not work.

Namely I made changes in the servrer.properties under that
group.initial.rebalance.delay.ms=3000 (default)
group.initial.rebalance.delay.ms=900 (changed one)

But the changed one not working and all the topics not coming up but they have 
change back to 3000 then all the topics coming up.

Please help us how do we achieve this.

With warm regards,



Vasu Rao
DevOps-Env
Email:vasudevanp@walgreens.com




Re: [DISCUSS] KIP-645: Replace abstract class Windows with a proper interface

2020-07-28 Thread John Roesler
Hi Sophie,

A quick update: I've pushed a commit to the POC PR
that includes the migration of Window to become a
data class instead of an abstract class. It's quite a bit
of code, but it does look like there is a clean
deprecation/migration path.

The basic idea is that we drop the "abstract" modifier
from Window an deprecate its constructor. We add
a public static `Window.withBounds(start,end)` to
replace the constructor.

Because the constructor is deprecated, existing
subclasses will continue to compile, but receive
a deprecation warning.

We'd also slightly modify EnumerableWindowDefinition
to _not_ be a parameterized class and instead
update windowsFor like this:
 Map windowsFor(final long timestamp)

Then, any existing caller that expects to get back
a subclass of windows during the deprecation period
would still get a valid return. For example, if you
had a custom window definition, and you
invoke this method in your tests, assigning it to a
subclass of Window, all your code would still compile,
but you would get deprecation warnings.

After the deprecation period, we could migrate Window
to be a final class with a private constructor safely.

If that sounds reasonable to you, I can update the
KIP accordingly.

Thanks,
-John

On Mon, Jul 27, 2020, at 22:11, John Roesler wrote:
> Thanks for the reply, Sophie.
> 
> Yes, I'd neglected to specify that Windows will implement maxSize()
> by delegating to size(). It's updated now. I'd also neglected to say that
> I plan to alter both windowBy methods to use the new interface now.
> Because Windows will implement the new interface, all implementations
> will continue to work with windowBy.
> So, yes, there are public methods changed, but no compatibility issues
> arise. Existing implementations will only get a deprecation warning.
> 
> The Window type is interesting. It actually seems to serve as just a data
> container. It almost doesn't need to be subclassed at all, since all
> implementations would just need to store the start and end bounds.
> As far as I can tell, the only purpose to subclass it is to implement
> "overlap(Window other)", to tell if the window is both the same type
> as and overlaps with the other window. But weirdly, this is unused
> in the codebase.
> 
> So one way we could go is to try and migrate it to just a final class,
> effectively a tuple of `(start, end)`.
> 
> However, another opportunity is to let it be a witness of the actual type
> of the window, so that you wouldn't be able to join a TimeWindow
> with a SessionWindow, for example.
> 
> However, because of covariance, it's more painful to change Window
> than Windows, so it might not be worth it right now. If anything, it
> would be more feasible to migrate toward the "simple data container"
> approach. What are your thoughts?
> 
> Thanks,
> -John
> 
> 
> On Mon, Jul 27, 2020, at 18:19, Sophie Blee-Goldman wrote:
> > Thanks for taking the time to really fill in the background details for
> > this KIP.
> > The Motivation section is very informative. Hopefully this will also serve
> > as a
> > warning against making similar such mistakes in the future :P
> > 
> > I notice that the `Window` class that
> > parametrizes EnumerableWindowDefinition
> > is also abstract. Did you consider migrating that to an interface as well?
> > 
> > Also, pretty awesome that we can solve the issue with varying fixed sized
> > windows
> > (eg calendar months) on the side. For users who may have already extended
> > Windows,
> > do you plan to just have Windows implement the new #maxSize method and
> > return the existing
> > size until Windows gets removed?
> > 
> > One final note: this seems to be implicitly implied throughout the KIP but
> > just to be clear,
> > you will be replacing any DSL methods that accept Windows with identical
> > DSL methods
> > that take the new EnumerableWindowDefinition as argument. So there is
> > nothing deprecated
> > and nothing added, but there are public methods changed. Is that right?
> > 
> > On Sun, Jul 26, 2020 at 1:23 PM John Roesler  wrote:
> > 
> > > Thanks Sophie and Boyang for asking for specifics.
> > >
> > > As far as I can tell, if we were to _remove_ all the non-public-method
> > > members from Windows, including any constructors, we are left with
> > > effectively an interface. I think this was my plan before. I don't think
> > > I realized at the time that it's possible to replace the entire class with
> > > an interface. Now I realize it is possible, hence the motivation to do it.
> > >
> > > We can choose either to maintain forever the tech debt of having to
> > > enforce that Windows look, sound, and act just like an interface, or we
> > > can just replace it with an interface and be done with it. I.e., the
> > > motivation is less maintenence burden for us and for users.
> > >
> > > Coincidentally, I had an interesting conversation with Matthias about
> > > this interface, and he made me realize that "fixed size" isn't the
> > > essential

Re: [DISCUSS] KIP-405: Kafka Tiered Storage

2020-07-28 Thread Jun Rao
Hi, Satish,

Thanks for the reply.

1001. In your example, I was thinking that you could just download the
latest leader epoch from the object store. After that you know the leader
should end with offset 1100. The leader will delete all its local data
before offset 1000 and start accepting new messages at offset 1100.
Consumer requests for messages before offset 1100 will be served from the
object store. The benefit with this approach is that it's simpler to reason
about who is the source of truth. The downside is slightly  increased
unavailability window during unclean leader election. Since unclean leader
elections are rare, I am not sure if this is a big concern.

1008. Yes, I think introducing sth like local.retention.ms seems more
consistent.

Jun

On Tue, Jul 28, 2020 at 2:30 AM Satish Duggana 
wrote:

> HI Jun,
> Thanks for your comments. We put our inline replies below.
>
> 1001. I was thinking that you could just use the tiered metadata to do the
> reconciliation. The tiered metadata contains offset ranges and epoch
> history. Those should be enough for reconciliation purposes.
>
> If we use remote storage as the source-of-truth during
> unclean-leader-election, it's possible that after reconciliation the
> remote storage will have more recent data than the new leader's local
> storage. For example, the new leader's latest message is offset 1000,
> while the remote storage has message 1100. In such a case, the new
> leader will have to download the messages from 1001 to 1100, before
> accepting new messages from producers. Otherwise, there would be a gap
> in the local data between 1000 and 1101.
>
> Moreover, with the current design, leader epoch history is stored in
> remote storage, rather than the metadata topic. We did consider saving
> epoch history in remote segment metadata. But the concern is that
> there is currently no limit for the epoch history size. Theoretically,
> if a user has a very long remote retention time and there are very
> frequent leadership changes, the leader epoch history can become too
> long to fit into a regular Kafka message.
>
>
> 1003.3 Having just a serverEndpoint string is probably not enough.
> Connecting to a Kafka cluster may need various security credentials. We can
> make RLMM configurable and pass in the properties through the configure()
> method. Ditto for RSM.
>
> RLMM and  RSM are already configurable and they take properties which
> start with "remote.log.metadata." and "remote.log.storage."
> respectively and a few others. We have listener-name as the config for
> RLMM and other properties(like security) can be sent as you suggested.
> We will update the KIP with the details.
>
>
> 1008.1 We started with log.retention.hours and log.retention.minutes, and
> added log.retention.ms later. If we are adding a new configuration, ms
> level config alone is enough and is simpler. We can build tools to make the
> configuration at different granularities easier. The definition of
> log.retention.ms is "The number of milliseconds to keep a log file before
> deleting it". The deletion is independent of whether tiering is enabled or
> not. If this changes to just the local portion of the data, we are changing
> the meaning of an existing configuration.
>
> We are fine with either way. We can go with log.retention. as the
> effective log retention instead of local log retention. With this
> convention, we need to introduce  local.log.retention instead of
> remote.log.retention.ms that we proposed. If log.retention.ms as -1
> then remote retention is also considered as unlimited but user should
> be able to set the local.retention.ms.
> So, we need to introduce local.log.retention.ms and
> local.log.retention.bytes which should  always  be <=
> log.retention.ms/bytes respectively.
>
>
>
> On Fri, Jul 24, 2020 at 3:37 AM Jun Rao  wrote:
> >
> > Hi, Satish,
> >
> > Thanks for the reply. A few quick comments below.
> >
> > 1001. I was thinking that you could just use the tiered metadata to do
> the
> > reconciliation. The tiered metadata contains offset ranges and epoch
> > history. Those should be enough for reconciliation purposes.
> >
> > 1003.3 Having just a serverEndpoint string is probably not enough.
> > Connecting to a Kafka cluster may need various security credentials. We
> can
> > make RLMM configurable and pass in the properties through the configure()
> > method. Ditto for RSM.
> >
> > 1008.1 We started with log.retention.hours and log.retention.minutes, and
> > added log.retention.ms later. If we are adding a new configuration, ms
> > level config alone is enough and is simpler. We can build tools to make
> the
> > configuration at different granularities easier. The definition of
> > log.retention.ms is "The number of milliseconds to keep a log file
> before
> > deleting it". The deletion is independent of whether tiering is enabled
> or
> > not. If this changes to just the local portion of the data, we are
> changing
> > the meaning of an 

Re: [DISCUSS] KIP-622 Add currentSystemTimeMs and currentStreamTimeMs to ProcessorContext

2020-07-28 Thread John Roesler
Thanks Matthias,

This is a really good point. It might be a bummer
to have to actually change the topology between
testing and production. Do you think we can rather
evolve the TimestampExtractor interface to let
Streams pass in the current system time, along with
the current record and the current partition time?

For example, we could add a new method:
long extract(
  ConsumerRecord record, 
  long partitionTime,
  long systemTime
);

Then, Streams could pass in the current system 
time and TopologyTestDriver could pass the mocked
time. Additionally, users who implement
TimestampExtractor on their own would be able to
deterministically unit-test their own implementation.

It's always a challenge to add to an interface without
breaking compatibility. In this case, it seems like
we could provide a default implementation that just
ignores the systemTime argument and calls
extract(record,  partitionTime) and also deprecate
the existing method. Then custom implementations
would get a deprecation warning telling them to
implement the other method, and when we remove
the deprecated extract(record, partitionTime), we can
also drop the default implementation from the new
method.

Specifically, what do you think about:
=
public interface TimestampExtractor {
/*...
 * @deprecated Since 2.7 Implement
 *   {@code extract(ConsumerRecord record, long 
partitionTime, long systemTime)} instead
 */
@Deprecated
long extract(
  ConsumerRecord record,
  long partitionTime
);

default long extract(
  ConsumerRecord record,
  long partitionTime,
  long systemTime) {
return extract(record, partitionTime);
}
}
=

Thanks,
-John

On Sun, Jul 26, 2020, at 15:47, Matthias J. Sax wrote:
> Hi,
> 
> I just had one more thought about an additional improvement we might
> want to include in this KIP.
> 
> Kafka Streams ships with a `WallclockTimestampExtractor` that just
> returns `System.currentTimeMillis()` and thus, cannot be mocked. And it
> seems that there is no way for a user to build a custom timestamps
> extractor that returns the TTD's mocked system time.
> 
> Thus, it might be nice, to add a `MockTimeExtractor` (only in the
> test-util package) that users could set and this `MockTimeExtractor`
> returns the mocked system time.
> 
> Thoughts?
> 
> 
> -Matthias
> 
> On 7/7/20 11:11 PM, Matthias J. Sax wrote:
> > I think, we don't need a default implementation for the new methods.
> > 
> > What would be the use-case to implement the  `ProcessorContext`
> > interface? In contract to, for example, `KeyValueStore`,
> > `ProcessorContext` is a use-only interface because it's never passed
> > into Kafka Streams, but only handed out to the user.
> > 
> > 
> > -Matthias
> > 
> > 
> > On 7/7/20 1:28 PM, William Bottrell wrote:
> >> Sure, I would appreciate help from Piotr creating an example.
> >>
> >> On Tue, Jul 7, 2020 at 12:03 PM Boyang Chen 
> >> wrote:
> >>
> >>> Hey John,
> >>>
> >>> since ProcessorContext is a public API, I couldn't be sure that people
> >>> won't try to extend it. Without a default implementation, user code
> >>> compilation will break.
> >>>
> >>> William and Piotr, it seems that we haven't added any example usage of the
> >>> new API, could we try to address that? It should help with the motivation
> >>> and follow-up meta comments as John proposed.
> >>>
> >>> Boyang
> >>>
> >>> On Mon, Jul 6, 2020 at 12:04 PM Matthias J. Sax  wrote:
> >>>
>  William,
> 
>  thanks for the KIP. LGMT. Feel free to start a vote.
> 
> 
>  -Matthias
> 
> 
>  On 7/4/20 10:14 AM, John Roesler wrote:
> > Hi Richard,
> >
> > It’s good to hear from you!
> >
> > Thanks for bringing up the wall-clock suppression feature. IIRC,
> >>> someone
>  actually started a KIP discussion for it already, but I don’t think it
> >>> went
>  to a vote. I don’t recall any technical impediment, just the lack of
>  availability to finish it up. Although there is some association, it
> >>> would
>  be good to keep the KIPs separate.
> >
> > Thanks,
> > John
> >
> > On Sat, Jul 4, 2020, at 10:05, Richard Yu wrote:
> >> Hi all,
> >>
> >> This reminds me of a previous issue I think that we were discussing.
> >> @John Roesler  I think you should
> >>> remember
>  this one.
> >>
> >> A while back, we were talking about having suppress operator emit
> >> records by wall-clock time instead of stream time.
> >> If we are adding this, wouldn't that make it more feasible for us to
> >> implement that feature for suppression?
> >>
> >> If I recall correctly, there actually had been quite a bit of user
> >> demand for such a feature.
> >> Might be good to include it in this KIP (or maybe use one of the prior
> >> KIPs for this feature).
> >>
> >> Best,
> 

Re: Request for access to create KIP

2020-07-28 Thread Boyang Chen
Permission granted

On Tue, Jul 28, 2020 at 9:01 AM John Thomas  wrote:

> johnthotekat
>
> 
> From: John Thomas 
> Sent: 27 July 2020 23:29
> To: dev@kafka.apache.org 
> Subject: Request for access to create KIP
>
> Hello Team,
>
> Could you please grant me access to Create KIP's. My ID is "johnthotekat"
> I'm a newbie stating to work on
> https://issues.apache.org/jira/browse/KAFKA-10316
>
> Thank you!
> John
>


[jira] [Created] (KAFKA-10320) Log metrics for future logs never have the is-future tag removed

2020-07-28 Thread Bob Barrett (Jira)
Bob Barrett created KAFKA-10320:
---

 Summary: Log metrics for future logs never have the is-future tag 
removed
 Key: KAFKA-10320
 URL: https://issues.apache.org/jira/browse/KAFKA-10320
 Project: Kafka
  Issue Type: Bug
Reporter: Bob Barrett






--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (KAFKA-10319) Fix unknown offset sum on trunk

2020-07-28 Thread John Roesler (Jira)
John Roesler created KAFKA-10319:


 Summary: Fix unknown offset sum on trunk
 Key: KAFKA-10319
 URL: https://issues.apache.org/jira/browse/KAFKA-10319
 Project: Kafka
  Issue Type: Sub-task
  Components: streams
Affects Versions: 2.7.0
Reporter: John Roesler
Assignee: Bruno Cadonna


Port [https://github.com/apache/kafka/pull/9066] to trunk



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


Re: Request for access to create KIP

2020-07-28 Thread John Thomas
johnthotekat


From: John Thomas 
Sent: 27 July 2020 23:29
To: dev@kafka.apache.org 
Subject: Request for access to create KIP

Hello Team,

Could you please grant me access to Create KIP's. My ID is "johnthotekat"
I'm a newbie stating to work on 
https://issues.apache.org/jira/browse/KAFKA-10316

Thank you!
John


Request for access to create KIP

2020-07-28 Thread John Thomas
Hello Team,

Could you please grant me access to Create KIP's. My ID is "johnthotekat"
I'm a newbie stating to work on 
https://issues.apache.org/jira/browse/KAFKA-10316

Thank you!
John


[VOTE] KIP-450: Sliding Window Aggregations in the DSL

2020-07-28 Thread Leah Thomas
Hi all,

I'd like to kick-off the vote for KIP-450
,
adding sliding window aggregations to the DSL. The discussion thread is here

.

Cheers,
Leah


[jira] [Created] (KAFKA-10318) Default API timeout must be enforced to be greater than request timeout just like in AdminClient

2020-07-28 Thread Gabor Somogyi (Jira)
Gabor Somogyi created KAFKA-10318:
-

 Summary: Default API timeout must be enforced to be greater than 
request timeout just like in AdminClient
 Key: KAFKA-10318
 URL: https://issues.apache.org/jira/browse/KAFKA-10318
 Project: Kafka
  Issue Type: Bug
Reporter: Gabor Somogyi


https://github.com/apache/kafka/blob/66563e712b0b9f84f673b262f2fb87c03110084d/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java#L545-L555




--
This message was sent by Atlassian Jira
(v8.3.4#803005)


Re: [VOTE] KIP-635: GetOffsetShell: support for multiple topics and consumer configuration override

2020-07-28 Thread Viktor Somogyi-Vass
+1 from me (non-binding), thanks for the KIP.

On Mon, Jul 27, 2020 at 10:02 AM Dániel Urbán  wrote:

> Hello everyone,
>
> I'd like to start a vote on KIP-635. The KIP enhances the GetOffsetShell
> tool by enabling querying multiple topic-partitions, adding new filtering
> options, and adding a config override option.
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-635%3A+GetOffsetShell%3A+support+for+multiple+topics+and+consumer+configuration+override
>
> The original discussion thread was named "[DISCUSS] KIP-308:
> GetOffsetShell: new KafkaConsumer API, support for multiple topics,
> minimize the number of requests to server". The id had to be changed as
> there was a collision, and the KIP also had to be renamed, as some of its
> motivations were outdated.
>
> Thanks,
> Daniel
>


Re: [VOTE] KIP-599: Throttle Create Topic, Create Partition and Delete Topic Operations

2020-07-28 Thread David Jacot
Hi all,

Just a quick update. We have made good progress regarding the implementation
of this KIP. The major parts are already in trunk modulo the new "rate"
implementation
which is still under development.

I would like to change the type of the `controller_mutations_rate` from a
Long to
a Double. I have made various experiments and the rate can be quite low,
especially
in clusters with a lot of tenants. Using a Long is quite limiting here to
fine tune small
rates. I hope that this change is fine for everyone.

Best,
David

On Mon, Jun 15, 2020 at 9:26 AM David Jacot  wrote:

> Hi all,
>
> The vote has passed with 5 binding votes (Gwen, Rajini, Mickael, Jun and
> Colin)
> and 2 non-binding votes (Tom, Anna).
>
> Thank you all for the fruitful discussion! I'd like to particularly thank
> Anna who has
> heavily contributed to the design of this KIP.
>
> Regards,
> David
>
> On Fri, Jun 12, 2020 at 10:08 PM Colin McCabe  wrote:
>
>> +1.  Thanks, David!
>>
>> best,
>> Colin
>>
>> On Thu, Jun 11, 2020, at 23:51, David Jacot wrote:
>> > Colin, Jun,
>> >
>> > Do the proposed error code and the updated KIP look good to you guys?
>> I’d
>> > like to wrap up and close the vote.
>> >
>> > Thanks,
>> > David
>> >
>> > Le mer. 10 juin 2020 à 14:50, David Jacot  a
>> écrit :
>> >
>> > > Hi Colin and Jun,
>> > >
>> > > I have no problem if we have to rewrite part of it when the new
>> controller
>> > > comes
>> > > out. I will be more than happy to help out.
>> > >
>> > > Regarding KIP-590, I think that we can cope with a principal as a
>> string
>> > > when the
>> > > time comes. The user entity name is defined with a string already.
>> > >
>> > > Regarding the name of the error, you have made a good point. I do
>> agree
>> > > that it
>> > > is important to differentiate the two cases. I propose the following
>> two
>> > > errors:
>> > > - THROTTLING_QUOTA_EXCEEDED - Throttling is slightly better than rate
>> as
>> > > we have quotas which are not rate (e.g. request quota). This one is
>> > > retryable
>> > > once the throttle time is passed.
>> > > - LIMIT_QUOTA_EXCEEDED - This one would indicate that the limit has
>> been
>> > > reached and is a final error.
>> > > We only need the former in this KIP. What do you think?
>> > >
>> > > Jun, I have added a few examples in the KIP. The new name works
>> exactly
>> > > like
>> > > the existing one once it is added to the accepted dynamic configs for
>> the
>> > > user
>> > > and the client entities. I have added a "Kafka Config Command"
>> chapter in
>> > > the
>> > > KIP. I will also open a Jira to not forget updating the AK
>> documentation
>> > > once
>> > > the KIP gets merged.
>> > >
>> > > Thanks,
>> > > David
>> > >
>> > > On Wed, Jun 10, 2020 at 3:03 AM Jun Rao  wrote:
>> > >
>> > >> Hi, Colin,
>> > >>
>> > >> Good point. Maybe sth like THROTTLING_QUOTA_VIOLATED will make this
>> clear.
>> > >>
>> > >> Hi, David,
>> > >>
>> > >> We added a new quota name in the KIP. You chose not to bump up the
>> version
>> > >> of DESCRIBE_CLIENT_QUOTAS and ALTER_CLIENT_QUOTAS, which seems ok
>> since
>> > >> the
>> > >> quota name is represented as a string. However, the new quota name
>> can be
>> > >> used in client tools for setting and listing the quota (
>> > >> https://kafka.apache.org/documentation/#quotas). Could you document
>> how
>> > >> the
>> > >> new name will be used in those tools?
>> > >>
>> > >> Thanks,
>> > >>
>> > >> Jun
>> > >>
>> > >> On Tue, Jun 9, 2020 at 3:37 PM Colin McCabe 
>> wrote:
>> > >>
>> > >> > On Tue, Jun 9, 2020, at 05:06, David Jacot wrote:
>> > >> > > Hi Colin,
>> > >> > >
>> > >> > > Thank you for your feedback.
>> > >> > >
>> > >> > > Jun has summarized the situation pretty well. Thanks Jun! I would
>> > >> like to
>> > >> > > complement it with the following points:
>> > >> > >
>> > >> > > 1. Indeed, when the quota is exceeded, the broker will reject the
>> > >> topic
>> > >> > > creations, partition creations and topics deletions that are
>> exceeding
>> > >> > > with the new QUOTA_VIOLATED error. The ThrottleTimeMs field will
>> > >> > > be populated accordingly to let the client know how long it must
>> wait.
>> > >> > >
>> > >> > > 2. I do agree that we actually want a mechanism to apply back
>> pressure
>> > >> > > to the clients. The KIP basically proposes a mechanism to
>> control and
>> > >> to
>> > >> > > limit the rate of operations before entering the controller. I
>> think
>> > >> that
>> > >> > > it is similar to your thinking but is enforced based on a defined
>> > >> > > instead of relying on the number of pending items in the
>> controller.
>> > >> > >
>> > >> > > 3. You mentioned an alternative idea in your comments that, if I
>> > >> > understood
>> > >> > > correctly, would bound the queue to limit the overload and reject
>> > >> work if
>> > >> > > the queue is full. I have been thinking about this as well but I
>> don't
>> > >> > think
>> > >> > > that it  works well in our case.
>> > >> > > - The first reason is 

Build failed in Jenkins: kafka-2.4-jdk8 #235

2020-07-28 Thread Apache Jenkins Server
See 


Changes:

[matthias] MINOR: Remove staticmethod tag to be able to use logger of instance


--
[...truncated 8.37 MB...]

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfKeyIsDifferentForCompareKeyValueTimestamp STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfKeyIsDifferentForCompareKeyValueTimestamp PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfKeyIsDifferentForCompareKeyValueTimestampWithProducerRecord STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfKeyIsDifferentForCompareKeyValueTimestampWithProducerRecord PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldPassIfValueIsEqualWithNullForCompareValueWithProducerRecord STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldPassIfValueIsEqualWithNullForCompareValueWithProducerRecord PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullReverseForCompareValueTimestamp STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullReverseForCompareValueTimestamp PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullReverseForCompareValue STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullReverseForCompareValue PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfTimestampIsDifferentForCompareValueTimestamp STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfTimestampIsDifferentForCompareValueTimestamp PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldPassIfKeyAndValueAndTimestampIsEqualWithNullForCompareKeyValueTimestampWithProducerRecord
 STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldPassIfKeyAndValueAndTimestampIsEqualWithNullForCompareKeyValueTimestampWithProducerRecord
 PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullForCompareKeyValueWithProducerRecord STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullForCompareKeyValueWithProducerRecord PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentForCompareKeyValueTimestampWithProducerRecord 
STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentForCompareKeyValueTimestampWithProducerRecord PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldNotAllowNullProducerRecordWithExpectedRecordForCompareValueTimestamp 
STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldNotAllowNullProducerRecordWithExpectedRecordForCompareValueTimestamp 
PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldNotAllowNullExpectedRecordForCompareValue STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldNotAllowNullExpectedRecordForCompareValue PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullReversForCompareKeyValueTimestampWithProducerRecord
 STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullReversForCompareKeyValueTimestampWithProducerRecord
 PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldNotAllowNullProducerRecordForCompareKeyValue STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldNotAllowNullProducerRecordForCompareKeyValue PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullReversForCompareKeyValueWithProducerRecord 
STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullReversForCompareKeyValueWithProducerRecord 
PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldPassIfKeyAndValueAndTimestampIsEqualForCompareKeyValueTimestamp STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldPassIfKeyAndValueAndTimestampIsEqualForCompareKeyValueTimestamp PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullForCompareKeyValueTimestampWithProducerRecord
 STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfValueIsDifferentWithNullForCompareKeyValueTimestampWithProducerRecord
 PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfKeyIsDifferentWithNullReversForCompareKeyValueWithProducerRecord 
STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldFailIfKeyIsDifferentWithNullReversForCompareKeyValueWithProducerRecord 
PASSED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldPassIfKeyAndValueIsEqualWithNullForCompareKeyValueWithProducerRecord 
STARTED

org.apache.kafka.streams.test.OutputVerifierTest > 
shouldPassIfKeyAndValueIsEqualWithNullForCompareKeyValueWithProducerRecord 
PASSED


Re: [DISCUSS] KIP-405: Kafka Tiered Storage

2020-07-28 Thread Satish Duggana
HI Jun,
Thanks for your comments. We put our inline replies below.

1001. I was thinking that you could just use the tiered metadata to do the
reconciliation. The tiered metadata contains offset ranges and epoch
history. Those should be enough for reconciliation purposes.

If we use remote storage as the source-of-truth during
unclean-leader-election, it's possible that after reconciliation the
remote storage will have more recent data than the new leader's local
storage. For example, the new leader's latest message is offset 1000,
while the remote storage has message 1100. In such a case, the new
leader will have to download the messages from 1001 to 1100, before
accepting new messages from producers. Otherwise, there would be a gap
in the local data between 1000 and 1101.

Moreover, with the current design, leader epoch history is stored in
remote storage, rather than the metadata topic. We did consider saving
epoch history in remote segment metadata. But the concern is that
there is currently no limit for the epoch history size. Theoretically,
if a user has a very long remote retention time and there are very
frequent leadership changes, the leader epoch history can become too
long to fit into a regular Kafka message.


1003.3 Having just a serverEndpoint string is probably not enough.
Connecting to a Kafka cluster may need various security credentials. We can
make RLMM configurable and pass in the properties through the configure()
method. Ditto for RSM.

RLMM and  RSM are already configurable and they take properties which
start with "remote.log.metadata." and "remote.log.storage."
respectively and a few others. We have listener-name as the config for
RLMM and other properties(like security) can be sent as you suggested.
We will update the KIP with the details.


1008.1 We started with log.retention.hours and log.retention.minutes, and
added log.retention.ms later. If we are adding a new configuration, ms
level config alone is enough and is simpler. We can build tools to make the
configuration at different granularities easier. The definition of
log.retention.ms is "The number of milliseconds to keep a log file before
deleting it". The deletion is independent of whether tiering is enabled or
not. If this changes to just the local portion of the data, we are changing
the meaning of an existing configuration.

We are fine with either way. We can go with log.retention. as the
effective log retention instead of local log retention. With this
convention, we need to introduce  local.log.retention instead of
remote.log.retention.ms that we proposed. If log.retention.ms as -1
then remote retention is also considered as unlimited but user should
be able to set the local.retention.ms.
So, we need to introduce local.log.retention.ms and
local.log.retention.bytes which should  always  be <=
log.retention.ms/bytes respectively.



On Fri, Jul 24, 2020 at 3:37 AM Jun Rao  wrote:
>
> Hi, Satish,
>
> Thanks for the reply. A few quick comments below.
>
> 1001. I was thinking that you could just use the tiered metadata to do the
> reconciliation. The tiered metadata contains offset ranges and epoch
> history. Those should be enough for reconciliation purposes.
>
> 1003.3 Having just a serverEndpoint string is probably not enough.
> Connecting to a Kafka cluster may need various security credentials. We can
> make RLMM configurable and pass in the properties through the configure()
> method. Ditto for RSM.
>
> 1008.1 We started with log.retention.hours and log.retention.minutes, and
> added log.retention.ms later. If we are adding a new configuration, ms
> level config alone is enough and is simpler. We can build tools to make the
> configuration at different granularities easier. The definition of
> log.retention.ms is "The number of milliseconds to keep a log file before
> deleting it". The deletion is independent of whether tiering is enabled or
> not. If this changes to just the local portion of the data, we are changing
> the meaning of an existing configuration.
>
> Jun
>
>
> On Thu, Jul 23, 2020 at 11:04 AM Satish Duggana 
> wrote:
>
> > Hi Jun,
> >
> > Thank you for the comments! Ying, Harsha and I discussed and put our
> > comments below.
> >
> >
> > 1001. The KIP described a few scenarios of unclean leader elections. This
> > is very useful, but I am wondering if this is the best approach. My
> > understanding of the proposed approach is to allow the new (unclean) leader
> > to take new messages immediately. While this increases availability, it
> > creates the problem that there could be multiple conflicting segments in
> > the remote store for the same offset range. This seems to make it harder
> > for RLMM to determine which archived log segments contain the correct data.
> > For example, an archived log segment could at one time be the correct data,
> > but be changed to incorrect data after an unclean leader election. An
> > alternative approach is to let the unclean leader use the archived data as
> 

Re: [VOTE] KIP-607: Add Metrics to Record the Memory Used by RocksDB to Kafka Streams

2020-07-28 Thread Bruno Cadonna

Thank you for voting.

The updated KIP-607 is accepted with
- 3 binding votes (John, Bill, and Guozhang)
- 1 non-binding vote (Navinder)

Best,
Bruno

On 27.07.20 20:06, Guozhang Wang wrote:

+1 (binding).

Thanks.

On Mon, Jul 27, 2020 at 9:24 AM Bill Bejeck  wrote:


Thanks for the KIP Bruno.

+1 (binding)

-Bill

On Mon, Jul 27, 2020 at 4:26 AM Navinder Brar
 wrote:


+1 (non-binding). Thanks for the KIP, Bruno.

~Navinder

 On Friday, 24 July, 2020, 08:41:03 pm IST, John Roesler <
vvcep...@apache.org> wrote:

  Thanks, Bruno!

I'm +1 (binding)

-John

On Fri, Jul 24, 2020, at 07:04, Bruno Cadonna wrote:

Hi,

After re-opening the discussion about





https://cwiki.apache.org/confluence/display/KAFKA/KIP-607%3A+Add+Metrics+to+Kafka+Streams+to+Report+Properties+of+RocksDB


I would like to re-open the voting for this KIP.

The discussion thread can be found here:





http://mail-archives.apache.org/mod_mbox/kafka-dev/202005.mbox/%3CCADR0NwzJBJa6WihnpmGj0R%2BYPVrojq4Kg_hOArNEytHAG-tZAQ%40mail.gmail.com%3E


Best,
Bruno

On 19.05.20 10:00, Bruno Cadonna wrote:

Thank you for voting!

This KIP passes with:
4 binding +1
1 non-binding +1
0 -1

Best,
Bruno

On Fri, May 15, 2020 at 11:34 PM Matthias J. Sax 

wrote:


+1 (binding)

-Matthias

On 5/15/20 11:48 AM, John Roesler wrote:

Thanks, Bruno!

I’m +1 (binding)

-John

On Fri, May 15, 2020, at 11:32, Sophie Blee-Goldman wrote:

Thanks Bruno! +1 (non-binding)

Sophie

On Fri, May 15, 2020 at 8:15 AM Bill Bejeck 

wrote:



Thanks for the KIP!

+1 (binding)

-Bill

On Fri, May 15, 2020 at 11:12 AM Guozhang Wang <

wangg...@gmail.com>

wrote:



+1.

Thanks!

On Fri, May 15, 2020 at 1:36 AM Bruno Cadonna <

br...@confluent.io



wrote:



Hi all,

I'd like to call for votes on

KIP-607: Add Metrics to Record the Memory Used by RocksDB to

Kafka

Streams


The KIP can be found here









https://cwiki.apache.org/confluence/display/KAFKA/KIP-607%3A+Add+Metrics+to+Kafka+Streams+to+Record+the+Memory+Used+by+RocksDB


The discussion thread can be found here:









http://mail-archives.apache.org/mod_mbox/kafka-dev/202005.mbox/%3CCADR0NwzJBJa6WihnpmGj0R%2BYPVrojq4Kg_hOArNEytHAG-tZAQ%40mail.gmail.com%3E


Best,
Bruno




--
-- Guozhang
















Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-28 Thread David Jacot
Hi Boyang,

Thanks for the update.

1./2. In KIP-599 (accepted and already in trunk), we throttle the
CreateTopicsRequest,
CreatePartitionsRequest, and DeleteTopicsRequests by muting the channel used
by the Admin client and setting the throttleTimeMs in the response. The
change that
you propose breaks this. If we distribute these requests to all brokers,
muting channels
does not make sense anymore.

Have we considered continuing sending controller requests to the broker
that hosts the
controller? We could continue to send requests to the broker listener and
redirect them
to the controller internally. This would still keep the controller
isolated. Another advantage
of doing so is that the forwarding on the same machine does not require to
go over the
network.

Could you elaborate a bit more on the motivation and the reasoning behind
this change?
Is there a requirement or a strong advantage that I have missed?

Best,
David

On Tue, Jul 28, 2020 at 6:48 AM Boyang Chen 
wrote:

> Hey there,
>
> I'm re-opening this thread because after some initial implementations
> started, we spotted some gaps in the approved KIP as well as some
> inconsistencies with KIP-631 controller. There are a couple of addendums to
> the existing KIP, specifically:
>
> 1. As the controller is foreseen to be only accessible to the brokers, the
> new admin client would not have direct access to the controller. It is
> guaranteed on the MetadataResponse level which no longer provides
> `ControllerId` to client side requests.
>
> 2. The broker would forward any direct ZK path mutation requests, including
> topic creation/deletion, reassignment, etc since we deprecate the direct
> controller access on the client side. No more protocol version bump is
> necessary for the configuration requests.
>
> 3. To make sure forwarding requests pass the authorization, broker
> principal CLUSTER_ACTION would be allowed to be used as an alternative
> authentication method for a variety of principal operations, including
> ALTER, ALTER_CONFIG, DELETE, etc. It is because the forwarding request
> needs to use the proxy broker's own principal, which is currently not
> supported to be used for many configuration change authentication listed
> above. The full list could be found in the KIP.
>
> 4. Add a new BROKER_AUTHORIZATION_FAILURE error code to indicate any
> internal security configuration failure, when the forwarded request failed
> authentication on the controller side.
>
> Let me know what you think. With such a major refinement of the KIP, I'm
> open for re-vote after discussions converge.
>
> Boyang
>
> On Wed, Jul 1, 2020 at 2:17 PM Boyang Chen 
> wrote:
>
> > Hey folks,
> >
> > I have also synced on the KIP-578 which was doing the partition limit, to
> > make sure the partition limit error code would be properly propagated
> once
> > it is done on top of KIP-590. Let me know if you have further questions
> or
> > concerns.
> >
> > Boyang
> >
> > On Tue, Jun 23, 2020 at 5:08 PM Boyang Chen 
> > wrote:
> >
> >> Thanks for the clarification, Colin and Ismael. Personally I also feel
> >> Option A is better to prioritize fixing the gap. Just to be clear, the
> >> proposed solution would be:
> >>
> >> 1. Bump the Metadata RPC version to return POLICY_VIOLATION. In the
> >> application level, we should swap the error message with the actual
> failure
> >> reason such as "violation of topic creation policy when attempting to
> auto
> >> create internal topic through MetadataRequest."
> >>
> >> 2. For older Metadata RPC, return AUTHORIZATION_FAILED to fail fast.
> >>
> >> Will address our other discussed points as well in the KIP, let me know
> >> if you have further questions.
> >>
> >> Thanks,
> >> Boyang
> >>
> >> On Tue, Jun 23, 2020 at 10:41 AM Ismael Juma  wrote:
> >>
> >>> Option A is basically what I was thinking. But with a slight
> adjustment:
> >>>
> >>> New versions of MetadataResponse return POLICY_VIOLATION, old versions
> >>> return AUTHORIZATION_FAILED. The latter works correctly with old Java
> >>> clients (i.e. the client fails fast and propagates the error), I've
> >>> tested
> >>> it. Adjust new clients to treat POLICY_VIOLATION like
> >>> AUTHORIZATION_FAILED,
> >>> but propagate the custom error message.
> >>>
> >>> Ismael
> >>>
> >>> On Mon, Jun 22, 2020 at 11:00 PM Colin McCabe 
> >>> wrote:
> >>>
> >>> > > > > On Fri, Jun 19, 2020 at 3:18 PM Ismael Juma  >
> >>> > wrote:
> >>> > > > >
> >>> > > > > > Hi Colin,
> >>> > > > > >
> >>> > > > > > The KIP states in the Compatibility section (not Future
> work):
> >>> > > > > >
> >>> > > > > > "To support the proxy of requests, we need to build a channel
> >>> for
> >>> > > > > > brokers to talk directly to the controller. This part of the
> >>> design
> >>> > > > > > is internal change only and won’t block the KIP progress."
> >>> > > > > >
> >>> > > > > > I am clarifying that this is not internal only due to the
> >>> config.
> >>> > If we
> >>> > > > > > say that this KIP depends on