Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-19 Thread Ismael Juma
Thanks for the KIP, Steven. A few minor comments:

1. The KIP seems to rely on the pull request for some of the details of the
proposal. Generally, the KIP should stand on its own. For example, the
public interfaces section should include the signature of interfaces and
methods being added.

2. Do we really need to deprecate `Function`? This will add build noise to
any library that builds with 1.1+ but also wants to support 0.11 and 1.0.
It may be worth just documenting that `FunctionInterface` is the one to use
for lambda support.

3. `FunctionInterface` is a bit of a clunky name. Due to lambdas, users
don't have to type the name themselves, so maybe it's fine as it is. An
alternative would be `BaseFunction` or something like that.

Ismael

On Tue, Dec 12, 2017 at 4:24 PM, Xavier Léauté  wrote:

> I'm fine with the whenComplete solution as well.
> On Tue, Dec 12, 2017 at 03:57 Tom Bentley  wrote:
>
> > Hi Steven,
> >
> > I am happy with adding whenComplete() instead of addWaiter(),
> >
> > Cheers,
> >
> > Tom
> >
> > On 12 December 2017 at 11:11, Steven Aerts 
> wrote:
> >
> > > Xavier, Colin and Tom
> > >
> > > can you line up on this?
> > > I don't really mind which solution is chosen, but I think it needs to
> be
> > > done be before I can close the vote.
> > >
> > > I want to help you with the implementation after a decision has been
> > made.
> > > Just let me know.
> > >
> > >
> > > Thanks,
> > >
> > >
> > >Steven
> > >
> > > Op di 12 dec. 2017 om 03:54 schreef Colin McCabe :
> > >
> > > > Thanks, Xavier we should definitely think about what happens when
> > > > exceptions are thrown from these functions.
> > > >
> > > > I would suggest maybe we should just implement whenComplete, rather
> > than
> > > > exposing addWaiter.  addWaiter was never intended as a public API,
> and
> > > > it's a little weird.  whenComplete is nice because it supports
> > chaining,
> > > > and should be more familiar to users of other async APIs.
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Fri, Dec 8, 2017, at 16:26, Xavier Léauté wrote:
> > > > > Hi Steven,
> > > > >
> > > > > I noticed you are making KafkaFuture.addWaiter(...) public as part
> of
> > > > > your
> > > > > PR. This is a very useful method to add – and you should mention it
> > in
> > > > > the
> > > > > KIP – however addWaiter currently doesn't guard against exceptions
> > > thrown
> > > > > inside of the BiConsumer function, which is something we should
> > > probably
> > > > > fix before making it public.
> > > > >
> > > > > I was about to make the necessary exception handling changes as
> part
> > of
> > > > > https://github.com/apache/kafka/pull/4308 until someone pointed
> out
> > > your
> > > > > KIP to me. Since you already have a PR out, it might be worth
> > > > > incorporating
> > > > > my fixes (and the extra docs), what do you think?
> > > > >
> > > > > I'll rebase my PR onto yours to make it easier to merge.
> > > > >
> > > > > Thanks!
> > > > > Xavier
> > > > >
> > > > >
> > > > > On Mon, Dec 4, 2017 at 4:03 AM Steven Aerts <
> steven.ae...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Tom,
> > > > > >
> > > > > > Thanks for the review.
> > > > > > updated the motivation a little bit, it's better, but I have to
> > admit
> > > > can
> > > > > > be improved.
> > > > > > I made addWaiters public.
> > > > > >
> > > > > > Enjoy,
> > > > > >
> > > > > > Steven
> > > > > >
> > > > > >
> > > > > >
> > > > > > Op ma 4 dec. 2017 om 11:01 schreef Tom Bentley <
> > > t.j.bent...@gmail.com
> > > > >:
> > > > > >
> > > > > > > Hi Steven,
> > > > > > >
> > > > > > > Thanks for updating the KIP. I have a couple of points:
> > > > > > >
> > > > > > > 1. Typo in the first sentence of the Motivation. Also what does
> > > > "empty
> > > > > > > public abstract classes with one abstract method" mean -- if
> it's
> > > > got one
> > > > > > > abstract method in what way is it empty?
> > > > > > > 2.From an entirely self-centred point of view, the main thing
> > > that's
> > > > > > > missing for my work in KIP-183 is that addWaiter() needs to be
> > > > public.
> > > > > > >
> > > > > > > Thanks again,
> > > > > > >
> > > > > > > Tom
> > > > > > >
> > > > > > > On 2 December 2017 at 10:07, Steven Aerts <
> > steven.ae...@gmail.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > I just made changes to the proposal of KIP-218, to make
> > > everything
> > > > more
> > > > > > > > backwards compatible as suggested by Collin.
> > > > > > > > For me it is now in a state where starts to become final.
> > > > > > > >
> > > > > > > > I propose to wait a few days so everybody can take a look and
> > > open
> > > > the
> > > > > > > > votes when I do not receive any major comments.
> > > > > > > >
> > > > > > > > Does that sound ok for you?
> > > > > > > >
> > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > 218%3A+Make+KafkaFuture.Function+java+8+lambda+compatible
>

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

2017-12-19 Thread Tom Bentley
+1

On 18 December 2017 at 23:28, Vahid S Hashemian 
wrote:

> +1
>
> Thanks for the KIP.
>
> --Vahid
>
>
>
> From:   Ted Yu 
> To: dev@kafka.apache.org
> Date:   12/18/2017 02:45 PM
> Subject:Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
> constructors public
>
>
>
> +1
>
> nit: via "copy and past" an 'e' is missing at the end.
>
> On Mon, Dec 18, 2017 at 2:38 PM, Matthias J. Sax 
> wrote:
>
> > Hi,
> >
> > I want to propose the following KIP:
> >
> https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.
> apache.org_confluence_display_KAFKA_KIP-2D&d=DwIBaQ&c=jf_
> iaSHvJObTbx-siA1ZOg&r=Q_itwloTQj3_xUKl7Nzswo6KE4Nj-
> kjJc7uSVcviKUc&m=JToRX4-HeVsRoOekIz18ht-YLMe-T21MttZTgbxB4ag&s=
> 6aZjPCc9e00raokVPKvx1BxwDOHyCuKNgtBXPMeoHy4&e=
>
> > 243%3A+Make+ProducerConfig+and+ConsumerConfig+constructors+public
> >
> >
> > This is a rather straight forward change, thus I skip the DISCUSS
> > thread and call for a vote immediately.
> >
> >
> > -Matthias
> >
> >
>
>
>
>
>


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

2017-12-19 Thread Jan Filipiak

Sorry for coming back at this so late.



On 11.12.2017 07:12, Colin McCabe wrote:

On Sun, Dec 10, 2017, at 22:10, Colin McCabe wrote:

On Fri, Dec 8, 2017, at 01:16, Jan Filipiak wrote:

Hi,

sorry for the late reply, busy times :-/

I would ask you one thing maybe. Since the timeout
argument seems to be settled I have no further argument
form your side except the "i don't want to".

Can you see that connection.max.idle.max is the exact time
that expresses "We expect the client to be away for this long,
and come back and continue"?

Hi Jan,

Sure, connection.max.idle.max is the exact time that we want to keep
around a TCP session.  TCP sessions are relatively cheap, so we can
afford to keep them around for 10 minutes by default.  Incremental fetch
state is less cheap, so we want to set a shorter timeout for it.  We
also want new TCP sessions to be able to reuse an existing incremental
fetch session rather than creating a new one and waiting for the old one
to time out.


also clarified some stuff inline

Best Jan




On 05.12.2017 23:14, Colin McCabe wrote:

On Tue, Dec 5, 2017, at 13:13, Jan Filipiak wrote:

Hi Colin

Addressing the topic of how to manage slots from the other thread.
With tcp connections all this comes for free essentially.

Hi Jan,

I don't think that it's accurate to say that cache management "comes for
free" by coupling the incremental fetch session with the TCP session.
When a new TCP session is started by a fetch request, you still have to
decide whether to grant that request an incremental fetch session or
not.  If your answer is that you always grant the request, I would argue
that you do not have cache management.

First I would say, the client has a big say in this. If the client
is not going to issue incremental he shouldn't ask for a cache
when the client ask for the cache we still have all options to deny.

To put it simply, we have to have some cache management above and beyond
just giving out an incremental fetch session to anyone who has a TCP
session.  Therefore, caching does not become simpler if you couple the
fetch session to the TCP session.
Simply giving out an fetch session for everyone with a connection is too 
simple,
but I think it plays well into the idea of consumers choosing to use the 
feature
therefore only enabling where it brings maximum gains 
(replicas,MirrorMakers)



I guess you could argue that timeouts are cache management, but I don't
find that argument persuasive.  Anyone could just create a lot of TCP
sessions and use a lot of resources, in that case.  So there is
essentially no limit on memory use.  In any case, TCP sessions don't
help us implement fetch session timeouts.

We still have all the options denying the request to keep the state.
What you want seems like a max connections / ip safeguard.
I can currently take down a broker with to many connections easily.



I still would argue we disable it by default and make a flag in the
broker to ask the leader to maintain the cache while replicating and also only
have it optional in consumers (default to off) so one can turn it on
where it really hurts.  MirrorMaker and audit consumers prominently.

I agree with Jason's point from earlier in the thread.  Adding extra
configuration knobs that aren't really necessary can harm usability.
Certainly asking people to manually turn on a feature "where it really
hurts" seems to fall in that category, when we could easily enable it
automatically for them.

This doesn't make much sense to me.

There are no tradeoffs to think about from the client's point of view:
it always wants an incremental fetch session.  So there is no benefit to
making the clients configure an extra setting.  Updating and managing
client configurations is also more difficult than managing broker
configurations for most users.


You also wanted to implement
a "turn of in case of bug"-knob. Having the client indicate if the
feauture will be used seems reasonable to me.,

True.  However, if there is a bug, we could also roll back the client,
so having this configuration knob is not strictly required.


Otherwise I left a few remarks in-line, which should help to understand
my view of the situation better

Best Jan


On 05.12.2017 08:06, Colin McCabe wrote:

On Mon, Dec 4, 2017, at 02:27, Jan Filipiak wrote:

On 03.12.2017 21:55, Colin McCabe wrote:

On Sat, Dec 2, 2017, at 23:21, Becket Qin wrote:

Thanks for the explanation, Colin. A few more questions.


The session epoch is not complex.  It's just a number which increments
on each incremental fetch.  The session epoch is also useful for
debugging-- it allows you to match up requests and responses when
looking at log files.

Currently each request in Kafka has a correlation id to help match the
requests and responses. Is epoch doing something differently?

Hi Becket,

The correlation ID is used within a single TCP session, to uniquely
associate a request with a response.  The correlation ID is not unique
(and has no meaning) outside 

Re: [DISCUSS] KIP-236 Interruptible Partition Reassignment

2017-12-19 Thread Tom Bentley
Hi Jun,

10. Another concern of mine is on consistency with the current pattern. The
> current pattern for change notification based on ZK is (1) we first write
> the actual value in the entity path and then write the change notification
> path, and (2)  the change notification path only includes what entity has
> changed but not the actual changes. If we want to follow this pattern for
> consistency, /admin/reassignment_requests/request_xxx will only have the
> partitions whose reassignment have changed, but not the actual
> reassignment.
>

Ah, I hadn't understood part (2). That means my concern about efficiency
with the current pattern is misplaced. There are still some interesting
differences in semantics, however:

a) The mechanism currently proposed in KIP-236 means that the controller is
the only writer to /admin/reassignments. This means it can include
information in these znodes that requesters might not know, or information
that's necessary to perform the reassignment but not necessary to describe
the request. While this could be handled using the current pattern it would
rely on all  writers to preserve any information added by the controller,
which seems complicated and hence fragile.

b) The current pattern for change notification doesn't cope with competing
writers to the entity path: If two processes write to the entity path
before the controller can read it (due to notification) then one set of
updates will be lost.

c) If a single writing process crashes after writing to the entity path,
but before writing to the notification path then the write will be lost.

I'm actually using point a) in my WIP (see below). Points b) and c) are
obviously edge cases.


> 11. Ok. I am not sure that I fully understand the description of that part.
> Does "assigned" refer to the current assignment? Could you also describe
> where the length of the original assignment is stored in ZK?
>

Sorry if the description is not clear. Yes, "assigned" referrs to the
currently assigned replicas (taken from the
ControllerContext.partitionReplicaAssignment). I would store the length of
the original assignment in the /admin/reassignments/$topic/$partition znode
(this is where the point (a) above is useful -- the requester shouldn't
know that this information is used by the controller).

I've updated the KIP to make these points clearer.


> 13. Hmm, I am not sure that the cancellation needs to be done for the whole
> batch. The reason that I brought this up is for consistency. The KIP allows
> override when using the new approach. It just seems that it's simpler to
> extend this model when resolving multiple changes between the old and the
> new approach.


Ah, I think I've been unclear on this point too. Currently the
ReassignPartitionsCommand enforces that you can't change reassignments, but
this doesn't stop other ZK clients making changes to
/admin/reassign_partitions directly and I believe some Kafka users do
indeed change reassignments in-flight by writing to
/admin/reassign_partitions. What I'm proposing doesn't break that at all.
The semantic I've implemented is only that the controller only refuses a
reassignment change if there is already one in-flight (i.e. in
/admin/reassignments/$topic/$partition) **via the other mechansim**. So if
you're using /admin/reassign_partitions and you change or cancel part of it
via /admin/reassign_partitions, that's OK. Likewise if you're using
/admin/reassignment_request/request_xxx and you change or cancel part of it
via another /admin/reassignment_request/request_xxx, that's OK.What you
can't do is change a request that was started via
/admin/reassign_partitions via /admin/reassignment_request/request_xxx, or
vice versa.

What I was thinking of when I replied is the case where, on controller
failover, /admin/reassign_partitions has been changed and
/admin/reassignment_request/request_xxx created (in the period when the new
controller was being elected, for example) with a common partition. In this
case we should apply a consistent rule to that used when the notification
happen in real time. Your suggestion to use the modification time of the
znode would work here too (except in the edge case where ZK writes to both
znodes happen within the same clock tick on the ZK server, so the mtimes
are the same).

Let me know if you think this is the right semantic and I'll try to clarify
the KIP.

Many thanks,

Tom

On 18 December 2017 at 18:12, Jun Rao  wrote:

> Hi, Tom,
>
> Thanks for the reply. A few more followup comments below.
>
> 10. Another concern of mine is on consistency with the current pattern. The
> current pattern for change notification based on ZK is (1) we first write
> the actual value in the entity path and then write the change notification
> path, and (2)  the change notification path only includes what entity has
> changed but not the actual changes. If we want to follow this pattern for
> consistency, /admin/reassignment_requests/request_xxx will only have the
> p

[jira] [Created] (KAFKA-6385) Rack awareness ignored by kafka-reassign-partitions

2017-12-19 Thread Gal Barak (JIRA)
Gal Barak created KAFKA-6385:


 Summary: Rack awareness ignored by kafka-reassign-partitions
 Key: KAFKA-6385
 URL: https://issues.apache.org/jira/browse/KAFKA-6385
 Project: Kafka
  Issue Type: Bug
Affects Versions: 1.0.0
 Environment: Ubuntu 16.04
Reporter: Gal Barak
Priority: Minor
 Attachments: actual.txt, topic-to-move.json

Hi,
It seems that the kafka-reassign-partitions script ignores rack awareness, when 
suggesting a new partition layout. Came across it when doing some initial 
testing with Kafka.

+To reproduce:+
#  Create a Kafka cluster with 3 brokers (1,2,3). Use 3 different racks 
(broker.rack definition. Example: "A", "B" and "C").
#* I used a non-root directory in zookeeper (i.e. - {{:2181,:2181,:2182/}})
#* The tested topic was automatically created, according to a default 
configuration of 12 partitions and 3 replicas per topic.
# Install a 4th broker, and assign it to the same rack as the 1st broker ("A").
# Create a topics-to-move.json file for a single topic. The file I used was 
uploaded as topic-to-move.json.
# Run the kafka-reassign-partitions script:
{{kafka-reassign-partitions --zookeeper :2181,:2181,:2182/ 
--topics-to-move-json-file  --broker-list "1,2,3,4" 
--generate}}

+Expected result:+
A suggested reassignment that makes sure that no partitions uses both broker 1 
and broker 4 as its replicas.

+Actual results of the command:+
The full result is attached as a file (actual.txt). It includes partitions with 
replicas that are on both brokers 1 and 4, which are two servers on the same 
rack.
Example: {"topic":"","partition":6,"replicas":[1,2,4]}

+Additional notes:+
* I did not test starting the cluster from scratch. The same behavior might be 
present when topic partitions are created automatically (in which case, the 
priority might be higher).
* I'm not sure it's related. But the original assignment seems to be 
problematic as well: If a single server (of the 3) failed, a different single 
server became the leader for all of its partitions. For example, if broker 1 
failed, server 2 became the leader for all of the partitions for which 1 was 
previously the leader, instead of having the load distributed evenly between 
brokers 2 and 3.





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


Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration

2017-12-19 Thread Rajini Sivaram
Hi Jason,

Thank you!

2. Updated the KIP:  Reconfigurable extends Configurable
4. Changed is_default to config_source in config_entry in the  protocol. It
will be less confusing that way. The method isDefault() will just
return configSource
== DEFAULT_CONFIG. Have also included the changes to the public classes in
the KIP.
6. It is a nice idea to have an automatically generated secret to avoid
yet another config. But I wasn't entirely sure, so went for an explicit
config instead (a bunch of them actually). I had two concerns (a) we might
have a password (like the delegation token master secret) that we want
to encrypt in future that is stored as a cluster-wide password. It will be
better if we can configure the broker secret  for that, even though for
that case we will have the same restriction that all brokers are configured
with the same secret. (b) broker writes to meta.properties, so there is a
possibility of losing the secret.


On Tue, Dec 19, 2017 at 12:47 AM, Jason Gustafson 
wrote:

> Hey Rajini,
>
> Thanks, makes sense. A couple replies:
>
> 2. I haven't changed the way Configurable is used. It is still used for
> > initial configuration (if the class implements it). Reconfigurable is
> used
> > at the moment only for reconfiguration. The reason I did it that way is
> > because for some of the internal components, the reconfiguration is
> handled
> > separately from initial configuration (we reconfigure classes which don't
> > implement Configurable). But if that is confusing, I can make
> > Reconfigurable
> > extend Configurable and add a dummy method in internal classes. What do
> you
> > think?
>
>
> I guess the slight mismatch comes from the difference in initialization
> between plugins and internal classes. For plugins, we only initialize state
> through configure() so it would be a little weird to have one which was
> Reconfigurable but not Configurable. Internal classes, on the other hand,
> probably have constructors which take the config values explicitly. If it
> worked analogously for reconfiguration, I would expect that the
> reconfigurable internal classes would have an explicit method and would not
> need Reconfigurable at all. That gives us a slightly nicer API for testing.
> That said, if the Reconfigurable API simplifies the internal usage quite a
> bit, then I have no complaint.
>
> 6. I hope not :-) We wouldn't want to store master secret in ZooKeeper. I
> > wasn't planning to add encryption for passwords in ZooKeeper initially
> and
> > I think that is ok for keystore passwords. But having started to
> implement
> > new listeners which require sasl.jaas.config, I don't think we can
> release
> > that with unencrypted passwords in ZooKeeper. We don't really need a
> master
> > secret that is same across all brokers since all the password configs at
> > the moment are per-broker configs. So I think I will add a new static
> > config to the KIP.
>
>
> Haha, agreed. If the configs are pre-broker, you might also consider
> generating a secret automatically (e.g. it could be added to
> meta.properties?).
>
> Thanks,
> Jason
>
> On Mon, Dec 18, 2017 at 4:07 PM, Rajini Sivaram 
> wrote:
>
> > Hi Jason,
> >
> > Thank you for reviewing the KIP.
> >
> > 1. ConfigDef is used for validating the type of the value and the
> > constraints. But I am doing a lot more validation of security configs.
> For
> > example, for keystore configuration update, validate() loads the keystore
> > and if it is an inter-broker listener, it validates the certificate chain
> > using the truststore for the listener. In the initial configuration case,
> > these errors would be detected when the server is started and the server
> > would just exit. For dynamic configuration, we want to validate as much
> as
> > possible before updating the config in ZooKeeper.
> >
> > 2. I haven't changed the way Configurable is used. It is still used for
> > initial configuration (if the class implements it). Reconfigurable is
> used
> > at the moment only for reconfiguration. The reason I did it that way is
> > because for some of the internal components, the reconfiguration is
> handled
> > separately from initial configuration (we reconfigure classes which don't
> > implement Configurable). But if that is confusing, I can make
> > Reconfigurable
> > extend Configurable and add a dummy method in internal classes. What do
> you
> > think?
> >
> > 3. At the moment, I am returning an empty list. Will add the classes to
> the
> > KIP.
> >
> > 4. I didn't want to change the existing API, so left the config entry as
> > is. When describing synonyms, the entry being described also included in
> > the synonym list with its config source.
> >
> > 5. Configs are validated in groups. validate(Map configs)
> > and reconfigure(Map > ?> configs) both provide all the configs (including those not being
> > altered). The validator for security configs validates the configs of a
> > listener. Validation is performed for altered configs in the

[jira] [Reopened] (KAFKA-6365) How to add a client to list of available clients?

2017-12-19 Thread Lev Gorodinski (JIRA)

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

Lev Gorodinski reopened KAFKA-6365:
---

Hi, looks I don't have edit permissions on 
https://cwiki.apache.org/confluence/display/KAFKA/Clients when I login using 
eulerfx.

> How to add a client to list of available clients?
> -
>
> Key: KAFKA-6365
> URL: https://issues.apache.org/jira/browse/KAFKA-6365
> Project: Kafka
>  Issue Type: Wish
>Reporter: Lev Gorodinski
>Priority: Trivial
>
> I'd like to add a client to: 
> https://cwiki.apache.org/confluence/display/KAFKA/Clients#Clients-.NET
> The client is: https://github.com/jet/kafunk
> .NET written in F# supports 0.8 0.9 0.10



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


[GitHub] kafka pull request #4297: KAFKA-6317: Maven artifact for kafka should not de...

2017-12-19 Thread asfgit
Github user asfgit closed the pull request at:

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


---


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

2017-12-19 Thread Jason Gustafson
Hey Dong,


> I think it is a good idea to let coordinator do the additional sanity check
> to ensure the leader epoch from OffsetCommitRequest never decreases. This
> can help us detect bug. The next question will be what should we do if
> OffsetCommitRequest provides a smaller leader epoch. One possible solution
> is to return a non-retriable error to consumer which will then be thrown to
> user application. But I am not sure it is worth doing it given its impact
> on the user. Maybe it will be safer to simply have an error message in the
> server log and allow offset commit to succeed. What do you think?


I think the check would only have value if you return an error when it
fails. It seems primarily useful to detect buggy consumer logic, so a
non-retriable error makes sense to me. Clients which don't implement this
capability can use the sentinel value and keep the current behavior.

It seems that FetchResponse includes leader epoch via the path
> FetchResponse -> MemoryRecords -> MutableRecordBatch -> DefaultRecordBatch
> -> partitionLeaderEpoch. Could this be an existing case where we expose the
> leader epoch to clients?


Right, in this case the client has no direct dependence on the field, but
it could still be argued that it is exposed (I had actually considered
stuffing this field into an opaque blob of bytes in the message format
which the client wasn't allowed to touch, but it didn't happen in the end).
I'm not opposed to using the leader epoch field here, I was just mentioning
that it does tie clients a bit tighter to something which could be
considered a Kafka internal implementation detail. It makes the protocol a
bit less intuitive as well since it is rather difficult to explain the edge
case it is protecting. That said, we've hit other scenarios where being
able to detect stale metadata in the client would be helpful, so I think it
might be worth the tradeoff.

-Jason

On Mon, Dec 18, 2017 at 6:09 PM, Dong Lin  wrote:

> Hey Jason,
>
> Thanks much for reviewing the KIP.
>
> I think it is a good idea to let coordinator do the additional sanity check
> to ensure the leader epoch from OffsetCommitRequest never decreases. This
> can help us detect bug. The next question will be what should we do if
> OffsetCommitRequest provides a smaller leader epoch. One possible solution
> is to return a non-retriable error to consumer which will then be thrown to
> user application. But I am not sure it is worth doing it given its impact
> on the user. Maybe it will be safer to simply have an error message in the
> server log and allow offset commit to succeed. What do you think?
>
> It seems that FetchResponse includes leader epoch via the path
> FetchResponse -> MemoryRecords -> MutableRecordBatch -> DefaultRecordBatch
> -> partitionLeaderEpoch. Could this be an existing case where we expose the
> leader epoch to clients?
>
> Thanks,
> Dong
>
>
>
> On Mon, Dec 18, 2017 at 3:27 PM, Jason Gustafson 
> wrote:
>
> > Hi Dong,
> >
> > Thanks for the KIP. Good job identifying the problem. One minor question
> I
> > had is whether the coordinator should enforce that the leader epoch
> > associated with an offset commit can only go forward for each partition?
> > Currently it looks like we just depend on the client for this, but since
> > we're caching the leader epoch anyway, it seems like a cheap safety
> > condition. To support old clients, you can always allow the commit if the
> > leader epoch is unknown.
> >
> > I agree that we shouldn't expose the leader epoch in OffsetAndMetadata in
> > the consumer API for what it's worth. As you have noted, it is more of an
> > implementation detail. By the same argument, it's also a bit unfortunate
> > that we have to expose it in the request API since that is nearly as
> > binding in terms of how it limits future iterations. I could be wrong,
> but
> > this appears to be the first case where clients will depend on the
> concept
> > of leader epoch. Might not be a big deal considering how deeply embedded
> > leader epochs already are in the inter-broker RPCs and the message format
> > itself, but just wanted to mention the fact that good encapsulation
> applies
> > to the client request API as well.
> >
> > Thanks,
> > Jason
> >
> > On Mon, Dec 18, 2017 at 1:58 PM, Dong Lin  wrote:
> >
> > > Hey Jun,
> > >
> > > Thanks much for your comments. These are very thoughtful ideas. Please
> > see
> > > my comments below.
> > >
> > > On Thu, Dec 14, 2017 at 6:38 PM, Jun Rao  wrote:
> > >
> > > > Hi, Dong,
> > > >
> > > > Thanks for the update. A few more comments below.
> > > >
> > > > 10. It seems that we need to return the leader epoch in the fetch
> > > response
> > > > as well When fetching data, we could be fetching data from a leader
> > epoch
> > > > older than what's returned in the metadata response. So, we want to
> use
> > > the
> > > > leader epoch associated with the offset being fetched for committing
> > > > offsets.
> > > >
> > >
> > > It seems that we may ha

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

2017-12-19 Thread Guozhang Wang
+1

On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley  wrote:

> +1
>
> On 18 December 2017 at 23:28, Vahid S Hashemian  >
> wrote:
>
> > +1
> >
> > Thanks for the KIP.
> >
> > --Vahid
> >
> >
> >
> > From:   Ted Yu 
> > To: dev@kafka.apache.org
> > Date:   12/18/2017 02:45 PM
> > Subject:Re: [VOTE] KIP-243: Make ProducerConfig and
> ConsumerConfig
> > constructors public
> >
> >
> >
> > +1
> >
> > nit: via "copy and past" an 'e' is missing at the end.
> >
> > On Mon, Dec 18, 2017 at 2:38 PM, Matthias J. Sax 
> > wrote:
> >
> > > Hi,
> > >
> > > I want to propose the following KIP:
> > >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.
> > apache.org_confluence_display_KAFKA_KIP-2D&d=DwIBaQ&c=jf_
> > iaSHvJObTbx-siA1ZOg&r=Q_itwloTQj3_xUKl7Nzswo6KE4Nj-
> > kjJc7uSVcviKUc&m=JToRX4-HeVsRoOekIz18ht-YLMe-T21MttZTgbxB4ag&s=
> > 6aZjPCc9e00raokVPKvx1BxwDOHyCuKNgtBXPMeoHy4&e=
> >
> > > 243%3A+Make+ProducerConfig+and+ConsumerConfig+constructors+public
> > >
> > >
> > > This is a rather straight forward change, thus I skip the DISCUSS
> > > thread and call for a vote immediately.
> > >
> > >
> > > -Matthias
> > >
> > >
> >
> >
> >
> >
> >
>



-- 
-- Guozhang


Re: [VOTE] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-12-19 Thread Guozhang Wang
Hello,

I'm calling for a re-vote on the additional API changes in this KIP. While
working on the implementation I found it's better to add the following
functions for programmability:

* Overloaded KafkaStreams constructor to allow overriding the `Time` object.
* Overloaded AbstractConfig#originalsWithPrefix() to allow specifying
whether or not to strip the prefix in the key of the returned map.

The corresponding wiki page (
https://cwiki.apache.org/confluence/display/KAFKA/KIP-220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier)
has been updated.

Gwen / Damian, could you take a quick look at the updated wiki page and
re-cast your vote?


Guozhang




On Mon, Nov 20, 2017 at 11:12 AM, Guozhang Wang  wrote:

> +1 from myself as well.
>
> I'm closing this KIP as accepted with 3 binding votes (Gwen, Damian, me)
> and 3 non-binding votes (Colin, Ted, Matthias).
>
>
> Guozhang
>
> On Mon, Nov 20, 2017 at 9:56 AM, Damian Guy  wrote:
>
>> +1
>>
>> On Mon, 20 Nov 2017 at 17:52 Gwen Shapira  wrote:
>>
>> > +1
>> >
>> > Make sense. We have a supplier for every other client type :)
>> >
>> > On Fri, Nov 17, 2017 at 1:33 PM Matthias J. Sax 
>> > wrote:
>> >
>> > > +1
>> > >
>> > > On 11/17/17 9:35 AM, Ted Yu wrote:
>> > > > +1
>> > > >
>> > > > On Fri, Nov 17, 2017 at 9:34 AM, Bill Bejeck 
>> > wrote:
>> > > >
>> > > >> +1
>> > > >>
>> > > >> Thanks,
>> > > >> Bill
>> > > >>
>> > > >> On Fri, Nov 17, 2017 at 12:13 PM, Colin McCabe > >
>> > > wrote:
>> > > >>
>> > > >>> +1 (non-binding)
>> > > >>>
>> > > >>> Colin
>> > > >>>
>> > > >>> On Tue, Nov 14, 2017, at 10:02, Guozhang Wang wrote:
>> > >  Hello folks,
>> > > 
>> > >  I have filed a new KIP on adding AdminClient into Streams for
>> > internal
>> > >  topic management.
>> > > 
>> > >  Please review and cast your vote on this thread.
>> > > 
>> > >  *https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > > >>> 220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier
>> > >  > > > >>> 220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier>*
>> > > 
>> > >  The discussion is in another thread so if you have detailed
>> > questions
>> > >  please chime in there.
>> > > 
>> > > 
>> > >  -- Guozhang
>> > > >>>
>> > > >>
>> > > >
>> > >
>> > >
>> >
>>
>
>
>
> --
> -- Guozhang
>



-- 
-- Guozhang


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

2017-12-19 Thread Colin McCabe
On Tue, Dec 19, 2017, at 02:16, Jan Filipiak wrote:
> Sorry for coming back at this so late.
> 
> 
> 
> On 11.12.2017 07:12, Colin McCabe wrote:
> > On Sun, Dec 10, 2017, at 22:10, Colin McCabe wrote:
> >> On Fri, Dec 8, 2017, at 01:16, Jan Filipiak wrote:
> >>> Hi,
> >>>
> >>> sorry for the late reply, busy times :-/
> >>>
> >>> I would ask you one thing maybe. Since the timeout
> >>> argument seems to be settled I have no further argument
> >>> form your side except the "i don't want to".
> >>>
> >>> Can you see that connection.max.idle.max is the exact time
> >>> that expresses "We expect the client to be away for this long,
> >>> and come back and continue"?
> >> Hi Jan,
> >>
> >> Sure, connection.max.idle.max is the exact time that we want to keep
> >> around a TCP session.  TCP sessions are relatively cheap, so we can
> >> afford to keep them around for 10 minutes by default.  Incremental fetch
> >> state is less cheap, so we want to set a shorter timeout for it.  We
> >> also want new TCP sessions to be able to reuse an existing incremental
> >> fetch session rather than creating a new one and waiting for the old one
> >> to time out.
> >>
> >>> also clarified some stuff inline
> >>>
> >>> Best Jan
> >>>
> >>>
> >>>
> >>>
> >>> On 05.12.2017 23:14, Colin McCabe wrote:
>  On Tue, Dec 5, 2017, at 13:13, Jan Filipiak wrote:
> > Hi Colin
> >
> > Addressing the topic of how to manage slots from the other thread.
> > With tcp connections all this comes for free essentially.
>  Hi Jan,
> 
>  I don't think that it's accurate to say that cache management "comes for
>  free" by coupling the incremental fetch session with the TCP session.
>  When a new TCP session is started by a fetch request, you still have to
>  decide whether to grant that request an incremental fetch session or
>  not.  If your answer is that you always grant the request, I would argue
>  that you do not have cache management.
> >>> First I would say, the client has a big say in this. If the client
> >>> is not going to issue incremental he shouldn't ask for a cache
> >>> when the client ask for the cache we still have all options to deny.
> >> To put it simply, we have to have some cache management above and beyond
> >> just giving out an incremental fetch session to anyone who has a TCP
> >> session.  Therefore, caching does not become simpler if you couple the
> >> fetch session to the TCP session.
> Simply giving out an fetch session for everyone with a connection is too 
> simple,
> but I think it plays well into the idea of consumers choosing to use the 
> feature
> therefore only enabling where it brings maximum gains 
> (replicas,MirrorMakers)
> >>
>  I guess you could argue that timeouts are cache management, but I don't
>  find that argument persuasive.  Anyone could just create a lot of TCP
>  sessions and use a lot of resources, in that case.  So there is
>  essentially no limit on memory use.  In any case, TCP sessions don't
>  help us implement fetch session timeouts.
> >>> We still have all the options denying the request to keep the state.
> >>> What you want seems like a max connections / ip safeguard.
> >>> I can currently take down a broker with to many connections easily.
> >>>
> >>>
> > I still would argue we disable it by default and make a flag in the
> > broker to ask the leader to maintain the cache while replicating and 
> > also only
> > have it optional in consumers (default to off) so one can turn it on
> > where it really hurts.  MirrorMaker and audit consumers prominently.
>  I agree with Jason's point from earlier in the thread.  Adding extra
>  configuration knobs that aren't really necessary can harm usability.
>  Certainly asking people to manually turn on a feature "where it really
>  hurts" seems to fall in that category, when we could easily enable it
>  automatically for them.
> >>> This doesn't make much sense to me.
> >> There are no tradeoffs to think about from the client's point of view:
> >> it always wants an incremental fetch session.  So there is no benefit to
> >> making the clients configure an extra setting.  Updating and managing
> >> client configurations is also more difficult than managing broker
> >> configurations for most users.
> >>
> >>> You also wanted to implement
> >>> a "turn of in case of bug"-knob. Having the client indicate if the
> >>> feauture will be used seems reasonable to me.,
> >> True.  However, if there is a bug, we could also roll back the client,
> >> so having this configuration knob is not strictly required.
> >>
> > Otherwise I left a few remarks in-line, which should help to understand
> > my view of the situation better
> >
> > Best Jan
> >
> >
> > On 05.12.2017 08:06, Colin McCabe wrote:
> >> On Mon, Dec 4, 2017, at 02:27, Jan Filipiak wrote:
> >>> On 03.12.2017 21:55, Colin McCabe wrote:
>  On Sat, Dec

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

2017-12-19 Thread Bill Bejeck
+1

On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang  wrote:

> +1
>
> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley 
> wrote:
>
> > +1
> >
> > On 18 December 2017 at 23:28, Vahid S Hashemian <
> vahidhashem...@us.ibm.com
> > >
> > wrote:
> >
> > > +1
> > >
> > > Thanks for the KIP.
> > >
> > > --Vahid
> > >
> > >
> > >
> > > From:   Ted Yu 
> > > To: dev@kafka.apache.org
> > > Date:   12/18/2017 02:45 PM
> > > Subject:Re: [VOTE] KIP-243: Make ProducerConfig and
> > ConsumerConfig
> > > constructors public
> > >
> > >
> > >
> > > +1
> > >
> > > nit: via "copy and past" an 'e' is missing at the end.
> > >
> > > On Mon, Dec 18, 2017 at 2:38 PM, Matthias J. Sax <
> matth...@confluent.io>
> > > wrote:
> > >
> > > > Hi,
> > > >
> > > > I want to propose the following KIP:
> > > >
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.
> > > apache.org_confluence_display_KAFKA_KIP-2D&d=DwIBaQ&c=jf_
> > > iaSHvJObTbx-siA1ZOg&r=Q_itwloTQj3_xUKl7Nzswo6KE4Nj-
> > > kjJc7uSVcviKUc&m=JToRX4-HeVsRoOekIz18ht-YLMe-T21MttZTgbxB4ag&s=
> > > 6aZjPCc9e00raokVPKvx1BxwDOHyCuKNgtBXPMeoHy4&e=
> > >
> > > > 243%3A+Make+ProducerConfig+and+ConsumerConfig+constructors+public
> > > >
> > > >
> > > > This is a rather straight forward change, thus I skip the DISCUSS
> > > > thread and call for a vote immediately.
> > > >
> > > >
> > > > -Matthias
> > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> >
>
>
>
> --
> -- Guozhang
>


[jira] [Resolved] (KAFKA-6365) How to add a client to list of available clients?

2017-12-19 Thread Lev Gorodinski (JIRA)

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

Lev Gorodinski resolved KAFKA-6365.
---
Resolution: Fixed

> How to add a client to list of available clients?
> -
>
> Key: KAFKA-6365
> URL: https://issues.apache.org/jira/browse/KAFKA-6365
> Project: Kafka
>  Issue Type: Wish
>Reporter: Lev Gorodinski
>Priority: Trivial
>
> I'd like to add a client to: 
> https://cwiki.apache.org/confluence/display/KAFKA/Clients#Clients-.NET
> The client is: https://github.com/jet/kafunk
> .NET written in F# supports 0.8 0.9 0.10



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


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

2017-12-19 Thread Apache Jenkins Server
See 




Re: [VOTE] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-12-19 Thread Matthias J. Sax
+1

On 12/19/17 9:23 AM, Guozhang Wang wrote:
> Hello,
> 
> I'm calling for a re-vote on the additional API changes in this KIP. While
> working on the implementation I found it's better to add the following
> functions for programmability:
> 
> * Overloaded KafkaStreams constructor to allow overriding the `Time` object.
> * Overloaded AbstractConfig#originalsWithPrefix() to allow specifying
> whether or not to strip the prefix in the key of the returned map.
> 
> The corresponding wiki page (
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier)
> has been updated.
> 
> Gwen / Damian, could you take a quick look at the updated wiki page and
> re-cast your vote?
> 
> 
> Guozhang
> 
> 
> 
> 
> On Mon, Nov 20, 2017 at 11:12 AM, Guozhang Wang  wrote:
> 
>> +1 from myself as well.
>>
>> I'm closing this KIP as accepted with 3 binding votes (Gwen, Damian, me)
>> and 3 non-binding votes (Colin, Ted, Matthias).
>>
>>
>> Guozhang
>>
>> On Mon, Nov 20, 2017 at 9:56 AM, Damian Guy  wrote:
>>
>>> +1
>>>
>>> On Mon, 20 Nov 2017 at 17:52 Gwen Shapira  wrote:
>>>
 +1

 Make sense. We have a supplier for every other client type :)

 On Fri, Nov 17, 2017 at 1:33 PM Matthias J. Sax 
 wrote:

> +1
>
> On 11/17/17 9:35 AM, Ted Yu wrote:
>> +1
>>
>> On Fri, Nov 17, 2017 at 9:34 AM, Bill Bejeck 
 wrote:
>>
>>> +1
>>>
>>> Thanks,
>>> Bill
>>>
>>> On Fri, Nov 17, 2017 at 12:13 PM, Colin McCabe >>>
> wrote:
>>>
 +1 (non-binding)

 Colin

 On Tue, Nov 14, 2017, at 10:02, Guozhang Wang wrote:
> Hello folks,
>
> I have filed a new KIP on adding AdminClient into Streams for
 internal
> topic management.
>
> Please review and cast your vote on this thread.
>
> *https://cwiki.apache.org/confluence/display/KAFKA/KIP-
 220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier
> *
>
> The discussion is in another thread so if you have detailed
 questions
> please chime in there.
>
>
> -- Guozhang

>>>
>>
>
>

>>>
>>
>>
>>
>> --
>> -- Guozhang
>>
> 
> 
> 



signature.asc
Description: OpenPGP digital signature


[jira] [Created] (KAFKA-6386) Deprecate KafkaStreams constructor talking StreamsConfig parameter

2017-12-19 Thread Matthias J. Sax (JIRA)
Matthias J. Sax created KAFKA-6386:
--

 Summary: Deprecate KafkaStreams constructor talking StreamsConfig 
parameter
 Key: KAFKA-6386
 URL: https://issues.apache.org/jira/browse/KAFKA-6386
 Project: Kafka
  Issue Type: Bug
  Components: streams
Affects Versions: 1.0.0
Reporter: Matthias J. Sax
Priority: Minor


Currently, {{KafkaStreams}} constructor has overloads that take either 
{{Properties}} or {{StreamsConfig}} a parameters.

Because {{StreamsConfig}} is immutable and is created from a {{Properties}} 
object itself, the constructors accepting {{StreamsConfig}} are not useful and 
add only boiler plate code. Thus, we should deprecate those constructors in 
order to remove them eventually.

This JIRA includes a public API changes and thus requires a KIP: 
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals



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


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

2017-12-19 Thread Dong Lin
Hey Jason,

Thanks for the comments. These make sense. I have updated the KIP to
include a new error INVALID_LEADER_EPOCH. This will be a non-retriable
error which may be thrown from consumer's API. When application receives
this exception, the only choice will be to revert Kafka client library to
an earlier version.

Previously I think it may be better to simply log an error because I am not
sure it is a good idea to force user to downgrade Kafka client library when
the error itself, e.g. smaller leader epoch, may not be that fatal. One the
other hand it could be argued that we don't know what else can go wrong in
the buggy client library and it may be a good reason to force user to
downgrade library.

Thanks,
Dong


On Tue, Dec 19, 2017 at 9:06 AM, Jason Gustafson  wrote:

> Hey Dong,
>
>
> > I think it is a good idea to let coordinator do the additional sanity
> check
> > to ensure the leader epoch from OffsetCommitRequest never decreases. This
> > can help us detect bug. The next question will be what should we do if
> > OffsetCommitRequest provides a smaller leader epoch. One possible
> solution
> > is to return a non-retriable error to consumer which will then be thrown
> to
> > user application. But I am not sure it is worth doing it given its impact
> > on the user. Maybe it will be safer to simply have an error message in
> the
> > server log and allow offset commit to succeed. What do you think?
>
>
> I think the check would only have value if you return an error when it
> fails. It seems primarily useful to detect buggy consumer logic, so a
> non-retriable error makes sense to me. Clients which don't implement this
> capability can use the sentinel value and keep the current behavior.
>
> It seems that FetchResponse includes leader epoch via the path
> > FetchResponse -> MemoryRecords -> MutableRecordBatch ->
> DefaultRecordBatch
> > -> partitionLeaderEpoch. Could this be an existing case where we expose
> the
> > leader epoch to clients?
>
>
> Right, in this case the client has no direct dependence on the field, but
> it could still be argued that it is exposed (I had actually considered
> stuffing this field into an opaque blob of bytes in the message format
> which the client wasn't allowed to touch, but it didn't happen in the end).
> I'm not opposed to using the leader epoch field here, I was just mentioning
> that it does tie clients a bit tighter to something which could be
> considered a Kafka internal implementation detail. It makes the protocol a
> bit less intuitive as well since it is rather difficult to explain the edge
> case it is protecting. That said, we've hit other scenarios where being
> able to detect stale metadata in the client would be helpful, so I think it
> might be worth the tradeoff.
>
> -Jason
>
> On Mon, Dec 18, 2017 at 6:09 PM, Dong Lin  wrote:
>
> > Hey Jason,
> >
> > Thanks much for reviewing the KIP.
> >
> > I think it is a good idea to let coordinator do the additional sanity
> check
> > to ensure the leader epoch from OffsetCommitRequest never decreases. This
> > can help us detect bug. The next question will be what should we do if
> > OffsetCommitRequest provides a smaller leader epoch. One possible
> solution
> > is to return a non-retriable error to consumer which will then be thrown
> to
> > user application. But I am not sure it is worth doing it given its impact
> > on the user. Maybe it will be safer to simply have an error message in
> the
> > server log and allow offset commit to succeed. What do you think?
> >
> > It seems that FetchResponse includes leader epoch via the path
> > FetchResponse -> MemoryRecords -> MutableRecordBatch ->
> DefaultRecordBatch
> > -> partitionLeaderEpoch. Could this be an existing case where we expose
> the
> > leader epoch to clients?
> >
> > Thanks,
> > Dong
> >
> >
> >
> > On Mon, Dec 18, 2017 at 3:27 PM, Jason Gustafson 
> > wrote:
> >
> > > Hi Dong,
> > >
> > > Thanks for the KIP. Good job identifying the problem. One minor
> question
> > I
> > > had is whether the coordinator should enforce that the leader epoch
> > > associated with an offset commit can only go forward for each
> partition?
> > > Currently it looks like we just depend on the client for this, but
> since
> > > we're caching the leader epoch anyway, it seems like a cheap safety
> > > condition. To support old clients, you can always allow the commit if
> the
> > > leader epoch is unknown.
> > >
> > > I agree that we shouldn't expose the leader epoch in OffsetAndMetadata
> in
> > > the consumer API for what it's worth. As you have noted, it is more of
> an
> > > implementation detail. By the same argument, it's also a bit
> unfortunate
> > > that we have to expose it in the request API since that is nearly as
> > > binding in terms of how it limits future iterations. I could be wrong,
> > but
> > > this appears to be the first case where clients will depend on the
> > concept
> > > of leader epoch. Might not be a big deal considering how deeply
> em

[GitHub] kafka pull request #4343: KAFKA-6383: complete shutdown for CREATED StreamTh...

2017-12-19 Thread rodesai
GitHub user rodesai opened a pull request:

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

KAFKA-6383: complete shutdown for CREATED StreamThreads

When transitioning StreamThread from CREATED to PENDING_SHUTDOWN
free up resources from the caller, rather than the stream thread,
since in this case the stream thread was never actually started.

In KakfaStreams.close, shut down the streams threads from the
close thread. StreamThread.shutdown may now block, so call this
from the close thread so that the timeout is honored.

*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*

*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*

### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation 
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/rodesai/kafka KAFKA-6383

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/4343.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #4343


commit ab98a90027189dc12e3df99caf3d4f2aab8fefa0
Author: Rohan Desai 
Date:   2017-12-19T18:14:20Z

KAFKA-6383: complete shutdown for CREATED StreamThreads

When transitioning StreamThread from CREATED to PENDING_SHUTDOWN
free up resources from the caller, rather than the stream thread,
since in this case the stream thread was never actually started.

In KakfaStreams.close, shut down the streams threads from the
close thread. StreamThread.shutdown may now block, so call this
from the close thread so that the timeout is honored.




---


Re: [VOTE] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-12-19 Thread Bill Bejeck
+1

On Tue, Dec 19, 2017 at 1:21 PM, Matthias J. Sax 
wrote:

> +1
>
> On 12/19/17 9:23 AM, Guozhang Wang wrote:
> > Hello,
> >
> > I'm calling for a re-vote on the additional API changes in this KIP.
> While
> > working on the implementation I found it's better to add the following
> > functions for programmability:
> >
> > * Overloaded KafkaStreams constructor to allow overriding the `Time`
> object.
> > * Overloaded AbstractConfig#originalsWithPrefix() to allow specifying
> > whether or not to strip the prefix in the key of the returned map.
> >
> > The corresponding wiki page (
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier)
> > has been updated.
> >
> > Gwen / Damian, could you take a quick look at the updated wiki page and
> > re-cast your vote?
> >
> >
> > Guozhang
> >
> >
> >
> >
> > On Mon, Nov 20, 2017 at 11:12 AM, Guozhang Wang 
> wrote:
> >
> >> +1 from myself as well.
> >>
> >> I'm closing this KIP as accepted with 3 binding votes (Gwen, Damian, me)
> >> and 3 non-binding votes (Colin, Ted, Matthias).
> >>
> >>
> >> Guozhang
> >>
> >> On Mon, Nov 20, 2017 at 9:56 AM, Damian Guy 
> wrote:
> >>
> >>> +1
> >>>
> >>> On Mon, 20 Nov 2017 at 17:52 Gwen Shapira  wrote:
> >>>
>  +1
> 
>  Make sense. We have a supplier for every other client type :)
> 
>  On Fri, Nov 17, 2017 at 1:33 PM Matthias J. Sax <
> matth...@confluent.io>
>  wrote:
> 
> > +1
> >
> > On 11/17/17 9:35 AM, Ted Yu wrote:
> >> +1
> >>
> >> On Fri, Nov 17, 2017 at 9:34 AM, Bill Bejeck 
>  wrote:
> >>
> >>> +1
> >>>
> >>> Thanks,
> >>> Bill
> >>>
> >>> On Fri, Nov 17, 2017 at 12:13 PM, Colin McCabe  
> > wrote:
> >>>
>  +1 (non-binding)
> 
>  Colin
> 
>  On Tue, Nov 14, 2017, at 10:02, Guozhang Wang wrote:
> > Hello folks,
> >
> > I have filed a new KIP on adding AdminClient into Streams for
>  internal
> > topic management.
> >
> > Please review and cast your vote on this thread.
> >
> > *https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>  220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier
> >   220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier>*
> >
> > The discussion is in another thread so if you have detailed
>  questions
> > please chime in there.
> >
> >
> > -- Guozhang
> 
> >>>
> >>
> >
> >
> 
> >>>
> >>
> >>
> >>
> >> --
> >> -- Guozhang
> >>
> >
> >
> >
>
>


Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration

2017-12-19 Thread Jason Gustafson
Hi Rajini,

4. Changed is_default to config_source in config_entry in the  protocol. It
> will be less confusing that way. The method isDefault() will just
> return configSource
>

Would we still include the active config in the list of synonyms?

6. It is a nice idea to have an automatically generated secret to avoid
> yet another config. But I wasn't entirely sure, so went for an explicit
> config instead (a bunch of them actually). I had two concerns (a) we might
> have a password (like the delegation token master secret) that we want
> to encrypt in future that is stored as a cluster-wide password. It will be
> better if we can configure the broker secret  for that, even though for
> that case we will have the same restriction that all brokers are configured
> with the same secret. (b) broker writes to meta.properties, so there is a
> possibility of losing the secret.


That's fair. I saw it as similar to auto-generation of the broker-id (if
you lose meta.properties, you lose the id also), but maybe it's better to
require an explicit config. If users don't provide a config secret, would
we store passwords unencrypted or would we forbid them from being altered?

Thanks,
Jason



On Tue, Dec 19, 2017 at 4:16 AM, Rajini Sivaram 
wrote:

> Hi Jason,
>
> Thank you!
>
> 2. Updated the KIP:  Reconfigurable extends Configurable
> 4. Changed is_default to config_source in config_entry in the  protocol. It
> will be less confusing that way. The method isDefault() will just
> return configSource
> == DEFAULT_CONFIG. Have also included the changes to the public classes in
> the KIP.
> 6. It is a nice idea to have an automatically generated secret to avoid
> yet another config. But I wasn't entirely sure, so went for an explicit
> config instead (a bunch of them actually). I had two concerns (a) we might
> have a password (like the delegation token master secret) that we want
> to encrypt in future that is stored as a cluster-wide password. It will be
> better if we can configure the broker secret  for that, even though for
> that case we will have the same restriction that all brokers are configured
> with the same secret. (b) broker writes to meta.properties, so there is a
> possibility of losing the secret.
>
>
> On Tue, Dec 19, 2017 at 12:47 AM, Jason Gustafson 
> wrote:
>
> > Hey Rajini,
> >
> > Thanks, makes sense. A couple replies:
> >
> > 2. I haven't changed the way Configurable is used. It is still used for
> > > initial configuration (if the class implements it). Reconfigurable is
> > used
> > > at the moment only for reconfiguration. The reason I did it that way is
> > > because for some of the internal components, the reconfiguration is
> > handled
> > > separately from initial configuration (we reconfigure classes which
> don't
> > > implement Configurable). But if that is confusing, I can make
> > > Reconfigurable
> > > extend Configurable and add a dummy method in internal classes. What do
> > you
> > > think?
> >
> >
> > I guess the slight mismatch comes from the difference in initialization
> > between plugins and internal classes. For plugins, we only initialize
> state
> > through configure() so it would be a little weird to have one which was
> > Reconfigurable but not Configurable. Internal classes, on the other hand,
> > probably have constructors which take the config values explicitly. If it
> > worked analogously for reconfiguration, I would expect that the
> > reconfigurable internal classes would have an explicit method and would
> not
> > need Reconfigurable at all. That gives us a slightly nicer API for
> testing.
> > That said, if the Reconfigurable API simplifies the internal usage quite
> a
> > bit, then I have no complaint.
> >
> > 6. I hope not :-) We wouldn't want to store master secret in ZooKeeper. I
> > > wasn't planning to add encryption for passwords in ZooKeeper initially
> > and
> > > I think that is ok for keystore passwords. But having started to
> > implement
> > > new listeners which require sasl.jaas.config, I don't think we can
> > release
> > > that with unencrypted passwords in ZooKeeper. We don't really need a
> > master
> > > secret that is same across all brokers since all the password configs
> at
> > > the moment are per-broker configs. So I think I will add a new static
> > > config to the KIP.
> >
> >
> > Haha, agreed. If the configs are pre-broker, you might also consider
> > generating a secret automatically (e.g. it could be added to
> > meta.properties?).
> >
> > Thanks,
> > Jason
> >
> > On Mon, Dec 18, 2017 at 4:07 PM, Rajini Sivaram  >
> > wrote:
> >
> > > Hi Jason,
> > >
> > > Thank you for reviewing the KIP.
> > >
> > > 1. ConfigDef is used for validating the type of the value and the
> > > constraints. But I am doing a lot more validation of security configs.
> > For
> > > example, for keystore configuration update, validate() loads the
> keystore
> > > and if it is an inter-broker listener, it validates the certificate
> chain
> > > using the trus

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

2017-12-19 Thread Jason Gustafson
Hey Dong,

Thanks for the updates. Just one question:

When application receives
> this exception, the only choice will be to revert Kafka client library to
> an earlier version.


Not sure I follow this. Wouldn't we just restart the consumer? That would
cause it to fetch the previous committed offset and then fetch the correct
metadata.

Thanks,
Jason

On Tue, Dec 19, 2017 at 10:36 AM, Dong Lin  wrote:

> Hey Jason,
>
> Thanks for the comments. These make sense. I have updated the KIP to
> include a new error INVALID_LEADER_EPOCH. This will be a non-retriable
> error which may be thrown from consumer's API. When application receives
> this exception, the only choice will be to revert Kafka client library to
> an earlier version.
>
> Previously I think it may be better to simply log an error because I am not
> sure it is a good idea to force user to downgrade Kafka client library when
> the error itself, e.g. smaller leader epoch, may not be that fatal. One the
> other hand it could be argued that we don't know what else can go wrong in
> the buggy client library and it may be a good reason to force user to
> downgrade library.
>
> Thanks,
> Dong
>
>
> On Tue, Dec 19, 2017 at 9:06 AM, Jason Gustafson 
> wrote:
>
> > Hey Dong,
> >
> >
> > > I think it is a good idea to let coordinator do the additional sanity
> > check
> > > to ensure the leader epoch from OffsetCommitRequest never decreases.
> This
> > > can help us detect bug. The next question will be what should we do if
> > > OffsetCommitRequest provides a smaller leader epoch. One possible
> > solution
> > > is to return a non-retriable error to consumer which will then be
> thrown
> > to
> > > user application. But I am not sure it is worth doing it given its
> impact
> > > on the user. Maybe it will be safer to simply have an error message in
> > the
> > > server log and allow offset commit to succeed. What do you think?
> >
> >
> > I think the check would only have value if you return an error when it
> > fails. It seems primarily useful to detect buggy consumer logic, so a
> > non-retriable error makes sense to me. Clients which don't implement this
> > capability can use the sentinel value and keep the current behavior.
> >
> > It seems that FetchResponse includes leader epoch via the path
> > > FetchResponse -> MemoryRecords -> MutableRecordBatch ->
> > DefaultRecordBatch
> > > -> partitionLeaderEpoch. Could this be an existing case where we expose
> > the
> > > leader epoch to clients?
> >
> >
> > Right, in this case the client has no direct dependence on the field, but
> > it could still be argued that it is exposed (I had actually considered
> > stuffing this field into an opaque blob of bytes in the message format
> > which the client wasn't allowed to touch, but it didn't happen in the
> end).
> > I'm not opposed to using the leader epoch field here, I was just
> mentioning
> > that it does tie clients a bit tighter to something which could be
> > considered a Kafka internal implementation detail. It makes the protocol
> a
> > bit less intuitive as well since it is rather difficult to explain the
> edge
> > case it is protecting. That said, we've hit other scenarios where being
> > able to detect stale metadata in the client would be helpful, so I think
> it
> > might be worth the tradeoff.
> >
> > -Jason
> >
> > On Mon, Dec 18, 2017 at 6:09 PM, Dong Lin  wrote:
> >
> > > Hey Jason,
> > >
> > > Thanks much for reviewing the KIP.
> > >
> > > I think it is a good idea to let coordinator do the additional sanity
> > check
> > > to ensure the leader epoch from OffsetCommitRequest never decreases.
> This
> > > can help us detect bug. The next question will be what should we do if
> > > OffsetCommitRequest provides a smaller leader epoch. One possible
> > solution
> > > is to return a non-retriable error to consumer which will then be
> thrown
> > to
> > > user application. But I am not sure it is worth doing it given its
> impact
> > > on the user. Maybe it will be safer to simply have an error message in
> > the
> > > server log and allow offset commit to succeed. What do you think?
> > >
> > > It seems that FetchResponse includes leader epoch via the path
> > > FetchResponse -> MemoryRecords -> MutableRecordBatch ->
> > DefaultRecordBatch
> > > -> partitionLeaderEpoch. Could this be an existing case where we expose
> > the
> > > leader epoch to clients?
> > >
> > > Thanks,
> > > Dong
> > >
> > >
> > >
> > > On Mon, Dec 18, 2017 at 3:27 PM, Jason Gustafson 
> > > wrote:
> > >
> > > > Hi Dong,
> > > >
> > > > Thanks for the KIP. Good job identifying the problem. One minor
> > question
> > > I
> > > > had is whether the coordinator should enforce that the leader epoch
> > > > associated with an offset commit can only go forward for each
> > partition?
> > > > Currently it looks like we just depend on the client for this, but
> > since
> > > > we're caching the leader epoch anyway, it seems like a cheap safety
> > > > condition. To support old cli

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

2017-12-19 Thread Dong Lin
Hey Jason,

Yeah this may sound a bit confusing. Let me explain my thoughts.

If there is no bug in the client library, after consumer rebalance or
consumer restart, consume will fetch the previously committed offset and
fetch the committed metadata until the leader epoch in the metadata >= the
leader epoch in the OffsetFetchResponse. Therefore, when consumer commits
offset later, the leader epoch in the OffsetCommitRequest should be larger
than the leader epoch from the previously committed offset. Does this sound
correct?

Given the above understanding, it seems to suggest that the only
explanation for this exception is that there is bug in the client library.
And due to this specific bug, I am not sure we can avoid this error by
simply restarting consumer. And because this error is non-retriable, user
may be forced to downgrade client library. Did I miss something here?

Thanks,
Dong


On Tue, Dec 19, 2017 at 11:19 AM, Jason Gustafson 
wrote:

> Hey Dong,
>
> Thanks for the updates. Just one question:
>
> When application receives
> > this exception, the only choice will be to revert Kafka client library to
> > an earlier version.
>
>
> Not sure I follow this. Wouldn't we just restart the consumer? That would
> cause it to fetch the previous committed offset and then fetch the correct
> metadata.
>
> Thanks,
> Jason
>
> On Tue, Dec 19, 2017 at 10:36 AM, Dong Lin  wrote:
>
> > Hey Jason,
> >
> > Thanks for the comments. These make sense. I have updated the KIP to
> > include a new error INVALID_LEADER_EPOCH. This will be a non-retriable
> > error which may be thrown from consumer's API. When application receives
> > this exception, the only choice will be to revert Kafka client library to
> > an earlier version.
> >
> > Previously I think it may be better to simply log an error because I am
> not
> > sure it is a good idea to force user to downgrade Kafka client library
> when
> > the error itself, e.g. smaller leader epoch, may not be that fatal. One
> the
> > other hand it could be argued that we don't know what else can go wrong
> in
> > the buggy client library and it may be a good reason to force user to
> > downgrade library.
> >
> > Thanks,
> > Dong
> >
> >
> > On Tue, Dec 19, 2017 at 9:06 AM, Jason Gustafson 
> > wrote:
> >
> > > Hey Dong,
> > >
> > >
> > > > I think it is a good idea to let coordinator do the additional sanity
> > > check
> > > > to ensure the leader epoch from OffsetCommitRequest never decreases.
> > This
> > > > can help us detect bug. The next question will be what should we do
> if
> > > > OffsetCommitRequest provides a smaller leader epoch. One possible
> > > solution
> > > > is to return a non-retriable error to consumer which will then be
> > thrown
> > > to
> > > > user application. But I am not sure it is worth doing it given its
> > impact
> > > > on the user. Maybe it will be safer to simply have an error message
> in
> > > the
> > > > server log and allow offset commit to succeed. What do you think?
> > >
> > >
> > > I think the check would only have value if you return an error when it
> > > fails. It seems primarily useful to detect buggy consumer logic, so a
> > > non-retriable error makes sense to me. Clients which don't implement
> this
> > > capability can use the sentinel value and keep the current behavior.
> > >
> > > It seems that FetchResponse includes leader epoch via the path
> > > > FetchResponse -> MemoryRecords -> MutableRecordBatch ->
> > > DefaultRecordBatch
> > > > -> partitionLeaderEpoch. Could this be an existing case where we
> expose
> > > the
> > > > leader epoch to clients?
> > >
> > >
> > > Right, in this case the client has no direct dependence on the field,
> but
> > > it could still be argued that it is exposed (I had actually considered
> > > stuffing this field into an opaque blob of bytes in the message format
> > > which the client wasn't allowed to touch, but it didn't happen in the
> > end).
> > > I'm not opposed to using the leader epoch field here, I was just
> > mentioning
> > > that it does tie clients a bit tighter to something which could be
> > > considered a Kafka internal implementation detail. It makes the
> protocol
> > a
> > > bit less intuitive as well since it is rather difficult to explain the
> > edge
> > > case it is protecting. That said, we've hit other scenarios where being
> > > able to detect stale metadata in the client would be helpful, so I
> think
> > it
> > > might be worth the tradeoff.
> > >
> > > -Jason
> > >
> > > On Mon, Dec 18, 2017 at 6:09 PM, Dong Lin  wrote:
> > >
> > > > Hey Jason,
> > > >
> > > > Thanks much for reviewing the KIP.
> > > >
> > > > I think it is a good idea to let coordinator do the additional sanity
> > > check
> > > > to ensure the leader epoch from OffsetCommitRequest never decreases.
> > This
> > > > can help us detect bug. The next question will be what should we do
> if
> > > > OffsetCommitRequest provides a smaller leader epoch. One possible
> > > solution
> > > > is to ret

[VOTE] KIP-231: Improve the Required ACL of ListGroups API

2017-12-19 Thread Vahid S Hashemian
I believe the concerns on this KIP have been addressed so far.
Therefore, I'd like to start a vote.

https://cwiki.apache.org/confluence/display/KAFKA/KIP-231%3A+Improve+the+Required+ACL+of+ListGroups+API

Thanks.
--Vahid



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

2017-12-19 Thread Jason Gustafson
I think you're saying that depending on the bug, in the worst case, you may
have to downgrade the client. I think that's fair. Note that one advantage
of making this a fatal error is that we'll be more likely to hit unexpected
edge cases in system tests.

-Jason

On Tue, Dec 19, 2017 at 11:26 AM, Dong Lin  wrote:

> Hey Jason,
>
> Yeah this may sound a bit confusing. Let me explain my thoughts.
>
> If there is no bug in the client library, after consumer rebalance or
> consumer restart, consume will fetch the previously committed offset and
> fetch the committed metadata until the leader epoch in the metadata >= the
> leader epoch in the OffsetFetchResponse. Therefore, when consumer commits
> offset later, the leader epoch in the OffsetCommitRequest should be larger
> than the leader epoch from the previously committed offset. Does this sound
> correct?
>
> Given the above understanding, it seems to suggest that the only
> explanation for this exception is that there is bug in the client library.
> And due to this specific bug, I am not sure we can avoid this error by
> simply restarting consumer. And because this error is non-retriable, user
> may be forced to downgrade client library. Did I miss something here?
>
> Thanks,
> Dong
>
>
> On Tue, Dec 19, 2017 at 11:19 AM, Jason Gustafson 
> wrote:
>
> > Hey Dong,
> >
> > Thanks for the updates. Just one question:
> >
> > When application receives
> > > this exception, the only choice will be to revert Kafka client library
> to
> > > an earlier version.
> >
> >
> > Not sure I follow this. Wouldn't we just restart the consumer? That would
> > cause it to fetch the previous committed offset and then fetch the
> correct
> > metadata.
> >
> > Thanks,
> > Jason
> >
> > On Tue, Dec 19, 2017 at 10:36 AM, Dong Lin  wrote:
> >
> > > Hey Jason,
> > >
> > > Thanks for the comments. These make sense. I have updated the KIP to
> > > include a new error INVALID_LEADER_EPOCH. This will be a non-retriable
> > > error which may be thrown from consumer's API. When application
> receives
> > > this exception, the only choice will be to revert Kafka client library
> to
> > > an earlier version.
> > >
> > > Previously I think it may be better to simply log an error because I am
> > not
> > > sure it is a good idea to force user to downgrade Kafka client library
> > when
> > > the error itself, e.g. smaller leader epoch, may not be that fatal. One
> > the
> > > other hand it could be argued that we don't know what else can go wrong
> > in
> > > the buggy client library and it may be a good reason to force user to
> > > downgrade library.
> > >
> > > Thanks,
> > > Dong
> > >
> > >
> > > On Tue, Dec 19, 2017 at 9:06 AM, Jason Gustafson 
> > > wrote:
> > >
> > > > Hey Dong,
> > > >
> > > >
> > > > > I think it is a good idea to let coordinator do the additional
> sanity
> > > > check
> > > > > to ensure the leader epoch from OffsetCommitRequest never
> decreases.
> > > This
> > > > > can help us detect bug. The next question will be what should we do
> > if
> > > > > OffsetCommitRequest provides a smaller leader epoch. One possible
> > > > solution
> > > > > is to return a non-retriable error to consumer which will then be
> > > thrown
> > > > to
> > > > > user application. But I am not sure it is worth doing it given its
> > > impact
> > > > > on the user. Maybe it will be safer to simply have an error message
> > in
> > > > the
> > > > > server log and allow offset commit to succeed. What do you think?
> > > >
> > > >
> > > > I think the check would only have value if you return an error when
> it
> > > > fails. It seems primarily useful to detect buggy consumer logic, so a
> > > > non-retriable error makes sense to me. Clients which don't implement
> > this
> > > > capability can use the sentinel value and keep the current behavior.
> > > >
> > > > It seems that FetchResponse includes leader epoch via the path
> > > > > FetchResponse -> MemoryRecords -> MutableRecordBatch ->
> > > > DefaultRecordBatch
> > > > > -> partitionLeaderEpoch. Could this be an existing case where we
> > expose
> > > > the
> > > > > leader epoch to clients?
> > > >
> > > >
> > > > Right, in this case the client has no direct dependence on the field,
> > but
> > > > it could still be argued that it is exposed (I had actually
> considered
> > > > stuffing this field into an opaque blob of bytes in the message
> format
> > > > which the client wasn't allowed to touch, but it didn't happen in the
> > > end).
> > > > I'm not opposed to using the leader epoch field here, I was just
> > > mentioning
> > > > that it does tie clients a bit tighter to something which could be
> > > > considered a Kafka internal implementation detail. It makes the
> > protocol
> > > a
> > > > bit less intuitive as well since it is rather difficult to explain
> the
> > > edge
> > > > case it is protecting. That said, we've hit other scenarios where
> being
> > > > able to detect stale metadata in the client would be helpful, so I
> >

Re: [DISCUSS] KIP 226 - Dynamic Broker Configuration

2017-12-19 Thread Rajini Sivaram
4. I wasn't sure what to do, so I left it in there, so that synonyms is a
self-contained list.

6. We will never store passwords unencrypted, we will forbid them from
being altered if the secret is not configured.

Thank you,

Rajini

On Tue, Dec 19, 2017 at 7:14 PM, Jason Gustafson  wrote:

> Hi Rajini,
>
> 4. Changed is_default to config_source in config_entry in the  protocol. It
> > will be less confusing that way. The method isDefault() will just
> > return configSource
> >
>
> Would we still include the active config in the list of synonyms?
>
> 6. It is a nice idea to have an automatically generated secret to avoid
> > yet another config. But I wasn't entirely sure, so went for an explicit
> > config instead (a bunch of them actually). I had two concerns (a) we
> might
> > have a password (like the delegation token master secret) that we want
> > to encrypt in future that is stored as a cluster-wide password. It will
> be
> > better if we can configure the broker secret  for that, even though for
> > that case we will have the same restriction that all brokers are
> configured
> > with the same secret. (b) broker writes to meta.properties, so there is a
> > possibility of losing the secret.
>
>
> That's fair. I saw it as similar to auto-generation of the broker-id (if
> you lose meta.properties, you lose the id also), but maybe it's better to
> require an explicit config. If users don't provide a config secret, would
> we store passwords unencrypted or would we forbid them from being altered?
>
> Thanks,
> Jason
>
>
>
> On Tue, Dec 19, 2017 at 4:16 AM, Rajini Sivaram 
> wrote:
>
> > Hi Jason,
> >
> > Thank you!
> >
> > 2. Updated the KIP:  Reconfigurable extends Configurable
> > 4. Changed is_default to config_source in config_entry in the  protocol.
> It
> > will be less confusing that way. The method isDefault() will just
> > return configSource
> > == DEFAULT_CONFIG. Have also included the changes to the public classes
> in
> > the KIP.
> > 6. It is a nice idea to have an automatically generated secret to avoid
> > yet another config. But I wasn't entirely sure, so went for an explicit
> > config instead (a bunch of them actually). I had two concerns (a) we
> might
> > have a password (like the delegation token master secret) that we want
> > to encrypt in future that is stored as a cluster-wide password. It will
> be
> > better if we can configure the broker secret  for that, even though for
> > that case we will have the same restriction that all brokers are
> configured
> > with the same secret. (b) broker writes to meta.properties, so there is a
> > possibility of losing the secret.
> >
> >
> > On Tue, Dec 19, 2017 at 12:47 AM, Jason Gustafson 
> > wrote:
> >
> > > Hey Rajini,
> > >
> > > Thanks, makes sense. A couple replies:
> > >
> > > 2. I haven't changed the way Configurable is used. It is still used for
> > > > initial configuration (if the class implements it). Reconfigurable is
> > > used
> > > > at the moment only for reconfiguration. The reason I did it that way
> is
> > > > because for some of the internal components, the reconfiguration is
> > > handled
> > > > separately from initial configuration (we reconfigure classes which
> > don't
> > > > implement Configurable). But if that is confusing, I can make
> > > > Reconfigurable
> > > > extend Configurable and add a dummy method in internal classes. What
> do
> > > you
> > > > think?
> > >
> > >
> > > I guess the slight mismatch comes from the difference in initialization
> > > between plugins and internal classes. For plugins, we only initialize
> > state
> > > through configure() so it would be a little weird to have one which was
> > > Reconfigurable but not Configurable. Internal classes, on the other
> hand,
> > > probably have constructors which take the config values explicitly. If
> it
> > > worked analogously for reconfiguration, I would expect that the
> > > reconfigurable internal classes would have an explicit method and would
> > not
> > > need Reconfigurable at all. That gives us a slightly nicer API for
> > testing.
> > > That said, if the Reconfigurable API simplifies the internal usage
> quite
> > a
> > > bit, then I have no complaint.
> > >
> > > 6. I hope not :-) We wouldn't want to store master secret in
> ZooKeeper. I
> > > > wasn't planning to add encryption for passwords in ZooKeeper
> initially
> > > and
> > > > I think that is ok for keystore passwords. But having started to
> > > implement
> > > > new listeners which require sasl.jaas.config, I don't think we can
> > > release
> > > > that with unencrypted passwords in ZooKeeper. We don't really need a
> > > master
> > > > secret that is same across all brokers since all the password configs
> > at
> > > > the moment are per-broker configs. So I think I will add a new static
> > > > config to the KIP.
> > >
> > >
> > > Haha, agreed. If the configs are pre-broker, you might also consider
> > > generating a secret automatically (e.g. it could be added to
> 

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-19 Thread Steven Aerts
Hello Ismael.


Thanks for you feedback.

> 1. The KIP seems to rely on the pull request for some of the details of the
> proposal. Generally, the KIP should stand on its own.

Looking back at what I wrote in the KIP, I agree that its style is too
descriptive
and relies too much on the content of the PR.
I will keep it in mind, and try to do better next time.  But as the
voting is over I
assume I better not alter it any more.

> 2. Do we really need to deprecate `Function`? This will add build noise to
> any library that builds with 1.1+ but also wants to support 0.11 and 1.0.

No we don't.  It is all a matter of how fast we can and want an api tagged with
@Evolving, to evolve.
As we know, that it will evolve again when KIP-118 (dropping java 7) is
implemented.

As the voting is over, I am rather reluctant to change it.  But if more people
agree with you, I still want to do it.


> 3. `FunctionInterface` is a bit of a clunky name. Due to lambdas, users
> don't have to type the name themselves, so maybe it's fine as it is. An
> alternative would be `BaseFunction` or something like that.

I share a little bit your feeling, as the best name for me would just be
`Function`.  But that one is taken.

Again, voting is over, so if more people agree with you, I still want to reopen
and change it.


-- 
Steven


Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-19 Thread Ismael Juma
Hi Steven,

As a general rule, we don't freeze KIPs after the vote passes. It's
reasonably common for things to come up during code review, for example. If
we think of improvements, we shouldn't refrain from doing them because of
of the vote. If we do minor changes after the KIP passes, we usually send a
follow-up to the vote thread and assume it's all good if no objections are
raised. Only significant changes require a vote from scratch (this tends to
be rare). More inline.

On Tue, Dec 19, 2017 at 7:58 PM, Steven Aerts 
wrote:
>
> > 1. The KIP seems to rely on the pull request for some of the details of
> the
> > proposal. Generally, the KIP should stand on its own.
>
> Looking back at what I wrote in the KIP, I agree that its style is too
> descriptive
> and relies too much on the content of the PR.
> I will keep it in mind, and try to do better next time.  But as the
> voting is over I
> assume I better not alter it any more.
>

I think we should fix this. At a minimum, the public interfaces section
should include the signature of interfaces and methods being added (as I
said before).

> 2. Do we really need to deprecate `Function`? This will add build noise to
> > any library that builds with 1.1+ but also wants to support 0.11 and 1.0.
>
> No we don't.  It is all a matter of how fast we can and want an api tagged
> with
> @Evolving, to evolve.
> As we know, that it will evolve again when KIP-118 (dropping java 7) is
> implemented.
>

For widely used APIs like the AdminClient, it's better to be conservative.
We can look at deprecations once we drop Java 7 so that we do them all at
once.


> > 3. `FunctionInterface` is a bit of a clunky name. Due to lambdas, users
> > don't have to type the name themselves, so maybe it's fine as it is. An
> > alternative would be `BaseFunction` or something like that.
>
> I share a little bit your feeling, as the best name for me would just be
> `Function`.  But that one is taken.
>

Yeah, it's a case of choosing the second best option.

Ismael


Re: [DISCUSS] KIP-240: AdminClient.listReassignments AdminClient.describeReassignments

2017-12-19 Thread Steven Aerts
Hello Tom,


when you were working out KIP-236, did you consider migrating the reassignment
state from zookeeper to an internal kafka topic, keyed by partition
and log compacted?

It would allow an admin client and controller to easily subscribe for
those changes,
without the need to extend the network protocol as discussed in KIP-240.

This is just a theoretical idea I wanted to share, as I can't find a
reason why it would
be a stupid idea.
But I assume that in practice, this will imply too much change to the
code base to be
viable.


Regards,


   Steven


2017-12-18 11:49 GMT+01:00 Tom Bentley :
> Hi Steven,
>
> I think it would be useful to be able to subscribe yourself on updates of
>> reassignment changes.
>
>
> I agree this would be really useful, but, to the extent I understand the
> networking underpinnings of the admin client, it might be difficult to do
> well in practice. Part of the problem is that you might "set a watch" (to
> borrow the ZK terminology) via one broker (or the controller), only for
> that broker to fail (or the controller be re-elected). Obviously you can
> detect the loss of connection and set a new watch via a different broker
> (or the new controller), but that couldn't be transparent to the user,
> because the AdminClient doesn't know what changed while it was
> disconnected/not watching.
>
> Another issue is that to avoid races you really need to combine fetching
> the current state with setting the watch (as is done in the native
> ZooKeeper API). I think there are lots of subtle issues of this sort which
> would need to be addressed to make something reliable.
>
> In the mean time, ZooKeeper already has a (proven and mature) API for
> watches, so there is, in principle, a good workaround. I say "in principle"
> because in the KIP-236 proposal right now the /admin/reassign_partitions
> znode is legacy and the reassignment is represented by
> /admin/reassigments/$topic/$partition. That naming scheme for the znode
> would make it harder for ZooKeeper clients like yours because such clients
> would need to set a child watch per topic. The original proposal for the
> naming scheme was /admin/reassigments/$topic-$partition, which would mean
> clients like yours would need only 1 child watch. The advantage of
> /admin/reassigments/$topic/$partition is it scales better. I don't
> currently know how well ZooKeeper copes with nodes with many children, so
> it's difficult for me weigh those two options, but I would be happy to
> switch back to /admin/reassigments/$topic-$partition if we could reassure
> ourselves it would scale OK to the reassignment sizes would people need in
> practice.
>
> Overall I would prefer not to tackle something like this in *this* KIP,
> though it could be something for a future KIP. Of course I'm happy to hear
> more discussion about this too!
>
> Cheers,
>
> Tom
>
>
> On 15 December 2017 at 18:51, Steven Aerts  wrote:
>
>> Tom,
>>
>>
>> I think it would be useful to be able to subscribe yourself on updates of
>> reassignment changes.
>> Our internal kafka supervisor and monitoring tools are currently subscribed
>> to these changes in zookeeper so they can babysit our clusters.
>>
>> I think it would be nice if we could receive these events through the
>> adminclient.
>> In the api proposal, you can only poll for changes.
>>
>> No clue how difficult it would be to implement, maybe you can piggyback on
>> some version number in the repartition messages or on zookeeper.
>>
>> This is just an idea, not a must have feature for me.  We can always poll
>> over
>> the proposed api.
>>
>>
>> Regards,
>>
>>
>>Steven
>>
>>
>> Op vr 15 dec. 2017 om 19:16 schreef Tom Bentley :
>>
>> > Hi,
>> >
>> > KIP-236 lays the foundations for AdminClient APIs to do with partition
>> > reassignment. I'd now like to start discussing KIP-240, which adds APIs
>> to
>> > the AdminClient to list and describe the current reassignments.
>> >
>> >
>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> 240%3A+AdminClient.listReassignments+AdminClient.describeReassignments
>> >
>> > Aside: I have fairly developed ideas for the API for starting a
>> > reassignment, but I intend to put that in a third KIP.
>> >
>> > Cheers,
>> >
>> > Tom
>> >
>>


[GitHub] kafka pull request #4344: KAFKA-6321: Consolidate calls to KafkaConsumer's `...

2017-12-19 Thread vahidhashemian
GitHub user vahidhashemian opened a pull request:

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

KAFKA-6321: Consolidate calls to KafkaConsumer's `beginningOffsets()` and 
`endOffsets()` in ConsumerGroupCommand

### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation 
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vahidhashemian/kafka KAFKA-6321

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/4344.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #4344


commit b69663447364a32a8d14ebd212f3f9ee99520fa0
Author: Vahid Hashemian 
Date:   2017-12-19T20:54:31Z

KAFKA-6321: Consolidate calls to KafkaConsumer's `beginningOffsets()` and 
`endOffsets()` in ConsumerGroupCommand




---


Re: [VOTE] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-12-19 Thread Damian Guy
+1

On Tue, 19 Dec 2017 at 19:13 Bill Bejeck  wrote:

> +1
>
> On Tue, Dec 19, 2017 at 1:21 PM, Matthias J. Sax 
> wrote:
>
> > +1
> >
> > On 12/19/17 9:23 AM, Guozhang Wang wrote:
> > > Hello,
> > >
> > > I'm calling for a re-vote on the additional API changes in this KIP.
> > While
> > > working on the implementation I found it's better to add the following
> > > functions for programmability:
> > >
> > > * Overloaded KafkaStreams constructor to allow overriding the `Time`
> > object.
> > > * Overloaded AbstractConfig#originalsWithPrefix() to allow specifying
> > > whether or not to strip the prefix in the key of the returned map.
> > >
> > > The corresponding wiki page (
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier)
> > > has been updated.
> > >
> > > Gwen / Damian, could you take a quick look at the updated wiki page and
> > > re-cast your vote?
> > >
> > >
> > > Guozhang
> > >
> > >
> > >
> > >
> > > On Mon, Nov 20, 2017 at 11:12 AM, Guozhang Wang 
> > wrote:
> > >
> > >> +1 from myself as well.
> > >>
> > >> I'm closing this KIP as accepted with 3 binding votes (Gwen, Damian,
> me)
> > >> and 3 non-binding votes (Colin, Ted, Matthias).
> > >>
> > >>
> > >> Guozhang
> > >>
> > >> On Mon, Nov 20, 2017 at 9:56 AM, Damian Guy 
> > wrote:
> > >>
> > >>> +1
> > >>>
> > >>> On Mon, 20 Nov 2017 at 17:52 Gwen Shapira  wrote:
> > >>>
> >  +1
> > 
> >  Make sense. We have a supplier for every other client type :)
> > 
> >  On Fri, Nov 17, 2017 at 1:33 PM Matthias J. Sax <
> > matth...@confluent.io>
> >  wrote:
> > 
> > > +1
> > >
> > > On 11/17/17 9:35 AM, Ted Yu wrote:
> > >> +1
> > >>
> > >> On Fri, Nov 17, 2017 at 9:34 AM, Bill Bejeck 
> >  wrote:
> > >>
> > >>> +1
> > >>>
> > >>> Thanks,
> > >>> Bill
> > >>>
> > >>> On Fri, Nov 17, 2017 at 12:13 PM, Colin McCabe <
> cmcc...@apache.org
> > 
> > > wrote:
> > >>>
> >  +1 (non-binding)
> > 
> >  Colin
> > 
> >  On Tue, Nov 14, 2017, at 10:02, Guozhang Wang wrote:
> > > Hello folks,
> > >
> > > I have filed a new KIP on adding AdminClient into Streams for
> >  internal
> > > topic management.
> > >
> > > Please review and cast your vote on this thread.
> > >
> > > *https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >  220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier
> > >  >  220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier>*
> > >
> > > The discussion is in another thread so if you have detailed
> >  questions
> > > please chime in there.
> > >
> > >
> > > -- Guozhang
> > 
> > >>>
> > >>
> > >
> > >
> > 
> > >>>
> > >>
> > >>
> > >>
> > >> --
> > >> -- Guozhang
> > >>
> > >
> > >
> > >
> >
> >
>


Re: [VOTE] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-12-19 Thread Gwen Shapira
+1

On Tue, Dec 19, 2017 at 1:06 PM Damian Guy  wrote:

> +1
>
> On Tue, 19 Dec 2017 at 19:13 Bill Bejeck  wrote:
>
> > +1
> >
> > On Tue, Dec 19, 2017 at 1:21 PM, Matthias J. Sax 
> > wrote:
> >
> > > +1
> > >
> > > On 12/19/17 9:23 AM, Guozhang Wang wrote:
> > > > Hello,
> > > >
> > > > I'm calling for a re-vote on the additional API changes in this KIP.
> > > While
> > > > working on the implementation I found it's better to add the
> following
> > > > functions for programmability:
> > > >
> > > > * Overloaded KafkaStreams constructor to allow overriding the `Time`
> > > object.
> > > > * Overloaded AbstractConfig#originalsWithPrefix() to allow specifying
> > > > whether or not to strip the prefix in the key of the returned map.
> > > >
> > > > The corresponding wiki page (
> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier)
> > > > has been updated.
> > > >
> > > > Gwen / Damian, could you take a quick look at the updated wiki page
> and
> > > > re-cast your vote?
> > > >
> > > >
> > > > Guozhang
> > > >
> > > >
> > > >
> > > >
> > > > On Mon, Nov 20, 2017 at 11:12 AM, Guozhang Wang 
> > > wrote:
> > > >
> > > >> +1 from myself as well.
> > > >>
> > > >> I'm closing this KIP as accepted with 3 binding votes (Gwen, Damian,
> > me)
> > > >> and 3 non-binding votes (Colin, Ted, Matthias).
> > > >>
> > > >>
> > > >> Guozhang
> > > >>
> > > >> On Mon, Nov 20, 2017 at 9:56 AM, Damian Guy 
> > > wrote:
> > > >>
> > > >>> +1
> > > >>>
> > > >>> On Mon, 20 Nov 2017 at 17:52 Gwen Shapira 
> wrote:
> > > >>>
> > >  +1
> > > 
> > >  Make sense. We have a supplier for every other client type :)
> > > 
> > >  On Fri, Nov 17, 2017 at 1:33 PM Matthias J. Sax <
> > > matth...@confluent.io>
> > >  wrote:
> > > 
> > > > +1
> > > >
> > > > On 11/17/17 9:35 AM, Ted Yu wrote:
> > > >> +1
> > > >>
> > > >> On Fri, Nov 17, 2017 at 9:34 AM, Bill Bejeck  >
> > >  wrote:
> > > >>
> > > >>> +1
> > > >>>
> > > >>> Thanks,
> > > >>> Bill
> > > >>>
> > > >>> On Fri, Nov 17, 2017 at 12:13 PM, Colin McCabe <
> > cmcc...@apache.org
> > > 
> > > > wrote:
> > > >>>
> > >  +1 (non-binding)
> > > 
> > >  Colin
> > > 
> > >  On Tue, Nov 14, 2017, at 10:02, Guozhang Wang wrote:
> > > > Hello folks,
> > > >
> > > > I have filed a new KIP on adding AdminClient into Streams for
> > >  internal
> > > > topic management.
> > > >
> > > > Please review and cast your vote on this thread.
> > > >
> > > > *https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > >  220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier
> > > >  > >  220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier>*
> > > >
> > > > The discussion is in another thread so if you have detailed
> > >  questions
> > > > please chime in there.
> > > >
> > > >
> > > > -- Guozhang
> > > 
> > > >>>
> > > >>
> > > >
> > > >
> > > 
> > > >>>
> > > >>
> > > >>
> > > >>
> > > >> --
> > > >> -- Guozhang
> > > >>
> > > >
> > > >
> > > >
> > >
> > >
> >
>


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

2017-12-19 Thread Jason Gustafson
Hey Dong,

One more thought came to mind. Have you considered edge cases around topic
deletion? I think currently if a topic is deleted and then re-created, the
leader epoch will start back at the beginning. It seems like that could
cause trouble for this solution. One thing that helps is that we have logic
to remove committed offsets for deleted topics, but there may not be any
guarantees on when that happens relative to when the metadata is updated on
all brokers. It seems like it could even happen that the topic is deleted
and recreated quickly enough that the consumer doesn't even "witness" the
deletion.

Thanks,
Jason

On Tue, Dec 19, 2017 at 11:40 AM, Jason Gustafson 
wrote:

> I think you're saying that depending on the bug, in the worst case, you
> may have to downgrade the client. I think that's fair. Note that one
> advantage of making this a fatal error is that we'll be more likely to hit
> unexpected edge cases in system tests.
>
> -Jason
>
> On Tue, Dec 19, 2017 at 11:26 AM, Dong Lin  wrote:
>
>> Hey Jason,
>>
>> Yeah this may sound a bit confusing. Let me explain my thoughts.
>>
>> If there is no bug in the client library, after consumer rebalance or
>> consumer restart, consume will fetch the previously committed offset and
>> fetch the committed metadata until the leader epoch in the metadata >= the
>> leader epoch in the OffsetFetchResponse. Therefore, when consumer commits
>> offset later, the leader epoch in the OffsetCommitRequest should be larger
>> than the leader epoch from the previously committed offset. Does this
>> sound
>> correct?
>>
>> Given the above understanding, it seems to suggest that the only
>> explanation for this exception is that there is bug in the client library.
>> And due to this specific bug, I am not sure we can avoid this error by
>> simply restarting consumer. And because this error is non-retriable, user
>> may be forced to downgrade client library. Did I miss something here?
>>
>> Thanks,
>> Dong
>>
>>
>> On Tue, Dec 19, 2017 at 11:19 AM, Jason Gustafson 
>> wrote:
>>
>> > Hey Dong,
>> >
>> > Thanks for the updates. Just one question:
>> >
>> > When application receives
>> > > this exception, the only choice will be to revert Kafka client
>> library to
>> > > an earlier version.
>> >
>> >
>> > Not sure I follow this. Wouldn't we just restart the consumer? That
>> would
>> > cause it to fetch the previous committed offset and then fetch the
>> correct
>> > metadata.
>> >
>> > Thanks,
>> > Jason
>> >
>> > On Tue, Dec 19, 2017 at 10:36 AM, Dong Lin  wrote:
>> >
>> > > Hey Jason,
>> > >
>> > > Thanks for the comments. These make sense. I have updated the KIP to
>> > > include a new error INVALID_LEADER_EPOCH. This will be a non-retriable
>> > > error which may be thrown from consumer's API. When application
>> receives
>> > > this exception, the only choice will be to revert Kafka client
>> library to
>> > > an earlier version.
>> > >
>> > > Previously I think it may be better to simply log an error because I
>> am
>> > not
>> > > sure it is a good idea to force user to downgrade Kafka client library
>> > when
>> > > the error itself, e.g. smaller leader epoch, may not be that fatal.
>> One
>> > the
>> > > other hand it could be argued that we don't know what else can go
>> wrong
>> > in
>> > > the buggy client library and it may be a good reason to force user to
>> > > downgrade library.
>> > >
>> > > Thanks,
>> > > Dong
>> > >
>> > >
>> > > On Tue, Dec 19, 2017 at 9:06 AM, Jason Gustafson 
>> > > wrote:
>> > >
>> > > > Hey Dong,
>> > > >
>> > > >
>> > > > > I think it is a good idea to let coordinator do the additional
>> sanity
>> > > > check
>> > > > > to ensure the leader epoch from OffsetCommitRequest never
>> decreases.
>> > > This
>> > > > > can help us detect bug. The next question will be what should we
>> do
>> > if
>> > > > > OffsetCommitRequest provides a smaller leader epoch. One possible
>> > > > solution
>> > > > > is to return a non-retriable error to consumer which will then be
>> > > thrown
>> > > > to
>> > > > > user application. But I am not sure it is worth doing it given its
>> > > impact
>> > > > > on the user. Maybe it will be safer to simply have an error
>> message
>> > in
>> > > > the
>> > > > > server log and allow offset commit to succeed. What do you think?
>> > > >
>> > > >
>> > > > I think the check would only have value if you return an error when
>> it
>> > > > fails. It seems primarily useful to detect buggy consumer logic, so
>> a
>> > > > non-retriable error makes sense to me. Clients which don't implement
>> > this
>> > > > capability can use the sentinel value and keep the current behavior.
>> > > >
>> > > > It seems that FetchResponse includes leader epoch via the path
>> > > > > FetchResponse -> MemoryRecords -> MutableRecordBatch ->
>> > > > DefaultRecordBatch
>> > > > > -> partitionLeaderEpoch. Could this be an existing case where we
>> > expose
>> > > > the
>> > > > > leader epoch to clients?
>> > > >
>> > > >
>> > 

Re: [VOTE] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-12-19 Thread Guozhang Wang
Thanks Gwen, Damian,

I will go ahead and merge the corresponding PR then.


Guozhang


On Tue, Dec 19, 2017 at 2:04 PM, Gwen Shapira  wrote:

> +1
>
> On Tue, Dec 19, 2017 at 1:06 PM Damian Guy  wrote:
>
> > +1
> >
> > On Tue, 19 Dec 2017 at 19:13 Bill Bejeck  wrote:
> >
> > > +1
> > >
> > > On Tue, Dec 19, 2017 at 1:21 PM, Matthias J. Sax <
> matth...@confluent.io>
> > > wrote:
> > >
> > > > +1
> > > >
> > > > On 12/19/17 9:23 AM, Guozhang Wang wrote:
> > > > > Hello,
> > > > >
> > > > > I'm calling for a re-vote on the additional API changes in this
> KIP.
> > > > While
> > > > > working on the implementation I found it's better to add the
> > following
> > > > > functions for programmability:
> > > > >
> > > > > * Overloaded KafkaStreams constructor to allow overriding the
> `Time`
> > > > object.
> > > > > * Overloaded AbstractConfig#originalsWithPrefix() to allow
> specifying
> > > > > whether or not to strip the prefix in the key of the returned map.
> > > > >
> > > > > The corresponding wiki page (
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > 220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier)
> > > > > has been updated.
> > > > >
> > > > > Gwen / Damian, could you take a quick look at the updated wiki page
> > and
> > > > > re-cast your vote?
> > > > >
> > > > >
> > > > > Guozhang
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On Mon, Nov 20, 2017 at 11:12 AM, Guozhang Wang <
> wangg...@gmail.com>
> > > > wrote:
> > > > >
> > > > >> +1 from myself as well.
> > > > >>
> > > > >> I'm closing this KIP as accepted with 3 binding votes (Gwen,
> Damian,
> > > me)
> > > > >> and 3 non-binding votes (Colin, Ted, Matthias).
> > > > >>
> > > > >>
> > > > >> Guozhang
> > > > >>
> > > > >> On Mon, Nov 20, 2017 at 9:56 AM, Damian Guy  >
> > > > wrote:
> > > > >>
> > > > >>> +1
> > > > >>>
> > > > >>> On Mon, 20 Nov 2017 at 17:52 Gwen Shapira 
> > wrote:
> > > > >>>
> > > >  +1
> > > > 
> > > >  Make sense. We have a supplier for every other client type :)
> > > > 
> > > >  On Fri, Nov 17, 2017 at 1:33 PM Matthias J. Sax <
> > > > matth...@confluent.io>
> > > >  wrote:
> > > > 
> > > > > +1
> > > > >
> > > > > On 11/17/17 9:35 AM, Ted Yu wrote:
> > > > >> +1
> > > > >>
> > > > >> On Fri, Nov 17, 2017 at 9:34 AM, Bill Bejeck <
> bbej...@gmail.com
> > >
> > > >  wrote:
> > > > >>
> > > > >>> +1
> > > > >>>
> > > > >>> Thanks,
> > > > >>> Bill
> > > > >>>
> > > > >>> On Fri, Nov 17, 2017 at 12:13 PM, Colin McCabe <
> > > cmcc...@apache.org
> > > > 
> > > > > wrote:
> > > > >>>
> > > >  +1 (non-binding)
> > > > 
> > > >  Colin
> > > > 
> > > >  On Tue, Nov 14, 2017, at 10:02, Guozhang Wang wrote:
> > > > > Hello folks,
> > > > >
> > > > > I have filed a new KIP on adding AdminClient into Streams
> for
> > > >  internal
> > > > > topic management.
> > > > >
> > > > > Please review and cast your vote on this thread.
> > > > >
> > > > > *https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > >  220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier
> > > > >  > > >  220%3A+Add+AdminClient+into+Kafka+Streams%27+
> ClientSupplier>*
> > > > >
> > > > > The discussion is in another thread so if you have detailed
> > > >  questions
> > > > > please chime in there.
> > > > >
> > > > >
> > > > > -- Guozhang
> > > > 
> > > > >>>
> > > > >>
> > > > >
> > > > >
> > > > 
> > > > >>>
> > > > >>
> > > > >>
> > > > >>
> > > > >> --
> > > > >> -- Guozhang
> > > > >>
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > >
> >
>



-- 
-- Guozhang


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

2017-12-19 Thread Randall Hauch (JIRA)
Randall Hauch created KAFKA-6387:


 Summary: Worker's producer and consumer configs should inherit 
from worker configs
 Key: KAFKA-6387
 URL: https://issues.apache.org/jira/browse/KAFKA-6387
 Project: Kafka
  Issue Type: Improvement
  Components: KafkaConnect
Affects Versions: 1.0.0
Reporter: Randall Hauch


Currently, the worker configuration file defines the connection properties for 
the three separate types of connections being made to the Kafka cluster:

# the worker group membership,
# producers for source connectors,
# the consumers for sink connectors. 

The configs are namespaced because to properly support things like interceptors 
where the configs for 2 and 3 would conflict (same config name, different 
value).

However, it would be beneficial when such control is not required for the 
producers and consumers to inherit the top-level configurations yet be able to 
override them with the {{producer.}} and {{consumer.}} namespaced 
configurations. This way the producer- and consumer-specific configurations 
need only be specified if/when they need to override the top-level 
configurations. This may be necessary, for example, to have different ACLs than 
the connector tasks compared to the producers and consumers.

This will require a minimal KIP to explain the new behavior. 



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


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

2017-12-19 Thread Jun Rao
Hi, Dong,

Thanks for the reply.

10. I was actually just thinking the case when the consumer consumes old
data. If the current leader epoch is 3 and the consumer is consuming
records generated in leader epoch 1, the epoch associated with the offset
should be 1. However, as you pointed out, the fetch response currently
includes the leader epoch for fetched data. So, this is already covered.

11. That's an interesting thought. What about the case when the offsets are
stored externally? When we restart a consumer and seek to an externally
stored offset, we won't know the leader epoch in the consumer. Do we need
another request to retrieve the leader epoch based on an offset and make
sure the info is up to date? Another related thing is that the leader epoch
that we want to associate the offset with ideally should be the epoch when
the data is fetched. For example, when all replicas lost data due to a
power failure or when there is an unclean leader election, the leader epoch
for a given offset may change over time on the broker. In those cases, a
consumer's offset may be in range, but is not in the same leader epoch for
the time when the data is fetched. We can potentially do a smarter offset
reset in those cases if we remember the epoch when the data is fetched.

Jun



On Mon, Dec 18, 2017 at 1:58 PM, Dong Lin  wrote:

> Hey Jun,
>
> Thanks much for your comments. These are very thoughtful ideas. Please see
> my comments below.
>
> On Thu, Dec 14, 2017 at 6:38 PM, Jun Rao  wrote:
>
> > Hi, Dong,
> >
> > Thanks for the update. A few more comments below.
> >
> > 10. It seems that we need to return the leader epoch in the fetch
> response
> > as well When fetching data, we could be fetching data from a leader epoch
> > older than what's returned in the metadata response. So, we want to use
> the
> > leader epoch associated with the offset being fetched for committing
> > offsets.
> >
>
> It seems that we may have two separate issues here. The first issue is that
> consumer uses metadata that is older than the one it uses before. The
> second issue is that consumer uses metadata which is newer than the
> corresponding leader epoch in the leader broker. We know that the
> OffsetOutOfRangeException described in this KIP can be prevented by
> avoiding the first issue. On the other hand, it seems that the
> OffsetOffsetOutOfRangeException can still happen even if we avoid the
> second issue -- if consumer uses an older version of metadata, the leader
> epoch in its metadata may equal the leader epoch in the broker even if the
> leader epoch in the broker is oudated.
>
> Given this understanding, I am not sure why we need to return the leader
> epoch in the fetch response. As long as consumer's metadata is not going
> back in version, I think we are good. Did I miss something here?
>
>
> >
> > 11. Should we now extend OffsetAndMetadata used in the offset commit api
> in
> > KafkaConsumer to include leader epoch? Similarly, should we return leader
> > epoch in endOffsets(), beginningOffsets() and position()? We probably
> need
> > to think about how to make the api backward compatible.
> >
>
> After thinking through this carefully, I think we probably don't want to
> extend OffsetAndMetadata to include leader epoch because leader epoch is
> kind of implementation detail which ideally should be hidden from user. The
> consumer can include leader epoch in the OffsetCommitRequest after taking
> offset from commitSync(final Map
> offsets). Similarly consumer can store leader epoch from
> OffsetFetchResponse and only provide offset to user via
> consumer.committed(topicPartition). This solution seems to work well and
> we
> don't have to make changes to consumer's public API. Does this sound OK?
>
>
> >
> > 12. It seems that we now need to store leader epoch in the offset topic.
> > Could you include the new schema for the value of the offset topic and
> add
> > upgrade notes?
>
>
> You are right. I have updated the KIP to specify the new schema for the
> value of the offset topic. Can you take another look?
>
> For existing messages in the offset topic, leader_epoch will be missing. We
> will use leader_epoch = -1 to indicate the missing leader_epoch. Then the
> consumer behavior will be the same as it is now because any leader_epoch in
> the MetadataResponse will be larger than the leader_epoch = -1 in the
> OffetFetchResponse. Thus we don't need specific procedure for upgrades due
> to this change in the offset topic schema. By "upgrade nodes", do you mean
> the sentences we need to include in the upgrade.html in the PR later?
>
>
> >
> > Jun
> >
> >
> > On Tue, Dec 12, 2017 at 5:19 PM, Dong Lin  wrote:
> >
> > > Hey Jun,
> > >
> > > I see. Sounds good. Yeah it is probably simpler to leave this to
> another
> > > KIP in the future.
> > >
> > > Thanks for all the comments. Since there is no further comment in the
> > > community, I will open the voting thread.
> > >
> > > Thanks,
> > > Dong
> > >
> > > On Mon, Dec 11, 

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

2017-12-19 Thread Apache Jenkins Server
See 


Changes:

[wangguoz] MINOR: Broker down for significant amt of time system test

--
Started by an SCM change
[EnvInject] - Loading node environment variables.
Building remotely on ubuntu-eu3 (ubuntu xenial) in workspace 

 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url 
 > https://git-wip-us.apache.org/repos/asf/kafka.git # timeout=10
Fetching upstream changes from https://git-wip-us.apache.org/repos/asf/kafka.git
 > git --version # timeout=10
 > git fetch --tags --progress 
 > https://git-wip-us.apache.org/repos/asf/kafka.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git rev-parse refs/remotes/origin/trunk^{commit} # timeout=10
 > git rev-parse refs/remotes/origin/origin/trunk^{commit} # timeout=10
Checking out Revision f3b9afe62265d7559ef65f5aa692feecc5fa8f25 
(refs/remotes/origin/trunk)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f f3b9afe62265d7559ef65f5aa692feecc5fa8f25
Commit message: "MINOR: Broker down for significant amt of time system test"
 > git rev-list 88c2b6849a5af2af74972c8b2e8431473542ca83 # timeout=10
ERROR: No tool found matching GRADLE_3_4_RC_2_HOME
Setting GRADLE_3_5_HOME=/home/jenkins/tools/gradle/3.5
[kafka-trunk-jdk7] $ /bin/bash -xe /tmp/jenkins1523141516202709991.sh
+ rm -rf 
+ /home/jenkins/tools/gradle/3.5/bin/gradle
/tmp/jenkins1523141516202709991.sh: line 4: 
/home/jenkins/tools/gradle/3.5/bin/gradle: No such file or directory
Build step 'Execute shell' marked build as failure
[FINDBUGS] Collecting findbugs analysis files...
ERROR: No tool found matching GRADLE_3_4_RC_2_HOME
Setting GRADLE_3_5_HOME=/home/jenkins/tools/gradle/3.5
[FINDBUGS] Searching for all files in 
 that match the pattern 
**/build/reports/findbugs/*.xml
[FINDBUGS] No files found. Configuration error?
ERROR: No tool found matching GRADLE_3_4_RC_2_HOME
Setting GRADLE_3_5_HOME=/home/jenkins/tools/gradle/3.5
ERROR: No tool found matching GRADLE_3_4_RC_2_HOME
Setting GRADLE_3_5_HOME=/home/jenkins/tools/gradle/3.5
 Using GitBlamer to create author and commit information for all 
warnings.
 GIT_COMMIT=f3b9afe62265d7559ef65f5aa692feecc5fa8f25, 
workspace=
[FINDBUGS] Computing warning deltas based on reference build #3048
Recording test results
ERROR: No tool found matching GRADLE_3_4_RC_2_HOME
Setting GRADLE_3_5_HOME=/home/jenkins/tools/gradle/3.5
ERROR: Step ‘Publish JUnit test result report’ failed: No test report files 
were found. Configuration error?
ERROR: No tool found matching GRADLE_3_4_RC_2_HOME
Setting GRADLE_3_5_HOME=/home/jenkins/tools/gradle/3.5
Not sending mail to unregistered user wangg...@gmail.com


[GitHub] kafka pull request #4313: MINOR: broker down for significant amt of time sys...

2017-12-19 Thread asfgit
Github user asfgit closed the pull request at:

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


---


[jira] [Created] (KAFKA-6388) Error while trying to roll a segment that already exists

2017-12-19 Thread David Hay (JIRA)
David Hay created KAFKA-6388:


 Summary: Error while trying to roll a segment that already exists
 Key: KAFKA-6388
 URL: https://issues.apache.org/jira/browse/KAFKA-6388
 Project: Kafka
  Issue Type: Bug
Affects Versions: 0.8.0
Reporter: David Hay
Assignee: Neha Narkhede
Priority: Blocker


I tried setting up a 5 broker 0.8 cluster and sending messages to 100s of 
topics on it. For a couple of topic partitions, the produce requests never 
succeed since they fail on the leader with the following error - 

[2012-12-05 22:54:05,711] WARN [Kafka Log on Broker 2], Newly rolled segment 
file 000
0.log already exists; deleting it first (kafka.log.Log)
[2012-12-05 22:54:05,711] WARN [Kafka Log on Broker 2], Newly rolled segment 
file 000
0.index already exists; deleting it first (kafka.log.Log)
[2012-12-05 22:54:05,715] ERROR [ReplicaFetcherThread-1-0-on-broker-2], Error 
due to  (kafka.server.R
eplicaFetcherThread)
kafka.common.KafkaException: Trying to roll a new log segment for topic 
partition NusWriteEvent-4 with start offset 0 while it already exsits
at kafka.log.Log.rollToOffset(Log.scala:456)
at kafka.log.Log.roll(Log.scala:434)
at kafka.log.Log.maybeRoll(Log.scala:423)
at kafka.log.Log.append(Log.scala:257)
at 
kafka.server.ReplicaFetcherThread.processPartitionData(ReplicaFetcherThread.scala:51)
at 
kafka.server.AbstractFetcherThread$$anonfun$doWork$5.apply(AbstractFetcherThread.scala:125)
at 
kafka.server.AbstractFetcherThread$$anonfun$doWork$5.apply(AbstractFetcherThread.scala:108)
at 
scala.collection.immutable.HashMap$HashMap1.foreach(HashMap.scala:125)
at 
scala.collection.immutable.HashMap$HashTrieMap.foreach(HashMap.scala:344)
at 
scala.collection.immutable.HashMap$HashTrieMap.foreach(HashMap.scala:344)
at 
kafka.server.AbstractFetcherThread.doWork(AbstractFetcherThread.scala:108)
at kafka.utils.ShutdownableThread.run(ShutdownableThread.scala:50)





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


Re: [VOTE] Allowing write access to GitHub repositories (aka GitBox)

2017-12-19 Thread Ismael Juma
GitBox migration will happen today. Committers, please make sure to
associate your github ID with your apache.org account via id.apache.org,
and make sure to enable 2 factor authentication in GitHub.

Ismael

On Fri, Dec 15, 2017 at 3:40 PM, Ismael Juma  wrote:

> Thanks to everyone who voted and contributed to the discussion.
>
> The vote passes with 7 binding votes (Damian, Rajini, Jason, Gwen,
> Guozhang, Sriram, Ismael) and 2 non-binding votes (Manikumar and Tom).
>
> I will file a JIRA ticket in the Apache Infra project requesting the
> migration to GitBox.
>
> Ismael
>
> On Thu, Dec 14, 2017 at 11:48 AM, Tom Bentley 
> wrote:
>
>> +1
>>
>> On 12 December 2017 at 20:38, Sriram Subramanian 
>> wrote:
>>
>> > +1
>> >
>> > On Tue, Dec 12, 2017 at 8:22 AM, Manikumar 
>> > wrote:
>> >
>> > > +1
>> > >
>> > > On Tue, Dec 12, 2017 at 9:49 PM, Rajini Sivaram <
>> rajinisiva...@gmail.com
>> > >
>> > > wrote:
>> > >
>> > > > +1
>> > > >
>> > > > Thanks, Ismael!
>> > > >
>> > > > On Tue, Dec 12, 2017 at 4:18 PM, Damian Guy 
>> > > wrote:
>> > > >
>> > > > > +1
>> > > > >
>> > > > > On Tue, 12 Dec 2017 at 15:47 Ismael Juma 
>> wrote:
>> > > > >
>> > > > > > Hi all,
>> > > > > >
>> > > > > > The Apache Infra team has started a new project earlier this
>> year
>> > > > called
>> > > > > > GitBox that supports two-way synchronization between GitHub and
>> > > > > > git-wip-us.apache.org and, most importantly, provides GitHub
>> write
>> > > > > access
>> > > > > > to committers. GitBox is not generally available yet, but
>> > individual
>> > > > > > projects can ask to be migrated.
>> > > > > >
>> > > > > > I would like to start a vote on migrating kafka and kafka-site
>> to
>> > > > GitBox
>> > > > > > and:
>> > > > > >
>> > > > > > 1. Providing GitHub write access to committers (this requires
>> dual
>> > > > factor
>> > > > > > authentication)
>> > > > > > 2. Allowing merges via the GitHub UI as well as the existing
>> merge
>> > > > script
>> > > > > > 3. Enabling protected branches for trunk and release branches so
>> > that
>> > > > > > merges via the GitHub UI can only be done if the tests pass and
>> the
>> > > PR
>> > > > > has
>> > > > > > been approved by a committer
>> > > > > > 4. Only allowing the "squash and merge" strategy for GitHub UI
>> > merges
>> > > > > > 5. Updating the merge script so that the GitHub git repo is the
>> > > target
>> > > > of
>> > > > > > the merge
>> > > > > > 6. Disallowing force pushes to trunk and release branches
>> > > > > >
>> > > > > > The discussion thread talks about some of the pros and cons
>> (mostly
>> > > > pros)
>> > > > > > of this change:
>> > > > > >
>> > > > > >
>> > > > > > https://lists.apache.org/thread.html/
>> > 7031168e7026222169c66fed29f520
>> > > > > 0fc4b561df28c242ccf706f326@%3Cdev.kafka.apache.org%3E
>> > > > > >
>> > > > > > The vote will run for 72 hours.
>> > > > > >
>> > > > > > Ismael
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>


[GitHub] kafka pull request #4315: KAFKA-6150: KIP-204 part III; Change repartition t...

2017-12-19 Thread asfgit
Github user asfgit closed the pull request at:

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


---


Re: [VOTE] Allowing write access to GitHub repositories (aka GitBox)

2017-12-19 Thread Ismael Juma
Forgot the link to the relevant Infra JIRA:
https://issues.apache.org/jira/browse/INFRA-15676

On Tue, Dec 19, 2017 at 11:59 PM, Ismael Juma  wrote:

> GitBox migration will happen today. Committers, please make sure to
> associate your github ID with your apache.org account via id.apache.org,
> and make sure to enable 2 factor authentication in GitHub.
>
> Ismael
>
> On Fri, Dec 15, 2017 at 3:40 PM, Ismael Juma  wrote:
>
>> Thanks to everyone who voted and contributed to the discussion.
>>
>> The vote passes with 7 binding votes (Damian, Rajini, Jason, Gwen,
>> Guozhang, Sriram, Ismael) and 2 non-binding votes (Manikumar and Tom).
>>
>> I will file a JIRA ticket in the Apache Infra project requesting the
>> migration to GitBox.
>>
>> Ismael
>>
>> On Thu, Dec 14, 2017 at 11:48 AM, Tom Bentley 
>> wrote:
>>
>>> +1
>>>
>>> On 12 December 2017 at 20:38, Sriram Subramanian 
>>> wrote:
>>>
>>> > +1
>>> >
>>> > On Tue, Dec 12, 2017 at 8:22 AM, Manikumar 
>>> > wrote:
>>> >
>>> > > +1
>>> > >
>>> > > On Tue, Dec 12, 2017 at 9:49 PM, Rajini Sivaram <
>>> rajinisiva...@gmail.com
>>> > >
>>> > > wrote:
>>> > >
>>> > > > +1
>>> > > >
>>> > > > Thanks, Ismael!
>>> > > >
>>> > > > On Tue, Dec 12, 2017 at 4:18 PM, Damian Guy 
>>> > > wrote:
>>> > > >
>>> > > > > +1
>>> > > > >
>>> > > > > On Tue, 12 Dec 2017 at 15:47 Ismael Juma 
>>> wrote:
>>> > > > >
>>> > > > > > Hi all,
>>> > > > > >
>>> > > > > > The Apache Infra team has started a new project earlier this
>>> year
>>> > > > called
>>> > > > > > GitBox that supports two-way synchronization between GitHub and
>>> > > > > > git-wip-us.apache.org and, most importantly, provides GitHub
>>> write
>>> > > > > access
>>> > > > > > to committers. GitBox is not generally available yet, but
>>> > individual
>>> > > > > > projects can ask to be migrated.
>>> > > > > >
>>> > > > > > I would like to start a vote on migrating kafka and kafka-site
>>> to
>>> > > > GitBox
>>> > > > > > and:
>>> > > > > >
>>> > > > > > 1. Providing GitHub write access to committers (this requires
>>> dual
>>> > > > factor
>>> > > > > > authentication)
>>> > > > > > 2. Allowing merges via the GitHub UI as well as the existing
>>> merge
>>> > > > script
>>> > > > > > 3. Enabling protected branches for trunk and release branches
>>> so
>>> > that
>>> > > > > > merges via the GitHub UI can only be done if the tests pass
>>> and the
>>> > > PR
>>> > > > > has
>>> > > > > > been approved by a committer
>>> > > > > > 4. Only allowing the "squash and merge" strategy for GitHub UI
>>> > merges
>>> > > > > > 5. Updating the merge script so that the GitHub git repo is the
>>> > > target
>>> > > > of
>>> > > > > > the merge
>>> > > > > > 6. Disallowing force pushes to trunk and release branches
>>> > > > > >
>>> > > > > > The discussion thread talks about some of the pros and cons
>>> (mostly
>>> > > > pros)
>>> > > > > > of this change:
>>> > > > > >
>>> > > > > >
>>> > > > > > https://lists.apache.org/thread.html/
>>> > 7031168e7026222169c66fed29f520
>>> > > > > 0fc4b561df28c242ccf706f326@%3Cdev.kafka.apache.org%3E
>>> > > > > >
>>> > > > > > The vote will run for 72 hours.
>>> > > > > >
>>> > > > > > Ismael
>>> > > > > >
>>> > > > >
>>> > > >
>>> > >
>>> >
>>>
>>
>>
>


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

2017-12-19 Thread Apache Jenkins Server
See 


Changes:

[wangguoz] MINOR: Broker down for significant amt of time system test

--
[...truncated 415.01 KB...]
kafka.utils.CoreUtilsTest > testReadInt STARTED

kafka.utils.CoreUtilsTest > testReadInt PASSED

kafka.utils.CoreUtilsTest > testAtomicGetOrUpdate STARTED

kafka.utils.CoreUtilsTest > testAtomicGetOrUpdate PASSED

kafka.utils.CoreUtilsTest > testUrlSafeBase64EncodeUUID STARTED

kafka.utils.CoreUtilsTest > testUrlSafeBase64EncodeUUID PASSED

kafka.utils.CoreUtilsTest > testCsvMap STARTED

kafka.utils.CoreUtilsTest > testCsvMap PASSED

kafka.utils.CoreUtilsTest > testInLock STARTED

kafka.utils.CoreUtilsTest > testInLock PASSED

kafka.utils.CoreUtilsTest > testTryAll STARTED

kafka.utils.CoreUtilsTest > testTryAll PASSED

kafka.utils.CoreUtilsTest > testSwallow STARTED

kafka.utils.CoreUtilsTest > testSwallow PASSED

kafka.utils.IteratorTemplateTest > testIterator STARTED

kafka.utils.IteratorTemplateTest > testIterator PASSED

kafka.utils.json.JsonValueTest > testJsonObjectIterator STARTED

kafka.utils.json.JsonValueTest > testJsonObjectIterator PASSED

kafka.utils.json.JsonValueTest > testDecodeLong STARTED

kafka.utils.json.JsonValueTest > testDecodeLong PASSED

kafka.utils.json.JsonValueTest > testAsJsonObject STARTED

kafka.utils.json.JsonValueTest > testAsJsonObject PASSED

kafka.utils.json.JsonValueTest > testDecodeDouble STARTED

kafka.utils.json.JsonValueTest > testDecodeDouble PASSED

kafka.utils.json.JsonValueTest > testDecodeOption STARTED

kafka.utils.json.JsonValueTest > testDecodeOption PASSED

kafka.utils.json.JsonValueTest > testDecodeString STARTED

kafka.utils.json.JsonValueTest > testDecodeString PASSED

kafka.utils.json.JsonValueTest > testJsonValueToString STARTED

kafka.utils.json.JsonValueTest > testJsonValueToString PASSED

kafka.utils.json.JsonValueTest > testAsJsonObjectOption STARTED

kafka.utils.json.JsonValueTest > testAsJsonObjectOption PASSED

kafka.utils.json.JsonValueTest > testAsJsonArrayOption STARTED

kafka.utils.json.JsonValueTest > testAsJsonArrayOption PASSED

kafka.utils.json.JsonValueTest > testAsJsonArray STARTED

kafka.utils.json.JsonValueTest > testAsJsonArray PASSED

kafka.utils.json.JsonValueTest > testJsonValueHashCode STARTED

kafka.utils.json.JsonValueTest > testJsonValueHashCode PASSED

kafka.utils.json.JsonValueTest > testDecodeInt STARTED

kafka.utils.json.JsonValueTest > testDecodeInt PASSED

kafka.utils.json.JsonValueTest > testDecodeMap STARTED

kafka.utils.json.JsonValueTest > testDecodeMap PASSED

kafka.utils.json.JsonValueTest > testDecodeSeq STARTED

kafka.utils.json.JsonValueTest > testDecodeSeq PASSED

kafka.utils.json.JsonValueTest > testJsonObjectGet STARTED

kafka.utils.json.JsonValueTest > testJsonObjectGet PASSED

kafka.utils.json.JsonValueTest > testJsonValueEquals STARTED

kafka.utils.json.JsonValueTest > testJsonValueEquals PASSED

kafka.utils.json.JsonValueTest > testJsonArrayIterator STARTED

kafka.utils.json.JsonValueTest > testJsonArrayIterator PASSED

kafka.utils.json.JsonValueTest > testJsonObjectApply STARTED

kafka.utils.json.JsonValueTest > testJsonObjectApply PASSED

kafka.utils.json.JsonValueTest > testDecodeBoolean STARTED

kafka.utils.json.JsonValueTest > testDecodeBoolean PASSED

kafka.producer.AsyncProducerTest > testFailedSendRetryLogic STARTED

kafka.producer.AsyncProducerTest > testFailedSendRetryLogic PASSED

kafka.producer.AsyncProducerTest > testQueueTimeExpired STARTED

kafka.producer.AsyncProducerTest > testQueueTimeExpired PASSED

kafka.producer.AsyncProducerTest > testPartitionAndCollateEvents STARTED

kafka.producer.AsyncProducerTest > testPartitionAndCollateEvents PASSED

kafka.producer.AsyncProducerTest > testBatchSize STARTED

kafka.producer.AsyncProducerTest > testBatchSize PASSED

kafka.producer.AsyncProducerTest > testSerializeEvents STARTED

kafka.producer.AsyncProducerTest > testSerializeEvents PASSED

kafka.producer.AsyncProducerTest > testProducerQueueSize STARTED

kafka.producer.AsyncProducerTest > testProducerQueueSize PASSED

kafka.producer.AsyncProducerTest > testRandomPartitioner STARTED

kafka.producer.AsyncProducerTest > testRandomPartitioner PASSED

kafka.producer.AsyncProducerTest > testInvalidConfiguration STARTED

kafka.producer.AsyncProducerTest > testInvalidConfiguration PASSED

kafka.producer.AsyncProducerTest > testInvalidPartition STARTED

kafka.producer.AsyncProducerTest > testInvalidPartition PASSED

kafka.producer.AsyncProducerTest > testNoBroker STARTED

kafka.producer.AsyncProducerTest > testNoBroker PASSED

kafka.producer.AsyncProducerTest > testProduceAfterClosed STARTED

kafka.producer.AsyncProducerTest > testProduceAfterClosed PASSED

kafka.producer.AsyncProducerTest > testJavaProducer STARTED

kafka.producer.AsyncProducerTest > testJavaProducer PASSED

kafka.producer.AsyncProducerTest > testIncompatibleEncoder STARTED

kafka.producer.AsyncPr

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

2017-12-19 Thread Apache Jenkins Server
See 


Changes:

[wangguoz] KAFKA-6150: KIP-204 part III; Change repartition topic segment size 
and

--
[...truncated 414.27 KB...]
kafka.utils.CoreUtilsTest > testReadInt STARTED

kafka.utils.CoreUtilsTest > testReadInt PASSED

kafka.utils.CoreUtilsTest > testAtomicGetOrUpdate STARTED

kafka.utils.CoreUtilsTest > testAtomicGetOrUpdate PASSED

kafka.utils.CoreUtilsTest > testUrlSafeBase64EncodeUUID STARTED

kafka.utils.CoreUtilsTest > testUrlSafeBase64EncodeUUID PASSED

kafka.utils.CoreUtilsTest > testCsvMap STARTED

kafka.utils.CoreUtilsTest > testCsvMap PASSED

kafka.utils.CoreUtilsTest > testInLock STARTED

kafka.utils.CoreUtilsTest > testInLock PASSED

kafka.utils.CoreUtilsTest > testTryAll STARTED

kafka.utils.CoreUtilsTest > testTryAll PASSED

kafka.utils.CoreUtilsTest > testSwallow STARTED

kafka.utils.CoreUtilsTest > testSwallow PASSED

kafka.utils.IteratorTemplateTest > testIterator STARTED

kafka.utils.IteratorTemplateTest > testIterator PASSED

kafka.utils.json.JsonValueTest > testJsonObjectIterator STARTED

kafka.utils.json.JsonValueTest > testJsonObjectIterator PASSED

kafka.utils.json.JsonValueTest > testDecodeLong STARTED

kafka.utils.json.JsonValueTest > testDecodeLong PASSED

kafka.utils.json.JsonValueTest > testAsJsonObject STARTED

kafka.utils.json.JsonValueTest > testAsJsonObject PASSED

kafka.utils.json.JsonValueTest > testDecodeDouble STARTED

kafka.utils.json.JsonValueTest > testDecodeDouble PASSED

kafka.utils.json.JsonValueTest > testDecodeOption STARTED

kafka.utils.json.JsonValueTest > testDecodeOption PASSED

kafka.utils.json.JsonValueTest > testDecodeString STARTED

kafka.utils.json.JsonValueTest > testDecodeString PASSED

kafka.utils.json.JsonValueTest > testJsonValueToString STARTED

kafka.utils.json.JsonValueTest > testJsonValueToString PASSED

kafka.utils.json.JsonValueTest > testAsJsonObjectOption STARTED

kafka.utils.json.JsonValueTest > testAsJsonObjectOption PASSED

kafka.utils.json.JsonValueTest > testAsJsonArrayOption STARTED

kafka.utils.json.JsonValueTest > testAsJsonArrayOption PASSED

kafka.utils.json.JsonValueTest > testAsJsonArray STARTED

kafka.utils.json.JsonValueTest > testAsJsonArray PASSED

kafka.utils.json.JsonValueTest > testJsonValueHashCode STARTED

kafka.utils.json.JsonValueTest > testJsonValueHashCode PASSED

kafka.utils.json.JsonValueTest > testDecodeInt STARTED

kafka.utils.json.JsonValueTest > testDecodeInt PASSED

kafka.utils.json.JsonValueTest > testDecodeMap STARTED

kafka.utils.json.JsonValueTest > testDecodeMap PASSED

kafka.utils.json.JsonValueTest > testDecodeSeq STARTED

kafka.utils.json.JsonValueTest > testDecodeSeq PASSED

kafka.utils.json.JsonValueTest > testJsonObjectGet STARTED

kafka.utils.json.JsonValueTest > testJsonObjectGet PASSED

kafka.utils.json.JsonValueTest > testJsonValueEquals STARTED

kafka.utils.json.JsonValueTest > testJsonValueEquals PASSED

kafka.utils.json.JsonValueTest > testJsonArrayIterator STARTED

kafka.utils.json.JsonValueTest > testJsonArrayIterator PASSED

kafka.utils.json.JsonValueTest > testJsonObjectApply STARTED

kafka.utils.json.JsonValueTest > testJsonObjectApply PASSED

kafka.utils.json.JsonValueTest > testDecodeBoolean STARTED

kafka.utils.json.JsonValueTest > testDecodeBoolean PASSED

kafka.producer.AsyncProducerTest > testFailedSendRetryLogic STARTED

kafka.producer.AsyncProducerTest > testFailedSendRetryLogic PASSED

kafka.producer.AsyncProducerTest > testQueueTimeExpired STARTED

kafka.producer.AsyncProducerTest > testQueueTimeExpired PASSED

kafka.producer.AsyncProducerTest > testPartitionAndCollateEvents STARTED

kafka.producer.AsyncProducerTest > testPartitionAndCollateEvents PASSED

kafka.producer.AsyncProducerTest > testBatchSize STARTED

kafka.producer.AsyncProducerTest > testBatchSize PASSED

kafka.producer.AsyncProducerTest > testSerializeEvents STARTED

kafka.producer.AsyncProducerTest > testSerializeEvents PASSED

kafka.producer.AsyncProducerTest > testProducerQueueSize STARTED

kafka.producer.AsyncProducerTest > testProducerQueueSize PASSED

kafka.producer.AsyncProducerTest > testRandomPartitioner STARTED

kafka.producer.AsyncProducerTest > testRandomPartitioner PASSED

kafka.producer.AsyncProducerTest > testInvalidConfiguration STARTED

kafka.producer.AsyncProducerTest > testInvalidConfiguration PASSED

kafka.producer.AsyncProducerTest > testInvalidPartition STARTED

kafka.producer.AsyncProducerTest > testInvalidPartition PASSED

kafka.producer.AsyncProducerTest > testNoBroker STARTED

kafka.producer.AsyncProducerTest > testNoBroker PASSED

kafka.producer.AsyncProducerTest > testProduceAfterClosed STARTED

kafka.producer.AsyncProducerTest > testProduceAfterClosed PASSED

kafka.producer.AsyncProducerTest > testJavaProducer STARTED

kafka.producer.AsyncProducerTest > testJavaProducer PASSED

kafka.producer.AsyncProducerTest > testIncompatibleEncoder STARTED

kafka.pr

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

2017-12-19 Thread Apache Jenkins Server
See 




[VOTE] KIP-227: Introduce Fetch Requests that are Incremental to Increase Partition Scalability

2017-12-19 Thread Colin McCabe
Hi all,

I'd like to start the vote on KIP-227: Incremental Fetch Requests.

The KIP is here: 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-227%3A+Introduce+Incremental+FetchRequests+to+Increase+Partition+Scalability

and discussion thread earlier: 
https://www.mail-archive.com/dev@kafka.apache.org/msg83115.html

thanks,
Colin


Unable to start Sink connectors[Confluent Kafka]

2017-12-19 Thread Somasundaram Sekar
Starting Sink connector on Confluent Kafka setup throws up the below
exception, understand that it is because of some configured interceptor,
any advice on how to disable it?





[2017-12-20 06:51:11,235] ERROR Task {} failed initialization and will not
be started. (org.apache.kafka.connect.runtimee

.WorkerSinkTask:115)

org.apache.kafka.connect.errors.ConnectException: Failed to create consumer

at
org.apache.kafka.connect.runtime.WorkerSinkTask.createConsumer(WorkerSinkTask.java:397)

at
org.apache.kafka.connect.runtime.WorkerSinkTask.initialize(WorkerSinkTask.java:112)

at
org.apache.kafka.connect.runtime.Worker.startTask(Worker.java:390)

at
org.apache.kafka.connect.runtime.distributed.DistributedHerder.startTask(DistributedHerder.java:830)

at
org.apache.kafka.connect.runtime.distributed.DistributedHerder.access$1600(DistributedHerder.java:100)

at
org.apache.kafka.connect.runtime.distributed.DistributedHerder$13.call(DistributedHerder.java:844)

at
org.apache.kafka.connect.runtime.distributed.DistributedHerder$13.call(DistributedHerder.java:840)

at java.util.concurrent.FutureTask.run(FutureTask.java:266)

at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)

at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)

at java.lang.Thread.run(Thread.java:748)

Caused by: org.apache.kafka.common.KafkaException: Failed to construct
kafka consumer

at
org.apache.kafka.clients.consumer.KafkaConsumer.(KafkaConsumer.java:765)

at
org.apache.kafka.clients.consumer.KafkaConsumer.(KafkaConsumer.java:602)

at
org.apache.kafka.clients.consumer.KafkaConsumer.(KafkaConsumer.java:585)

at
org.apache.kafka.connect.runtime.WorkerSinkTask.createConsumer(WorkerSinkTask.java:395)

... 10 more

*Caused by: org.apache.kafka.common.KafkaException:
io.confluent.monitoring.clients.interceptor.MonitoringConsumerIntercee*

*ptor ClassNotFoundException exception occurred*

*at
org.apache.kafka.common.config.AbstractConfig.getConfiguredInstances(AbstractConfig.java:288)*

*at
org.apache.kafka.common.config.AbstractConfig.getConfiguredInstances(AbstractConfig.java:263)*



*Regards,*

*Somasundaram S*

-- 
*Disclaimer*: This e-mail is intended to be delivered only to the named 
addressee(s). If this information is received by anyone other than the 
named addressee(s), the recipient(s) should immediately notify 
i...@tigeranalytics.com and promptly delete the transmitted material from 
your computer and server.   In no event shall this material be read, used, 
stored, or retained by anyone other than the named addressee(s) without the 
express written consent of the sender or the named addressee(s). Computer 
viruses can be transmitted viaemail. The recipient should check this email and 
any attachments for viruses. The company accepts no liability for any 
damage caused by any virus transmitted by this email.