Re: [ANNOUNCE] New Kafka PMC member: Dong Lin

[Viktor Somogyi-Vass]

Congrats Dong! :)

On Tue, Aug 21, 2018 at 10:09 AM James Cheng  wrote:

> Congrats Dong!
>
> -James
>
> > On Aug 20, 2018, at 3:54 AM, Ismael Juma  wrote:
> >
> > Hi everyone,
> >
> > Dong Lin became a committer in March 2018. Since then, he has remained
> > active in the community and contributed a number of patches, reviewed
> > several pull requests and participated in numerous KIP discussions. I am
> > happy to announce that Dong is now a member of the
> > Apache Kafka PMC.
> >
> > Congratulation Dong! Looking forward to your future contributions.
> >
> > Ismael, on behalf of the Apache Kafka PMC
>
>

[VOTE] KIP-336: Consolidate ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer

[Viktor Somogyi-Vass]

Hi All,

I'd like to start a vote on this KIP (
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=87298242)
which aims to refactor ExtendedSerializer/Serializer and
ExtendedDeserializer/Deserializer.

To summarize what's the motivation:

When headers were introduced by KIP-82 the ExtendedSerializer and
ExtendedDeserializer classes were created in order to keep interface
compatibility but still add `T deserialize(String topic, Headers headers,
byte[] data);` and `byte[] serialize(String topic, Headers headers, T
data);` methods that consume the headers for serialization/deserialization.
The reason for doing so was that Kafka at that time needed be compatbile
with Java 7. Since we're not compiling on Java 7 anymore (KAFKA-4423) we'll
try consolidate the way we're using these in a backward compatible fashion:
deprecating the Extended* classes and moving the aforementioned methods up
in the class hierarchy.

I'd be happy to get votes or additional feedback on this.

Viktor

Re: [VOTE] KIP-336: Consolidate ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer

[Viktor Somogyi-Vass]

Hi Ismael,

Regarding the deprecation of the 2 parameter method: should we do this with
the Serializer interface as well?

I've updated the "Rejected Alternatives" with a few.
I've added this circular reference one too but actually there's a way
(pretty heavyweight) by adding a guard class that prevents recursive
invocation of either methods. I've tried this out but it seems to me an
overshoot. So just for the sake of completeness I'll copy it here. :)

public interface Deserializer extends Closeable {

class Guard {

private Set objects = Collections.synchronizedSet(new
HashSet<>()); // might as well use concurrent hashmap

private void methodCallInProgress(Object x) {
objects.add(x);
}

private boolean isMethodCallInProgress(Object x) {
return objects.contains(x);
}

private void clearMethodCallInProgress(Object x) {
objects.remove(x);
}

private  T guard(Supplier supplier) {
if (GUARD.isMethodCallInProgress(this)) {
throw new IllegalStateException("You must implement one of
the deserialize methods");
} else {
try {
GUARD.methodCallInProgress(this);
return supplier.get();
} finally {
GUARD.clearMethodCallInProgress(this);
}
}
}
}

Guard GUARD = new Guard();

void configure(Map configs, boolean isKey);

default T deserialize(String topic, byte[] data) {
return GUARD.guard(() -> deserialize(topic, null, data));
}

default T deserialize(String topic, Headers headers, byte[] data) {
return GUARD.guard(() -> deserialize(topic, data));
}

@Override
void close();
}


Cheers,
Viktor

On Thu, Aug 23, 2018 at 3:50 PM Ismael Juma  wrote:

> Also, we may consider deprecating the deserialize method that does not take
> headers. Yes, it's a convenience, but it also adds confusion.
>
> Ismael
>
> On Thu, Aug 23, 2018 at 6:48 AM Ismael Juma  wrote:
>
> > I think the KIP needs the rejected alternatives section to have more
> > detail. For example, another option would be something like the
> following,
> > which works great as long as one overrides one of the methods, but pretty
> > bad if one doesn't. :)
> >
> > default T deserialize(String topic, byte[] data) {
> > return deserialize(topic, null, data);
> > }
> >
> > default T deserialize(String topic, Headers headers, byte[] data) { //
> > This is the new method
> > return deserialize(topic, data);
> > }
> >
> >
> > On Thu, Aug 23, 2018 at 3:57 AM Viktor Somogyi-Vass <
> > viktorsomo...@gmail.com> wrote:
> >
> >> Hi Jason,
> >>
> >> Thanks for the feedback.
> >> 1. I chose to return null here because according to the documentation it
> >> may return null data, therefore the users of this methods are perpared
> for
> >> getting a null. Thinking of it though it may be better to throw an
> >> exception by default because it'd indicate a programming error. However,
> >> would that be a backward incompatible change? I'm simply thinking of
> this
> >> because this is a new behavior that we'd introduce but I'm not sure yet
> if
> >> it'd cause problems.
> >> Do you think it'd make sense to do the same in `serialize`?
> >> 2. Yes, I believe that is covered in KIP-331:
> >>
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-331+Add+default+implementation+to+close%28%29+and+configure%28%29+for+Serializer%2C+Deserializer+and+Serde
> >>
> >> Cheers,
> >> Viktor
> >>
> >> On Wed, Aug 22, 2018 at 6:11 PM Jason Gustafson 
> >> wrote:
> >>
> >> > Hey Viktor,
> >> >
> >> > This is a nice cleanup. Just a couple quick questions:
> >> >
> >> > 1. Rather than returning null for the default `deserialize(topic,
> >> data)`,
> >> > would it be better to throw UnsupportedOperationException? I assume
> that
> >> > internally we'll always invoke the api which takes headers. Similarly
> >> for
> >> > `serialize(topic, data)`.
> >> > 2. Would it make sense to have default no-op implementations for
> >> > `configure` and `close`?
> >> >
> >> > Thanks,
> >> > Jason
> >> >
> >> > On Wed, Aug 22, 2018 at 5:27 AM, Satish Duggana <
> >> satish.dugg...@gmail.com>
> >> > wrote:
> >> >
> >> > > +1
> >> > >
> >> > > On W

Re: [VOTE] KIP-336: Consolidate ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer

[Viktor Somogyi-Vass]

I think in the first draft I didn't provide an implementation for them as
it seemed very simple and straightforward. I looked up a couple of
implementations of the ExtendedSerializers on github and the general
behavior seems to be that they delegate to the 2 argument (headerless)
method:

https://github.com/khoitnm/practice-spring-kafka-grpc/blob/a6fc9b3395762c4889807baedd822f4653d5dcdd/kafka-common/src/main/java/org/tnmk/common/kafka/serialization/protobuf/ProtobufSerializer.java
https://github.com/hong-zhu/nxgen/blob/5cf1427d4e1a8f5c7fab47955af32a0d4f4873af/nxgen-kafka-client/src/main/java/nxgen/kafka/client/event/serdes/EventSerializer.java
https://github.com/jerry-jx/spring-kafka/blob/ac323ec5b8b9a0ca975db2f7322ff6878fce481a/spring-kafka/src/main/java/org/springframework/kafka/support/serializer/JsonSerializer.java
https://github.com/enzobonggio/nonblocking-kafka/blob/bc1a379b2d9593b46cf9604063bc5b38e2785d19/src/main/java/com/example/kafka/producer/CustomJsonSerializer.java

Of course 4 example is not representative but it shows that these users
usually delegate to the "headerless" (2 argument) method. I've tried to
look it up on other code search sites but haven't had much luck so far.
Given these examples and the way they implement them I'd say it's more
common to delegate to the headerless method, that's why I think it's a good
approach for us too. Now having a default implementation for that is again
a good question. I think current use cases wouldn't change in either case
(unless we deprecate the headerless one).
For the new use cases it depends what do we want to propagate going
forward. Do we want only one method to exist or two? As Ismael highlighted
it might be confusing if we have 2 methods, both with default
implementation and in this case we want to push the 3 argument one for
users.

So I see three possible ways:
1.) Don't provide a default implementation for the headerless method. This
supports the current implementations and encourages the delegation style in
future implementations. This might be the simplest option.
2.) Provide a default implementation for the headerless method. This would
be a bit confusing, so we'd likely push the use of the 3 parameter method
and deprecate the headerless. This would however further litter the code
base with deprecation warnings as we're using the headerless method in a
lot of places (think of the current serializers/deserializers). So in this
case we would want to clean up the code base a little where we can and may
remove the headerless method entirely in Kafka 3. But they would hang
around until that point. I think in this case the implementation for the
headerless is a detail question as that is deprecated so we don't expect
new implementations to use that method.
If we decide to move this way, we have explored two options so far:
returning null / empty array or throwing exceptions. (And I honestly
started to like the latter as calling that with no real implementation is
really a programming error.)
3.) We can do it in multiple steps. In the first step we do 1 and later 2.
I think it would also make sense as the Kafka code base heavily uses the
headerless method still (think of the existing serializers/deserializers)
and it would give us time to eliminate/change those use cases.

Cheers,
Viktor

On Thu, Aug 23, 2018 at 11:55 PM Jason Gustafson  wrote:

> To clarify, what I am suggesting is to only remove the default
> implementation for these methods. So users would be required to implement
> serialize(topic, data) and deserialize(topic, data).
>
> -Jason
>
> On Thu, Aug 23, 2018 at 1:48 PM, Jason Gustafson 
> wrote:
>
> > Hey Viktor,
> >
> > Thinking about it a little more, I wonder if we should just not provide a
> > default method for serialize(topic, data) and deserialize(topic, data).
> > Implementing these methods is a trivial burden for users and it feels
> like
> > there's no good solution which allows both methods to have default
> > implementations.
> >
> > Also, ack on KIP-331. Thanks for the pointer.
> >
> > -Jason
> >
> > On Thu, Aug 23, 2018 at 12:30 PM, Viktor Somogyi-Vass <
> > viktorsomo...@gmail.com> wrote:
> >
> >> Hi Ismael,
> >>
> >> Regarding the deprecation of the 2 parameter method: should we do this
> >> with
> >> the Serializer interface as well?
> >>
> >> I've updated the "Rejected Alternatives" with a few.
> >> I've added this circular reference one too but actually there's a way
> >> (pretty heavyweight) by adding a guard class that prevents recursive
> >> invocation of either methods. I've tried this out but it seems to me an
> >> overshoot. So just for the sake of completeness I'll copy it here. :)
> >>
> >> public interface Deserializer extends Cl

Re: [DISCUSS] KIP-363: Allow performance tools to print final results to output file

[Viktor Somogyi-Vass]

Hi Attila,

Thanks for the KIP, I think overall it looks good. I have three comments:
1. Would you mind adding an example? (Later on we'd need anyway for the
public doc.)
2. Do you want to add any 3rd party CSV reader/writer library or will you
implement that too?
3. What is the separator or is that configurable?

Cheers,
Viktor

On Fri, Aug 24, 2018 at 8:18 AM Kevin Lu  wrote:

> Hi Attila,
>
> Thanks for the KIP.
>
> I think this would be a useful feature. Every time I have to benchmark
> using these performance tools, I end up redirecting the output to a file
> anyways.
>
> Just a couple minor questions...
>
> 1. If the configured file already exists, what would be the outcome? My
> intuition is that the performance tool will spit out some type of error and
> quit as we do not want to accidentally overwrite files.
>
> 2. Will the performance tool still output directly to shell if this option
> is specified?
>
> Regards,
> Kevin
>
> On Wed, Aug 22, 2018 at 12:16 PM Attila Sasvári 
> wrote:
>
> > Hi all,
> >
> > I have created a minor KIP to allow consumer and producer performance
> tools
> > to print final results to output file in CSV format.
> >
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-363%3A+Allow+performance+tools+to+print+final+results+to+output+file
> >
> > Please take a look and share your thoughts!
> >
> > Thanks,
> > Attila
> >
>

Re: [VOTE] KIP-336: Consolidate ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer

[Viktor Somogyi-Vass]

Sure, I've added it. I'll also do the testing today.

On Mon, Aug 27, 2018 at 5:03 PM Ismael Juma  wrote:

> Thanks Viktor. I think it would be good to verify that existing
> ExtendedSerializer implementations work without recompiling. This could be
> done as a manual test. If you agree, I suggest adding it to the testing
> plan section.
>
> Ismael
>
> On Mon, Aug 27, 2018 at 7:57 AM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> wrote:
>
> > Thanks guys, I've updated my KIP with this info (so to keep solution #1).
> > If you find it good enough, please vote as well or let me know if you
> think
> > something is missing.
> >
> > On Sat, Aug 25, 2018 at 1:14 AM Ismael Juma  wrote:
> >
> > > I'm OK with 1 too. It makes me a bit sad that we don't have a path for
> > > removing the method without headers, but it seems like the simplest and
> > > least confusing option (I am assuming that headers are not needed in
> the
> > > serializers in the common case).
> > >
> > > Ismael
> > >
> > > On Fri, Aug 24, 2018 at 2:42 PM Jason Gustafson 
> > > wrote:
> > >
> > > > Hey Viktor,
> > > >
> > > > Good summary. I agree that option 1) seems like the simplest choice
> > and,
> > > as
> > > > you note, we can always add the default implementation later. I'll
> > leave
> > > > Ismael to make a case for the circular forwarding approach ;)
> > > >
> > > > -Jason
> > > >
> > > > On Fri, Aug 24, 2018 at 3:02 AM, Viktor Somogyi-Vass <
> > > > viktorsomo...@gmail.com> wrote:
> > > >
> > > > > I think in the first draft I didn't provide an implementation for
> > them
> > > as
> > > > > it seemed very simple and straightforward. I looked up a couple of
> > > > > implementations of the ExtendedSerializers on github and the
> general
> > > > > behavior seems to be that they delegate to the 2 argument
> > (headerless)
> > > > > method:
> > > > >
> > > > > https://github.com/khoitnm/practice-spring-kafka-grpc/blob/
> > > > > a6fc9b3395762c4889807baedd822f4653d5dcdd/kafka-common/src/
> > > > > main/java/org/tnmk/common/kafka/serialization/protobuf/
> > > > > ProtobufSerializer.java
> > > > >
> > https://github.com/hong-zhu/nxgen/blob/5cf1427d4e1a8f5c7fab47955af32a
> > > > > 0d4f4873af/nxgen-kafka-client/src/main/java/nxgen/kafka/
> > > > > client/event/serdes/EventSerializer.java
> > > > > https://github.com/jerry-jx/spring-kafka/blob/
> > > > > ac323ec5b8b9a0ca975db2f7322ff6878fce481a/spring-kafka/src/
> > > > >
> > > >
> > >
> >
> main/java/org/springframework/kafka/support/serializer/JsonSerializer.java
> > > > > https://github.com/enzobonggio/nonblocking-kafka/blob/
> > > > > bc1a379b2d9593b46cf9604063bc5b38e2785d19/src/main/java/com/
> > > > > example/kafka/producer/CustomJsonSerializer.java
> > > > >
> > > > > Of course 4 example is not representative but it shows that these
> > users
> > > > > usually delegate to the "headerless" (2 argument) method. I've
> tried
> > to
> > > > > look it up on other code search sites but haven't had much luck so
> > far.
> > > > > Given these examples and the way they implement them I'd say it's
> > more
> > > > > common to delegate to the headerless method, that's why I think
> it's
> > a
> > > > good
> > > > > approach for us too. Now having a default implementation for that
> is
> > > > again
> > > > > a good question. I think current use cases wouldn't change in
> either
> > > case
> > > > > (unless we deprecate the headerless one).
> > > > > For the new use cases it depends what do we want to propagate going
> > > > > forward. Do we want only one method to exist or two? As Ismael
> > > > highlighted
> > > > > it might be confusing if we have 2 methods, both with default
> > > > > implementation and in this case we want to push the 3 argument one
> > for
> > > > > users.
> > > > >
> > > > > So I see three possible ways:
> > > > > 1.) Don't provide a default implementation for the headerless
> method.
> > > > This
> > > > > supports the cu

Re: [DISCUSS] KIP-357: Add support to list ACLs per principal

[Viktor Somogyi-Vass]

Hi Manikumar,

Implementation-wise is it just a filter over the returned ACL listing or do
you plan to add new methods to the Authorizer as well?

Thanks,
Viktor

On Fri, Aug 17, 2018 at 9:18 PM Priyank Shah  wrote:

> +1(non-binding)
>
> Thanks.
> Priyank
>
> On 8/16/18, 6:01 AM, "Manikumar"  wrote:
>
> Hi all,
>
> I have created a minor KIP to add support to list ACLs per principal
> using
> AclCommand (kafka-acls.sh)
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-357%3A++Add+support+to+list+ACLs+per+principal
>
> Please take a look.
>
> Thanks,
>
>
>

Re: [VOTE] KIP-336: Consolidate ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer

[Viktor Somogyi-Vass]

Hi Jason,

Thanks for the feedback.
1. I chose to return null here because according to the documentation it
may return null data, therefore the users of this methods are perpared for
getting a null. Thinking of it though it may be better to throw an
exception by default because it'd indicate a programming error. However,
would that be a backward incompatible change? I'm simply thinking of this
because this is a new behavior that we'd introduce but I'm not sure yet if
it'd cause problems.
Do you think it'd make sense to do the same in `serialize`?
2. Yes, I believe that is covered in KIP-331:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-331+Add+default+implementation+to+close%28%29+and+configure%28%29+for+Serializer%2C+Deserializer+and+Serde

Cheers,
Viktor

On Wed, Aug 22, 2018 at 6:11 PM Jason Gustafson  wrote:

> Hey Viktor,
>
> This is a nice cleanup. Just a couple quick questions:
>
> 1. Rather than returning null for the default `deserialize(topic, data)`,
> would it be better to throw UnsupportedOperationException? I assume that
> internally we'll always invoke the api which takes headers. Similarly for
> `serialize(topic, data)`.
> 2. Would it make sense to have default no-op implementations for
> `configure` and `close`?
>
> Thanks,
> Jason
>
> On Wed, Aug 22, 2018 at 5:27 AM, Satish Duggana 
> wrote:
>
> > +1
> >
> > On Wed, Aug 22, 2018 at 4:45 PM, Ted Yu  wrote:
> >
> > > +1
> > >  Original message From: Kamal Chandraprakash <
> > > kamal.chandraprak...@gmail.com> Date: 8/22/18  3:19 AM  (GMT-08:00)
> To:
> > > dev@kafka.apache.org Subject: Re: [VOTE] KIP-336: Consolidate
> > > ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer
> > > +1
> > >
> > > Thanks for the KIP!
> > >
> > > On Wed, Aug 22, 2018 at 2:48 PM Viktor Somogyi-Vass <
> > > viktorsomo...@gmail.com>
> > > wrote:
> > >
> > > > Hi All,
> > > >
> > > > I'd like to start a vote on this KIP (
> > > > https://cwiki.apache.org/confluence/pages/viewpage.
> > > action?pageId=87298242)
> > > > which aims to refactor ExtendedSerializer/Serializer and
> > > > ExtendedDeserializer/Deserializer.
> > > >
> > > > To summarize what's the motivation:
> > > >
> > > > When headers were introduced by KIP-82 the ExtendedSerializer and
> > > > ExtendedDeserializer classes were created in order to keep interface
> > > > compatibility but still add `T deserialize(String topic, Headers
> > headers,
> > > > byte[] data);` and `byte[] serialize(String topic, Headers headers, T
> > > > data);` methods that consume the headers for
> > > serialization/deserialization.
> > > > The reason for doing so was that Kafka at that time needed be
> > compatbile
> > > > with Java 7. Since we're not compiling on Java 7 anymore (KAFKA-4423)
> > > we'll
> > > > try consolidate the way we're using these in a backward compatible
> > > fashion:
> > > > deprecating the Extended* classes and moving the aforementioned
> methods
> > > up
> > > > in the class hierarchy.
> > > >
> > > > I'd be happy to get votes or additional feedback on this.
> > > >
> > > > Viktor
> > > >
> > >
> >
>

Re: [VOTE] KIP-336: Consolidate ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer

[Viktor Somogyi-Vass]

Thanks guys, I've updated my KIP with this info (so to keep solution #1).
If you find it good enough, please vote as well or let me know if you think
something is missing.

On Sat, Aug 25, 2018 at 1:14 AM Ismael Juma  wrote:

> I'm OK with 1 too. It makes me a bit sad that we don't have a path for
> removing the method without headers, but it seems like the simplest and
> least confusing option (I am assuming that headers are not needed in the
> serializers in the common case).
>
> Ismael
>
> On Fri, Aug 24, 2018 at 2:42 PM Jason Gustafson 
> wrote:
>
> > Hey Viktor,
> >
> > Good summary. I agree that option 1) seems like the simplest choice and,
> as
> > you note, we can always add the default implementation later. I'll leave
> > Ismael to make a case for the circular forwarding approach ;)
> >
> > -Jason
> >
> > On Fri, Aug 24, 2018 at 3:02 AM, Viktor Somogyi-Vass <
> > viktorsomo...@gmail.com> wrote:
> >
> > > I think in the first draft I didn't provide an implementation for them
> as
> > > it seemed very simple and straightforward. I looked up a couple of
> > > implementations of the ExtendedSerializers on github and the general
> > > behavior seems to be that they delegate to the 2 argument (headerless)
> > > method:
> > >
> > > https://github.com/khoitnm/practice-spring-kafka-grpc/blob/
> > > a6fc9b3395762c4889807baedd822f4653d5dcdd/kafka-common/src/
> > > main/java/org/tnmk/common/kafka/serialization/protobuf/
> > > ProtobufSerializer.java
> > > https://github.com/hong-zhu/nxgen/blob/5cf1427d4e1a8f5c7fab47955af32a
> > > 0d4f4873af/nxgen-kafka-client/src/main/java/nxgen/kafka/
> > > client/event/serdes/EventSerializer.java
> > > https://github.com/jerry-jx/spring-kafka/blob/
> > > ac323ec5b8b9a0ca975db2f7322ff6878fce481a/spring-kafka/src/
> > >
> >
> main/java/org/springframework/kafka/support/serializer/JsonSerializer.java
> > > https://github.com/enzobonggio/nonblocking-kafka/blob/
> > > bc1a379b2d9593b46cf9604063bc5b38e2785d19/src/main/java/com/
> > > example/kafka/producer/CustomJsonSerializer.java
> > >
> > > Of course 4 example is not representative but it shows that these users
> > > usually delegate to the "headerless" (2 argument) method. I've tried to
> > > look it up on other code search sites but haven't had much luck so far.
> > > Given these examples and the way they implement them I'd say it's more
> > > common to delegate to the headerless method, that's why I think it's a
> > good
> > > approach for us too. Now having a default implementation for that is
> > again
> > > a good question. I think current use cases wouldn't change in either
> case
> > > (unless we deprecate the headerless one).
> > > For the new use cases it depends what do we want to propagate going
> > > forward. Do we want only one method to exist or two? As Ismael
> > highlighted
> > > it might be confusing if we have 2 methods, both with default
> > > implementation and in this case we want to push the 3 argument one for
> > > users.
> > >
> > > So I see three possible ways:
> > > 1.) Don't provide a default implementation for the headerless method.
> > This
> > > supports the current implementations and encourages the delegation
> style
> > in
> > > future implementations. This might be the simplest option.
> > > 2.) Provide a default implementation for the headerless method. This
> > would
> > > be a bit confusing, so we'd likely push the use of the 3 parameter
> method
> > > and deprecate the headerless. This would however further litter the
> code
> > > base with deprecation warnings as we're using the headerless method in
> a
> > > lot of places (think of the current serializers/deserializers). So in
> > this
> > > case we would want to clean up the code base a little where we can and
> > may
> > > remove the headerless method entirely in Kafka 3. But they would hang
> > > around until that point. I think in this case the implementation for
> the
> > > headerless is a detail question as that is deprecated so we don't
> expect
> > > new implementations to use that method.
> > > If we decide to move this way, we have explored two options so far:
> > > returning null / empty array or throwing exceptions. (And I honestly
> > > started to like the latter as calling that with no real implementation
> is
> > > really a programming error.)
>

Re: [VOTE] KIP-363: Allow performance tools to print final results to output file

[Viktor Somogyi-Vass]

+1 (non-binding),

Thanks for the KIP!

Viktor

On Wed, Sep 5, 2018 at 10:14 AM Attila Sasvári  wrote:

> Hi All,
>
> I'd like to start the vote on KIP-363:
> *
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-363%3A+Allow+performance+tools+to+print+final+results+to+output+file
> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-363%3A+Allow+performance+tools+to+print+final+results+to+output+file
> >*
> .
>
> Thanks in advance for reviewing.
>
> Best regards,
> - Attila
>

Re: [VOTE] KIP-336: Consolidate ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer

[Viktor Somogyi-Vass]

Hi All,

Many thanks for participating the discussion and voting. The KIP has passed
with 3 binding votes (Jason, Harsha, Ismael) and 2 non-binding (Attila and
Manikumar).
If you have time, please also have a look at the corresponding code review:
https://github.com/apache/kafka/pull/5494

Cheers,
Viktor

On Thu, Sep 6, 2018 at 11:03 PM Ismael Juma  wrote:

> Thanks, +1 (binding).
>
> On Mon, Sep 3, 2018 at 6:28 AM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> wrote:
>
> > Apologies, miscounted the binding votes but the good news is that we need
> > only one.
> >
> > Cheers,
> > Viktor
> >
> > On Mon, Sep 3, 2018 at 11:09 AM Viktor Somogyi-Vass <
> > viktorsomo...@gmail.com>
> > wrote:
> >
> > > Hi All,
> > >
> > > I have completed my binary compatibility testing on this as well.
> Created
> > > a small script & example to make this reproducible:
> > > https://gist.github.com/viktorsomogyi/391defca73e7a46a2c6a40bc699231d4
> .
> > >
> > > Also, thanks for the votes so far, we're still awaiting for 2 binding.
> > >
> > > Cheers,
> > > Viktor
> > >
> > > On Thu, Aug 30, 2018 at 5:09 PM Harsha  wrote:
> > >
> > >> +1.
> > >> Thanks,
> > >> Harsha
> > >>
> > >> On Thu, Aug 30, 2018, at 4:19 AM, Attila Sasvári wrote:
> > >> > Thanks for the KIP and the updates Viktor!
> > >> >
> > >> > +1 (non-binding)
> > >> >
> > >> >
> > >> >
> > >> > On Wed, Aug 29, 2018 at 10:44 AM Manikumar <
> manikumar.re...@gmail.com
> > >
> > >> > wrote:
> > >> >
> > >> > > +1 (non-binding)
> > >> > >
> > >> > > Thanks for the KIP.
> > >> > >
> > >> > > On Wed, Aug 29, 2018 at 1:41 AM Jason Gustafson <
> ja...@confluent.io
> > >
> > >> > > wrote:
> > >> > >
> > >> > > > +1 Thanks for the updates.
> > >> > > >
> > >> > > > On Tue, Aug 28, 2018 at 1:15 AM, Viktor Somogyi-Vass <
> > >> > > > viktorsomo...@gmail.com> wrote:
> > >> > > >
> > >> > > > > Sure, I've added it. I'll also do the testing today.
> > >> > > > >
> > >> > > > > On Mon, Aug 27, 2018 at 5:03 PM Ismael Juma <
> ism...@juma.me.uk>
> > >> wrote:
> > >> > > > >
> > >> > > > > > Thanks Viktor. I think it would be good to verify that
> > existing
> > >> > > > > > ExtendedSerializer implementations work without recompiling.
> > >> This
> > >> > > could
> > >> > > > > be
> > >> > > > > > done as a manual test. If you agree, I suggest adding it to
> > the
> > >> > > testing
> > >> > > > > > plan section.
> > >> > > > > >
> > >> > > > > > Ismael
> > >> > > > > >
> > >> > > > > > On Mon, Aug 27, 2018 at 7:57 AM Viktor Somogyi-Vass <
> > >> > > > > > viktorsomo...@gmail.com>
> > >> > > > > > wrote:
> > >> > > > > >
> > >> > > > > > > Thanks guys, I've updated my KIP with this info (so to
> keep
> > >> > > solution
> > >> > > > > #1).
> > >> > > > > > > If you find it good enough, please vote as well or let me
> > >> know if
> > >> > > you
> > >> > > > > > think
> > >> > > > > > > something is missing.
> > >> > > > > > >
> > >> > > > > > > On Sat, Aug 25, 2018 at 1:14 AM Ismael Juma <
> > >> ism...@juma.me.uk>
> > >> > > > wrote:
> > >> > > > > > >
> > >> > > > > > > > I'm OK with 1 too. It makes me a bit sad that we don't
> > have
> > >> a
> > >> > > path
> > >> > > > > for
> > >> > > > > > > > removing the method without headers, but it seems like
> the
> > >> > > simplest
> > >> > > > > and
> > >> > > > > &

Re: [ANNOUNCE] New Committer: Manikumar Reddy

[Viktor Somogyi-Vass]

Congratulations Manikumar, well deserved!

On Fri, 12 Oct 2018, 06:30 Andras Beni, 
wrote:

> Congratulations, Manikumar!
>
> Srinivas Reddy  ezt írta (időpont: 2018. okt.
> 12., P 3:00):
>
> > Congratulations Mani. We'll deserved 
> >
> > -
> > Srinivas
> >
> > - Typed on tiny keys. pls ignore typos.{mobile app}
> >
> > On Fri 12 Oct, 2018, 01:39 Jason Gustafson,  wrote:
> >
> > > Hi all,
> > >
> > > The PMC for Apache Kafka has invited Manikumar Reddy as a committer and
> > we
> > > are
> > > pleased to announce that he has accepted!
> > >
> > > Manikumar has contributed 134 commits including significant work to add
> > > support for delegation tokens in Kafka:
> > >
> > > KIP-48:
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-48+Delegation+token+support+for+Kafka
> > > KIP-249
> > > <
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-48+Delegation+token+support+for+KafkaKIP-249
> > >
> > > :
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-249%3A+Add+Delegation+Token+Operations+to+KafkaAdminClient
> > >
> > > He has broad experience working with many of the core components in
> Kafka
> > > and he has reviewed over 80 PRs. He has also made huge progress
> > addressing
> > > some of our technical debt.
> > >
> > > We appreciate the contributions and we are looking forward to more.
> > > Congrats Manikumar!
> > >
> > > Jason, on behalf of the Apache Kafka PMC
> > >
> >
>

Re: [DISCUSS] KIP-377: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi Colin,

Thanks, it makes sense and simplifies this KIP tremendously. I'll move this
section to the rejected alternatives with a note that KIP-142 will have
this feature.
On a similar note: is there a KIP for describe topics protocol or have you
been thinking about it? I guess there it's the same problem, we often don't
want to forward the entire metadata.

Viktor

On Fri, Oct 12, 2018 at 12:03 PM Colin McCabe  wrote:

> Hi Viktor,
>
> Thanks for bumping this thread.
>
> I think we should just focus on transitioning the TopicCommand to use
> AdminClient, and talk about protocol changes in a separate KIP.  Protocol
> changes often involve a lot of discussion.  This does mean that we couldn't
> implement the "list topics under deletion" feature when using AdminClient
> at the moment.  We could add a note to the tool output indicating this.
>
> We should move the protocol discussion to a separate thread.  Probably
> also look at KIP-142 as well.
>
> best,
> Colin
>
>
> On Tue, Oct 9, 2018, at 07:45, Viktor Somogyi-Vass wrote:
> > Hi All,
> >
> > Would like to bump this as the conversation sank a little bit, but more
> > importantly I'd like to validate my plans/ideas on extending the Metadata
> > protocol. I was thinking about two other alternatives, namely:
> > 1. Create a ListTopicUnderDeletion protocol. This however would be
> > unnecessary: it'd have one very narrow functionality which we can't
> extend.
> > I'd make sense to have a list topics or describe topics protocol where we
> > can list/describe topics under deletion but for normal listing/describing
> > we already use the metadata, so it would be a duplication of
> functionality.
> > 2. DeleteTopicsResponse could return the topics under deletion if the
> > request's argument list is empty which might make sense at the first
> look,
> > but actually we'd mix the query functionality with the delete
> functionality
> > which is counterintuitive.
> >
> > Even though most clients won't need these "limbo" topics (which are under
> > deletion) in the foreseeable future, it can be considered as part of the
> > cluster state or metadata and to me it makes sense. Also it doesn't have
> a
> > big overhead in the response size as typically users don't delete topics
> > too often as far as I experienced.
> >
> > I'd be happy to receive some ideas/feedback on this.
> >
> > Cheers,
> > Viktor
> >
> >
> > On Fri, Sep 28, 2018 at 4:51 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> > wrote:
> >
> > > Hi All,
> > >
> > > I made an update to the KIP. Just in short:
> > > Currently KafkaAdminClient.describeTopics() and
> > > KafkaAdminClient.listTopics() uses the Metadata protocol to acquire
> topic
> > > information. The returned response however won't contain the topics
> that
> > > are under deletion but couldn't complete yet (for instance because of
> some
> > > replicas offline), therefore it is not possible to implement the
> current
> > > command's "marked for deletion" feature. To get around this I
> introduced
> > > some changes in the Metadata protocol.
> > >
> > > Thanks,
> > > Viktor
> > >
> > > On Fri, Sep 28, 2018 at 4:48 PM Viktor Somogyi-Vass <
> > > viktorsomo...@gmail.com> wrote:
> > >
> > >> Hi Mickael,
> > >>
> > >> Thanks for the feedback, I also think that many customers wanted this
> for
> > >> a long time.
> > >>
> > >> Cheers,
> > >> Viktor
> > >>
> > >> On Fri, Sep 28, 2018 at 11:45 AM Mickael Maison <
> mickael.mai...@gmail.com>
> > >> wrote:
> > >>
> > >>> Hi Viktor,
> > >>> Thanks for taking this task!
> > >>> This is a very nice change as it will allow users to use this tool in
> > >>> many Cloud environments where direct zookeeper access is not
> > >>> available.
> > >>>
> > >>>
> > >>> On Thu, Sep 27, 2018 at 10:34 AM Viktor Somogyi-Vass
> > >>>  wrote:
> > >>> >
> > >>> > Hi All,
> > >>> >
> > >>> > This is the continuation of the old KIP-375 with the same title:
> > >>> >
> > >>>
> https://lists.apache.org/thread.html/dc71d08de8cd2f082765be22c9f88bc9f8b39bb8e0929a3a4394e9da@%3Cdev.kafka.apache.org%3E
> > >>> >
> > >>> > The problem there was that two KIPs w

[VOTE] KIP-377: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi All,

I'd like to start a vote on KIP-377:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-377%3A+TopicCommand+to+use+AdminClient.

Summary:
The KIP basically proposes to add --bootstrap-server and
--command-config option to TopicsCommand and implement topic
administration with AdminClient in a backwards compatible way (so
wouldn't drop or change the --zookeeper option usage).

I'd appreciate any votes or feedback.

Viktor

Re: [DISCUSS] KIP-377: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi All,

Colin, thanks for the heads-up. I'll rethink this metadata protocol thing
as in a global sense there might be other options as you mentioned and
start separate a discussion.

I'll start a vote soon as the KIP itself is relatively simple.

Viktor

On Tue, Oct 23, 2018 at 3:33 AM Kevin Lu  wrote:

> Hi Viktor,
>
> +1 to this KIP.
>
> I would very much like to see AdminClient in TopicCommand. This would also
> allow us to efficiently implement new features like the "--under-min-isr"
> option I proposed in KIP-351
> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-351%3A+Add+--under-min-isr+option+to+describe+topics+command
> >
> .
>
> Thanks.
>
> Regards,
> Kevin
>
> On Sat, Oct 20, 2018 at 10:52 PM Colin McCabe  wrote:
>
> > Hi Viktor,
> >
> > Sounds good.  If you want to propose a way of improving the metadata
> > protocol so that "[deleted]" could be supported, you could probably
> create
> > that KIP in parallel.
> >
> > The last KIP in that area that I can remember is KIP-142, which didn't
> get
> > adopted (yet?)
> >
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-142%3A+Add+ListTopicsRequest+to+efficiently+list+all+the+topics+in+a+cluster
> >
> > There have been other discussions though.  In general there are a lot of
> > features that would be nice to have in the metadata protocol (pagniation,
> > regexes, skip stuff we don't need).
> >
> > best,
> > Colin
> >
> >
> > On Tue, Oct 16, 2018, at 10:11, Viktor Somogyi-Vass wrote:
> > > Hi Colin,
> > >
> > > Thanks, it makes sense and simplifies this KIP tremendously. I'll move
> > this
> > > section to the rejected alternatives with a note that KIP-142 will have
> > > this feature.
> > > On a similar note: is there a KIP for describe topics protocol or have
> > you
> > > been thinking about it? I guess there it's the same problem, we often
> > don't
> > > want to forward the entire metadata.
> > >
> > > Viktor
> > >
> > > On Fri, Oct 12, 2018 at 12:03 PM Colin McCabe 
> > wrote:
> > >
> > > > Hi Viktor,
> > > >
> > > > Thanks for bumping this thread.
> > > >
> > > > I think we should just focus on transitioning the TopicCommand to use
> > > > AdminClient, and talk about protocol changes in a separate KIP.
> > Protocol
> > > > changes often involve a lot of discussion.  This does mean that we
> > couldn't
> > > > implement the "list topics under deletion" feature when using
> > AdminClient
> > > > at the moment.  We could add a note to the tool output indicating
> this.
> > > >
> > > > We should move the protocol discussion to a separate thread.
> Probably
> > > > also look at KIP-142 as well.
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Tue, Oct 9, 2018, at 07:45, Viktor Somogyi-Vass wrote:
> > > > > Hi All,
> > > > >
> > > > > Would like to bump this as the conversation sank a little bit, but
> > more
> > > > > importantly I'd like to validate my plans/ideas on extending the
> > Metadata
> > > > > protocol. I was thinking about two other alternatives, namely:
> > > > > 1. Create a ListTopicUnderDeletion protocol. This however would be
> > > > > unnecessary: it'd have one very narrow functionality which we can't
> > > > extend.
> > > > > I'd make sense to have a list topics or describe topics protocol
> > where we
> > > > > can list/describe topics under deletion but for normal
> > listing/describing
> > > > > we already use the metadata, so it would be a duplication of
> > > > functionality.
> > > > > 2. DeleteTopicsResponse could return the topics under deletion if
> the
> > > > > request's argument list is empty which might make sense at the
> first
> > > > look,
> > > > > but actually we'd mix the query functionality with the delete
> > > > functionality
> > > > > which is counterintuitive.
> > > > >
> > > > > Even though most clients won't need these "limbo" topics (which are
> > under
> > > > > deletion) in the foreseeable future, it can be considered as part
> of
> > the
> > > > > cluster state or metadata and to me it makes sense. A

Re: [VOTE] KIP-377: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi All,

I'm closing the vote as there has been a few days without any more
feedback and the KIP collected 3 binding votes (Gwen, Harsha and
Colin) and 2 non-binding (Mickael and Kevin) - therefore it got
accepted.
I'll update its status and work on the implementation (I have a WIP PR
but that still needs some changes to be done). As soon as it's ready
I'll ping this thread if anyone interested in the review.

Thank you all for participating the discussion and voting.

Viktor
On Thu, Oct 25, 2018 at 10:53 PM Gwen Shapira  wrote:
>
> +1 (binding)
> Thanks for working on this.
>
> On Wed, Oct 24, 2018, 7:28 AM Viktor Somogyi-Vass 
> wrote:
>
> > Hi All,
> >
> > I'd like to start a vote on KIP-377:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-377%3A+TopicCommand+to+use+AdminClient
> > .
> >
> > Summary:
> > The KIP basically proposes to add --bootstrap-server and
> > --command-config option to TopicsCommand and implement topic
> > administration with AdminClient in a backwards compatible way (so
> > wouldn't drop or change the --zookeeper option usage).
> >
> > I'd appreciate any votes or feedback.
> >
> > Viktor
> >

Re: [VOTE] KIP-377: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Thanks for the votes so far.

@Colin: yup, I've added this to the KIP. Thanks.
On Thu, Oct 25, 2018 at 2:30 AM Colin McCabe  wrote:
>
> Thanks, Viktor.  +1 (binding).
>
> One note: can we add a deprecation warning when --zookeeper is used, to 
> indicate that this option will be phased out in the future?
>
> best,
> Colin
>
> On Wed, Oct 24, 2018, at 05:47, Mickael Maison wrote:
> > +1 (non-binding)
> > Thanks for the KIP!
> > On Wed, Oct 24, 2018 at 1:28 PM Viktor Somogyi-Vass
> >  wrote:
> > >
> > > Hi All,
> > >
> > > I'd like to start a vote on KIP-377:
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-377%3A+TopicCommand+to+use+AdminClient.
> > >
> > > Summary:
> > > The KIP basically proposes to add --bootstrap-server and
> > > --command-config option to TopicsCommand and implement topic
> > > administration with AdminClient in a backwards compatible way (so
> > > wouldn't drop or change the --zookeeper option usage).
> > >
> > > I'd appreciate any votes or feedback.
> > >
> > > Viktor

Re: [VOTE] KIP-374: Add '--help' option to all available Kafka CLI commands

[Viktor Somogyi-Vass]

+1 (non-binding)

Thanks for this KIP.

Viktor

On Wed, Oct 31, 2018 at 2:43 PM Srinivas Reddy 
wrote:

> Hi All,
>
> I would like to call for a vote on KIP-374:
> https://cwiki.apache.org/confluence/x/FgSQBQ
>
> Summary:
> Currently, the '--help' option is recognized by some Kafka commands
> but not all. To provide a consistent user experience, it would be nice to
> add a '--help' option to all Kafka commands.
>
> I'd appreciate any votes or feedback.
>
> --
> Srinivas Reddy
>
> http://mrsrinivas.com/
>
>
> (Sent via gmail web)
>

Re: [DISCUSS] KIP-377: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi All,

Would like to bump this as the conversation sank a little bit, but more
importantly I'd like to validate my plans/ideas on extending the Metadata
protocol. I was thinking about two other alternatives, namely:
1. Create a ListTopicUnderDeletion protocol. This however would be
unnecessary: it'd have one very narrow functionality which we can't extend.
I'd make sense to have a list topics or describe topics protocol where we
can list/describe topics under deletion but for normal listing/describing
we already use the metadata, so it would be a duplication of functionality.
2. DeleteTopicsResponse could return the topics under deletion if the
request's argument list is empty which might make sense at the first look,
but actually we'd mix the query functionality with the delete functionality
which is counterintuitive.

Even though most clients won't need these "limbo" topics (which are under
deletion) in the foreseeable future, it can be considered as part of the
cluster state or metadata and to me it makes sense. Also it doesn't have a
big overhead in the response size as typically users don't delete topics
too often as far as I experienced.

I'd be happy to receive some ideas/feedback on this.

Cheers,
Viktor


On Fri, Sep 28, 2018 at 4:51 PM Viktor Somogyi-Vass 
wrote:

> Hi All,
>
> I made an update to the KIP. Just in short:
> Currently KafkaAdminClient.describeTopics() and
> KafkaAdminClient.listTopics() uses the Metadata protocol to acquire topic
> information. The returned response however won't contain the topics that
> are under deletion but couldn't complete yet (for instance because of some
> replicas offline), therefore it is not possible to implement the current
> command's "marked for deletion" feature. To get around this I introduced
> some changes in the Metadata protocol.
>
> Thanks,
> Viktor
>
> On Fri, Sep 28, 2018 at 4:48 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
>> Hi Mickael,
>>
>> Thanks for the feedback, I also think that many customers wanted this for
>> a long time.
>>
>> Cheers,
>> Viktor
>>
>> On Fri, Sep 28, 2018 at 11:45 AM Mickael Maison 
>> wrote:
>>
>>> Hi Viktor,
>>> Thanks for taking this task!
>>> This is a very nice change as it will allow users to use this tool in
>>> many Cloud environments where direct zookeeper access is not
>>> available.
>>>
>>>
>>> On Thu, Sep 27, 2018 at 10:34 AM Viktor Somogyi-Vass
>>>  wrote:
>>> >
>>> > Hi All,
>>> >
>>> > This is the continuation of the old KIP-375 with the same title:
>>> >
>>> https://lists.apache.org/thread.html/dc71d08de8cd2f082765be22c9f88bc9f8b39bb8e0929a3a4394e9da@%3Cdev.kafka.apache.org%3E
>>> >
>>> > The problem there was that two KIPs were created around the same time
>>> and I
>>> > chose to reorganize mine a bit and give it a new number to avoid
>>> > duplication.
>>> >
>>> > The KIP summary here once again:
>>> >
>>> > I wrote up a relatively simple KIP about improving the Kafka protocol
>>> and
>>> > the TopicCommand tool to support the new Java based AdminClient and
>>> > hopefully to deprecate the Zookeeper side of it.
>>> >
>>> > I would be happy to receive some opinions about this. In general I
>>> think
>>> > this would be an important addition as this is one of the few left but
>>> > important tools that still uses direct Zookeeper connection.
>>> >
>>> > Here is the link for the KIP:
>>> >
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-377%3A+TopicCommand+to+use+AdminClient
>>> >
>>> > Cheers,
>>> > Viktor
>>>
>>

Re: [VOTE] KIP-336: Consolidate ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer

[Viktor Somogyi-Vass]

Hi All,

I have completed my binary compatibility testing on this as well. Created a
small script & example to make this reproducible:
https://gist.github.com/viktorsomogyi/391defca73e7a46a2c6a40bc699231d4 .

Also, thanks for the votes so far, we're still awaiting for 2 binding.

Cheers,
Viktor

On Thu, Aug 30, 2018 at 5:09 PM Harsha  wrote:

> +1.
> Thanks,
> Harsha
>
> On Thu, Aug 30, 2018, at 4:19 AM, Attila Sasvári wrote:
> > Thanks for the KIP and the updates Viktor!
> >
> > +1 (non-binding)
> >
> >
> >
> > On Wed, Aug 29, 2018 at 10:44 AM Manikumar 
> > wrote:
> >
> > > +1 (non-binding)
> > >
> > > Thanks for the KIP.
> > >
> > > On Wed, Aug 29, 2018 at 1:41 AM Jason Gustafson 
> > > wrote:
> > >
> > > > +1 Thanks for the updates.
> > > >
> > > > On Tue, Aug 28, 2018 at 1:15 AM, Viktor Somogyi-Vass <
> > > > viktorsomo...@gmail.com> wrote:
> > > >
> > > > > Sure, I've added it. I'll also do the testing today.
> > > > >
> > > > > On Mon, Aug 27, 2018 at 5:03 PM Ismael Juma 
> wrote:
> > > > >
> > > > > > Thanks Viktor. I think it would be good to verify that existing
> > > > > > ExtendedSerializer implementations work without recompiling. This
> > > could
> > > > > be
> > > > > > done as a manual test. If you agree, I suggest adding it to the
> > > testing
> > > > > > plan section.
> > > > > >
> > > > > > Ismael
> > > > > >
> > > > > > On Mon, Aug 27, 2018 at 7:57 AM Viktor Somogyi-Vass <
> > > > > > viktorsomo...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Thanks guys, I've updated my KIP with this info (so to keep
> > > solution
> > > > > #1).
> > > > > > > If you find it good enough, please vote as well or let me know
> if
> > > you
> > > > > > think
> > > > > > > something is missing.
> > > > > > >
> > > > > > > On Sat, Aug 25, 2018 at 1:14 AM Ismael Juma  >
> > > > wrote:
> > > > > > >
> > > > > > > > I'm OK with 1 too. It makes me a bit sad that we don't have a
> > > path
> > > > > for
> > > > > > > > removing the method without headers, but it seems like the
> > > simplest
> > > > > and
> > > > > > > > least confusing option (I am assuming that headers are not
> needed
> > > > in
> > > > > > the
> > > > > > > > serializers in the common case).
> > > > > > > >
> > > > > > > > Ismael
> > > > > > > >
> > > > > > > > On Fri, Aug 24, 2018 at 2:42 PM Jason Gustafson <
> > > > ja...@confluent.io>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hey Viktor,
> > > > > > > > >
> > > > > > > > > Good summary. I agree that option 1) seems like the
> simplest
> > > > choice
> > > > > > > and,
> > > > > > > > as
> > > > > > > > > you note, we can always add the default implementation
> later.
> > > > I'll
> > > > > > > leave
> > > > > > > > > Ismael to make a case for the circular forwarding approach
> ;)
> > > > > > > > >
> > > > > > > > > -Jason
> > > > > > > > >
> > > > > > > > > On Fri, Aug 24, 2018 at 3:02 AM, Viktor Somogyi-Vass <
> > > > > > > > > viktorsomo...@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > > I think in the first draft I didn't provide an
> implementation
> > > > for
> > > > > > > them
> > > > > > > > as
> > > > > > > > > > it seemed very simple and straightforward. I looked up a
> > > couple
> > > > > of
> > > > > > > > > > implementations of the ExtendedSerializers on github and
> the
> > > > > > general
> > > > > > > > > &

Re: [VOTE] KIP-336: Consolidate ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer

[Viktor Somogyi-Vass]

Apologies, miscounted the binding votes but the good news is that we need
only one.

Cheers,
Viktor

On Mon, Sep 3, 2018 at 11:09 AM Viktor Somogyi-Vass 
wrote:

> Hi All,
>
> I have completed my binary compatibility testing on this as well. Created
> a small script & example to make this reproducible:
> https://gist.github.com/viktorsomogyi/391defca73e7a46a2c6a40bc699231d4 .
>
> Also, thanks for the votes so far, we're still awaiting for 2 binding.
>
> Cheers,
> Viktor
>
> On Thu, Aug 30, 2018 at 5:09 PM Harsha  wrote:
>
>> +1.
>> Thanks,
>> Harsha
>>
>> On Thu, Aug 30, 2018, at 4:19 AM, Attila Sasvári wrote:
>> > Thanks for the KIP and the updates Viktor!
>> >
>> > +1 (non-binding)
>> >
>> >
>> >
>> > On Wed, Aug 29, 2018 at 10:44 AM Manikumar 
>> > wrote:
>> >
>> > > +1 (non-binding)
>> > >
>> > > Thanks for the KIP.
>> > >
>> > > On Wed, Aug 29, 2018 at 1:41 AM Jason Gustafson 
>> > > wrote:
>> > >
>> > > > +1 Thanks for the updates.
>> > > >
>> > > > On Tue, Aug 28, 2018 at 1:15 AM, Viktor Somogyi-Vass <
>> > > > viktorsomo...@gmail.com> wrote:
>> > > >
>> > > > > Sure, I've added it. I'll also do the testing today.
>> > > > >
>> > > > > On Mon, Aug 27, 2018 at 5:03 PM Ismael Juma 
>> wrote:
>> > > > >
>> > > > > > Thanks Viktor. I think it would be good to verify that existing
>> > > > > > ExtendedSerializer implementations work without recompiling.
>> This
>> > > could
>> > > > > be
>> > > > > > done as a manual test. If you agree, I suggest adding it to the
>> > > testing
>> > > > > > plan section.
>> > > > > >
>> > > > > > Ismael
>> > > > > >
>> > > > > > On Mon, Aug 27, 2018 at 7:57 AM Viktor Somogyi-Vass <
>> > > > > > viktorsomo...@gmail.com>
>> > > > > > wrote:
>> > > > > >
>> > > > > > > Thanks guys, I've updated my KIP with this info (so to keep
>> > > solution
>> > > > > #1).
>> > > > > > > If you find it good enough, please vote as well or let me
>> know if
>> > > you
>> > > > > > think
>> > > > > > > something is missing.
>> > > > > > >
>> > > > > > > On Sat, Aug 25, 2018 at 1:14 AM Ismael Juma <
>> ism...@juma.me.uk>
>> > > > wrote:
>> > > > > > >
>> > > > > > > > I'm OK with 1 too. It makes me a bit sad that we don't have
>> a
>> > > path
>> > > > > for
>> > > > > > > > removing the method without headers, but it seems like the
>> > > simplest
>> > > > > and
>> > > > > > > > least confusing option (I am assuming that headers are not
>> needed
>> > > > in
>> > > > > > the
>> > > > > > > > serializers in the common case).
>> > > > > > > >
>> > > > > > > > Ismael
>> > > > > > > >
>> > > > > > > > On Fri, Aug 24, 2018 at 2:42 PM Jason Gustafson <
>> > > > ja...@confluent.io>
>> > > > > > > > wrote:
>> > > > > > > >
>> > > > > > > > > Hey Viktor,
>> > > > > > > > >
>> > > > > > > > > Good summary. I agree that option 1) seems like the
>> simplest
>> > > > choice
>> > > > > > > and,
>> > > > > > > > as
>> > > > > > > > > you note, we can always add the default implementation
>> later.
>> > > > I'll
>> > > > > > > leave
>> > > > > > > > > Ismael to make a case for the circular forwarding
>> approach ;)
>> > > > > > > > >
>> > > > > > > > > -Jason
>> > > > > > > > >
>> > > > > > > > > On Fri, Aug 24, 2018 at 3:02 AM, Viktor Somogyi-Vass <
>> > > > > > > > > viktorsomo.

Re: [ANNOUNCE] New Kafka PMC member: Dong Lin

[Viktor Somogyi-Vass]

Congrats Dong!

On Sun, Sep 16, 2018 at 2:44 PM Matt Farmer  wrote:

> Congrats Dong!
>
> On Sat, Sep 15, 2018 at 4:40 PM Bill Bejeck  wrote:
>
> > Congrats!
> >
> > On Sat, Sep 15, 2018 at 1:27 PM Jorge Esteban Quilcate Otoya <
> > quilcate.jo...@gmail.com> wrote:
> >
> > > Congratulations!!
> > >
> > > El sáb., 15 sept. 2018 a las 15:18, Dongjin Lee ()
> > > escribió:
> > >
> > > > Congratulations!
> > > >
> > > > Best,
> > > > Dongjin
> > > >
> > > > On Sat, Sep 15, 2018 at 3:00 PM Colin McCabe 
> > wrote:
> > > >
> > > > > Congratulations, Dong Lin!
> > > > >
> > > > > best,
> > > > > Colin
> > > > >
> > > > > On Wed, Aug 22, 2018, at 05:26, Satish Duggana wrote:
> > > > > > Congrats Dong Lin!
> > > > > >
> > > > > > On Wed, Aug 22, 2018 at 10:08 AM, Abhimanyu Nagrath <
> > > > > > abhimanyunagr...@gmail.com> wrote:
> > > > > >
> > > > > > > Congratulations, Dong!
> > > > > > >
> > > > > > > On Wed, Aug 22, 2018 at 6:20 AM Dhruvil Shah <
> > dhru...@confluent.io
> > > >
> > > > > wrote:
> > > > > > >
> > > > > > > > Congratulations, Dong!
> > > > > > > >
> > > > > > > > On Tue, Aug 21, 2018 at 4:38 PM Jason Gustafson <
> > > > ja...@confluent.io>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Congrats!
> > > > > > > > >
> > > > > > > > > On Tue, Aug 21, 2018 at 10:03 AM, Ray Chiang <
> > > rchi...@apache.org
> > > > >
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Congrats Dong!
> > > > > > > > > >
> > > > > > > > > > -Ray
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On 8/21/18 9:33 AM, Becket Qin wrote:
> > > > > > > > > >
> > > > > > > > > >> Congrats, Dong!
> > > > > > > > > >>
> > > > > > > > > >> On Aug 21, 2018, at 11:03 PM, Eno Thereska <
> > > > > eno.there...@gmail.com>
> > > > > > > > > >>> wrote:
> > > > > > > > > >>>
> > > > > > > > > >>> Congrats Dong!
> > > > > > > > > >>>
> > > > > > > > > >>> Eno
> > > > > > > > > >>>
> > > > > > > > > >>> On Tue, Aug 21, 2018 at 7:05 AM, Ted Yu <
> > > yuzhih...@gmail.com
> > > > >
> > > > > > > wrote:
> > > > > > > > > >>>
> > > > > > > > > >>> Congratulation Dong!
> > > > > > > > > >>>>
> > > > > > > > > >>>> On Tue, Aug 21, 2018 at 1:59 AM Viktor Somogyi-Vass <
> > > > > > > > > >>>> viktorsomo...@gmail.com>
> > > > > > > > > >>>> wrote:
> > > > > > > > > >>>>
> > > > > > > > > >>>> Congrats Dong! :)
> > > > > > > > > >>>>>
> > > > > > > > > >>>>> On Tue, Aug 21, 2018 at 10:09 AM James Cheng <
> > > > > > > wushuja...@gmail.com
> > > > > > > > >
> > > > > > > > > >>>>>
> > > > > > > > > >>>> wrote:
> > > > > > > > > >>>>
> > > > > > > > > >>>>> Congrats Dong!
> > > > > > > > > >>>>>>
> > > > > > > > > >>>>>> -James
> > > > > > > > > >>>>>>
> > > > > > > > > >>>>>> On Aug 20, 2018, at 3:54 AM, Ismael Juma <
> > > > ism...@juma.me.uk
> > > > > >
> > > > > > > > wrote:
> > > > > > > > > >>>>>>>
> > > > > > > > > >>>>>>> Hi everyone,
> > > > > > > > > >>>>>>>
> > > > > > > > > >>>>>>> Dong Lin became a committer in March 2018. Since
> > then,
> > > he
> > > > > has
> > > > > > > > > >>>>>>>
> > > > > > > > > >>>>>> remained
> > > > > > > > > >>>>
> > > > > > > > > >>>>> active in the community and contributed a number of
> > > > patches,
> > > > > > > > reviewed
> > > > > > > > > >>>>>>> several pull requests and participated in numerous
> > KIP
> > > > > > > > > discussions. I
> > > > > > > > > >>>>>>>
> > > > > > > > > >>>>>> am
> > > > > > > > > >>>>>
> > > > > > > > > >>>>>> happy to announce that Dong is now a member of the
> > > > > > > > > >>>>>>> Apache Kafka PM
> > > > > > > > > >>>>>>>
> > > > > > > > > >>>>>>> Congratulation Dong! Looking forward to your future
> > > > > > > > contributions.
> > > > > > > > > >>>>>>>
> > > > > > > > > >>>>>>> Ismael, on behalf of the Apache Kafka PMC
> > > > > > > > > >>>>>>>
> > > > > > > > > >>>>>>
> > > > > > > > > >>>>>>
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > *Dongjin Lee*
> > > >
> > > > *A hitchhiker in the mathematical world.*
> > > >
> > > > *github:  <http://goog_969573159/>github.com/dongjinleekr
> > > > <http://github.com/dongjinleekr>linkedin:
> > > kr.linkedin.com/in/dongjinleekr
> > > > <http://kr.linkedin.com/in/dongjinleekr>slideshare:
> > > > www.slideshare.net/dongjinleekr
> > > > <http://www.slideshare.net/dongjinleekr>*
> > > >
> > >
> >
>

Re: [DISCUSS]: KIP-339: Create a new ModifyConfigs API

[Viktor Somogyi-Vass]

I think so, I'm +1 on this.

On Sat, Sep 15, 2018 at 8:14 AM Colin McCabe  wrote:

> On Wed, Aug 15, 2018, at 05:04, Viktor Somogyi wrote:
> > Hi,
> >
> > To weigh-in, I agree with Colin on the API naming, overloads shouldn't
> > change behavior. I think all of the Java APIs I've used so far followed
> > this principle and I think we shouldn't diverge.
> >
> > Also I think I have an entry about this incremental thing in KIP-248. It
> > died off a bit at voting (I guess 2.0 came quick) but I was about to
> revive
> > and restructure it a bit. If you remember it would have done something
> > similar. Back then we discussed an "incremental_update" flag would have
> > been sufficient to keep backward compatibility with the protocol. Since
> > here you designed a new protocol I think I'll remove this bit from my KIP
> > and also align the other parts/namings to yours so we'll have a more
> > unified interface on this front.
> >
> > At last, one minor comment: is throttling a part of your protocol
> similarly
> > to alterConfigs?
>
> It should cover the same ground as alterConfigs, basically.
>
> Does it make sense to start a VOTE thread on this?
>
> best,
> Colin
>
> >
> > Viktor
> >
> >
> > On Fri, Jul 20, 2018 at 8:05 PM Colin McCabe  wrote:
> >
> > > I updated the KIP.
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-339%3A+Create+a+new+IncrementalAlterConfigs+API
> > >
> > > Updates:
> > > * Use "incrementalAlterConfigs" rather than "modifyConfigs," for
> > > consistency with the other "alter" APIs.
> > > * Implement Magnus' idea of supporting "append" and "subtract" on
> > > configuration keys that contain lists.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Mon, Jul 16, 2018, at 14:12, Colin McCabe wrote:
> > > > Hi Magnus,
> > > >
> > > > Thanks for taking a look.
> > > >
> > > > On Mon, Jul 16, 2018, at 11:43, Magnus Edenhill wrote:
> > > > > Thanks for driving this KIP, Colin.
> > > > >
> > > > > I agree with Dong that a new similar modifyConfigs API (and
> protocol
> > > API)
> > > > > is confusing and that
> > > > > we should try to extend the current alterConfigs interface to
> support
> > > the
> > > > > incremental mode instead,
> > > > > deprecating the non-incremental mode in the process.
> > > >
> > > > In the longer term, I think that the non-incremental mode should
> > > > definitely go away, and not be an option at all.  That's why I don't
> > > > think of this KIP as "adding more options  to AlterConfigs" but as
> > > > getting rid of a broken API.  I've described a lot of reasons why
> non-
> > > > incremental mode is broken.  I've also described why the brokenness
> is
> > > > subtle and an easy trap for newbies to fall into.  Hopefully everyone
> > > > agrees that getting rid of non-incremental mode completely should be
> the
> > > > eventual goal.
> > > >
> > > > I do not think that having a different name for modifyConfigs is
> > > > confusing.  "Deleting all the configs and then setting some
> designated
> > > > ones" is a very different operation from "modifying some
> > > > configurations".  Giving the two operations different names expresses
> > > > the fact  that they really are very different.  Would it be less
> > > > confusing if the new function were called alterConfigsIncremental
> rather
> > > > than modifyConfigs?
> > > >
> > > > I think it's important to have a new name for the new function.  If
> the
> > > > names are the same, how can we even explain to users which API they
> > > > should or should not use?  "Use the three argument overload, or the
> two
> > > > argument overload where the Options class is not the final parameter"
> > > > That is not user-friendly.
> > > >
> > > > You could say that some of the overloads would be deprecated.
> However,
> > > > my experience as a Hadoop developer is that most users simply don't
> care
> > > > about deprecation warnings.  They will use autocomplete in their IDEs
> > > > and use whatever function seems to have the parameters they need.
> > > > Hadoop and Kafka themselves use plenty of deprecated APIs.  But
> somehow
> > > > we expect that our users have much more respect for @deprecated than
> we
> > > > ourselves do.
> > > >
> > > > I would further argue that function overloads in Java are intended to
> > > > provide additional parameters, not to fundamentally change the
> semantics
> > > > of a function.  If you have two functions int addTwoNumbers(int a,
> int
> > > > b) and int addTwoNumbers(int a, int b, boolean verbose), they should
> > > > both add together two numbers.  And a user should be able to expect
> that
> > > > the plain old addTwoNumbers is equivalent to either
> > > > addTwoNumbers(verbose=true) or addTwoNumbers(verbose=false), not a
> > > > totally different operation.
> > > >
> > > > Every time programmers violate this contract, it inevitably leads to
> > > > misunderstanding.  One example is how in HDFS there are multiple
> > > > function overloads for renaming a file.  Depending on 

[DISCUSS] KIP-375: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi All,

I wrote up a relatively simple KIP about improving the Kafka protocol and
the TopicCommand tool to support the new Java based AdminClient and
hopefully to deprecate the Zookeeper side of it.

I would be happy to receive some opinions about this. In general I think
this would be an important addition as this is one of the few left but
important tools that still uses direct Zookeeper connection.

Here is the link for the KIP:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-375%3A+TopicCommand+to+use+AdminClient

Thanks,
Viktor

Re: [DISCUSS] KIP-375: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi Manikumar,

Thanks for raising this to my attention. I must have missed this in the
AdminClient, so you're right, we don't need this new protocol at all. I've
moved it to the rejected alternatives section.

Thanks,
Viktor

On Wed, Sep 26, 2018 at 8:24 AM Manikumar  wrote:

> Hi Viktor,
>
> We already have API (AdminClient.createPartitions,
> CreatePartitionsRequest/Response ) to increase the number of partitions of
> the topics.
> So, we may not need the protocol changes proposed in the KIP.  Let me know,
> If I am missing anything.
>
> Thanks,
>
>
> On Tue, Sep 25, 2018 at 2:45 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> wrote:
>
> > Hi Eno,
> >
> > Thanks for the question. Basically KAFKA-5561 aims for a bigger task: not
> > to add functionality to the existing TopicCommand but rewrite it in the
> > tools module in Java. KIP-375 only aims for extending the existing
> > TopicCommand's capability, so it is a smaller and backward compatible
> step
> > in the direction of having the tools communicating via the Kafka
> protocol.
> > Other commands, such as ConfigCommand (partly), ConsumerGroupCommand,
> > LogDirsCommand, DelegationTokenCommand also moved this direction, so I
> > think it makes sense to do it for TopicCommand as well.
> >
> > Cheers,
> > Viktor
> >
> > On Tue, Sep 25, 2018 at 10:48 AM Eno Thereska 
> > wrote:
> >
> > > This would be very useful.
> > > Could you clarify a bit the difference to
> > > https://issues.apache.org/jira/browse/KAFKA-5561 since I didn't get it
> > > from
> > > the JIRA notes. It's fine if you pick up that work but wanted to make
> > sure
> > > we're not duplicating efforts.
> > >
> > > Thanks
> > > Eno
> > >
> > > On Mon, Sep 24, 2018 at 8:26 PM, Viktor Somogyi-Vass <
> > > viktorsomo...@gmail.com> wrote:
> > >
> > > > Hi Gwen,
> > > >
> > > > Thanks for your feedback. It is the latter, so passing extra
> connection
> > > > properties for the admin client. I'll try to make that clearer in the
> > > KIP.
> > > > The same option name is used in the ConfigCommand, so that's why I
> > named
> > > it
> > > > "command-config".
> > > >
> > > > Cheers,
> > > > Viktor
> > > >
> > > >
> > > > On Mon, Sep 24, 2018 at 8:18 PM Gwen Shapira 
> > wrote:
> > > >
> > > > > The "use admin client" part is amazing and thank you.
> > > > >
> > > > > I'm confused about "commandConfig" - is this a list of
> configurations
> > > for
> > > > > use with --config option? Or a list of properties for connecting to
> > > > brokers
> > > > > (like SSL and such)? If the former, it seems unrelated.
> > > > >
> > > > > On Mon, Sep 24, 2018 at 7:25 AM Viktor Somogyi-Vass <
> > > > > viktorsomo...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi All,
> > > > > >
> > > > > > I wrote up a relatively simple KIP about improving the Kafka
> > protocol
> > > > and
> > > > > > the TopicCommand tool to support the new Java based AdminClient
> and
> > > > > > hopefully to deprecate the Zookeeper side of it.
> > > > > >
> > > > > > I would be happy to receive some opinions about this. In general
> I
> > > > think
> > > > > > this would be an important addition as this is one of the few
> left
> > > but
> > > > > > important tools that still uses direct Zookeeper connection.
> > > > > >
> > > > > > Here is the link for the KIP:
> > > > > >
> > > > > >
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > 375%3A+TopicCommand+to+use+AdminClient
> > > > > >
> > > > > > Thanks,
> > > > > > Viktor
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > *Gwen Shapira*
> > > > > Product Manager | Confluent
> > > > > 650.450.2760 | @gwenshap
> > > > > Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
> > > > > <http://www.confluent.io/blog>
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] KIP-375 Kafka Clients - make Metadata#TOPIC_EXPIRY_MS configurable

[Viktor Somogyi-Vass]

Hi Pavel,

May I kindly ask you to increment your KIP number? :)
As it turns out I have already created one with this number a little bit
earlier. Apologies for the inconvenience.

Viktor

On Wed, Sep 26, 2018 at 4:48 PM Pavel Moukhataev 
wrote:

>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-375%3A+Kafka+Clients+-+make+Metadata%23TOPIC_EXPIRY_MS+configurable
>
> I'd like to introduce new feature for kafka client:
> Making org.apache.kafka.clients.Metadata#TOPIC_EXPIRY_MS configurable
>
> The problem is: if application sends records to some topic rarely then
> topic metadata gets expired and sending thread is blocked by waiting topic
> metadata.
>
> Easy fix of this exact problem is to make Metadata#TOPIC_EXPIRY_MS
> configurable and set it value higher than record sending period.
>
> --
> Pavel
> +7-903-258-5544
> skype://pavel.moukhataev
>

Re: [DISCUSS] KIP-375: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi Eno,

Thanks for the question. Basically KAFKA-5561 aims for a bigger task: not
to add functionality to the existing TopicCommand but rewrite it in the
tools module in Java. KIP-375 only aims for extending the existing
TopicCommand's capability, so it is a smaller and backward compatible step
in the direction of having the tools communicating via the Kafka protocol.
Other commands, such as ConfigCommand (partly), ConsumerGroupCommand,
LogDirsCommand, DelegationTokenCommand also moved this direction, so I
think it makes sense to do it for TopicCommand as well.

Cheers,
Viktor

On Tue, Sep 25, 2018 at 10:48 AM Eno Thereska 
wrote:

> This would be very useful.
> Could you clarify a bit the difference to
> https://issues.apache.org/jira/browse/KAFKA-5561 since I didn't get it
> from
> the JIRA notes. It's fine if you pick up that work but wanted to make sure
> we're not duplicating efforts.
>
> Thanks
> Eno
>
> On Mon, Sep 24, 2018 at 8:26 PM, Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
> > Hi Gwen,
> >
> > Thanks for your feedback. It is the latter, so passing extra connection
> > properties for the admin client. I'll try to make that clearer in the
> KIP.
> > The same option name is used in the ConfigCommand, so that's why I named
> it
> > "command-config".
> >
> > Cheers,
> > Viktor
> >
> >
> > On Mon, Sep 24, 2018 at 8:18 PM Gwen Shapira  wrote:
> >
> > > The "use admin client" part is amazing and thank you.
> > >
> > > I'm confused about "commandConfig" - is this a list of configurations
> for
> > > use with --config option? Or a list of properties for connecting to
> > brokers
> > > (like SSL and such)? If the former, it seems unrelated.
> > >
> > > On Mon, Sep 24, 2018 at 7:25 AM Viktor Somogyi-Vass <
> > > viktorsomo...@gmail.com>
> > > wrote:
> > >
> > > > Hi All,
> > > >
> > > > I wrote up a relatively simple KIP about improving the Kafka protocol
> > and
> > > > the TopicCommand tool to support the new Java based AdminClient and
> > > > hopefully to deprecate the Zookeeper side of it.
> > > >
> > > > I would be happy to receive some opinions about this. In general I
> > think
> > > > this would be an important addition as this is one of the few left
> but
> > > > important tools that still uses direct Zookeeper connection.
> > > >
> > > > Here is the link for the KIP:
> > > >
> > > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 375%3A+TopicCommand+to+use+AdminClient
> > > >
> > > > Thanks,
> > > > Viktor
> > > >
> > >
> > >
> > > --
> > > *Gwen Shapira*
> > > Product Manager | Confluent
> > > 650.450.2760 | @gwenshap
> > > Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
> > > <http://www.confluent.io/blog>
> > >
> >
>

Re: [ANNOUNCE] New committer: Colin McCabe

[Viktor Somogyi-Vass]

Congrats Colin, well deserved! :)

On Tue, Sep 25, 2018 at 10:58 AM Stanislav Kozlovski 
wrote:

> Congrats Colin!
>
> On Tue, Sep 25, 2018 at 9:51 AM Edoardo Comar  wrote:
>
> > Congratulations Colin !
> > --
> >
> > Edoardo Comar
> >
> > IBM Event Streams
> > IBM UK Ltd, Hursley Park, SO21 2JN
> >
> >
> >
> >
> > From:   Ismael Juma 
> > To: Kafka Users , dev 
> > Date:   25/09/2018 09:40
> > Subject:[ANNOUNCE] New committer: Colin McCabe
> >
> >
> >
> > Hi all,
> >
> > The PMC for Apache Kafka has invited Colin McCabe as a committer and we
> > are
> > pleased to announce that he has accepted!
> >
> > Colin has contributed 101 commits and 8 KIPs including significant
> > improvements to replication, clients, code quality and testing. A few
> > highlights were KIP-97 (Improved Clients Compatibility Policy), KIP-117
> > (AdminClient), KIP-227 (Incremental FetchRequests to Increase Partition
> > Scalability), the introduction of findBugs and adding Trogdor (fault
> > injection and benchmarking tool).
> >
> > In addition, Colin has reviewed 38 pull requests and participated in more
> > than 50 KIP discussions.
> >
> > Thank you for your contributions Colin! Looking forward to many more. :)
> >
> > Ismael, for the Apache Kafka PMC
> >
> >
> >
> > Unless stated otherwise above:
> > IBM United Kingdom Limited - Registered in England and Wales with number
> > 741598.
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
> 3AU
> >
>
>
> --
> Best,
> Stanislav
>

Re: [DISCUSS] KIP-375: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi All,

A technical update: around the time I created this KIP, another one got
created with the same number and therefore to avoid collisions in KIP
numbers I chose to increment my KIP's number, so the new URL where it is
available is:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-377%3A+TopicCommand+to+use+AdminClient

I will also start a new discussion thread (and link this one), so please
post comments there. Apologies for any confusion.

Cheers,
Viktor

On Wed, Sep 26, 2018 at 12:26 PM Viktor Somogyi-Vass <
viktorsomo...@gmail.com> wrote:

> Hi Manikumar,
>
> Thanks for raising this to my attention. I must have missed this in the
> AdminClient, so you're right, we don't need this new protocol at all. I've
> moved it to the rejected alternatives section.
>
> Thanks,
> Viktor
>
> On Wed, Sep 26, 2018 at 8:24 AM Manikumar 
> wrote:
>
>> Hi Viktor,
>>
>> We already have API (AdminClient.createPartitions,
>> CreatePartitionsRequest/Response ) to increase the number of partitions of
>> the topics.
>> So, we may not need the protocol changes proposed in the KIP.  Let me
>> know,
>> If I am missing anything.
>>
>> Thanks,
>>
>>
>> On Tue, Sep 25, 2018 at 2:45 PM Viktor Somogyi-Vass <
>> viktorsomo...@gmail.com>
>> wrote:
>>
>> > Hi Eno,
>> >
>> > Thanks for the question. Basically KAFKA-5561 aims for a bigger task:
>> not
>> > to add functionality to the existing TopicCommand but rewrite it in the
>> > tools module in Java. KIP-375 only aims for extending the existing
>> > TopicCommand's capability, so it is a smaller and backward compatible
>> step
>> > in the direction of having the tools communicating via the Kafka
>> protocol.
>> > Other commands, such as ConfigCommand (partly), ConsumerGroupCommand,
>> > LogDirsCommand, DelegationTokenCommand also moved this direction, so I
>> > think it makes sense to do it for TopicCommand as well.
>> >
>> > Cheers,
>> > Viktor
>> >
>> > On Tue, Sep 25, 2018 at 10:48 AM Eno Thereska 
>> > wrote:
>> >
>> > > This would be very useful.
>> > > Could you clarify a bit the difference to
>> > > https://issues.apache.org/jira/browse/KAFKA-5561 since I didn't get
>> it
>> > > from
>> > > the JIRA notes. It's fine if you pick up that work but wanted to make
>> > sure
>> > > we're not duplicating efforts.
>> > >
>> > > Thanks
>> > > Eno
>> > >
>> > > On Mon, Sep 24, 2018 at 8:26 PM, Viktor Somogyi-Vass <
>> > > viktorsomo...@gmail.com> wrote:
>> > >
>> > > > Hi Gwen,
>> > > >
>> > > > Thanks for your feedback. It is the latter, so passing extra
>> connection
>> > > > properties for the admin client. I'll try to make that clearer in
>> the
>> > > KIP.
>> > > > The same option name is used in the ConfigCommand, so that's why I
>> > named
>> > > it
>> > > > "command-config".
>> > > >
>> > > > Cheers,
>> > > > Viktor
>> > > >
>> > > >
>> > > > On Mon, Sep 24, 2018 at 8:18 PM Gwen Shapira 
>> > wrote:
>> > > >
>> > > > > The "use admin client" part is amazing and thank you.
>> > > > >
>> > > > > I'm confused about "commandConfig" - is this a list of
>> configurations
>> > > for
>> > > > > use with --config option? Or a list of properties for connecting
>> to
>> > > > brokers
>> > > > > (like SSL and such)? If the former, it seems unrelated.
>> > > > >
>> > > > > On Mon, Sep 24, 2018 at 7:25 AM Viktor Somogyi-Vass <
>> > > > > viktorsomo...@gmail.com>
>> > > > > wrote:
>> > > > >
>> > > > > > Hi All,
>> > > > > >
>> > > > > > I wrote up a relatively simple KIP about improving the Kafka
>> > protocol
>> > > > and
>> > > > > > the TopicCommand tool to support the new Java based AdminClient
>> and
>> > > > > > hopefully to deprecate the Zookeeper side of it.
>> > > > > >
>> > > > > > I would be happy to receive some opinions about this. In
>> general I
>> > > > think
>> > > > > > this would be an important addition as this is one of the few
>> left
>> > > but
>> > > > > > important tools that still uses direct Zookeeper connection.
>> > > > > >
>> > > > > > Here is the link for the KIP:
>> > > > > >
>> > > > > >
>> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > > > 375%3A+TopicCommand+to+use+AdminClient
>> > > > > >
>> > > > > > Thanks,
>> > > > > > Viktor
>> > > > > >
>> > > > >
>> > > > >
>> > > > > --
>> > > > > *Gwen Shapira*
>> > > > > Product Manager | Confluent
>> > > > > 650.450.2760 | @gwenshap
>> > > > > Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
>> > > > > <http://www.confluent.io/blog>
>> > > > >
>> > > >
>> > >
>> >
>>
>

Re: [DISCUSS] KIP-375 Kafka Clients - make Metadata#TOPIC_EXPIRY_MS configurable

[Viktor Somogyi-Vass]

Hi Pavel,

I have changed my KIP's number to 377 so all sorted out.

Cheers,
Viktor

On Wed, Sep 26, 2018 at 4:53 PM Viktor Somogyi-Vass 
wrote:

> Hi Pavel,
>
> May I kindly ask you to increment your KIP number? :)
> As it turns out I have already created one with this number a little bit
> earlier. Apologies for the inconvenience.
>
> Viktor
>
> On Wed, Sep 26, 2018 at 4:48 PM Pavel Moukhataev 
> wrote:
>
>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-375%3A+Kafka+Clients+-+make+Metadata%23TOPIC_EXPIRY_MS+configurable
>>
>> I'd like to introduce new feature for kafka client:
>> Making org.apache.kafka.clients.Metadata#TOPIC_EXPIRY_MS configurable
>>
>> The problem is: if application sends records to some topic rarely then
>> topic metadata gets expired and sending thread is blocked by waiting topic
>> metadata.
>>
>> Easy fix of this exact problem is to make Metadata#TOPIC_EXPIRY_MS
>> configurable and set it value higher than record sending period.
>>
>> --
>> Pavel
>> +7-903-258-5544
>> skype://pavel.moukhataev
>>
>

[DISCUSS] KIP-377: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi All,

This is the continuation of the old KIP-375 with the same title:
https://lists.apache.org/thread.html/dc71d08de8cd2f082765be22c9f88bc9f8b39bb8e0929a3a4394e9da@%3Cdev.kafka.apache.org%3E

The problem there was that two KIPs were created around the same time and I
chose to reorganize mine a bit and give it a new number to avoid
duplication.

The KIP summary here once again:

I wrote up a relatively simple KIP about improving the Kafka protocol and
the TopicCommand tool to support the new Java based AdminClient and
hopefully to deprecate the Zookeeper side of it.

I would be happy to receive some opinions about this. In general I think
this would be an important addition as this is one of the few left but
important tools that still uses direct Zookeeper connection.

Here is the link for the KIP:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-377%3A+TopicCommand+to+use+AdminClient

Cheers,
Viktor

Re: [DISCUSS] KIP-375: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi Gwen,

Thanks for your feedback. It is the latter, so passing extra connection
properties for the admin client. I'll try to make that clearer in the KIP.
The same option name is used in the ConfigCommand, so that's why I named it
"command-config".

Cheers,
Viktor


On Mon, Sep 24, 2018 at 8:18 PM Gwen Shapira  wrote:

> The "use admin client" part is amazing and thank you.
>
> I'm confused about "commandConfig" - is this a list of configurations for
> use with --config option? Or a list of properties for connecting to brokers
> (like SSL and such)? If the former, it seems unrelated.
>
> On Mon, Sep 24, 2018 at 7:25 AM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> wrote:
>
> > Hi All,
> >
> > I wrote up a relatively simple KIP about improving the Kafka protocol and
> > the TopicCommand tool to support the new Java based AdminClient and
> > hopefully to deprecate the Zookeeper side of it.
> >
> > I would be happy to receive some opinions about this. In general I think
> > this would be an important addition as this is one of the few left but
> > important tools that still uses direct Zookeeper connection.
> >
> > Here is the link for the KIP:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-375%3A+TopicCommand+to+use+AdminClient
> >
> > Thanks,
> > Viktor
> >
>
>
> --
> *Gwen Shapira*
> Product Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
> <http://www.confluent.io/blog>
>

Re: [DISCUSS] KIP-377: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi All,

I made an update to the KIP. Just in short:
Currently KafkaAdminClient.describeTopics() and
KafkaAdminClient.listTopics() uses the Metadata protocol to acquire topic
information. The returned response however won't contain the topics that
are under deletion but couldn't complete yet (for instance because of some
replicas offline), therefore it is not possible to implement the current
command's "marked for deletion" feature. To get around this I introduced
some changes in the Metadata protocol.

Thanks,
Viktor

On Fri, Sep 28, 2018 at 4:48 PM Viktor Somogyi-Vass 
wrote:

> Hi Mickael,
>
> Thanks for the feedback, I also think that many customers wanted this for
> a long time.
>
> Cheers,
> Viktor
>
> On Fri, Sep 28, 2018 at 11:45 AM Mickael Maison 
> wrote:
>
>> Hi Viktor,
>> Thanks for taking this task!
>> This is a very nice change as it will allow users to use this tool in
>> many Cloud environments where direct zookeeper access is not
>> available.
>>
>>
>> On Thu, Sep 27, 2018 at 10:34 AM Viktor Somogyi-Vass
>>  wrote:
>> >
>> > Hi All,
>> >
>> > This is the continuation of the old KIP-375 with the same title:
>> >
>> https://lists.apache.org/thread.html/dc71d08de8cd2f082765be22c9f88bc9f8b39bb8e0929a3a4394e9da@%3Cdev.kafka.apache.org%3E
>> >
>> > The problem there was that two KIPs were created around the same time
>> and I
>> > chose to reorganize mine a bit and give it a new number to avoid
>> > duplication.
>> >
>> > The KIP summary here once again:
>> >
>> > I wrote up a relatively simple KIP about improving the Kafka protocol
>> and
>> > the TopicCommand tool to support the new Java based AdminClient and
>> > hopefully to deprecate the Zookeeper side of it.
>> >
>> > I would be happy to receive some opinions about this. In general I think
>> > this would be an important addition as this is one of the few left but
>> > important tools that still uses direct Zookeeper connection.
>> >
>> > Here is the link for the KIP:
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-377%3A+TopicCommand+to+use+AdminClient
>> >
>> > Cheers,
>> > Viktor
>>
>

Re: [DISCUSS] KIP-377: TopicCommand to use AdminClient

[Viktor Somogyi-Vass]

Hi Mickael,

Thanks for the feedback, I also think that many customers wanted this for a
long time.

Cheers,
Viktor

On Fri, Sep 28, 2018 at 11:45 AM Mickael Maison 
wrote:

> Hi Viktor,
> Thanks for taking this task!
> This is a very nice change as it will allow users to use this tool in
> many Cloud environments where direct zookeeper access is not
> available.
>
>
> On Thu, Sep 27, 2018 at 10:34 AM Viktor Somogyi-Vass
>  wrote:
> >
> > Hi All,
> >
> > This is the continuation of the old KIP-375 with the same title:
> >
> https://lists.apache.org/thread.html/dc71d08de8cd2f082765be22c9f88bc9f8b39bb8e0929a3a4394e9da@%3Cdev.kafka.apache.org%3E
> >
> > The problem there was that two KIPs were created around the same time
> and I
> > chose to reorganize mine a bit and give it a new number to avoid
> > duplication.
> >
> > The KIP summary here once again:
> >
> > I wrote up a relatively simple KIP about improving the Kafka protocol and
> > the TopicCommand tool to support the new Java based AdminClient and
> > hopefully to deprecate the Zookeeper side of it.
> >
> > I would be happy to receive some opinions about this. In general I think
> > this would be an important addition as this is one of the few left but
> > important tools that still uses direct Zookeeper connection.
> >
> > Here is the link for the KIP:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-377%3A+TopicCommand+to+use+AdminClient
> >
> > Cheers,
> > Viktor
>

Re: Failing test on PR

[Viktor Somogyi-Vass]

Hi Mateusz,

Just write a comment that only says "retest this please" (without the
quotation marks).

Best,
Viktor

On Wed, Mar 27, 2019 at 1:02 PM Mateusz Zakarczemny 
wrote:

> Hi,
> I'm working on https://github.com/apache/kafka/pull/4807 PR. Last PR build
> failed in some random place. My changes are related to console consumer and
> the job failed in
>
> *kafka.api.ConsumerBounceTest.testRollingBrokerRestartsWithSmallerMaxGroupSizeConfigDisruptsBigGroup*
> I'm assuming it's some flaky test. Is there any way to rerun Jenkins PR
> build? Or I should proceed in a other way?
>
> Regards,
> Mateusz Zakarczemny
>

[jira] [Created] (KAFKA-8164) Improve test passing rate by rerunning flaky tests

[Viktor Somogyi-Vass (JIRA)]

Viktor Somogyi-Vass created KAFKA-8164:
--

 Summary: Improve test passing rate by rerunning flaky tests
 Key: KAFKA-8164
 URL: https://issues.apache.org/jira/browse/KAFKA-8164
 Project: Kafka
  Issue Type: Improvement
Affects Versions: 2.3.0
Reporter: Viktor Somogyi-Vass
Assignee: Viktor Somogyi-Vass


Failing flaky tests are often a problem:
* pull requests need to be rerun to achieve a green build
* rerunning the whole build for a PR is resource and time consuming
* green build give high confidence when releasing (and well, generally too) but 
flaky tests often erode this and annoying for the developers

The aim of this JIRA is to provide an improvement which automatically retries 
any tests that are failed for the first run.



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

Re: Speeding up integration tests

[Viktor Somogyi-Vass]

Hi All,

I've created a PR for what we have internally for retrying flaky tests. Any
reviews and ideas are welcome: https://github.com/apache/kafka/pull/6506
It's basically collects the failed classes and reruns them at the end. If
they successful it overwrites the test report.

Thanks,
Viktor

On Mon, Mar 11, 2019 at 1:05 PM Stanislav Kozlovski 
wrote:

> I agree with Ron.
> I think improving the framework with a configurable number of retries on
> some tests will yield the highest ROI in terms of passing builds.
>
> On Fri, Mar 8, 2019 at 10:48 PM Ron Dagostino  wrote:
>
> > It's a classic problem: you can't string N things together serially and
> > expect high reliability.  5,000 tests in a row isn't going to give you a
> > bunch of 9's.  It feels to me that the test frameworks themselves should
> > support a more robust model -- like a way to tag a test as "retry me up
> to
> > N times before you really consider me a failure" or something like that.
> >
> > Ron
> >
> > On Fri, Mar 8, 2019 at 11:40 AM Stanislav Kozlovski <
> > stanis...@confluent.io>
> > wrote:
> >
> > > > We internally have an improvement for a half a year now which reruns
> > the
> > > flaky test classes at the end of the test gradle task, lets you know
> that
> > > they were rerun and probably flaky. It fails the build only if the
> second
> > > run of the test class was also unsuccessful. I think it works pretty
> > good,
> > > we mostly have green builds. If there is interest, I can try to
> > contribute
> > > that.
> > >
> > > That does sound very intriguing. Does it rerun the test classes that
> > failed
> > > or some known, marked classes? If it is the former, I can see a lot of
> > > value in having that automated in our PR builds. I wonder what others
> > think
> > > of this
> > >
> > > On Thu, Feb 28, 2019 at 6:04 PM Viktor Somogyi-Vass <
> > > viktorsomo...@gmail.com>
> > > wrote:
> > >
> > > > Hey All,
> > > >
> > > > Thanks for the loads of ideas.
> > > >
> > > > @Stanislav, @Sonke
> > > > I probably left it out from my email but I really imagined this as a
> > > > case-by-case basis change. If we think that it wouldn't cause
> problems,
> > > > then it might be applied. That way we'd limit the blast radius
> > somewhat.
> > > > The 1 hour gain is really just the most optimistic scenario, I'm
> almost
> > > > sure that not every test could be transformed to use a common
> cluster.
> > > > We internally have an improvement for a half a year now which reruns
> > the
> > > > flaky test classes at the end of the test gradle task, lets you know
> > that
> > > > they were rerun and probably flaky. It fails the build only if the
> > second
> > > > run of the test class was also unsuccessful. I think it works pretty
> > > good,
> > > > we mostly have green builds. If there is interest, I can try to
> > > contribute
> > > > that.
> > > >
> > > > >I am also extremely annoyed at times by the amount of coffee I have
> to
> > > > drink before tests finish
> > > > Just please don't get a heart attack :)
> > > >
> > > > @Ron, @Colin
> > > > You bring up a very good point that it is easier and frees up more
> > > > resources if we just run change specific tests and it's good to know
> > > that a
> > > > similar solution (meaning using a shared resource for testing) have
> > > failed
> > > > elsewhere. I second Ron on the test categorization though, although
> as
> > a
> > > > first attempt I think using a flaky retry + running only the
> necessary
> > > > tests would help in both time saving and effectiveness. Also it would
> > be
> > > > easier to achieve.
> > > >
> > > > @Ismael
> > > > Yea, it'd be interesting to profile the startup/shutdown, I've never
> > done
> > > > that. Perhaps I'll set some time apart for that :). It's definitely
> > true
> > > > though that if we see a significant delay there we wouldn't just
> > improve
> > > > the efficiency of the tests but also customer experience.
> > > >
> > > > Best,
> > > > Viktor
> > > >
> > > >
> > > >
> > > > On Thu, Feb 28, 2019 at 8:12 AM Ismael Juma 
> wrote:
> > > >

Re: [VOTE] KIP-392: Allow consumers to fetch from the closest replica

[Viktor Somogyi-Vass]

+1 (non-binding) very good proposal.

On Mon, Mar 25, 2019 at 7:19 PM David Arthur  wrote:

> +1
>
> Thanks, Jason!
>
> On Mon, Mar 25, 2019 at 1:23 PM Eno Thereska 
> wrote:
>
> > +1 (non-binding)
> > Thanks for updating the KIP and addressing my previous comments.
> >
> > Eno
> >
> > On Mon, Mar 25, 2019 at 4:35 PM Ryanne Dolan 
> > wrote:
> >
> > > +1 (non-binding)
> > >
> > > Great stuff, thanks.
> > >
> > > Ryanne
> > >
> > > On Mon, Mar 25, 2019, 11:08 AM Jason Gustafson 
> > wrote:
> > >
> > > > Hi All, discussion on the KIP seems to have died down, so I'd like to
> > go
> > > > ahead and start a vote. Here is a link to the KIP:
> > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-392%3A+Allow+consumers+to+fetch+from+closest+replica
> > > > .
> > > >
> > > > +1 from me (duh)
> > > >
> > > > -Jason
> > > >
> > >
> >
>
>
> --
> David Arthur
>

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

[Viktor Somogyi-Vass]

ocal disk until the segment
> is copied and any requests to the data in these files will be served from
> local disk. So I don't think we need to be aggressive and optimize the this
> copy segment to remote path.
>
>
>
> Hi Viktor,
>  Thanks for the comments.
>
> "I have a rather technical question to this. How do you plan to package
> this
> extension? Does this mean that Kafka will depend on HDFS?
> I think it'd be nice to somehow separate this off to a different package in
> the project so that it could be built and released separately from the main
> Kafka packages."
>
> We would like all of this code to be part of Apache Kafka . In early days
> of Kafka, there is external module which used to contain kafka to hdfs copy
> tools and dependencies.  We would like to have RLM (class implementation)
> and RSM(interface) to be in core and as you suggested, implementation of
> RSM could be in another package so that the dependencies of RSM won't come
> into Kafka's classpath unless someone explicity configures them.
>
>
> Thanks,
> Harsha
>
>
>
>
>
>
>
>
> On Mon, Apr 1, 2019, at 1:02 AM, Viktor Somogyi-Vass wrote:
> > Hey Harsha,
> >
> > I have a rather technical question to this. How do you plan to package
> this
> > extension? Does this mean that Kafka will depend on HDFS?
> > I think it'd be nice to somehow separate this off to a different package
> in
> > the project so that it could be built and released separately from the
> main
> > Kafka packages.
> > This decoupling would be useful when direct dependency on HDFS (or other
> > implementations) is not needed and would also encourage decoupling for
> > other storage implementations.
> >
> > Best,
> > Viktor
> >
> > On Mon, Apr 1, 2019 at 3:44 AM Ambud Sharma 
> wrote:
> >
> > > Hi Harsha,
> > >
> > > Thank you for proposing this KIP. We are looking forward to this
> feature as
> > > well.
> > >
> > > A few questions around the design & implementation:
> > >
> > > 1. Wouldn't implicit checking for old offsets in remote location if not
> > > found locally on the leader i.e. do we really need remote index files?
> > > Since the storage path for a given topic would presumably be constant
> > > across all the brokers, the remote topic-partition path could simply be
> > > checked to see if there are any segment file names that would meet the
> > > offset requirements for a Consumer Fetch Request. RSM implementations
> could
> > > optionally cache this information.
> > >
> > > 2. Would it make sense to create an internal compacted Kafka topic to
> > > publish & record remote segment information? This would enable the
> > > followers to get updates about new segments rather than running list()
> > > operations on remote storage to detect new segments which may be
> expensive.
> > >
> > > 3. For RLM to scan local segment rotations are you thinking of
> leveraging
> > > java.nio.file.WatchService or simply running listFiles() on a periodic
> > > basis? Since WatchService implementation is heavily OS dependent it
> might
> > > create some complications around missing FS Events.
> > >
> > > Thanks.
> > > Ambud
> > >
> > > On Thu, Mar 28, 2019 at 8:04 AM Ron Dagostino 
> wrote:
> > >
> > > > Hi Harsha.  I'm excited about this potential feature.  Did you
> consider
> > > > storing the information about the remote segments in a Kafka topic as
> > > > opposed to in the remote storage itself?  The topic would need
> infinite
> > > > retention (or it would need to be compacted) so as not to itself be
> sent
> > > to
> > > > cold storage, but assuming that topic would fit on local disk for all
> > > time
> > > > (an open question as to whether this is acceptable or not) it feels
> like
> > > > the most natural way to communicate information among brokers -- more
> > > > natural than having them poll the remote storage systems, at least.
> > > >
> > > > To add to Eric's question/confusion about where logic lives (RLM vs.
> > > RSM),
> > > > I think it would be helpful to explicitly identify in the KIP that
> the
> > > RLM
> > > > delegates to the RSM since the RSM is part of the public API and is
> the
> > > > pluggable piece.  For example, instead of saying "RLM will ship the
> log
> > > > segment files that are older than a conf

Re: [DISCUSS] KIP-434: Add Replica Fetcher and Log Cleaner Count Metrics

[Viktor Somogyi-Vass]

Hey Folks,

Since there were no discussions on this in the past two weeks I'll create a
VOTE thread soon.

Thanks,
Viktor

On Thu, Mar 14, 2019 at 7:05 PM Viktor Somogyi-Vass 
wrote:

> Hey Stanislav,
>
> Sorry for the delay on this. In the meantime I realized that the dead
> fetchers won't be removed from the fetcher map, so it's very easy to figure
> out how many dead and alive there are. I can collect them on broker level
> which I think gives a good enough information if there is a problem with a
> given broker. You do raise a good point with your idea that it's helpful to
> know which fetcher is acting up, although I find that solution less
> feasible due to the number of metrics generated. For this I'd use a JMX
> method that'd return a list of problematic fetcher threads but I'm not sure
> if we have to extend the scope of this KIP that much.
>
> Best,
> Viktor
>
> On Mon, Mar 4, 2019 at 7:22 PM Stanislav Kozlovski 
> wrote:
>
>> Hey Viktor,
>>
>> > however displaying the thread count (be it alive or dead) would still
>> add
>> extra information regarding the failure, that a thread died during
>> cleanup.
>>
>> I agree, I think it's worth adding.
>>
>>
>> > Doing this on the replica fetchers though would be a bit harder as the
>> number of replica fetchers is the (brokers-to-fetch-from *
>> fetchers-per-broker) and we don't really maintain the capacity information
>> or any kind of cluster information and I'm not sure we should.
>>
>> Perhaps we could split the metric per broker that is being fetched from?
>> i.e each replica fetcher would have a `dead-fetcher-threads` metric that
>> has the broker-id it's fetching from as a tag?
>> It would solve an observability question which I think is very important -
>> are we replicating from this broker at all?
>> On the other hand, this could potentially produce a lot of metric data
>> with
>> a big cluster, so that is definitely something to consider as well.
>>
>> All in all, I think this is a great KIP and very much needed in my
>> opinion.
>> I can't wait to see this roll out
>>
>> Best,
>> Stanislav
>>
>> On Mon, Feb 25, 2019 at 10:29 AM Viktor Somogyi-Vass <
>> viktorsomo...@gmail.com> wrote:
>>
>> > Hi Stanislav,
>> >
>> > Thanks for the feedback and sharing that discussion thread.
>> >
>> > I read your KIP and the discussion on it too and it seems like that'd
>> cover
>> > the same motivation I had with the log-cleaner-thread-count metric. This
>> > supposed to tell the count of the alive threads which might differ from
>> the
>> > config (I could've used a better name :) ). Now I'm thinking that using
>> > uncleanable-bytes, uncleanable-partition-count together with
>> > time-since-last-run would mostly cover the motivation I have in this
>> KIP,
>> > however displaying the thread count (be it alive or dead) would still
>> add
>> > extra information regarding the failure, that a thread died during
>> cleanup.
>> >
>> > You had a very good idea about instead of the alive threads, display the
>> > dead ones! That way we wouldn't need
>> log-cleaner-current-live-thread-rate
>> > just a "dead-log-cleaner-thread-count" as it it would make easy to
>> trigger
>> > warnings based on that (if it's even > 0 then we can say there's a
>> > potential problem).
>> > Doing this on the replica fetchers though would be a bit harder as the
>> > number of replica fetchers is the (brokers-to-fetch-from *
>> > fetchers-per-broker) and we don't really maintain the capacity
>> information
>> > or any kind of cluster information and I'm not sure we should. It would
>> add
>> > too much responsibility to the class and wouldn't be a rock-solid
>> solution
>> > but I guess I have to look into it more.
>> >
>> > I don't think that restarting the cleaner threads would be helpful as
>> the
>> > problems I've seen mostly are non-recoverable and requires manual user
>> > intervention and partly I agree what Colin said on the KIP-346
>> discussion
>> > thread about the problems experienced with HDFS.
>> >
>> > Best,
>> > Viktor
>> >
>> >
>> > On Fri, Feb 22, 2019 at 5:03 PM Stanislav Kozlovski <
>> > stanis...@confluent.io>
>> > wrote:
>> >
>> > > Hey Viktor,
>> > >
>> > > First off, thanks for the KIP! I think that it is almost always a good
>>

[VOTE] KIP-434: Dead replica fetcher and log cleaner metrics

[Viktor Somogyi-Vass]

Hi All,

I'd like to start a vote on KIP-434.
This basically would add a metrics to count dead threads in
ReplicaFetcherManager and LogCleaner to allow monitoring systems to alert
based on this.

The KIP link:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics
The
PR: https://github.com/apache/kafka/pull/6514

I'd be happy to receive any votes or additional feedback/reviews too.

Thanks,
Viktor

Re: [VOTE] KIP-434: Add Replica Fetcher and Log Cleaner Count Metrics

[Viktor Somogyi-Vass]

Thanks for the heads-up Gwen, I'll resend it with a different title.

On Thu, Mar 28, 2019 at 6:26 PM Gwen Shapira  wrote:

> Gmail automatically folded the vote into the old discussion thread. I
> suggest resending with a sufficiently different title so no one will miss
> it.
>
> On Thu, Mar 28, 2019 at 8:05 AM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> wrote:
>
> > Hi All,
> >
> > I'd like to start a vote on KIP-434.
> > This basically would add a metrics to count dead threads in
> > ReplicaFetcherManager and LogCleaner to allow monitoring systems to alert
> > based on this.
> >
> > The KIP link:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics
> > The
> > PR: https://github.com/apache/kafka/pull/6514
> >
> > I'd be happy to receive any votes or additional feedback/reviews too.
> >
> > Thanks,
> > Viktor
> >
>
>
> --
> *Gwen Shapira*
> Product Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
> <http://www.confluent.io/blog>
>

Re: [VOTE] KIP-434: Dead replica fetcher and log cleaner metrics

[Viktor Somogyi-Vass]

Hi Dhruvil,

Thanks for the feedback and the vote. I fixed the typo in the KIP.
The naming is interesting though. Unfortunately kafka overall is not
consistent in metric naming but at least I tried to be consistent among the
other metrics used in LogManager

On Thu, Mar 28, 2019 at 7:32 PM Dhruvil Shah  wrote:

> Thanks for the KIP, Viktor! This is a useful addition. +1 overall.
>
> Minor nits:
> > I propose to add three gauge: DeadFetcherThreadCount for the fetcher
> threads, log-cleaner-dead-thread-count for the log cleaner.
> I think you meant two instead of three.
>
> Also, would it make sense to name these metrics consistency, something like
> `log-cleaner-dead-thread-count` and `replica-fetcher-dead-thread-count`?
>
> Thanks,
> Dhruvil
>
> On Thu, Mar 28, 2019 at 11:27 AM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
> > Hi All,
> >
> > I'd like to start a vote on KIP-434.
> > This basically would add a metrics to count dead threads in
> > ReplicaFetcherManager and LogCleaner to allow monitoring systems to alert
> > based on this.
> >
> > The KIP link:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics
> > The
> > PR: https://github.com/apache/kafka/pull/6514
> >
> > I'd be happy to receive any votes or additional feedback/reviews too.
> >
> > Thanks,
> > Viktor
> >
>

Re: [VOTE] KIP-434: Dead replica fetcher and log cleaner metrics

[Viktor Somogyi-Vass]

Sorry, the end of the message cut off.

So I tried to be consistent with the convention in LogManager, hence the
hyphens and in AbstractFetcherManager, hence the camel case. It would be
nice though to decide with one convention across the whole project, however
it requires a major refactor (especially for the components that leverage
metrics for monitoring).

Thanks,
Viktor

On Thu, Mar 28, 2019 at 8:44 PM Viktor Somogyi-Vass 
wrote:

> Hi Dhruvil,
>
> Thanks for the feedback and the vote. I fixed the typo in the KIP.
> The naming is interesting though. Unfortunately kafka overall is not
> consistent in metric naming but at least I tried to be consistent among the
> other metrics used in LogManager
>
> On Thu, Mar 28, 2019 at 7:32 PM Dhruvil Shah  wrote:
>
>> Thanks for the KIP, Viktor! This is a useful addition. +1 overall.
>>
>> Minor nits:
>> > I propose to add three gauge: DeadFetcherThreadCount for the fetcher
>> threads, log-cleaner-dead-thread-count for the log cleaner.
>> I think you meant two instead of three.
>>
>> Also, would it make sense to name these metrics consistency, something
>> like
>> `log-cleaner-dead-thread-count` and `replica-fetcher-dead-thread-count`?
>>
>> Thanks,
>> Dhruvil
>>
>> On Thu, Mar 28, 2019 at 11:27 AM Viktor Somogyi-Vass <
>> viktorsomo...@gmail.com> wrote:
>>
>> > Hi All,
>> >
>> > I'd like to start a vote on KIP-434.
>> > This basically would add a metrics to count dead threads in
>> > ReplicaFetcherManager and LogCleaner to allow monitoring systems to
>> alert
>> > based on this.
>> >
>> > The KIP link:
>> >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics
>> > The
>> > PR: https://github.com/apache/kafka/pull/6514
>> >
>> > I'd be happy to receive any votes or additional feedback/reviews too.
>> >
>> > Thanks,
>> > Viktor
>> >
>>
>

[VOTE] KIP-434: Add Replica Fetcher and Log Cleaner Count Metrics

[Viktor Somogyi-Vass]

Hi All,

I'd like to start a vote on KIP-434.
This basically would add a metrics to count dead threads in
ReplicaFetcherManager and LogCleaner to allow monitoring systems to alert
based on this.

The KIP link:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics
The
PR: https://github.com/apache/kafka/pull/6514

I'd be happy to receive any votes or additional feedback/reviews too.

Thanks,
Viktor

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

[Viktor Somogyi-Vass]

Hey Harsha,

I have a rather technical question to this. How do you plan to package this
extension? Does this mean that Kafka will depend on HDFS?
I think it'd be nice to somehow separate this off to a different package in
the project so that it could be built and released separately from the main
Kafka packages.
This decoupling would be useful when direct dependency on HDFS (or other
implementations) is not needed and would also encourage decoupling for
other storage implementations.

Best,
Viktor

On Mon, Apr 1, 2019 at 3:44 AM Ambud Sharma  wrote:

> Hi Harsha,
>
> Thank you for proposing this KIP. We are looking forward to this feature as
> well.
>
> A few questions around the design & implementation:
>
> 1. Wouldn't implicit checking for old offsets in remote location if not
> found locally on the leader i.e. do we really need remote index files?
> Since the storage path for a given topic would presumably be constant
> across all the brokers, the remote topic-partition path could simply be
> checked to see if there are any segment file names that would meet the
> offset requirements for a Consumer Fetch Request. RSM implementations could
> optionally cache this information.
>
> 2. Would it make sense to create an internal compacted Kafka topic to
> publish & record remote segment information? This would enable the
> followers to get updates about new segments rather than running list()
> operations on remote storage to detect new segments which may be expensive.
>
> 3. For RLM to scan local segment rotations are you thinking of leveraging
> java.nio.file.WatchService or simply running listFiles() on a periodic
> basis? Since WatchService implementation is heavily OS dependent it might
> create some complications around missing FS Events.
>
> Thanks.
> Ambud
>
> On Thu, Mar 28, 2019 at 8:04 AM Ron Dagostino  wrote:
>
> > Hi Harsha.  I'm excited about this potential feature.  Did you consider
> > storing the information about the remote segments in a Kafka topic as
> > opposed to in the remote storage itself?  The topic would need infinite
> > retention (or it would need to be compacted) so as not to itself be sent
> to
> > cold storage, but assuming that topic would fit on local disk for all
> time
> > (an open question as to whether this is acceptable or not) it feels like
> > the most natural way to communicate information among brokers -- more
> > natural than having them poll the remote storage systems, at least.
> >
> > To add to Eric's question/confusion about where logic lives (RLM vs.
> RSM),
> > I think it would be helpful to explicitly identify in the KIP that the
> RLM
> > delegates to the RSM since the RSM is part of the public API and is the
> > pluggable piece.  For example, instead of saying "RLM will ship the log
> > segment files that are older than a configurable time to remote storage"
> I
> > think it would be better to say "RLM identifies log segment files that
> are
> > older than a configurable time and delegates to the configured RSM to
> ship
> > them to remote storage" (or something like that -- just make it clear
> that
> > the RLM is delegating to the configured RSM).
> >
> > Ron
> >
> >
> >
> >
> >
> > On Thu, Mar 28, 2019 at 6:12 AM Eno Thereska 
> > wrote:
> >
> > > Thanks Harsha,
> > >
> > > A couple of comments:
> > >
> > > Performance & durability
> > > --
> > > - would be good to have more discussion on performance implications of
> > > tiering. Copying the data from the local storage to the remote storage
> is
> > > going to be expensive in terms of network bandwidth and will affect
> > > foreground traffic to Kafka potentially reducing its throughput and
> > > latency.
> > > - throttling the copying of the data above might be a solution, however
> > if
> > > you have a few TB of data to move to the slower remote tier the risk is
> > > that the movement will never complete on time under high Kafka load. Do
> > we
> > > need a scheduler to use idle time to do the copying?
> > > - Have you considered having two options: 1) a slow tier only (e.g.,
> all
> > > the data on HDFS) and 2) a fast tier only like Kafka today. This would
> > > avoid copying data between the tiers. Customers that can tolerate a
> > slower
> > > tier with a better price/GB can just choose option (1). Would be good
> to
> > > put in Alternatives considered.
> > >
> > > Topic configs
> > > --
> > > - related to performance but also availability, we need to discuss the
> > > replication mode for the remote tier. For example, if the Kafka topics
> > used
> > > to have 3-way replication, will they continue to have 3-way replication
> > on
> > > the remote tier? Will the user configure that replication? In S3 for
> > > example, one can choose from different S3 tiers like STD or SIA, but
> > there
> > > is no direct control over the replication factor like in Kafka.
> > > - how will security and ACLs be configured for the remote tier. E.g.,
> if
> > > user 

Speeding up integration tests

[Viktor Somogyi-Vass]

Hi Folks,

I've been observing lately that unit tests usually take 2.5 hours to run
and a very big portion of these are the core tests where a new cluster is
spun up for every test. This takes most of the time. I ran a test
(TopicCommandWithAdminClient with 38 test inside) through the profiler and
it shows for instance that running the whole class itself took 10 minutes
and 37 seconds where the useful time was 5 minutes 18 seconds. That's a
100% overhead. Without profiler the whole class takes 7 minutes and 48
seconds, so the useful time would be between 3-4 minutes. This is a bigger
test though, most of them won't take this much.
There are 74 classes that implement KafkaServerTestHarness and just running
:core:integrationTest takes almost 2 hours.

I think we could greatly speed up these integration tests by just creating
the cluster once per class and perform the tests on separate methods. I
know that this a little bit contradicts to the principle that tests should
be independent but it seems like recreating clusters for each is a very
expensive operation. Also if the tests are acting on different resources
(different topics, etc.) then it might not hurt their independence. There
might be cases of course where this is not possible but I think there could
be a lot where it is.

In the optimal case we could cut the testing time back by approximately an
hour. This would save resources and give quicker feedback for PR builds.

What are your thoughts?
Has anyone thought about this or were there any attempts made?

Best,
Viktor

Re: Speeding up integration tests

[Viktor Somogyi-Vass]

Hey All,

Thanks for the loads of ideas.

@Stanislav, @Sonke
I probably left it out from my email but I really imagined this as a
case-by-case basis change. If we think that it wouldn't cause problems,
then it might be applied. That way we'd limit the blast radius somewhat.
The 1 hour gain is really just the most optimistic scenario, I'm almost
sure that not every test could be transformed to use a common cluster.
We internally have an improvement for a half a year now which reruns the
flaky test classes at the end of the test gradle task, lets you know that
they were rerun and probably flaky. It fails the build only if the second
run of the test class was also unsuccessful. I think it works pretty good,
we mostly have green builds. If there is interest, I can try to contribute
that.

>I am also extremely annoyed at times by the amount of coffee I have to
drink before tests finish
Just please don't get a heart attack :)

@Ron, @Colin
You bring up a very good point that it is easier and frees up more
resources if we just run change specific tests and it's good to know that a
similar solution (meaning using a shared resource for testing) have failed
elsewhere. I second Ron on the test categorization though, although as a
first attempt I think using a flaky retry + running only the necessary
tests would help in both time saving and effectiveness. Also it would be
easier to achieve.

@Ismael
Yea, it'd be interesting to profile the startup/shutdown, I've never done
that. Perhaps I'll set some time apart for that :). It's definitely true
though that if we see a significant delay there we wouldn't just improve
the efficiency of the tests but also customer experience.

Best,
Viktor



On Thu, Feb 28, 2019 at 8:12 AM Ismael Juma  wrote:

> It's an idea that has come up before and worth exploring eventually.
> However, I'd first try to optimize the server startup/shutdown process. If
> we measure where the time is going, maybe some opportunities will present
> themselves.
>
> Ismael
>
> On Wed, Feb 27, 2019, 3:09 AM Viktor Somogyi-Vass  >
> wrote:
>
> > Hi Folks,
> >
> > I've been observing lately that unit tests usually take 2.5 hours to run
> > and a very big portion of these are the core tests where a new cluster is
> > spun up for every test. This takes most of the time. I ran a test
> > (TopicCommandWithAdminClient with 38 test inside) through the profiler
> and
> > it shows for instance that running the whole class itself took 10 minutes
> > and 37 seconds where the useful time was 5 minutes 18 seconds. That's a
> > 100% overhead. Without profiler the whole class takes 7 minutes and 48
> > seconds, so the useful time would be between 3-4 minutes. This is a
> bigger
> > test though, most of them won't take this much.
> > There are 74 classes that implement KafkaServerTestHarness and just
> running
> > :core:integrationTest takes almost 2 hours.
> >
> > I think we could greatly speed up these integration tests by just
> creating
> > the cluster once per class and perform the tests on separate methods. I
> > know that this a little bit contradicts to the principle that tests
> should
> > be independent but it seems like recreating clusters for each is a very
> > expensive operation. Also if the tests are acting on different resources
> > (different topics, etc.) then it might not hurt their independence. There
> > might be cases of course where this is not possible but I think there
> could
> > be a lot where it is.
> >
> > In the optimal case we could cut the testing time back by approximately
> an
> > hour. This would save resources and give quicker feedback for PR builds.
> >
> > What are your thoughts?
> > Has anyone thought about this or were there any attempts made?
> >
> > Best,
> > Viktor
> >
>

[DISCUSS] KIP-435: Incremental Partition Reassignment

[Viktor Somogyi-Vass]

Hi Folks,

I've created a KIP about an improvement of the reassignment algorithm we
have. It aims to enable partition-wise incremental reassignment. The
motivation for this is to avoid excess load that the current replication
algorithm implicitly carries as in that case there are points in the
algorithm where both the new and old replica set could be online and
replicating which puts double (or almost double) pressure on the brokers
which could cause problems.
Instead my proposal would slice this up into several steps where each step
is calculated based on the final target replicas and the current replica
assignment taking into account scenarios where brokers could be offline and
when there are not enough replicas to fulfil the min.insync.replica
requirement.

The link to the KIP:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-435%3A+Incremental+Partition+Reassignment

I'd be happy to receive any feedback.

An important note is that this KIP and another one, KIP-236 that is about
interruptible reassignment (
https://cwiki.apache.org/confluence/display/KAFKA/KIP-236%3A+Interruptible+Partition+Reassignment)
should be compatible.

Thanks,
Viktor

Re: [DISCUSS] KIP-236 Interruptible Partition Reassignment

[Viktor Somogyi-Vass]

I've published the above mentioned KIP here:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-435%3A+Incremental+Partition+Reassignment
Will start a discussion about it soon.

On Fri, Feb 22, 2019 at 12:45 PM Viktor Somogyi-Vass <
viktorsomo...@gmail.com> wrote:

> Hi Folks,
>
> I also have a pending active work on the incremental partition
> reassignment stuff here: https://issues.apache.org/jira/browse/KAFKA-6794
> I think it would be good to cooperate on this to make both work compatible
> with each other.
>
> I'll write up a KIP about this today so it'll be easier to see how to fit
> the two together. Basically in my work I operate on the
> /admin/reassign_partitions node on a fully compatible way, meaning I won't
> change it just calculate each increment based on that and the current state
> of the ISR set for the partition in reassignment.
> I hope we could collaborate on this.
>
> Viktor
>
> On Thu, Feb 21, 2019 at 9:04 PM Harsha  wrote:
>
>> Thanks George. LGTM.
>> Jun & Tom, Can you please take a look at the updated KIP.
>> Thanks,
>> Harsha
>>
>> On Wed, Feb 20, 2019, at 12:18 PM, George Li wrote:
>> > Hi,
>> >
>> > After discussing with Tom, Harsha and I are picking up KIP-236 <
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-236%3A+Interruptible+Partition+Reassignment>.
>> The work focused on safely/cleanly cancel / rollback pending reassignments
>> in a timely fashion. Pull Request #6296 <
>> https://github.com/apache/kafka/pull/6296> Still working on more
>> integration/system tests.
>> >
>> > Please review and provide feedbacks/suggestions.
>> >
>> > Thanks,
>> > George
>> >
>> >
>> > On Saturday, December 23, 2017, 0:51:13 GMT, Jun Rao 
>> wrote:
>> >
>> > Hi, Tom,
>>
>> Thanks for the reply.
>>
>> 10. That's a good thought. Perhaps it's better to get rid of
>> /admin/reassignment_requests
>> too. The window when a controller is not available is small. So, we can
>> just failed the admin client if the controller is not reachable after the
>> timeout.
>>
>> 13. With the changes in 10, the old approach is handled through ZK
>> callback
>> and the new approach is through Kafka RPC. The ordering between the two is
>> kind of arbitrary. Perhaps the ordering can just be based on the order
>> that
>> the reassignment is added to the controller request queue. From there, we
>> can either do the overriding or the prevention.
>>
>> Jun
>>
>>
>> On Fri, Dec 22, 2017 at 7:31 AM, Tom Bentley 
>> wrote:
>>
>> > Hi Jun,
>> >
>> > Thanks for responding, my replies are inline:
>> >
>> > 10. You explanation makes sense. My remaining concern is the additional
>> ZK
>> > > writes in the proposal. With the proposal, we will need to do
>> following
>> > > writes in ZK.
>> > >
>> > > a. write new assignment in /admin/reassignment_requests
>> > >
>> > > b. write new assignment and additional metadata in
>> > > /admin/reassignments/$topic/$partition
>> > >
>> > > c. write old + new assignment  in /brokers/topics/[topic]
>> > >
>> > > d. write new assignment in /brokers/topics/[topic]
>> > >
>> > > e. delete /admin/reassignments/$topic/$partition
>> > >
>> > > So, there are quite a few ZK writes. I am wondering if it's better to
>> > > consolidate the info in /admin/reassignments/$topic/$partition into
>> > > /brokers/topics/[topic].
>> > > For example, we can just add some new JSON fields in
>> > > /brokers/topics/[topic]
>> > > to remember the new assignment and potentially the original replica
>> count
>> > > when doing step c. Those fields with then be removed in step d. That
>> way,
>> > > we can get rid of step b and e, saving 2 ZK writes per partition.
>> > >
>> >
>> > This seems like a great idea to me.
>> >
>> > It might also be possible to get rid of the /admin/reassignment_requests
>> > subtree too. I've not yet published the ideas I have for the AdminClient
>> > API for reassigning partitions, but given the existence of such an API,
>> the
>> > route to starting a reassignment would be the AdminClient, and not
>> > zookeeper. In that case there is no need for
>> /admin/reassignment_requests
>> > at all. The only drawback that I can see is that while it's curr

Re: [DISCUSS] KIP-236 Interruptible Partition Reassignment

[Viktor Somogyi-Vass]

Read through the KIP and I have one comment:

It seems like it is not looking strictly for cancellation but also
implements rolling back to the original. I think it'd be much simpler to
generate a reassignment json on cancellation that contains the original
assignment and start a new partition reassignment completely. This way the
reassignment algorithm (whatever it is) could be reused as a whole. Did you
consider this or are there any obstacles that prevents doing this?

Regards,
Viktor

On Fri, Feb 22, 2019 at 2:24 PM Viktor Somogyi-Vass 
wrote:

> I've published the above mentioned KIP here:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-435%3A+Incremental+Partition+Reassignment
> Will start a discussion about it soon.
>
> On Fri, Feb 22, 2019 at 12:45 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
>> Hi Folks,
>>
>> I also have a pending active work on the incremental partition
>> reassignment stuff here: https://issues.apache.org/jira/browse/KAFKA-6794
>> I think it would be good to cooperate on this to make both work
>> compatible with each other.
>>
>> I'll write up a KIP about this today so it'll be easier to see how to fit
>> the two together. Basically in my work I operate on the
>> /admin/reassign_partitions node on a fully compatible way, meaning I won't
>> change it just calculate each increment based on that and the current state
>> of the ISR set for the partition in reassignment.
>> I hope we could collaborate on this.
>>
>> Viktor
>>
>> On Thu, Feb 21, 2019 at 9:04 PM Harsha  wrote:
>>
>>> Thanks George. LGTM.
>>> Jun & Tom, Can you please take a look at the updated KIP.
>>> Thanks,
>>> Harsha
>>>
>>> On Wed, Feb 20, 2019, at 12:18 PM, George Li wrote:
>>> > Hi,
>>> >
>>> > After discussing with Tom, Harsha and I are picking up KIP-236 <
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-236%3A+Interruptible+Partition+Reassignment>.
>>> The work focused on safely/cleanly cancel / rollback pending reassignments
>>> in a timely fashion. Pull Request #6296 <
>>> https://github.com/apache/kafka/pull/6296> Still working on more
>>> integration/system tests.
>>> >
>>> > Please review and provide feedbacks/suggestions.
>>> >
>>> > Thanks,
>>> > George
>>> >
>>> >
>>> > On Saturday, December 23, 2017, 0:51:13 GMT, Jun Rao 
>>> wrote:
>>> >
>>> > Hi, Tom,
>>>
>>> Thanks for the reply.
>>>
>>> 10. That's a good thought. Perhaps it's better to get rid of
>>> /admin/reassignment_requests
>>> too. The window when a controller is not available is small. So, we can
>>> just failed the admin client if the controller is not reachable after the
>>> timeout.
>>>
>>> 13. With the changes in 10, the old approach is handled through ZK
>>> callback
>>> and the new approach is through Kafka RPC. The ordering between the two
>>> is
>>> kind of arbitrary. Perhaps the ordering can just be based on the order
>>> that
>>> the reassignment is added to the controller request queue. From there, we
>>> can either do the overriding or the prevention.
>>>
>>> Jun
>>>
>>>
>>> On Fri, Dec 22, 2017 at 7:31 AM, Tom Bentley 
>>> wrote:
>>>
>>> > Hi Jun,
>>> >
>>> > Thanks for responding, my replies are inline:
>>> >
>>> > 10. You explanation makes sense. My remaining concern is the
>>> additional ZK
>>> > > writes in the proposal. With the proposal, we will need to do
>>> following
>>> > > writes in ZK.
>>> > >
>>> > > a. write new assignment in /admin/reassignment_requests
>>> > >
>>> > > b. write new assignment and additional metadata in
>>> > > /admin/reassignments/$topic/$partition
>>> > >
>>> > > c. write old + new assignment  in /brokers/topics/[topic]
>>> > >
>>> > > d. write new assignment in /brokers/topics/[topic]
>>> > >
>>> > > e. delete /admin/reassignments/$topic/$partition
>>> > >
>>> > > So, there are quite a few ZK writes. I am wondering if it's better to
>>> > > consolidate the info in /admin/reassignments/$topic/$partition into
>>> > > /brokers/topics/[topic].
>>> > > For example, we can just add some new JSO

Re: [DISCUSS] KIP-236 Interruptible Partition Reassignment

[Viktor Somogyi-Vass]

Hi Folks,

I also have a pending active work on the incremental partition reassignment
stuff here: https://issues.apache.org/jira/browse/KAFKA-6794
I think it would be good to cooperate on this to make both work compatible
with each other.

I'll write up a KIP about this today so it'll be easier to see how to fit
the two together. Basically in my work I operate on the
/admin/reassign_partitions node on a fully compatible way, meaning I won't
change it just calculate each increment based on that and the current state
of the ISR set for the partition in reassignment.
I hope we could collaborate on this.

Viktor

On Thu, Feb 21, 2019 at 9:04 PM Harsha  wrote:

> Thanks George. LGTM.
> Jun & Tom, Can you please take a look at the updated KIP.
> Thanks,
> Harsha
>
> On Wed, Feb 20, 2019, at 12:18 PM, George Li wrote:
> > Hi,
> >
> > After discussing with Tom, Harsha and I are picking up KIP-236 <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-236%3A+Interruptible+Partition+Reassignment>.
> The work focused on safely/cleanly cancel / rollback pending reassignments
> in a timely fashion. Pull Request #6296 <
> https://github.com/apache/kafka/pull/6296> Still working on more
> integration/system tests.
> >
> > Please review and provide feedbacks/suggestions.
> >
> > Thanks,
> > George
> >
> >
> > On Saturday, December 23, 2017, 0:51:13 GMT, Jun Rao 
> wrote:
> >
> > Hi, Tom,
>
> Thanks for the reply.
>
> 10. That's a good thought. Perhaps it's better to get rid of
> /admin/reassignment_requests
> too. The window when a controller is not available is small. So, we can
> just failed the admin client if the controller is not reachable after the
> timeout.
>
> 13. With the changes in 10, the old approach is handled through ZK callback
> and the new approach is through Kafka RPC. The ordering between the two is
> kind of arbitrary. Perhaps the ordering can just be based on the order that
> the reassignment is added to the controller request queue. From there, we
> can either do the overriding or the prevention.
>
> Jun
>
>
> On Fri, Dec 22, 2017 at 7:31 AM, Tom Bentley 
> wrote:
>
> > Hi Jun,
> >
> > Thanks for responding, my replies are inline:
> >
> > 10. You explanation makes sense. My remaining concern is the additional
> ZK
> > > writes in the proposal. With the proposal, we will need to do following
> > > writes in ZK.
> > >
> > > a. write new assignment in /admin/reassignment_requests
> > >
> > > b. write new assignment and additional metadata in
> > > /admin/reassignments/$topic/$partition
> > >
> > > c. write old + new assignment  in /brokers/topics/[topic]
> > >
> > > d. write new assignment in /brokers/topics/[topic]
> > >
> > > e. delete /admin/reassignments/$topic/$partition
> > >
> > > So, there are quite a few ZK writes. I am wondering if it's better to
> > > consolidate the info in /admin/reassignments/$topic/$partition into
> > > /brokers/topics/[topic].
> > > For example, we can just add some new JSON fields in
> > > /brokers/topics/[topic]
> > > to remember the new assignment and potentially the original replica
> count
> > > when doing step c. Those fields with then be removed in step d. That
> way,
> > > we can get rid of step b and e, saving 2 ZK writes per partition.
> > >
> >
> > This seems like a great idea to me.
> >
> > It might also be possible to get rid of the /admin/reassignment_requests
> > subtree too. I've not yet published the ideas I have for the AdminClient
> > API for reassigning partitions, but given the existence of such an API,
> the
> > route to starting a reassignment would be the AdminClient, and not
> > zookeeper. In that case there is no need for /admin/reassignment_requests
> > at all. The only drawback that I can see is that while it's currently
> > possible to trigger a reassignment even during a controller
> > election/failover that would no longer be the case if all requests had to
> > go via the controller.
> >
> >
> > > 11. What you described sounds good. We could potentially optimize the
> > > dropped replicas a bit more. Suppose that assignment [0,1,2] is first
> > > changed to [1,2,3] and then to [2,3,4]. When initiating the second
> > > assignment, we may end up dropping replica 3 and only to restart it
> > again.
> > > In this case, we could only drop a replica if it's not going to be
> added
> > > back again.
> > >
> >
> > I had missed that, thank you! I will update the proposed algorithm to
> > prevent this.
> >
> >
> > > 13. Since this is a corner case, we can either prevent or allow
> > overriding
> > > with old/new mechanisms. To me, it seems that allowing is simpler to
> > > implement, the order in /admin/reassignment_requests determines the
> > > ordering the of override, whether that's initiated by the new way or
> the
> > > old way.
> > >
> >
> > That makes sense except for the corner case where:
> >
> > * There is no current controller and
> > * Writes to both the new and old znodes happen
> >
> > On election of the new controller, for those 

[jira] [Created] (KAFKA-7981) Add Replica Fetcher and Log Cleaner Count Metrics

[Viktor Somogyi-Vass (JIRA)]

Viktor Somogyi-Vass created KAFKA-7981:
--

 Summary: Add Replica Fetcher and Log Cleaner Count Metrics
 Key: KAFKA-7981
 URL: https://issues.apache.org/jira/browse/KAFKA-7981
 Project: Kafka
  Issue Type: Improvement
  Components: metrics
Affects Versions: 2.3.0
Reporter: Viktor Somogyi-Vass
Assignee: Viktor Somogyi-Vass






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

[DISCUSS] KIP-434: Add Replica Fetcher and Log Cleaner Count Metrics

[Viktor Somogyi-Vass]

Hi All,

I'd like to start a discussion about exposing count gauge metrics for the
replica fetcher and log cleaner thread counts. It isn't a long KIP and the
motivation is very simple: monitoring the thread counts in these cases
would help with the investigation of various issues and might help in
preventing more serious issues when a broker is in a bad state. Such a
scenario that we seen with users is that their disk fills up as the log
cleaner died for some reason and couldn't recover (like log corruption). In
this case an early warning would help in the root cause analysis process as
well as enable detecting and resolving the problem early on.

The KIP is here:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics

I'd be happy to receive any feedback on this.

Regards,
Viktor

Re: [DISCUSS] KIP-434: Add Replica Fetcher and Log Cleaner Count Metrics

[Viktor Somogyi-Vass]

Hi Stanislav,

Thanks for the feedback and sharing that discussion thread.

I read your KIP and the discussion on it too and it seems like that'd cover
the same motivation I had with the log-cleaner-thread-count metric. This
supposed to tell the count of the alive threads which might differ from the
config (I could've used a better name :) ). Now I'm thinking that using
uncleanable-bytes, uncleanable-partition-count together with
time-since-last-run would mostly cover the motivation I have in this KIP,
however displaying the thread count (be it alive or dead) would still add
extra information regarding the failure, that a thread died during cleanup.

You had a very good idea about instead of the alive threads, display the
dead ones! That way we wouldn't need log-cleaner-current-live-thread-rate
just a "dead-log-cleaner-thread-count" as it it would make easy to trigger
warnings based on that (if it's even > 0 then we can say there's a
potential problem).
Doing this on the replica fetchers though would be a bit harder as the
number of replica fetchers is the (brokers-to-fetch-from *
fetchers-per-broker) and we don't really maintain the capacity information
or any kind of cluster information and I'm not sure we should. It would add
too much responsibility to the class and wouldn't be a rock-solid solution
but I guess I have to look into it more.

I don't think that restarting the cleaner threads would be helpful as the
problems I've seen mostly are non-recoverable and requires manual user
intervention and partly I agree what Colin said on the KIP-346 discussion
thread about the problems experienced with HDFS.

Best,
Viktor


On Fri, Feb 22, 2019 at 5:03 PM Stanislav Kozlovski 
wrote:

> Hey Viktor,
>
> First off, thanks for the KIP! I think that it is almost always a good idea
> to have more metrics. Observability never hurts.
>
> In regards to the LogCleaner:
> * Do we need to know log-cleaner-thread-count? That should always be equal
> to "log.cleaner.threads" if I'm not mistaken.
> * log-cleaner-current-live-thread-rate -  We already have the
> "time-since-last-run-ms" metric which can let you know if something is
> wrong with the log cleaning
> As you said, we would like to have these two new metrics in order to
> understand when a partial failure has happened - e.g only 1/3 log cleaner
> threads are alive. I'm wondering if it may make more sense to either:
> a) restart the threads when they die
> b) add a metric which shows the dead thread count. You should probably
> always have a low-level alert in the case that any threads have died
>
> We had discussed a similar topic about thread revival and metrics in
> KIP-346. Have you had a chance to look over that discussion? Here is the
> mailing discussion for that -
>
> http://mail-archives.apache.org/mod_mbox/kafka-dev/201807.mbox/%3ccanzzngyr_22go9swl67hedcm90xhvpyfzy_tezhz1mrizqk...@mail.gmail.com%3E
>
> Best,
> Stanislav
>
>
>
> On Fri, Feb 22, 2019 at 11:18 AM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
> > Hi All,
> >
> > I'd like to start a discussion about exposing count gauge metrics for the
> > replica fetcher and log cleaner thread counts. It isn't a long KIP and
> the
> > motivation is very simple: monitoring the thread counts in these cases
> > would help with the investigation of various issues and might help in
> > preventing more serious issues when a broker is in a bad state. Such a
> > scenario that we seen with users is that their disk fills up as the log
> > cleaner died for some reason and couldn't recover (like log corruption).
> In
> > this case an early warning would help in the root cause analysis process
> as
> > well as enable detecting and resolving the problem early on.
> >
> > The KIP is here:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics
> >
> > I'd be happy to receive any feedback on this.
> >
> > Regards,
> > Viktor
> >
>
>
> --
> Best,
> Stanislav
>

Re: [DISCUSS] KIP-435: Incremental Partition Reassignment

[Viktor Somogyi-Vass]

Hi Harsha,

As far as I understand KIP-236 it's about enabling reassignment
cancellation and as a future plan providing a queue of replica reassignment
steps to allow manual reassignment chains. While I agree that the
reassignment chain has a specific use case that allows fine grain control
over reassignment process, My proposal on the other hand doesn't talk about
cancellation but it only provides an automatic way to incrementalize an
arbitrary reassignment which I think fits the general use case where users
don't want that level of control but still would like a balanced way of
reassignments. Therefore I think it's still relevant as an improvement of
the current algorithm.
Nevertheless I'm happy to add my ideas to KIP-236 as I think it would be a
great improvement to Kafka.

Cheers,
Viktor

On Fri, Feb 22, 2019 at 5:05 PM Harsha  wrote:

> Hi Viktor,
> There is already KIP-236 for the same feature and George made
> a PR for this as well.
> Lets consolidate these two discussions. If you have any cases that are not
> being solved by KIP-236 can you please mention them in that thread. We can
> address as part of KIP-236.
>
> Thanks,
> Harsha
>
> On Fri, Feb 22, 2019, at 5:44 AM, Viktor Somogyi-Vass wrote:
> > Hi Folks,
> >
> > I've created a KIP about an improvement of the reassignment algorithm we
> > have. It aims to enable partition-wise incremental reassignment. The
> > motivation for this is to avoid excess load that the current replication
> > algorithm implicitly carries as in that case there are points in the
> > algorithm where both the new and old replica set could be online and
> > replicating which puts double (or almost double) pressure on the brokers
> > which could cause problems.
> > Instead my proposal would slice this up into several steps where each
> step
> > is calculated based on the final target replicas and the current replica
> > assignment taking into account scenarios where brokers could be offline
> and
> > when there are not enough replicas to fulfil the min.insync.replica
> > requirement.
> >
> > The link to the KIP:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-435%3A+Incremental+Partition+Reassignment
> >
> > I'd be happy to receive any feedback.
> >
> > An important note is that this KIP and another one, KIP-236 that is
> > about
> > interruptible reassignment (
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-236%3A+Interruptible+Partition+Reassignment
> )
> > should be compatible.
> >
> > Thanks,
> > Viktor
> >
>

Re: [DISCUSS] KIP-433: Provide client API version to authorizer

[Viktor Somogyi-Vass]

Hi Ying,

I was also thinking about your KIP yesterday I second Gwen on this. There
are several other authorization components that are compatible with Kafka
and putting this in there means that each and every component has to
implement this functionality.
At the first read I thought that this functionality could be a good
extension of the API_VERSIONS protocol where based on a dynamic config
(producer.min.api.version or consumer.min.api.version) the brokers would
reject certain clients that are too old.  And based on the motivation, to
avoid certain bugs with certain producer/consumer version, we might not
need finer grained control over this which I think also plays for the
config approach.

Cheers,
Viktor

On Sat, Feb 23, 2019 at 12:06 AM Ying Zheng  wrote:

> Hi Gwen,
>
> Thank you for the quick feedback!
>
> It's a good point that broker configuration can be dynamic and is more
> convenient. Technically, anything inside the authorizer can also be
> dynamic. For example, the SimpleAclAuthorizer in Kafka stores ACLs in
> Zookeeper, which can be dynamically changed with CLI.
>
>
>
>
>
> On Fri, Feb 22, 2019 at 2:41 PM Gwen Shapira  wrote:
>
> > Link, for convenience:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-433%3A+Provide+client+API+version+to+authorizer
> >
> > I actually prefer the first rejected alternative (add a
> > configuration). While you are right that configuration is inherently
> > less flexible, putting the logic in the authorizer means that an admin
> > that wants to limit the allowed client API versions has to implement
> > an authorizer. This is more challenging than changing a config (and
> > AFAIK, can't be done dynamically - configs can be dynamic and the
> > admin can avoid a restart).
> >
> > Would be interested to hear what others think.
> >
> > Gwen
> >
> > On Fri, Feb 22, 2019 at 2:11 PM Ying Zheng 
> wrote:
> > >
> > >
> >
> >
> > --
> > Gwen Shapira
> > Product Manager | Confluent
> > 650.450.2760 | @gwenshap
> > Follow us: Twitter | blog
> >
>

Re: [DISCUSS] KIP-236 Interruptible Partition Reassignment

[Viktor Somogyi-Vass]

Hey George,

Thanks for the prompt response, it makes sense. I'll try to keep your code
changes on top of my list and help reviewing that. :)
Regarding the incremental reassignment: I don't mind either to discuss it
as part of this or in a separate conversation but I think a separate one
could be better because both discussions can be long and keeping them
separated would limit the scope and make them more digestible and focused.
If the community decides to discuss it here then I think I'll put KIP-435
on hold or rejected and add my ideas here.
If the community decides to discuss it in a different KIP I think it's a
good idea to move the planned future work part into KIP-435 and rework
that. Maybe we can co-author it as I think both works could be
complementary to each other.
In any case I'd be absolutely interested in what others think.

A few questions regarding the rollback algorithm:
1. At step 2 how do you elect the leader?
1.1. Would it be always the original leader?
1.2. What if some brokers that are in OAR are down?
2. I still have doubts that we need to do the reassignment backwards during
rollback. For instance if we decide to cancel the reassignment at step #8
where replicas in OAR - RAR are offline and start the rollback, then how do
we make a replica from OAR online again before electing a leader as
described in step #2 of the rollback algorithm?
3. Does the algorithm defend against crashes? Is it able to continue after
a controller failover?
4. I think it would be a good addition if you could add few example
scenarios for rollback.

Best,
Viktor


On Fri, Feb 22, 2019 at 7:04 PM George Li  wrote:

> Hi Viktor,
>
>
> Thanks for reading and provide feedbacks on KIP-236.
>
>
> For reassignments, one can generate a json for new assignments and another
> json with "original" assignments for rollback purpose.  In production
> cluster, from our experience, we need to submit the reassignments in
> batches with throttle/staggering to minimize the impact to the cluster.
> Some large topic/partition couple with throttle can take pretty long time
> for the new replica to be in ISR to complete reassignment in that batch.
> Currently during this,  Kafka does not allow cancelling the pending
> reassignments cleanly.  Even you have the json with the "original"
> assignments to rollback, it has to wait till current reassignment to
> complete, then submit it as reassignments to rollback. If the current
> reassignment is causing impact to production, we would like the
> reassignments to be cancelled/rollbacked cleanly/safely/quickly.  This is
> the main goal of KIP-236.
>
>
> The original KIP-236 by Tom Bentley also proposed the incremental
> reassignments, to submit new reassignments while the current reassignments
> is still going on. This is scaled back to put under "Planned Future
> Changes" section of KIP-236, so we can expedite this Reassignment
> Cancellation/Rollback feature out to the community.
>
>
> The main idea incremental reassignment is to allow submit new
> reassignments in another ZK node /admin/reassign_partitions_queue  and
> merge it with current pending reassignments in /admin/reassign_partitions.
> In case of same topic/partition in both ZK node, the conflict resolution is
> to cancel the current reassignment in /admin/reassign_partitions, and move
> the same topic/partition from /admin/reassign_partitions_queue  as new
> reassignment.
>
>
> If there is enough interest from the community, this "Planned Future
> Changes" for incremental reassignments can also be delivered in KIP-236,
> otherwise, another KIP.  The current PR:
> https://github.com/apache/kafka/pull/6296  only focuses/addresses the
> pending Reassignment Cancellation/Rollback.
>
>
> Hope this answers your questions.
>
>
> Thanks,
> George
>
>
> On Friday, February 22, 2019, 6:51:14 AM PST, Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
>
> Read through the KIP and I have one comment:
>
> It seems like it is not looking strictly for cancellation but also
> implements rolling back to the original. I think it'd be much simpler to
> generate a reassignment json on cancellation that contains the original
> assignment and start a new partition reassignment completely. This way the
> reassignment algorithm (whatever it is) could be reused as a whole. Did you
> consider this or are there any obstacles that prevents doing this?
>
> Regards,
> Viktor
>
> On Fri, Feb 22, 2019 at 2:24 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> wrote:
>
> > I've published the above mentioned KIP here:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-435%3A+Incremental+Partition+Reassignment
> > Will start a discussion about it soon.
> &g

Re: [VOTE] 2.2.0 RC0

[Viktor Somogyi-Vass]

Hi Matthias,

I've noticed a minor line break issue in the upgrade docs. I've created a
small PR for that: https://github.com/apache/kafka/pull/6320

Best,
Viktor

On Sun, Feb 24, 2019 at 10:16 PM Stephane Maarek 
wrote:

> Hi Matthias
>
> Thanks for this
> Running through the list of KIPs. I think this is not included in 2.2:
>
> - Allow clients to suppress auto-topic-creation
>
> Regards
> Stephane
>
> On Sun, Feb 24, 2019 at 1:03 AM Matthias J. Sax 
> wrote:
>
> > Hello Kafka users, developers and client-developers,
> >
> > This is the first candidate for the release of Apache Kafka 2.2.0.
> >
> > This is a minor release with the follow highlight:
> >
> >  - Added SSL support for custom principle name
> >  - Allow SASL connections to periodically re-authenticate
> >  - Improved consumer group management
> >- default group.id is `null` instead of empty string
> >  - Add --under-min-isr option to describe topics command
> >  - Allow clients to suppress auto-topic-creation
> >  - API improvement
> >- Producer: introduce close(Duration)
> >- AdminClient: introduce close(Duration)
> >- Kafka Streams: new flatTransform() operator in Streams DSL
> >- KafkaStreams (and other classed) now implement AutoClosable to
> > support try-with-resource
> >- New Serdes and default method implementations
> >  - Kafka Streams exposed internal client.id via ThreadMetadata
> >  - Metric improvements:  All `-min`, `-avg` and `-max` metrics will now
> > output `NaN` as default value
> >
> >
> > Release notes for the 2.2.0 release:
> > http://home.apache.org/~mjsax/kafka-2.2.0-rc0/RELEASE_NOTES.html
> >
> > *** Please download, test and vote by Friday, March 1, 9am PST.
> >
> > Kafka's KEYS file containing PGP keys we use to sign the release:
> > http://kafka.apache.org/KEYS
> >
> > * Release artifacts to be voted upon (source and binary):
> > http://home.apache.org/~mjsax/kafka-2.2.0-rc0/
> >
> > * Maven artifacts to be voted upon:
> > https://repository.apache.org/content/groups/staging/org/apache/kafka/
> >
> > * Javadoc:
> > http://home.apache.org/~mjsax/kafka-2.2.0-rc0/javadoc/
> >
> > * Tag to be voted upon (off 2.2 branch) is the 2.2.0 tag:
> > https://github.com/apache/kafka/releases/tag/2.2.0-rc0
> >
> > * Documentation:
> > https://kafka.apache.org/22/documentation.html
> >
> > * Protocol:
> > https://kafka.apache.org/22/protocol.html
> >
> > * Successful Jenkins builds for the 2.2 branch:
> > Unit/integration tests: https://builds.apache.org/job/kafka-2.2-jdk8/31/
> >
> > * System tests:
> > https://jenkins.confluent.io/job/system-test-kafka/job/2.2/
> >
> >
> >
> >
> > Thanks,
> >
> > -Matthias
> >
> >
>

Re: [DISCUSS] KIP-236 Interruptible Partition Reassignment

[Viktor Somogyi-Vass]

b.com/apache/kafka/pull/6296>
>
>
> rollbackReassignedPartitionLeaderIfRequired(topicPartition, 
> reassignedPartitionContext)
>
> During "pending" reassignment, e.g.  (1,2,3) => (4,2,5)  normally, the
> leader (in this case broker_id 1) will remain as the leader until all
> replicas (1,2,3,4,5) in ISR, then the leader will be switched to 4.
> However, in one scenario, if let's say new replica 4 is already caught up
> in ISR, and somehow original leader 1 is down or bounced.  4 could become
> the new leader.
>
> rollbackReassignedPartitionLeaderIfRequired() will do a leadership election 
> using PreferredReplicaPartitionLeaderElectionStrategy
>   among brokers in OAR (Original Assigned Replicas set in memory).
>
> > 1.1. Would it be always the original leader?
>
> Not necessarily,  if the original preferred leader is down, it can be
> other brokers in OAR which are in ISR.
>
> > 1.2. What if some brokers that are in OAR are down?
>
> If some brokers in OAR are down, the topic/partition will have URP (Under
> Replicated Partition). The client deciding to do reassignment should be
> clear what the current state of the cluster is, what brokers are down, what
> are up, what reassignment is trying to accomplish. e.g. reassignment from
> down brokers to new brokers(?)
>
>
> > 2. I still have doubts that we need to do the reassignment backwards
> during rollback. For instance if we decide to cancel the reassignment at
> step > #8 where replicas in OAR - RAR are offline and start the rollback,
> then how do we make a replica from OAR online again before electing a
> leader as described in step #2 of the rollback algorithm?
> > 3. Does the algorithm defend against crashes? Is it able to continue
> after a controller failover?
> > 4. I think it would be a good addition if you could add few example
> scenarios for rollback.
>
>
> yes. shouldTriggerReassignCancelOnControllerStartup() is the integration
> test to simulate controller failover while cancelling pending
> reassignments.  I will try to add controller failover scenario in a
> ducktape system test.
>
> You do raise a good point here. If the cluster is in a very BAD shape,
> e.g. None of the OAR brokers are online,  but some new broker in RAR is in
> ISR and is current leader, it make senses not to rollback to keep that
> topic/partition online. However, if none of brokers in  RAR is online, it
> may make sense to rollback to OAR and remove it from
> /admin/reassign_partitions, since the cluster state is already so bad, that
> topic/partition is offline anyway no matter rollback or not.
>
> I will add a check before cancel/rollback a topic/partition's pending
> reassignment by checking whether at least one broker of OAR is in ISR, so
> that it can be elected as leader,  if not, skip that topic/partition
> reassignment cancellation and raise an exception.
>
> I will list a few more scenarios for rollback.
>
> What additional scenarios for rollback you and others can think of?
>
>
> Thanks,
> George
>
>
> On Monday, February 25, 2019, 3:53:33 AM PST, Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
>
> Hey George,
>
> Thanks for the prompt response, it makes sense. I'll try to keep your code
> changes on top of my list and help reviewing that. :)
> Regarding the incremental reassignment: I don't mind either to discuss it
> as part of this or in a separate conversation but I think a separate one
> could be better because both discussions can be long and keeping them
> separated would limit the scope and make them more digestible and focused.
> If the community decides to discuss it here then I think I'll put KIP-435
> on hold or rejected and add my ideas here.
> If the community decides to discuss it in a different KIP I think it's a
> good idea to move the planned future work part into KIP-435 and rework
> that. Maybe we can co-author it as I think both works could be
> complementary to each other.
> In any case I'd be absolutely interested in what others think.
>
> A few questions regarding the rollback algorithm:
> 1. At step 2 how do you elect the leader?
> 1.1. Would it be always the original leader?
> 1.2. What if some brokers that are in OAR are down?
> 2. I still have doubts that we need to do the reassignment backwards
> during rollback. For instance if we decide to cancel the reassignment at
> step #8 where replicas in OAR - RAR are offline and start the rollback,
> then how do we make a replica from OAR online again before electing a
> leader as described in step #2 of the rollback algorithm?
> 3. Does the algorithm defend against crashes? Is it able to continue after
> a controller failover?
> 4. I think 

Re: [DISCUSS] KIP-434: Add Replica Fetcher and Log Cleaner Count Metrics

[Viktor Somogyi-Vass]

Hey Stanislav,

Sorry for the delay on this. In the meantime I realized that the dead
fetchers won't be removed from the fetcher map, so it's very easy to figure
out how many dead and alive there are. I can collect them on broker level
which I think gives a good enough information if there is a problem with a
given broker. You do raise a good point with your idea that it's helpful to
know which fetcher is acting up, although I find that solution less
feasible due to the number of metrics generated. For this I'd use a JMX
method that'd return a list of problematic fetcher threads but I'm not sure
if we have to extend the scope of this KIP that much.

Best,
Viktor

On Mon, Mar 4, 2019 at 7:22 PM Stanislav Kozlovski 
wrote:

> Hey Viktor,
>
> > however displaying the thread count (be it alive or dead) would still add
> extra information regarding the failure, that a thread died during cleanup.
>
> I agree, I think it's worth adding.
>
>
> > Doing this on the replica fetchers though would be a bit harder as the
> number of replica fetchers is the (brokers-to-fetch-from *
> fetchers-per-broker) and we don't really maintain the capacity information
> or any kind of cluster information and I'm not sure we should.
>
> Perhaps we could split the metric per broker that is being fetched from?
> i.e each replica fetcher would have a `dead-fetcher-threads` metric that
> has the broker-id it's fetching from as a tag?
> It would solve an observability question which I think is very important -
> are we replicating from this broker at all?
> On the other hand, this could potentially produce a lot of metric data with
> a big cluster, so that is definitely something to consider as well.
>
> All in all, I think this is a great KIP and very much needed in my opinion.
> I can't wait to see this roll out
>
> Best,
> Stanislav
>
> On Mon, Feb 25, 2019 at 10:29 AM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
> > Hi Stanislav,
> >
> > Thanks for the feedback and sharing that discussion thread.
> >
> > I read your KIP and the discussion on it too and it seems like that'd
> cover
> > the same motivation I had with the log-cleaner-thread-count metric. This
> > supposed to tell the count of the alive threads which might differ from
> the
> > config (I could've used a better name :) ). Now I'm thinking that using
> > uncleanable-bytes, uncleanable-partition-count together with
> > time-since-last-run would mostly cover the motivation I have in this KIP,
> > however displaying the thread count (be it alive or dead) would still add
> > extra information regarding the failure, that a thread died during
> cleanup.
> >
> > You had a very good idea about instead of the alive threads, display the
> > dead ones! That way we wouldn't need log-cleaner-current-live-thread-rate
> > just a "dead-log-cleaner-thread-count" as it it would make easy to
> trigger
> > warnings based on that (if it's even > 0 then we can say there's a
> > potential problem).
> > Doing this on the replica fetchers though would be a bit harder as the
> > number of replica fetchers is the (brokers-to-fetch-from *
> > fetchers-per-broker) and we don't really maintain the capacity
> information
> > or any kind of cluster information and I'm not sure we should. It would
> add
> > too much responsibility to the class and wouldn't be a rock-solid
> solution
> > but I guess I have to look into it more.
> >
> > I don't think that restarting the cleaner threads would be helpful as the
> > problems I've seen mostly are non-recoverable and requires manual user
> > intervention and partly I agree what Colin said on the KIP-346 discussion
> > thread about the problems experienced with HDFS.
> >
> > Best,
> > Viktor
> >
> >
> > On Fri, Feb 22, 2019 at 5:03 PM Stanislav Kozlovski <
> > stanis...@confluent.io>
> > wrote:
> >
> > > Hey Viktor,
> > >
> > > First off, thanks for the KIP! I think that it is almost always a good
> > idea
> > > to have more metrics. Observability never hurts.
> > >
> > > In regards to the LogCleaner:
> > > * Do we need to know log-cleaner-thread-count? That should always be
> > equal
> > > to "log.cleaner.threads" if I'm not mistaken.
> > > * log-cleaner-current-live-thread-rate -  We already have the
> > > "time-since-last-run-ms" metric which can let you know if something is
> > > wrong with the log cleaning
> > > As you said, we would like to have these two new metrics in order to
> > > understand when a partial failure has happened -

Re: [DISCUSS] KIP-236 Interruptible Partition Reassignment

[Viktor Somogyi-Vass]

est_topic-15 -> Map(replicas ->
> Buffer(1, 3, 5), original_replicas -> Buffer(1, 3, 4)), test_topic-11 ->
> Map(replicas -> Buffer(2, 3, 5), original_replicas -> Buffer(2, 3, 4)))
> Successfully submitted cancellation of reassignments.
> The cancelled pending reassignments throttle was removed.
> Please run --verify to have the previous reassignments (not just the
> cancelled reassignments in progress) throttle removed.
>
>
>
>
>
> On Tuesday, March 5, 2019, 8:20:25 AM PST, Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
>
> Hey George,
>
> Sorry for the delay.
> I'll answer point-by-point:
> 1.2: I think it's fine. As you say we presume that the client knows the
> state of the cluster before doing the reassignment, so we can presume the
> same for cancellation.
> 2.: One follow-up question: if the reassignment cancellation gets
> interrupted and a failover happens after step #2 but before step #3, how
> will the new controller continue? At this stage Zookeeper would contain
> OAR + RAR, however the brokers will have the updated LeaderAndIsr about
> OAR, so they won't know about RAR. I would suppose the new controller would
> start from the beginning as it only knows what's in Zookeeper. Is that true?
> 2.1: Another interesting question that are what are those replicas are
> doing which are online but not part of the leader and ISR? Are they still
> replicating? Are they safe to lie around for the time being?
>
> Best,
> Viktor
>
>
> On Fri, Mar 1, 2019 at 8:40 PM George Li  wrote:
>
> Hi Jun,
>
> Could you help review KIP-236 when you have time?  Thanks.
>
>
>
> Hi Becket,
>
> Since you filed https://issues.apache.org/jira/browse/KAFKA-6304 to
> request this feature.  Could you also help review and comment on KIP-236 ?
> Thanks.
>
>
>
>
> Hi Viktor,
>
> I have updated https://github.com/apache/kafka/pull/6296  and also
> KIP-236:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-236%3A+Interruptible+Partition+Reassignment
>  Please
> see below new section "Skip Reassignment Cancellation Scenarios":
>
> Skip Reassignment Cancellation Scenarios
>
> There are a couple scenarios that the Pending reassignments in
> /admin/reassign_partitions can not be cancelled / rollback.
>
>
>1. If the "original_replicas"  is missing for the topic/partition in
>/admin/reassign_partitions .  In this case, the pending reassignment
>cancelled will be skipped.  Because there is no way to reset to the
>original replicas.  The reasons this can happened  could be:
>   1. if either the user/client is tampering /admin/reassign_partitions 
> directly,
>   and does not have the "original_replicas" for the topic
>   2. if the user/client is using incorrect versions of the admin
>   client to submit for reassignments.   The Kafka software should be 
> upgraded
>   not just for all the brokers in the cluster.  but also on the host that 
> is
>   used to submit reassignments.
>
>   2. If all the "original_replicas" brokers are not in ISR,  and some
>brokers in the "new_replicas" are not offline for the topic/partition in
>the pending reassignments.   In this case, it's better to skip this topic's
>pending reassignment  cancellation/rollback,  otherwise, it will become
>offline.  However,  if all the brokers in "original_replicas" are offline
>AND  all the brokers in "new_replicas" are also offline for this
>topic/partition,  then the cluster is in such a bad state, the
>topic/partition is currently offline anyway,  it will cancel/rollback this
>topic pending reassignments back to the "original_replicas".
>
>
>
>
>
>
> What other scenarios others can think of that reassignment cancellation
> should be skipped?  Thanks
>
>
>
> Hi All,
>
>
> Another issues I would like to raise is the removing of throttle for the
> Cancelled Reassignments.  Currently the remove throttle code is in the
> Admin Client.  Since the pending reassignments are cancelled /rollback,
> the throttle would not be removed by running admin client with --verify
> option to remove the throttle.  My approached is to remove the throttle in
> the admin client after the reassignments cancellation.  But I feel it's
> better to move this in Controller (in controller failover scenario).
>
>
>
> Thanks,
>
> George
>
>
>
>
> On Monday, February 25, 2019, 11:40:08 AM PST, George Li <
> sql_consult...@yahoo.com> wrote:
>
>
> Hi Viktor,
>
> Thanks for the response.  Good questions!  answers below:
>
> > A few qu

Re: [DISCUSS] KIP-438: Expose task, connector IDs in Connect API

[Viktor Somogyi-Vass]

I'm generally +1 on this, although have a couple of basic questions.
Am I getting that right that you basically want to expose the task id from
ConnectorTaskId? And if so, then I guess you'll provide the implementation
too?

Thanks,
Viktor

On Tue, Mar 5, 2019 at 6:49 PM Ryanne Dolan  wrote:

> Hey y'all, please consider the following small KIP:
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-438%3A+Expose+task%2C+connector+IDs+in+Connect+API
>
> Thanks!
> Ryanne
>

Re: [DISCUSSION] KIP-422: Add support for user/client configuration in the Kafka Admin Client

[Viktor Somogyi-Vass]

Hi Guys,

I wanted to reject that KIP, split it up and revamp it as in the meantime
there were some overlapping works I just didn't get to it due to other
higher priority work.
One of the splitted KIPs would have been the quota part of that and I'd be
happy if that lived in this KIP if Yaodong thinks it's worth to
incorporate. I'd be also happy to rebase that wire protocol and contribute
it to this KIP.

Viktor

On Wed, Feb 13, 2019 at 7:14 PM Jun Rao  wrote:

> Hi, Yaodong,
>
> Thanks for the KIP. As Stan mentioned earlier, it seems that this is
> mostly covered by KIP-248, which was originally proposed by Victor.
>
> Hi, Victor,
>
> Do you still plan to work on KIP-248? It seems that you already got pretty
> far on that. If not, would you mind letting Yaodong take over this?
>
> For both KIP-248 and KIP-422, one thing that I found missing is the
> support for customized quota (
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-257+-+Configurable+Quota+Management).
> With KIP-257, it's possible for one to construct a customized quota defined
> through a map of metric tags. It would be useful to support that in the
> AdminClient API and the wire protocol.
>
> Hi, Sonke,
>
> I think the proposal is to support the user/clientId level quota through
> an AdminClient api. The user can be obtained from any existing
> authentication mechanisms.
>
> Thanks,
>
> Jun
>
> On Thu, Feb 7, 2019 at 5:59 AM Sönke Liebau
>  wrote:
>
>> Hi Yaodong,
>>
>> thanks for the KIP!
>>
>> If I understand your intentions correctly then this KIP would only
>> address a fairly specific use case, namely SASL-PLAIN with users
>> defined in Zookeeper. For all other authentication mechanisms like
>> SSL, SASL-GSSAPI or SASL-PLAIN with users defined in jaas files I
>> don't see how the AdminClient could directly create new users.
>> Is this correct, or am I missing something?
>>
>> Best regards,
>> Sönke
>>
>> On Thu, Feb 7, 2019 at 2:47 PM Stanislav Kozlovski
>>  wrote:
>> >
>> > This KIP seems to duplicate some of the functionality proposed in
>> KIP-248
>> > <
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-248+-+Create+New+ConfigCommand+That+Uses+The+New+AdminClient
>> >.
>> > KIP-248 has been stuck in a vote thread since July 2018.
>> >
>> > Viktor, do you plan on working on the KIP?
>> >
>> > On Thu, Feb 7, 2019 at 1:27 PM Stanislav Kozlovski <
>> stanis...@confluent.io>
>> > wrote:
>> >
>> > > Hey there Yaodong, thanks for the KIP!
>> > >
>> > > I'm not too familiar with the user/client configurations we currently
>> > > allow, is there a KIP describing the initial feature? If there is, it
>> would
>> > > be useful to include in KIP-422.
>> > >
>> > > I also didn't see any authorization in the PR, have we thought about
>> > > needing to authorize the alter/describe requests per the user/client?
>> > >
>> > > Thanks,
>> > > Stanislav
>> > >
>> > > On Fri, Jan 25, 2019 at 5:47 PM Yaodong Yang > >
>> > > wrote:
>> > >
>> > >> Hi folks,
>> > >>
>> > >> I've published KIP-422 which is about adding support for user/client
>> > >> configurations in the Kafka Admin Client.
>> > >>
>> > >> Basically the story here is to allow KafkaAdminClient to configure
>> the
>> > >> user
>> > >> or client configurations for users, instead of requiring users to
>> directly
>> > >> talk to ZK.
>> > >>
>> > >> The link for this KIP is
>> > >> following:
>> > >>
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97555704
>> > >>
>> > >> I'd be happy to receive some feedback about the KIP I published.
>> > >>
>> > >> --
>> > >> Best,
>> > >> Yaodong Yang
>> > >>
>> > >
>> > >
>> > > --
>> > > Best,
>> > > Stanislav
>> > >
>> >
>> >
>> > --
>> > Best,
>> > Stanislav
>>
>>
>>
>> --
>> Sönke Liebau
>> Partner
>> Tel. +49 179 7940878
>> OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany
>>
>

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi-Vass]

Hi Everyone,

Sorry for dropping the ball on this. I'd like to discard this KIP as there
were some overlapping works in the meanwhile and I think now some design
decisions could be made differently. I'll try to revamp this and take the
parts that are not implemented yet and compile them into smaller KIPs.

Viktor

On Thu, Jul 5, 2018 at 6:28 PM Rajini Sivaram 
wrote:

> Hi Magnus,
>
> Thanks for pointing that out. I haven't been keeping up-to-date with all
> the changes and I have no idea how we got here. I had requested the change
> to use SET/ADD/DELETE way back in February (
>
> http://mail-archives.apache.org/mod_mbox/kafka-dev/201802.mbox/%3cCAOJcB39amNDgf+8EO5fyA0JvpcUEQZBHu=ujhasj4zhjf-b...@mail.gmail.com%3e
> )
> and I thought the KIP was updated. Non-incremental updates are pretty much
> useless for broker config updates, and the KIP as-is doesn't work for
> broker configs until ADD/DELETE is reintroduced. DescribeConfigs doesn't
> return sensitive configs, so apart from atomicity, any sequence of changes
> that requires non-incremental updates simply doesn't work for broker
> configs.
>
> Hi Viktor, Are you still working on this KIP?
>
> Regards,
>
> Rajini
>
> On Wed, Jul 4, 2018 at 12:14 PM, Magnus Edenhill 
> wrote:
>
> > There are some concerns about the incremental option that needs to be
> > discussed further.
> >
> > I believe everyone agrees on the need for incremental updates, allowing a
> > client
> > to only alter the configuration it provides in an atomic fashion.
> > The proposal adds a request-level incremental bool for this purpose,
> which
> > is good.
> >
> > But I suspect this might not be enough, and thus suggest that we should
> > extend
> > the per-config-entry struct with a mode field that tells the broker how
> to
> > alter
> > the given configuration entry:
> >  - append - append value to entry (if no previous value it acts like set)
> >  - set - overwrite value
> >  - delete - delete configuration entry / revert to default.
> >
> > If we don't do this, the incremental mode can only be used in "append"
> > mode,
> > and a client that wishes to overwrite property A, delete B, and append to
> > C,
> > will need to issue three requests:
> >  - 1. DescribeConfigs to get the current config.
> >  - 2. AlterConfigs(incremental=False) to overwrite config property A and
> > delete B.
> >  - 3. AlterConfigs(incremental=True) to append to config property C.
> >
> > This makes the configuration update non-atomic, which incremental is set
> > out to fix,
> > any configuration changes made by another client between 1 and 2 would be
> > lost at 2.
> >
> >
> > This also needs to be exposed in the Admin API to make the user intention
> > clear,
> > ConfigEntry should be extended with a new constructor that takes the mode
> > parameter: append, set, or delete.
> > The existing constructor should default to set/overwrite (as in the
> > existing pre-incremental case).
> > If an application issues an AlterConfigs() request with append or delete
> > ConfigEntrys and the broker does not support KIP-248,
> > the request should fail locally in the client.
> >
> > For reference, this is how it is exposed in the corresponding C API:
> > https://github.com/edenhill/librdkafka/blob/master/src/rdkafka.h#L5200
> >
> >
> >
> > 2018-07-04 11:28 GMT+02:00 Rajini Sivaram :
> >
> > > Hi Viktor,
> > >
> > > Where are we with this KIP? Is it just waiting for votes? We should try
> > and
> > > get this in earlier in the release cycle this time.
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> > > On Mon, May 21, 2018 at 7:44 AM, Viktor Somogyi <
> viktorsomo...@gmail.com
> > >
> > > wrote:
> > >
> > > > Hi All,
> > > >
> > > > I'd like to ask the community to please vote for this as the KIP
> > > > freeze is tomorrow.
> > > >
> > > > Thank you very much,
> > > > Viktor
> > > >
> > > > On Mon, May 21, 2018 at 9:39 AM, Viktor Somogyi <
> > viktorsomo...@gmail.com
> > > >
> > > > wrote:
> > > > > Hi Colin,
> > > > >
> > > > > Sure, I'll add a note.
> > > > > Thanks for your vote.
> > > > >
> > > > > Viktor
> > > > >
> > > > > On Sat, May 19, 2018 at 1:01 AM, Colin McCabe 
> > > > wrote:
> > > > >> Hi Viktor,
> > > > >>
> > > > >> Thanks, this looks good.
> > > > >>
> > > > >> The boolean should default to false if not set, to ensure that
> > > existing
> > > > clients continue to work as-is, right?  Might be good to add a note
> > > > specifying that.
> > > > >>
> > > > >> +1 (non-binding)
> > > > >>
> > > > >> best,
> > > > >> Colin
> > > > >>
> > > > >> On Fri, May 18, 2018, at 08:16, Viktor Somogyi wrote:
> > > > >>> Updated KIP-248:
> > > > >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > 248+-+Create+New+ConfigCommand+That+Uses+The+New+AdminClient
> > > > >>>
> > > > >>> I'd like to ask project members, committers and contributors to
> > vote
> > > > >>> as this would be a useful improvement in Kafka.
> > > > >>>
> > > > >>> Sections changed:
> > > > >>> - Public interfaces: added the 

Re: [ANNOUNCE] New Committer: Randall Hauch

[Viktor Somogyi-Vass]

Congrats Randall! :)

On Fri, Feb 15, 2019 at 10:15 AM Satish Duggana 
wrote:

> Congratulations Randall!
>
> On Fri, Feb 15, 2019 at 1:51 PM Mickael Maison 
> wrote:
> >
> > Congrats Randall!
> >
> > On Fri, Feb 15, 2019 at 6:37 AM James Cheng 
> wrote:
> > >
> > > Congrats, Randall! Well deserved!
> > >
> > > -James
> > >
> > > Sent from my iPhone
> > >
> > > > On Feb 14, 2019, at 6:16 PM, Guozhang Wang 
> wrote:
> > > >
> > > > Hello all,
> > > >
> > > > The PMC of Apache Kafka is happy to announce another new committer
> joining
> > > > the project today: we have invited Randall Hauch as a project
> committer and
> > > > he has accepted.
> > > >
> > > > Randall has been participating in the Kafka community for the past 3
> years,
> > > > and is well known as the founder of the Debezium project, a popular
> project
> > > > for database change-capture streams using Kafka (https://debezium.io).
> More
> > > > recently he has become the main person keeping Kafka Connect moving
> > > > forward, participated in nearly all KIP discussions and QAs on the
> mailing
> > > > list. He's authored 6 KIPs and authored 50 pull requests and
> conducted over
> > > > a hundred reviews around Kafka Connect, and has also been
> evangelizing
> > > > Kafka Connect at several Kafka Summit venues.
> > > >
> > > >
> > > > Thank you very much for your contributions to the Connect community
> Randall
> > > > ! And looking forward to many more :)
> > > >
> > > >
> > > > Guozhang, on behalf of the Apache Kafka PMC
>

Re: [ANNOUNCE] New Committer: Bill Bejeck

[Viktor Somogyi-Vass]

Congrats Bill! :)

On Thu, Feb 14, 2019 at 4:50 PM Dongjin Lee  wrote:

> Congrats Bill, and thank you again for your great book and contributions on
> Kafka Streams!
>
> Best,
> Dongjin
>
> On Thu, 14 Feb 2019 at 7:31 PM Kamal Chandraprakash <
> kamal.chandraprak...@gmail.com> wrote:
>
> > Congratulations Bill!
> >
> > On Thu, Feb 14, 2019 at 3:55 PM Ivan Ponomarev
>  > >
> > wrote:
> >
> > > Congratulations, Bill!
> > >
> > > Your 'Kafka Streams in Action' is a great book. These months it is
> > > always travelling with me in my backpack with my laptop ))
> > >
> > > Regards,
> > >
> > > Ivan
> > >
> > > 14.02.2019 3:56, Guozhang Wang пишет:
> > > > Hello all,
> > > >
> > > > The PMC of Apache Kafka is happy to announce that we've added Bill
> > Bejeck
> > > > as our newest project committer.
> > > >
> > > > Bill has been active in the Kafka community since 2015. He has made
> > > > significant contributions to the Kafka Streams project with more than
> > 100
> > > > PRs and 4 authored KIPs, including the streams topology optimization
> > > > framework. Bill's also very keen on tightening Kafka's unit test /
> > system
> > > > tests coverage, which is a great value to our project codebase.
> > > >
> > > > In addition, Bill has been very active in evangelizing Kafka for
> stream
> > > > processing in the community. He has given several Kafka meetup talks
> in
> > > the
> > > > past year, including a presentation at Kafka Summit SF. He's also
> > > authored
> > > > a book about Kafka Streams (
> > > > https://www.manning.com/books/kafka-streams-in-action), as well as
> > > various
> > > > of posts in public venues like DZone as well as his personal blog (
> > > > http://codingjunkie.net/).
> > > >
> > > > We really appreciate the contributions and are looking forward to see
> > > more
> > > > from him. Congratulations, Bill !
> > > >
> > > >
> > > > Guozhang, on behalf of the Apache Kafka PMC
> > > >
> > >
> > >
> >
> --
> *Dongjin Lee*
>
> *A hitchhiker in the mathematical world.*
> *github:  github.com/dongjinleekr
> linkedin: kr.linkedin.com/in/dongjinleekr
> speakerdeck:
> speakerdeck.com/dongjin
> *
>

Re: [VOTE] 2.1.1 RC1

[Viktor Somogyi-Vass]

Hi,

Ran the ducktapes but the streams upgrade tests failed because the dev
version was not updated. This will be fixed in
https://github.com/apache/kafka/pull/6217. Once it's merged I'll rerun them.

Viktor

On Wed, 30 Jan 2019, 22:19 Jonathan Santilli  Hello,
>
> I have downloaded the source code for tag *2.1.1-rc1* (667980043). Executed
> integration and unit tests:
>
> *BUILD SUCCESSFUL in 25m 34s*
> *136 actionable tasks: 133 executed, 3 up-to-date*
>
>
> Also, I have downloaded the artifact from
> http://home.apache.org/~cmccabe/kafka-2.1.1-rc1/kafka_2.11-2.1.1.tgz and
> ran a cluster of 3 Brokers.
> Kept Kafka-monitor running for about 1 hour without any issues.
>
> +1
>
> Thanks for the release Colin.
> --
> Jonathan Santilli
>
>
> On Wed, Jan 30, 2019 at 8:18 PM Eno Thereska 
> wrote:
>
> > I couldn't repro locally, that was on an m3.large. And it's not happening
> > anymore. Might be a transient issue.
> >
> > Thanks,
> > Eno
> >
> > On Wed, Jan 30, 2019 at 6:46 PM Colin McCabe  wrote:
> >
> > > (+all lists)
> > >
> > > Hi Eno,
> > >
> > > Thanks for testing this.
> > >
> > > Those tests passed in the Jenkins build we did here:
> > > https://builds.apache.org/job/kafka-2.1-jdk8/118/
> > >
> > > Perhaps there is an environment issue at play here?  Do you get the
> same
> > > failures running those tests on the 2.1 release?
> > >
> > > Best,
> > > Colin
> > >
> > > On Wed, Jan 30, 2019, at 09:11, Eno Thereska wrote:
> > > > Hi Colin,
> > > >
> > > > I've been running the tests and so far I get the following failures.
> > Are
> > > > they known?
> > > >
> > > > kafka.server.ReplicaManagerQuotasTest >
> > > shouldGetBothMessagesIfQuotasAllow
> > > > FAILED
> > > > kafka.server.ReplicaManagerQuotasTest >
> > > > testCompleteInDelayedFetchWithReplicaThrottling FAILED
> > > > kafka.server.ReplicaManagerQuotasTest >
> > > > shouldExcludeSubsequentThrottledPartitions FAILED
> > > > kafka.server.ReplicaManagerQuotasTest >
> > > > shouldGetNoMessagesIfQuotasExceededOnSubsequentPartitions FAILED
> > > > kafka.server.ReplicaManagerQuotasTest >
> > > > shouldIncludeInSyncThrottledReplicas FAILED
> > > >
> > > > Thanks
> > > > Eno
> > > >
> > > > On Sun, Jan 27, 2019 at 9:46 PM Colin McCabe 
> > wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > This is the second candidate for release of Apache Kafka 2.1.1.
> This
> > > > > release includes many bug fixes for Apache Kafka 2.1.
> > > > >
> > > > > Compared to rc0, this release includes the following changes:
> > > > > * MINOR: Upgrade ducktape to 0.7.5 (#6197)
> > > > > * KAFKA-7837: Ensure offline partitions are picked up as soon as
> > > possible
> > > > > when shrinking ISR
> > > > > * tests/kafkatest/__init__.py now contains __version__ = '2.1.1'
> > rather
> > > > > than '2.1.1.dev0'
> > > > > * Maven artifacts should be properly staged this time
> > > > > * I have added my GPG key to https://kafka.apache.org/KEYS
> > > > >
> > > > > Check out the release notes here:
> > > > > http://home.apache.org/~cmccabe/kafka-2.1.1-rc1/RELEASE_NOTES.html
> > > > >
> > > > > The vote will go until Friday, February 1st.
> > > > >
> > > > > * Release artifacts to be voted upon (source and binary):
> > > > > http://home.apache.org/~cmccabe/kafka-2.1.1-rc1/
> > > > >
> > > > > * Maven artifacts to be voted upon:
> > > > > https://repository.apache.org/content/groups/staging/
> > > > >
> > > > > * Javadoc:
> > > > > http://home.apache.org/~cmccabe/kafka-2.1.1-rc1/javadoc/
> > > > >
> > > > > * Tag to be voted upon (off 2.1 branch) is the 2.1.1 tag:
> > > > > https://github.com/apache/kafka/releases/tag/2.1.1-rc1
> > > > >
> > > > > * Successful Jenkins builds for the 2.1 branch:
> > > > > Unit/integration tests:
> > > https://builds.apache.org/job/kafka-2.1-jdk8/118/
> > > > >
> > > > > thanks,
> > > > > Colin
> > > > >
> > > >
> > >
> >
>
>
> --
> Santilli Jonathan
>

Re: [DISCUSS] KIP-435: Incremental Partition Reassignment

[Viktor Somogyi-Vass]

Fri, Apr 5, 2019, at 19:34, George Li wrote:
> > > > > >  Hi Jason / Viktor,
> > > > > >
> > > > > > For the URP during a reassignment,  if the "original_replicas" is
> > kept
> > > > > > for the current pending reassignment. I think it will be very
> easy
> > to
> > > > > > compare that with the topic/partition's ISR.  If all
> > > > > > "original_replicas" are in ISR, then URP should be 0 for that
> > > > > > topic/partition.
> > > > > >
> > > > > > It would be also nice to separate the metrics MaxLag/TotalLag for
> > > > > > Reassignments. I think that will also require "original_replicas"
> > (the
> > > > > > topic/partition's replicas just before reassignment when the AR
> > > > > > (Assigned Replicas) is set to Set(original_replicas) +
> > > > > > Set(new_replicas_in_reassign_partitions) ).
> > > > > >
> > > > > > Thanks,
> > > > > > George
> > > > > >
> > > > > > On Friday, April 5, 2019, 6:29:55 PM PDT, Jason Gustafson
> > > > > >  wrote:
> > > > > >
> > > > > >  Hi Viktor,
> > > > > >
> > > > > > Thanks for writing this up. As far as questions about overlap
> with
> > > > > KIP-236,
> > > > > > I agree it seems mostly orthogonal. I think KIP-236 may have had
> a
> > > > larger
> > > > > > initial scope, but now it focuses on cancellation and batching is
> > left
> > > > > for
> > > > > > future work.
> > > > > >
> > > > > > With that said, I think we may not actually need a KIP for the
> > current
> > > > > > proposal since it doesn't change any APIs. To make it more
> > generally
> > > > > > useful, however, it would be nice to handle batching at the
> > partition
> > > > > level
> > > > > > as well as Jun suggests. The basic question is at what level
> > should the
> > > > > > batching be determined. You could rely on external processes
> (e.g.
> > > > cruise
> > > > > > control) or it could be built into the controller. There are
> > tradeoffs
> > > > > > either way, but I think it simplifies such tools if it is handled
> > > > > > internally. Then it would be much safer to submit a larger
> > reassignment
> > > > > > even just using the simple tools that come with Kafka.
> > > > > >
> > > > > > By the way, since you are looking into some of the reassignment
> > logic,
> > > > > > another problem that we might want to address is the misleading
> > way we
> > > > > > report URPs during a reassignment. I had a naive proposal for
> this
> > > > > > previously, but it didn't really work
> > > > > >
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-352%3A+Distinguish+URPs+caused+by+reassignment
> > > > > .
> > > > > > Potentially fixing that could fall under this work as well if you
> > think
> > > > > > it
> > > > > > makes sense.
> > > > > >
> > > > > > Best,
> > > > > > Jason
> > > > > >
> > > > > > On Thu, Apr 4, 2019 at 4:49 PM Jun Rao  wrote:
> > > > > >
> > > > > > > Hi, Viktor,
> > > > > > >
> > > > > > > Thanks for the KIP. A couple of comments below.
> > > > > > >
> > > > > > > 1. Another potential thing to do reassignment incrementally is
> to
> > > > move
> > > > > a
> > > > > > > batch of partitions at a time, instead of all partitions. This
> > may
> > > > > lead to
> > > > > > > less data replication since by the time the first batch of
> > partitions
> > > > > have
> > > > > > > been completely moved, some data of the next batch may have
> been
> > > > > deleted
> > > > > > > due to retention and doesn't need to be replicated.
> > > > > > >
> > > > > > > 2. "Update CR in Zookeeper with TR for th

Re: [DISCUSS] KIP-435: Incremental Partition Reassignment

[Viktor Somogyi-Vass]

A follow up on the batching topic to clarify my points above.

Generally I think that batching should be a core feature as Colin said the
controller should possess all information that are related.
Also Cruise Control (or really any 3rd party admin system) might build upon
this to give more holistic approach to balance brokers. We may cater them
with APIs that act like building blocks to make their life easier like
incrementalization, batching, cancellation and rollback but I think the
more advanced we go we'll need more advanced control surface and Kafka's
basic tooling might not be suitable for that.

Best,
Viktor


On Mon, 15 Apr 2019, 18:22 Viktor Somogyi-Vass, 
wrote:

> Hey Guys,
>
> I'll reply to you all in this email:
>
> @Jun:
> 1. yes, it'd be a good idea to add this feature, I'll write this into the
> KIP. I was actually thinking about introducing a dynamic config called
> reassignment.parallel.partition.count and
> reassignment.parallel.replica.count. The first property would control how
> many partition reassignment can we do concurrently. The second would go one
> level in granularity and would control how many replicas do we want to move
> for a given partition. Also one more thing that'd be useful to fix is that
> a given list of partition -> replica list would be executed in the same
> order (from first to last) so it's overall predictable and the user would
> have some control over the order of reassignments should be specified as
> the JSON is still assembled by the user.
> 2. the /kafka/brokers/topics/{topic} znode to be specific. I'll update the
> KIP to contain this.
>
> @Jason:
> I think building this functionality into Kafka would definitely benefit
> all the users and that CC as well as it'd simplify their software as you
> said. As I understand the main advantage of CC and other similar softwares
> are to give high level features for automatic load balancing. Reliability,
> stability and predictability of the reassignment should be a core feature
> of Kafka. I think the incrementalization feature would make it more stable.
> I would consider cancellation too as a core feature and we can leave the
> gate open for external tools to feed in their reassignment json as they
> want. I was also thinking about what are the set of features we can provide
> for Kafka but I think the more advanced we go the more need there is for an
> administrative UI component.
> Regarding KIP-352: Thanks for pointing this out, I didn't see this
> although lately I was also thinking about the throttling aspect of it.
> Would be a nice add-on to Kafka since though the above configs provide some
> level of control, it'd be nice to put an upper cap on the bandwidth and
> make it monitorable.
>
> Viktor
>
> On Wed, Apr 10, 2019 at 2:57 AM Jason Gustafson 
> wrote:
>
>> Hi Colin,
>>
>> On a related note, what do you think about the idea of storing the
>> > reassigning replicas in
>> > /brokers/topics/[topic]/partitions/[partitionId]/state, rather than in
>> the
>> > reassignment znode?  I don't think this requires a major change to the
>> > proposal-- when the controller becomes aware that it should do a
>> > reassignment, the controller could make the changes.  This also helps
>> keep
>> > the reassignment znode from getting larger, which has been a problem.
>>
>>
>> Yeah, I think it's a good idea to store the reassignment state at a finer
>> level. I'm not sure the LeaderAndIsr znode is the right one though.
>> Another
>> option is /brokers/topics/{topic}. That is where we currently store the
>> replica assignment. I think we basically want to represent both the
>> current
>> state and the desired state. This would also open the door to a cleaner
>> way
>> to update a reassignment while it is still in progress.
>>
>> -Jason
>>
>>
>>
>>
>> On Mon, Apr 8, 2019 at 11:14 PM George Li > .invalid>
>> wrote:
>>
>> >  Hi Colin / Jason,
>> >
>> > Reassignment should really be doing a batches.  I am not too worried
>> about
>> > reassignment znode getting larger.  In a real production environment,
>> too
>> > many concurrent reassignment and too frequent submission of
>> reassignments
>> > seemed to cause latency spikes of kafka cluster.  So
>> > batching/staggering/throttling of submitting reassignments is
>> recommended.
>> >
>> > In KIP-236,  The "originalReplicas" are only kept for the current
>> > reassigning partitions (small #), and kept in memory of the controller
>> > context partitionsBeingReassigned as well as in the znode
>> > /admi

Re: [ANNOUNCE] Apache Kafka 2.2.1

[Viktor Somogyi-Vass]

Thanks Vahid!

On Tue, Jun 4, 2019 at 5:20 PM Colin McCabe  wrote:

> Thanks, Vahid.
>
> best,
> Colin
>
> On Mon, Jun 3, 2019, at 07:23, Vahid Hashemian wrote:
> > The Apache Kafka community is pleased to announce the release for Apache
> > Kafka 2.2.1
> >
> > This is a bugfix release for Kafka 2.2.0. All of the changes in this
> > release can be found in the release notes:
> > https://www.apache.org/dist/kafka/2.2.1/RELEASE_NOTES.html
> >
> > You can download the source and binary release from:
> > https://kafka.apache.org/downloads#2.2.1
> >
> >
> ---
> >
> > Apache Kafka is a distributed streaming platform with four core APIs:
> >
> > ** The Producer API allows an application to publish a stream records to
> > one or more Kafka topics.
> >
> > ** The Consumer API allows an application to subscribe to one or more
> > topics and process the stream of records produced to them.
> >
> > ** The Streams API allows an application to act as a stream processor,
> > consuming an input stream from one or more topics and producing an output
> > stream to one or more output topics, effectively transforming the input
> > streams to output streams.
> >
> > ** The Connector API allows building and running reusable producers or
> > consumers that connect Kafka topics to existing applications or data
> > systems. For example, a connector to a relational database might capture
> > every change to a table.
> >
> > With these APIs, Kafka can be used for two broad classes of application:
> >
> > ** Building real-time streaming data pipelines that reliably get data
> > between systems or applications.
> >
> > ** Building real-time streaming applications that transform or react to
> the
> > streams of data.
> >
> > Apache Kafka is in use at large and small companies worldwide, including
> > Capital One, Goldman Sachs, ING, LinkedIn, Netflix, Pinterest, Rabobank,
> > Target, The New York Times, Uber, Yelp, and Zalando, among others.
> >
> > A big thank you for the following 30 contributors to this release!
> >
> > Anna Povzner, Arabelle Hou, A. Sophie Blee-Goldman, Bill Bejeck, Bob
> > Barrett, Chris Egerton, Colin Patrick McCabe, Cyrus Vafadari, Dhruvil
> Shah,
> > Doroszlai, Attila, Guozhang Wang, huxi, Jason Gustafson, John Roesler,
> > Konstantine Karantasis, Kristian Aurlien, Lifei Chen, Magesh Nandakumar,
> > Manikumar Reddy, Massimo Siani, Matthias J. Sax, Nicholas Parker,
> pkleindl,
> > Rajini Sivaram, Randall Hauch, Sebastián Ortega, Vahid Hashemian,
> Victoria
> > Bialas, Yaroslav Klymko, Zhanxiang (Patrick) Huang
> >
> > We welcome your help and feedback. For more information on how to report
> > problems, and to get involved, visit the project website at
> > https://kafka.apache.org/
> >
> > Thank you!
> >
> > Regards,
> > --Vahid Hashemian
> >
>

Re: [VOTE] KIP-434: Dead replica fetcher and log cleaner metrics

[Viktor Somogyi-Vass]

Hi Folks,

This vote sunk a bit, I'd like to draw some attention to this again in the
hope I get some feedback or votes.

Thanks,
Viktor

On Tue, May 7, 2019 at 4:28 PM Harsha  wrote:

> Thanks for the kip. LGTM +1.
>
> -Harsha
>
> On Mon, Apr 29, 2019, at 8:14 AM, Viktor Somogyi-Vass wrote:
> > Hi Jason,
> >
> > I too agree this is more of a problem in older versions and therefore we
> > could backport it. Were you thinking of any specific versions? I guess
> the
> > 2.x and 1.x versions are definitely targets here but I was thinking that
> we
> > might not want to further.
> >
> > Viktor
> >
> > On Mon, Apr 29, 2019 at 12:55 AM Stanislav Kozlovski <
> stanis...@confluent.io>
> > wrote:
> >
> > > Thanks for the work done, Viktor! +1 (non-binding)
> > >
> > > I strongly agree with Jason that this monitoring-focused KIP is worth
> > > porting back to older versions. I am sure users will find it very
> useful
> > >
> > > Best,
> > > Stanislav
> > >
> > > On Fri, Apr 26, 2019 at 9:38 PM Jason Gustafson 
> > > wrote:
> > >
> > > > Thanks, that works for me. +1
> > > >
> > > > By the way, we don't normally port KIPs to older releases, but I
> wonder
> > > if
> > > > it's worth making an exception here. From recent experience, it
> tends to
> > > be
> > > > the older versions that are more prone to fetcher failures. Thoughts?
> > > >
> > > > -Jason
> > > >
> > > > On Fri, Apr 26, 2019 at 5:18 AM Viktor Somogyi-Vass <
> > > > viktorsomo...@gmail.com>
> > > > wrote:
> > > >
> > > > > Let me have a second thought, I'll just add the clientId instead to
> > > > follow
> > > > > the convention, so it'll change DeadFetcherThreadCount but with the
> > > > > clientId tag.
> > > > >
> > > > > On Fri, Apr 26, 2019 at 11:29 AM Viktor Somogyi-Vass <
> > > > > viktorsomo...@gmail.com> wrote:
> > > > >
> > > > > > Hi Jason,
> > > > > >
> > > > > > Yea I think it could make sense. In this case I would rename the
> > > > > > DeadFetcherThreadCount to DeadReplicaFetcherThreadCount and
> introduce
> > > > the
> > > > > > metric you're referring to as DeadLogDirFetcherThreadCount.
> > > > > > I'll update the KIP to reflect this.
> > > > > >
> > > > > > Viktor
> > > > > >
> > > > > > On Thu, Apr 25, 2019 at 8:07 PM Jason Gustafson <
> ja...@confluent.io>
> > > > > > wrote:
> > > > > >
> > > > > >> Hi Viktor,
> > > > > >>
> > > > > >> This looks good. Just one question I had is whether we may as
> well
> > > > cover
> > > > > >> the log dir fetchers as well.
> > > > > >>
> > > > > >> Thanks,
> > > > > >> Jason
> > > > > >>
> > > > > >>
> > > > > >> On Thu, Apr 25, 2019 at 7:46 AM Viktor Somogyi-Vass <
> > > > > >> viktorsomo...@gmail.com>
> > > > > >> wrote:
> > > > > >>
> > > > > >> > Hi Folks,
> > > > > >> >
> > > > > >> > This thread sunk a bit but I'd like to bump it hoping to get
> some
> > > > > >> feedback
> > > > > >> > and/or votes.
> > > > > >> >
> > > > > >> > Thanks,
> > > > > >> > Viktor
> > > > > >> >
> > > > > >> > On Thu, Mar 28, 2019 at 8:47 PM Viktor Somogyi-Vass <
> > > > > >> > viktorsomo...@gmail.com>
> > > > > >> > wrote:
> > > > > >> >
> > > > > >> > > Sorry, the end of the message cut off.
> > > > > >> > >
> > > > > >> > > So I tried to be consistent with the convention in
> LogManager,
> > > > hence
> > > > > >> the
> > > > > >> > > hyphens and in AbstractFetcherManager, hence the camel
> case. It
> > > > > would
> > > > > >> be
> > > > > >> > > nice thoug

Re: [DISCUSS] KIP-435: Incremental Partition Reassignment

[Viktor Somogyi-Vass]

Hi All,

I have added another improvement to this, which is to limit the parallel
leader movements. I think I'll soon (maybe late this week or early next)
start a vote on this too if there are no additional feedback.

Thanks,
Viktor

On Mon, Apr 29, 2019 at 1:26 PM Viktor Somogyi-Vass 
wrote:

> Hi Folks,
>
> I've updated the KIP with the batching which would work on both replica
> and partition level. To explain it briefly: for instance if the replica
> level is set to 2 and partition level is set to 3, then 2x3=6 replica
> reassignment would be in progress at the same time. In case of reassignment
> for a single partition from (0, 1, 2, 3, 4) to (5, 6, 7, 8, 9) we would
> form the batches (0, 1) → (5, 6); (2, 3) → (7, 8) and 4 → 9 and would
> execute the reassignment in this order.
>
> Let me know what you think.
>
> Best,
> Viktor
>
> On Mon, Apr 15, 2019 at 7:01 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
>> A follow up on the batching topic to clarify my points above.
>>
>> Generally I think that batching should be a core feature as Colin said
>> the controller should possess all information that are related.
>> Also Cruise Control (or really any 3rd party admin system) might build
>> upon this to give more holistic approach to balance brokers. We may cater
>> them with APIs that act like building blocks to make their life easier like
>> incrementalization, batching, cancellation and rollback but I think the
>> more advanced we go we'll need more advanced control surface and Kafka's
>> basic tooling might not be suitable for that.
>>
>> Best,
>> Viktor
>>
>>
>> On Mon, 15 Apr 2019, 18:22 Viktor Somogyi-Vass, 
>> wrote:
>>
>>> Hey Guys,
>>>
>>> I'll reply to you all in this email:
>>>
>>> @Jun:
>>> 1. yes, it'd be a good idea to add this feature, I'll write this into
>>> the KIP. I was actually thinking about introducing a dynamic config called
>>> reassignment.parallel.partition.count and
>>> reassignment.parallel.replica.count. The first property would control how
>>> many partition reassignment can we do concurrently. The second would go one
>>> level in granularity and would control how many replicas do we want to move
>>> for a given partition. Also one more thing that'd be useful to fix is that
>>> a given list of partition -> replica list would be executed in the same
>>> order (from first to last) so it's overall predictable and the user would
>>> have some control over the order of reassignments should be specified as
>>> the JSON is still assembled by the user.
>>> 2. the /kafka/brokers/topics/{topic} znode to be specific. I'll update
>>> the KIP to contain this.
>>>
>>> @Jason:
>>> I think building this functionality into Kafka would definitely benefit
>>> all the users and that CC as well as it'd simplify their software as you
>>> said. As I understand the main advantage of CC and other similar softwares
>>> are to give high level features for automatic load balancing. Reliability,
>>> stability and predictability of the reassignment should be a core feature
>>> of Kafka. I think the incrementalization feature would make it more stable.
>>> I would consider cancellation too as a core feature and we can leave the
>>> gate open for external tools to feed in their reassignment json as they
>>> want. I was also thinking about what are the set of features we can provide
>>> for Kafka but I think the more advanced we go the more need there is for an
>>> administrative UI component.
>>> Regarding KIP-352: Thanks for pointing this out, I didn't see this
>>> although lately I was also thinking about the throttling aspect of it.
>>> Would be a nice add-on to Kafka since though the above configs provide some
>>> level of control, it'd be nice to put an upper cap on the bandwidth and
>>> make it monitorable.
>>>
>>> Viktor
>>>
>>> On Wed, Apr 10, 2019 at 2:57 AM Jason Gustafson 
>>> wrote:
>>>
>>>> Hi Colin,
>>>>
>>>> On a related note, what do you think about the idea of storing the
>>>> > reassigning replicas in
>>>> > /brokers/topics/[topic]/partitions/[partitionId]/state, rather than
>>>> in the
>>>> > reassignment znode?  I don't think this requires a major change to the
>>>> > proposal-- when the controller becomes aware that it should do a
>>>> > reassignment, the controller could make the changes.  This also helps
&

[jira] [Created] (KAFKA-8566) Force Topic Deletion

[Viktor Somogyi-Vass (JIRA)]

Viktor Somogyi-Vass created KAFKA-8566:
--

 Summary: Force Topic Deletion
 Key: KAFKA-8566
 URL: https://issues.apache.org/jira/browse/KAFKA-8566
 Project: Kafka
  Issue Type: Improvement
Affects Versions: 2.3.0
Reporter: Viktor Somogyi-Vass
Assignee: Viktor Somogyi-Vass


Forcing topic deletion is sometimes useful if normal topic deletion gets 
stucked. In this case we want to remove the topic data from zookeeper anyway 
and also the segment files from the online brokers. On offline brokers it could 
be done on startup somehow.



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

Re: [VOTE] KIP-455: Create an Administrative API for Replica Reassignment

[Viktor Somogyi-Vass]

+1 (non-binding)

Thanks Colin, this is great stuff. Does a jira (or maybe even a PR :) ) for
this exist yet?

Viktor

On Thu, May 9, 2019 at 7:23 AM Colin McCabe  wrote:

> Hi all,
>
> I'd like to start the vote for KIP-455: Create an Administrative API for
> Replica Reassignment.  I think this KIP is important since it will unlock
> many follow-on improvements to Kafka reassignment (see the "Future work"
> section, plus a lot of the other discussions we've had recently about
> reassignment).  It also furthers the important KIP-4 goal of removing
> direct access to ZK.
>
> I made a few changes based on the discussion in the [DISCUSS] thread.  As
> Robert suggested, I removed the need to explicitly cancel a reassignment
> for a partition before setting up a different reassignment for that
> specific partition.  I also simplified the API a bit by adding a
> PartitionReassignment class which is used by both the alter and list APIs.
>
> I modified the proposal so that we now deprecate the old znode-based API
> rather than removing it completely.  That should give external rebalancing
> tools some time to transition to the new API.
>
> To clarify a question Viktor asked, I added a note that the
> kafka-reassign-partitions.sh will now use a --bootstrap-server argument to
> contact the admin APIs.
>
> thanks,
> Colin
>

Re: [DISCUSS] KIP-435: Incremental Partition Reassignment

[Viktor Somogyi-Vass]

Hi Folks,

I've updated the KIP with the batching which would work on both replica and
partition level. To explain it briefly: for instance if the replica level
is set to 2 and partition level is set to 3, then 2x3=6 replica
reassignment would be in progress at the same time. In case of reassignment
for a single partition from (0, 1, 2, 3, 4) to (5, 6, 7, 8, 9) we would
form the batches (0, 1) → (5, 6); (2, 3) → (7, 8) and 4 → 9 and would
execute the reassignment in this order.

Let me know what you think.

Best,
Viktor

On Mon, Apr 15, 2019 at 7:01 PM Viktor Somogyi-Vass 
wrote:

> A follow up on the batching topic to clarify my points above.
>
> Generally I think that batching should be a core feature as Colin said the
> controller should possess all information that are related.
> Also Cruise Control (or really any 3rd party admin system) might build
> upon this to give more holistic approach to balance brokers. We may cater
> them with APIs that act like building blocks to make their life easier like
> incrementalization, batching, cancellation and rollback but I think the
> more advanced we go we'll need more advanced control surface and Kafka's
> basic tooling might not be suitable for that.
>
> Best,
> Viktor
>
>
> On Mon, 15 Apr 2019, 18:22 Viktor Somogyi-Vass, 
> wrote:
>
>> Hey Guys,
>>
>> I'll reply to you all in this email:
>>
>> @Jun:
>> 1. yes, it'd be a good idea to add this feature, I'll write this into the
>> KIP. I was actually thinking about introducing a dynamic config called
>> reassignment.parallel.partition.count and
>> reassignment.parallel.replica.count. The first property would control how
>> many partition reassignment can we do concurrently. The second would go one
>> level in granularity and would control how many replicas do we want to move
>> for a given partition. Also one more thing that'd be useful to fix is that
>> a given list of partition -> replica list would be executed in the same
>> order (from first to last) so it's overall predictable and the user would
>> have some control over the order of reassignments should be specified as
>> the JSON is still assembled by the user.
>> 2. the /kafka/brokers/topics/{topic} znode to be specific. I'll update
>> the KIP to contain this.
>>
>> @Jason:
>> I think building this functionality into Kafka would definitely benefit
>> all the users and that CC as well as it'd simplify their software as you
>> said. As I understand the main advantage of CC and other similar softwares
>> are to give high level features for automatic load balancing. Reliability,
>> stability and predictability of the reassignment should be a core feature
>> of Kafka. I think the incrementalization feature would make it more stable.
>> I would consider cancellation too as a core feature and we can leave the
>> gate open for external tools to feed in their reassignment json as they
>> want. I was also thinking about what are the set of features we can provide
>> for Kafka but I think the more advanced we go the more need there is for an
>> administrative UI component.
>> Regarding KIP-352: Thanks for pointing this out, I didn't see this
>> although lately I was also thinking about the throttling aspect of it.
>> Would be a nice add-on to Kafka since though the above configs provide some
>> level of control, it'd be nice to put an upper cap on the bandwidth and
>> make it monitorable.
>>
>> Viktor
>>
>> On Wed, Apr 10, 2019 at 2:57 AM Jason Gustafson 
>> wrote:
>>
>>> Hi Colin,
>>>
>>> On a related note, what do you think about the idea of storing the
>>> > reassigning replicas in
>>> > /brokers/topics/[topic]/partitions/[partitionId]/state, rather than in
>>> the
>>> > reassignment znode?  I don't think this requires a major change to the
>>> > proposal-- when the controller becomes aware that it should do a
>>> > reassignment, the controller could make the changes.  This also helps
>>> keep
>>> > the reassignment znode from getting larger, which has been a problem.
>>>
>>>
>>> Yeah, I think it's a good idea to store the reassignment state at a finer
>>> level. I'm not sure the LeaderAndIsr znode is the right one though.
>>> Another
>>> option is /brokers/topics/{topic}. That is where we currently store the
>>> replica assignment. I think we basically want to represent both the
>>> current
>>> state and the desired state. This would also open the door to a cleaner
>>> way
>>> to update a reassignment while it is still in progress.
>&g

Re: [VOTE] KIP-434: Dead replica fetcher and log cleaner metrics

[Viktor Somogyi-Vass]

Hi Jason,

I too agree this is more of a problem in older versions and therefore we
could backport it. Were you thinking of any specific versions? I guess the
2.x and 1.x versions are definitely targets here but I was thinking that we
might not want to further.

Viktor

On Mon, Apr 29, 2019 at 12:55 AM Stanislav Kozlovski 
wrote:

> Thanks for the work done, Viktor! +1 (non-binding)
>
> I strongly agree with Jason that this monitoring-focused KIP is worth
> porting back to older versions. I am sure users will find it very useful
>
> Best,
> Stanislav
>
> On Fri, Apr 26, 2019 at 9:38 PM Jason Gustafson 
> wrote:
>
> > Thanks, that works for me. +1
> >
> > By the way, we don't normally port KIPs to older releases, but I wonder
> if
> > it's worth making an exception here. From recent experience, it tends to
> be
> > the older versions that are more prone to fetcher failures. Thoughts?
> >
> > -Jason
> >
> > On Fri, Apr 26, 2019 at 5:18 AM Viktor Somogyi-Vass <
> > viktorsomo...@gmail.com>
> > wrote:
> >
> > > Let me have a second thought, I'll just add the clientId instead to
> > follow
> > > the convention, so it'll change DeadFetcherThreadCount but with the
> > > clientId tag.
> > >
> > > On Fri, Apr 26, 2019 at 11:29 AM Viktor Somogyi-Vass <
> > > viktorsomo...@gmail.com> wrote:
> > >
> > > > Hi Jason,
> > > >
> > > > Yea I think it could make sense. In this case I would rename the
> > > > DeadFetcherThreadCount to DeadReplicaFetcherThreadCount and introduce
> > the
> > > > metric you're referring to as DeadLogDirFetcherThreadCount.
> > > > I'll update the KIP to reflect this.
> > > >
> > > > Viktor
> > > >
> > > > On Thu, Apr 25, 2019 at 8:07 PM Jason Gustafson 
> > > > wrote:
> > > >
> > > >> Hi Viktor,
> > > >>
> > > >> This looks good. Just one question I had is whether we may as well
> > cover
> > > >> the log dir fetchers as well.
> > > >>
> > > >> Thanks,
> > > >> Jason
> > > >>
> > > >>
> > > >> On Thu, Apr 25, 2019 at 7:46 AM Viktor Somogyi-Vass <
> > > >> viktorsomo...@gmail.com>
> > > >> wrote:
> > > >>
> > > >> > Hi Folks,
> > > >> >
> > > >> > This thread sunk a bit but I'd like to bump it hoping to get some
> > > >> feedback
> > > >> > and/or votes.
> > > >> >
> > > >> > Thanks,
> > > >> > Viktor
> > > >> >
> > > >> > On Thu, Mar 28, 2019 at 8:47 PM Viktor Somogyi-Vass <
> > > >> > viktorsomo...@gmail.com>
> > > >> > wrote:
> > > >> >
> > > >> > > Sorry, the end of the message cut off.
> > > >> > >
> > > >> > > So I tried to be consistent with the convention in LogManager,
> > hence
> > > >> the
> > > >> > > hyphens and in AbstractFetcherManager, hence the camel case. It
> > > would
> > > >> be
> > > >> > > nice though to decide with one convention across the whole
> > project,
> > > >> > however
> > > >> > > it requires a major refactor (especially for the components that
> > > >> leverage
> > > >> > > metrics for monitoring).
> > > >> > >
> > > >> > > Thanks,
> > > >> > > Viktor
> > > >> > >
> > > >> > > On Thu, Mar 28, 2019 at 8:44 PM Viktor Somogyi-Vass <
> > > >> > > viktorsomo...@gmail.com> wrote:
> > > >> > >
> > > >> > >> Hi Dhruvil,
> > > >> > >>
> > > >> > >> Thanks for the feedback and the vote. I fixed the typo in the
> > KIP.
> > > >> > >> The naming is interesting though. Unfortunately kafka overall
> is
> > > not
> > > >> > >> consistent in metric naming but at least I tried to be
> consistent
> > > >> among
> > > >> > the
> > > >> > >> other metrics used in LogManager
> > > >> > >>
> > > >> > >> On Thu, Mar 28, 2019 at 7:32 PM Dhruvil Shah <
&g

[jira] [Created] (KAFKA-8330) Separate Replica and Reassignment Throttling

[Viktor Somogyi-Vass (JIRA)]

Viktor Somogyi-Vass created KAFKA-8330:
--

 Summary: Separate Replica and Reassignment Throttling
 Key: KAFKA-8330
 URL: https://issues.apache.org/jira/browse/KAFKA-8330
 Project: Kafka
  Issue Type: Improvement
  Components: core, replication
Affects Versions: 2.3.0
Reporter: Viktor Somogyi-Vass
Assignee: Viktor Somogyi-Vass


Currently Kafka doesn't separate reassignment related replication from ISR 
replication. That is dangerous because if replication is throttled during 
reassignment and some replicas fall out of ISR they could get further behind 
while reassignment is also slower. We need a method that throttles reassignment 
related replication only.

A KIP is in progress for this.



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

Re: Cleaning up command line tools argument parsing a little

[Viktor Somogyi-Vass]

Just to add: I wasn't too pushy by raising a KIP for this as so far I had
the experience that the community is fine with them or at least people
learned to live with the current tools but if there's a need I'd be happy
to jump back working on it or helping you if you have time :)

On Wed, May 8, 2019 at 11:35 AM Viktor Somogyi-Vass 
wrote:

> Hi Sönke,
>
> In KAFKA-1774 I have created a Kafka Shell java tool that unfortunately
> didn't get much attention so far from the creators of the jira. It works
> similarly to what Colin mentioned, like "kafka.sh topics create -n my-topic
> -p 3 -r 3" or has long names like "kafka.sh topics create --name my-topic
> --partitions 3 --replicas 3". The bootstrap server everywhere defaults to
> :9092 or reads up the configs from a path so in many scenarios you can omit
> it, also it uses the admin client of course and all in all provides a much
> better experience than the current tools. It has interactive mode and help
> too. Wanted to implement "code completion" too but for that I'd have to
> exercise the code a little bit more :).
> It is currently based on a quite old trunk but if you think it's
> interesting I can rebase it and we can continue with raising a KIP.
> https://github.com/viktorsomogyi/kafka/tree/kafka_shell
>
> Viktor
>
> On Tue, May 7, 2019 at 11:10 AM Sönke Liebau
>  wrote:
>
>> Hi Colin,
>>
>> thanks!
>> While I personally don't like the current command line tools I
>> realistically think we'll be stuck with them for a while yet, so a cleanup
>> might make sense.
>> So I'll start looking into that.
>>
>> Regarding a central entrypoint, that would indeed be brilliant and I'd
>> love
>> to work on that, but I currently have enough other open things to look at,
>> so I won't draw that one to myself as well for now.
>>
>> But I'll keep it in mind for when some time frees up :)
>>
>> Best regards,
>> Sönke
>>
>>
>>
>> Colin McCabe  schrieb am Di., 7. Mai 2019, 00:56:
>>
>> > On Mon, May 6, 2019, at 10:21, Sönke Liebau wrote:
>> > > Hi Colin,
>> > >
>> > > it was my intention to keep the structure of the commands mostly
>> intact
>> > > while doing the refactoring - if that is possible, have not really
>> > checked
>> > > yet to be honest.
>> > >
>> > > But what I wanted to try and do is recreate the current parsing with
>> > > argparse as much as possible. And in the process simply adding
>> synonyms,
>> > > for example make the kafka-console-producer understand a
>> > > bootstrap-parameter in addition to broker-list.
>> > > There is a bit of custom logic about which parameters go together
>> etc. in
>> > > the current classes, so output may look different here and there, but
>> in
>> > > principle I do believe that it should be possible to recreate the
>> current
>> > > structure.
>> >
>> > Sounds like a good idea.  Thanks for the clarification.
>> >
>> > >
>> > > If there is an appetite for a new, hadoop-like entrypoint anyway, then
>> > all
>> > > of this might be "wasted" effort, or rather effort better spent
>> though,
>> > you
>> > > are right.
>> >
>> > I don't think anyone is working on a new entry point right now -- or if
>> > they are, they haven't said anything yet :)
>> >
>> > I just wanted to mention it as a possible approach in case you wanted to
>> > do a bigger project.
>> >
>> > best,
>> > Colin
>> >
>> > >
>> > > Best regards,
>> > > Sönke
>> > >
>> > >
>> > >
>> > > On Mon, May 6, 2019 at 7:13 PM Colin McCabe 
>> wrote:
>> > >
>> > > > Hi Sönke,
>> > > >
>> > > > #2 is a bit tough because people have come to rely on the way the
>> > commands
>> > > > are structured right now.
>> > > >
>> > > > If we want to make big changes, it might be easier just to create a
>> > > > separate tool and deprecate the old one(s).  One thing we've talked
>> > about
>> > > > doing in the past is creating a single entry point for all the tool
>> > > > functionality, kind of like hadoop did with the "hadoop" command  Or
>> > git
>> > > > with the "git" command, etc.  Then we could deprecate the standalone
>> >

Re: Cleaning up command line tools argument parsing a little

[Viktor Somogyi-Vass]

Hi Sönke,

In KAFKA-1774 I have created a Kafka Shell java tool that unfortunately
didn't get much attention so far from the creators of the jira. It works
similarly to what Colin mentioned, like "kafka.sh topics create -n my-topic
-p 3 -r 3" or has long names like "kafka.sh topics create --name my-topic
--partitions 3 --replicas 3". The bootstrap server everywhere defaults to
:9092 or reads up the configs from a path so in many scenarios you can omit
it, also it uses the admin client of course and all in all provides a much
better experience than the current tools. It has interactive mode and help
too. Wanted to implement "code completion" too but for that I'd have to
exercise the code a little bit more :).
It is currently based on a quite old trunk but if you think it's
interesting I can rebase it and we can continue with raising a KIP.
https://github.com/viktorsomogyi/kafka/tree/kafka_shell

Viktor

On Tue, May 7, 2019 at 11:10 AM Sönke Liebau
 wrote:

> Hi Colin,
>
> thanks!
> While I personally don't like the current command line tools I
> realistically think we'll be stuck with them for a while yet, so a cleanup
> might make sense.
> So I'll start looking into that.
>
> Regarding a central entrypoint, that would indeed be brilliant and I'd love
> to work on that, but I currently have enough other open things to look at,
> so I won't draw that one to myself as well for now.
>
> But I'll keep it in mind for when some time frees up :)
>
> Best regards,
> Sönke
>
>
>
> Colin McCabe  schrieb am Di., 7. Mai 2019, 00:56:
>
> > On Mon, May 6, 2019, at 10:21, Sönke Liebau wrote:
> > > Hi Colin,
> > >
> > > it was my intention to keep the structure of the commands mostly intact
> > > while doing the refactoring - if that is possible, have not really
> > checked
> > > yet to be honest.
> > >
> > > But what I wanted to try and do is recreate the current parsing with
> > > argparse as much as possible. And in the process simply adding
> synonyms,
> > > for example make the kafka-console-producer understand a
> > > bootstrap-parameter in addition to broker-list.
> > > There is a bit of custom logic about which parameters go together etc.
> in
> > > the current classes, so output may look different here and there, but
> in
> > > principle I do believe that it should be possible to recreate the
> current
> > > structure.
> >
> > Sounds like a good idea.  Thanks for the clarification.
> >
> > >
> > > If there is an appetite for a new, hadoop-like entrypoint anyway, then
> > all
> > > of this might be "wasted" effort, or rather effort better spent though,
> > you
> > > are right.
> >
> > I don't think anyone is working on a new entry point right now -- or if
> > they are, they haven't said anything yet :)
> >
> > I just wanted to mention it as a possible approach in case you wanted to
> > do a bigger project.
> >
> > best,
> > Colin
> >
> > >
> > > Best regards,
> > > Sönke
> > >
> > >
> > >
> > > On Mon, May 6, 2019 at 7:13 PM Colin McCabe 
> wrote:
> > >
> > > > Hi Sönke,
> > > >
> > > > #2 is a bit tough because people have come to rely on the way the
> > commands
> > > > are structured right now.
> > > >
> > > > If we want to make big changes, it might be easier just to create a
> > > > separate tool and deprecate the old one(s).  One thing we've talked
> > about
> > > > doing in the past is creating a single entry point for all the tool
> > > > functionality, kind of like hadoop did with the "hadoop" command  Or
> > git
> > > > with the "git" command, etc.  Then we could deprecate the standalone
> > > > commands and remove them after enough time had passed-- kind of like
> > the
> > > > old consumer.
> > > >
> > > > On the other hand, a more incremental change would be standardizing
> > flags
> > > > a bit.  So for example, at least setting it up so that there is a
> > standard
> > > > way of supplying bootstrap brokers, etc.  We could keep the old flags
> > > > around for a while as variants to ease the transition.
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Sun, May 5, 2019, at 00:54, Sönke Liebau wrote:
> > > > > Hi Colin,
> > > > >
> > > > > I totally agree! Especially the differently named bootstrap server
> > > > options
> > > > > have been annoying me a long time.
> > > > >
> > > > > I'd propose a two-step approach:
> > > > > 1. Add new default options objects similar to CommandLineUtils and
> > > > > CommandDefaultOptions (based on argparse4j) but in the clients
> > project,
> > > > as
> > > > > this is referenced by all command line tools as far as I can tell
> > > > > 2. Refactor tools one by one to use these new helper classes (and
> > thus
> > > > > argparse) and add standardized synonyms for parameters as necessary
> > > > >
> > > > > I think for step 1 we can get away with no KIP, as this doesn't
> > change
> > > > any
> > > > > public interfaces or behavior.
> > > > > Step 2 probably needs a KIP as we are adding new parameters? We can
> > pick
> > > > up
> > > > > KIP-14 again 

Re: [DISCUSS] KIP-455: Create an Administrative API for Replica Reassignment

[Viktor Somogyi-Vass]

Hi Colin,

Thanks for explaining all this, it makes sense.

Viktor

On Sun, May 5, 2019 at 8:18 AM Colin McCabe  wrote:

> On Thu, May 2, 2019, at 09:35, Viktor Somogyi-Vass wrote:
> > Hey Colin & George,
> >
> > Thinking on George's points I was wondering if it's feasible to submit a
> > big reassignment to the controller and thus Zookeeper as frequent writes
> > are slow as the quorum has to synchronize. Perhaps it should be the
> > responsibility of KIP-435 <https://issues.apache.org/jira/browse/KIP-435>
> but
> > I'd like to note it here as we're changing the current znode layout in
> this
> > KIP.
>
> Hi Viktor,
>
> This is similar conceptually to if we lose a broker from the cluster.  In
> that case, we have to remove that node from the ISR of all the partitions
> it has, which means updating O(partitions_on_node) znodes.  It's also
> similar to completing a reassignment in the existing Kafka version, and
> updating the partition znodes to reflect new nodes joining the ISR for
> various partitions.  While you are right that ZK is a low-bandwidth system,
> in general writing, to a few thousand ZNodes over the course of a second or
> two is OK.
>
> The existing reassignment znode requires the whole plan to fit within a
> single znode.  The maximum znodes size of 1 megabyte by default, and almost
> nobody reconfigures this.  Assuming about 100 bytes per reassignment, we
> can't get many more than about 10,000 partitions in a reassignment today in
> any case.  The current scalability bottleneck is much more on the side of
> "can kafka actually handle a huge amount of extra traffic due to ongoing
> reassignments"?
>
> That does bring up a good point, though-- we may want to have a "maximum
> concurrent reassignments" to avoid a common scenario that happens now,
> where people accidentally submit a plan that's way too big.  But this is
> not to protect ZooKeeper-- it is to protect the brokers.
>
> > I think ideally we should add these writes in batches to zookeeper and
> > otherwise store it in a replicated internal topic
> > (__partition_reassignments). That would solve the scalability problem as
> > the failover controller would be able to read it up very quickly and also
> > we would spread the writes in Zookeeper over time. Just the current,
> > actively replicated partitions should be present under
> > /brokers/topics/[topic]/partitions/[partitionId]/state, so those
> partitions
> > will know if they have to do reassignment (even in case of a broker
> > bounce). The controller on the other hand could regain its state by
> reading
> > up the last produced message from this __partition_reassignments topic
> and
> > reading up the Zookeeper state to figure out which batch its currently
> > doing (supposing it goes sequentially in the given reassignment).
>
> As I wrote in my reply to the other email, this is not needed because
> we're not adding any controller startup overhead beyond what already
> exists.  We do have some plans to optimize this, but it's outside the scope
> of this KIP.
>
> > I'll think a little bit more about this to fill out any gaps there are
> and
> > perhaps add it to my KIP. That being said probably we'll need to make
> some
> > benchmarking first if this bulk read-write causes a problem at all to
> avoid
> > premature optimisation. I generally don't really worry about reading up
> > this new information as the controller would read up the assignment
> anyway
> > in initializeControllerContext().
>
> Right, the controller will read those znodes on startup anyway.
>
> >
> > A question on SubmitPartitionReassignmentsRequest and its connection with
> > KIP-435 <https://cwiki.apache.org/confluence/display/KAFKA/KIP-435>.
> Would
> > the list of topic-partitions have the same ordering on the client side as
> > well as the broker side? I think it would be an advantage as the user
> would
> > know in which order the reassignment would be performed. I think it's
> > useful when it comes to incrementalization as they'd be able to figure
> out
> > what replicas will be in one batch (given they know about the batch
> size).
>
> The big advantage of doing batching on the controller is that the
> controller has more information about what is going on in the cluster.  So
> it can schedule reassignments in a more optimal way.  For instance, it can
> schedule reassignments so that the load is distributed evenly across
> nodes.  This advantage is lost if we have to adhere to a rigid ordering
> that is set up in advance.  We don't know exactly when anything will
> complete in any case.  Just

Re: [DISCUSS] KIP-455: Create an Administrative API for Replica Reassignment

[Viktor Somogyi-Vass]

Hey Colin & George,

Thinking on George's points I was wondering if it's feasible to submit a
big reassignment to the controller and thus Zookeeper as frequent writes
are slow as the quorum has to synchronize. Perhaps it should be the
responsibility of KIP-435  but
I'd like to note it here as we're changing the current znode layout in this
KIP.
I think ideally we should add these writes in batches to zookeeper and
otherwise store it in a replicated internal topic
(__partition_reassignments). That would solve the scalability problem as
the failover controller would be able to read it up very quickly and also
we would spread the writes in Zookeeper over time. Just the current,
actively replicated partitions should be present under
/brokers/topics/[topic]/partitions/[partitionId]/state, so those partitions
will know if they have to do reassignment (even in case of a broker
bounce). The controller on the other hand could regain its state by reading
up the last produced message from this __partition_reassignments topic and
reading up the Zookeeper state to figure out which batch its currently
doing (supposing it goes sequentially in the given reassignment).
I'll think a little bit more about this to fill out any gaps there are and
perhaps add it to my KIP. That being said probably we'll need to make some
benchmarking first if this bulk read-write causes a problem at all to avoid
premature optimisation. I generally don't really worry about reading up
this new information as the controller would read up the assignment anyway
in initializeControllerContext().

A question on SubmitPartitionReassignmentsRequest and its connection with
KIP-435 . Would
the list of topic-partitions have the same ordering on the client side as
well as the broker side? I think it would be an advantage as the user would
know in which order the reassignment would be performed. I think it's
useful when it comes to incrementalization as they'd be able to figure out
what replicas will be in one batch (given they know about the batch size).

Viktor

On Wed, May 1, 2019 at 8:33 AM George Li 
wrote:

>  Hi Colin,
>
> Thanks for KIP-455!  yes. KIP-236, etc. will depend on it.  It is the good
> direction to go for the RPC.
>
> Regarding storing the new reassignments & original replicas at the
> topic/partition level.  I have some concerns when controller is failing
> over, and the scalability of scanning the active reassignments from ZK
> topic/partition level nodes. Please see my reply to Jason in the KIP-236
> thread.
>
> Once the decision is made where new reassignment and original replicas is
> stored, I will modify KIP-236 accordingly for how to cancel/rollback the
> reassignments.
>
> Thanks,
> George
>
>
> On Monday, April 15, 2019, 6:07:44 PM PDT, Colin McCabe <
> cmcc...@apache.org> wrote:
>
>  Hi all,
>
> We've been having discussions on a few different KIPs (KIP-236, KIP-435,
> etc.) about what the Admin Client replica reassignment API should look
> like.  The current API is really hard to extend and maintain, which is a
> big source of problems.  I think it makes sense to have a KIP that
> establishes a clean API that we can use and extend going forward, so I
> posted KIP-455.  Take a look.  :)
>
> best,
> Colin
>

Re: [VOTE] KIP-434: Dead replica fetcher and log cleaner metrics

[Viktor Somogyi-Vass]

Let me have a second thought, I'll just add the clientId instead to follow
the convention, so it'll change DeadFetcherThreadCount but with the
clientId tag.

On Fri, Apr 26, 2019 at 11:29 AM Viktor Somogyi-Vass <
viktorsomo...@gmail.com> wrote:

> Hi Jason,
>
> Yea I think it could make sense. In this case I would rename the
> DeadFetcherThreadCount to DeadReplicaFetcherThreadCount and introduce the
> metric you're referring to as DeadLogDirFetcherThreadCount.
> I'll update the KIP to reflect this.
>
> Viktor
>
> On Thu, Apr 25, 2019 at 8:07 PM Jason Gustafson 
> wrote:
>
>> Hi Viktor,
>>
>> This looks good. Just one question I had is whether we may as well cover
>> the log dir fetchers as well.
>>
>> Thanks,
>> Jason
>>
>>
>> On Thu, Apr 25, 2019 at 7:46 AM Viktor Somogyi-Vass <
>> viktorsomo...@gmail.com>
>> wrote:
>>
>> > Hi Folks,
>> >
>> > This thread sunk a bit but I'd like to bump it hoping to get some
>> feedback
>> > and/or votes.
>> >
>> > Thanks,
>> > Viktor
>> >
>> > On Thu, Mar 28, 2019 at 8:47 PM Viktor Somogyi-Vass <
>> > viktorsomo...@gmail.com>
>> > wrote:
>> >
>> > > Sorry, the end of the message cut off.
>> > >
>> > > So I tried to be consistent with the convention in LogManager, hence
>> the
>> > > hyphens and in AbstractFetcherManager, hence the camel case. It would
>> be
>> > > nice though to decide with one convention across the whole project,
>> > however
>> > > it requires a major refactor (especially for the components that
>> leverage
>> > > metrics for monitoring).
>> > >
>> > > Thanks,
>> > > Viktor
>> > >
>> > > On Thu, Mar 28, 2019 at 8:44 PM Viktor Somogyi-Vass <
>> > > viktorsomo...@gmail.com> wrote:
>> > >
>> > >> Hi Dhruvil,
>> > >>
>> > >> Thanks for the feedback and the vote. I fixed the typo in the KIP.
>> > >> The naming is interesting though. Unfortunately kafka overall is not
>> > >> consistent in metric naming but at least I tried to be consistent
>> among
>> > the
>> > >> other metrics used in LogManager
>> > >>
>> > >> On Thu, Mar 28, 2019 at 7:32 PM Dhruvil Shah 
>> > >> wrote:
>> > >>
>> > >>> Thanks for the KIP, Viktor! This is a useful addition. +1 overall.
>> > >>>
>> > >>> Minor nits:
>> > >>> > I propose to add three gauge: DeadFetcherThreadCount for the
>> fetcher
>> > >>> threads, log-cleaner-dead-thread-count for the log cleaner.
>> > >>> I think you meant two instead of three.
>> > >>>
>> > >>> Also, would it make sense to name these metrics consistency,
>> something
>> > >>> like
>> > >>> `log-cleaner-dead-thread-count` and
>> > `replica-fetcher-dead-thread-count`?
>> > >>>
>> > >>> Thanks,
>> > >>> Dhruvil
>> > >>>
>> > >>> On Thu, Mar 28, 2019 at 11:27 AM Viktor Somogyi-Vass <
>> > >>> viktorsomo...@gmail.com> wrote:
>> > >>>
>> > >>> > Hi All,
>> > >>> >
>> > >>> > I'd like to start a vote on KIP-434.
>> > >>> > This basically would add a metrics to count dead threads in
>> > >>> > ReplicaFetcherManager and LogCleaner to allow monitoring systems
>> to
>> > >>> alert
>> > >>> > based on this.
>> > >>> >
>> > >>> > The KIP link:
>> > >>> >
>> > >>> >
>> > >>>
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics
>> > >>> > The
>> > >>> > PR: https://github.com/apache/kafka/pull/6514
>> > >>> >
>> > >>> > I'd be happy to receive any votes or additional feedback/reviews
>> too.
>> > >>> >
>> > >>> > Thanks,
>> > >>> > Viktor
>> > >>> >
>> > >>>
>> > >>
>> >
>>
>

Re: [DISCUSS] KIP-435: Incremental Partition Reassignment

[Viktor Somogyi-Vass]

Hi Colin,

Certainly there will be some interaction and good idea with that you said,
I've added it to my KIP.
Will start a new discussion thread and link this one.

Viktor

On Wed, Jun 26, 2019 at 11:39 PM Colin McCabe  wrote:

> Hi Viktor,
>
> Good point.  Sorry, I should have read the KIP more closely.
>
> It would be good to change the title of the mail thread to reflect the new
> title of the KIP, "Internal Partition Reassignment Batching."
>
> I do think there will be some interaction with KIP-455 here.  One example
> is that we'll want a way of knowing what target replicas are currently
> being worked on.  So maybe we'll have to add a field to the structures
> returned by listPartitionReassignments.
>
> best,
> Colin
>
>
> On Wed, Jun 26, 2019, at 06:20, Viktor Somogyi-Vass wrote:
> > Hey Colin,
> >
> > I think there's some confusion here so I might change the name of this.
> So
> > KIP-435 is about the internal batching of reassignments (so purely a
> > controller change) and not about client side APIs. As per this moment
> these
> > kind of improvements are listed on KIP-455's future work section so in my
> > understanding KIP-455 won't touch that :).
> > Let me know if I'm missing any points here.
> >
> > Viktor
> >
> > On Tue, Jun 25, 2019 at 9:02 PM Colin McCabe  wrote:
> >
> > > Hi Viktor,
> > >
> > > Now that the 2.3 release is over, we're going to be turning our
> attention
> > > back to working on KIP-455, which provides an API for partition
> > > reassignment, and also solves the incremental reassignment problem.
> Sorry
> > > about the pause, but I had to focus on the stuff that was going into
> 2.3.
> > >
> > > I think last time we talked about this, the consensus was that KIP-455
> > > supersedes KIP-435, since KIP-455 supports incremental reassignment.
> We
> > > also don't want to add more technical debt in the form of a new
> > > ZooKeeper-based API that we'll have to support for a while.  So let's
> focus
> > > on KIP-455 here.  We have more resources now so I think we'll be able
> to
> > > get it done soonish.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Tue, Jun 25, 2019, at 08:09, Viktor Somogyi-Vass wrote:
> > > > Hi All,
> > > >
> > > > I have added another improvement to this, which is to limit the
> parallel
> > > > leader movements. I think I'll soon (maybe late this week or early
> next)
> > > > start a vote on this too if there are no additional feedback.
> > > >
> > > > Thanks,
> > > > Viktor
> > > >
> > > > On Mon, Apr 29, 2019 at 1:26 PM Viktor Somogyi-Vass <
> > > viktorsomo...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Folks,
> > > > >
> > > > > I've updated the KIP with the batching which would work on both
> replica
> > > > > and partition level. To explain it briefly: for instance if the
> replica
> > > > > level is set to 2 and partition level is set to 3, then 2x3=6
> replica
> > > > > reassignment would be in progress at the same time. In case of
> > > reassignment
> > > > > for a single partition from (0, 1, 2, 3, 4) to (5, 6, 7, 8, 9) we
> would
> > > > > form the batches (0, 1) → (5, 6); (2, 3) → (7, 8) and 4 → 9 and
> would
> > > > > execute the reassignment in this order.
> > > > >
> > > > > Let me know what you think.
> > > > >
> > > > > Best,
> > > > > Viktor
> > > > >
> > > > > On Mon, Apr 15, 2019 at 7:01 PM Viktor Somogyi-Vass <
> > > > > viktorsomo...@gmail.com> wrote:
> > > > >
> > > > >> A follow up on the batching topic to clarify my points above.
> > > > >>
> > > > >> Generally I think that batching should be a core feature as Colin
> said
> > > > >> the controller should possess all information that are related.
> > > > >> Also Cruise Control (or really any 3rd party admin system) might
> build
> > > > >> upon this to give more holistic approach to balance brokers. We
> may
> > > cater
> > > > >> them with APIs that act like building blocks to make their life
> > > easier like
> > > > >> incrementalization, batching, cancellation and rollback but I
> think
&g

[DISCUSS] KIP-435: Internal Partition Reassignment Batching

[Viktor Somogyi-Vass]

Hi All,

I've renamed my KIP as its name was a bit confusing so we'll continue it in
this thread.
The previous thread for record:
https://lists.apache.org/thread.html/0e97e30271f80540d4da1947bba94832639767e511a87bb2ba530fe7@%3Cdev.kafka.apache.org%3E

A short summary of the KIP:
In case of a vast partition reassignment (thousands of partitions at once)
Kafka can collapse under the increased replication traffic. This KIP will
mitigate it by introducing internal batching done by the controller.
Besides putting a bandwidth limit on the replication it is useful to batch
partition movements as fewer number of partitions will use the available
bandwidth for reassignment and they finish faster.
The main control handles are:
- the number of parallel leader movements,
- the number of parallel partition movements
- and the number of parallel replica movements.

Thank you for the feedback and ideas so far in the previous thread and I'm
happy to receive more.

Regards,
Viktor

Re: [DISCUSS] KIP-455: Create an Administrative API for Replica Reassignment

[Viktor Somogyi-Vass]

Hey Colin,

So in my understanding this is how the controller handles a reassignment in
a simple scenario:
1. When an AlterPartitionReassignmentRequest arrives it updates the
partitions' ZK data in
/brokers/topics/[topic]/partitions/[partitionId]/state with targetReplicas
2. Sends out LeaderAndIsr requests to the reassigned partitions to update
their targetReplica set
3. Starts the new replicas
4. Waits until a new replica comes in sync, mark them online
5. Sends out an UpdateMetadata request to the replicas that the new
partitions became ISR
6. Updates the partition state in ZK by removing the new ISR from the
targetReplica set
7. Sends out a LeaderAndIsr request with the targetReplicas not containing
the new ISR. This request may contain a change in leadership info if leader
change is needed
8. If the reassigned replica has to be deleted, then take them offline and
then non-existent
9. Send out an UpdateMetadata request with the new ISR information

Is this correct? Let me know if I missed something, I just wanted to
assemble the picture a little bit. I think it would be useful to add this
(or the corrected version if needed :) ) to the KIP with an example maybe
as Jason suggested. Would help a lot in understandability.

Regards,
Viktor

On Wed, Jun 26, 2019 at 11:12 PM Colin McCabe  wrote:

> On Wed, Jun 26, 2019, at 12:02, Jason Gustafson wrote:
> > Hi Colin,
> >
> > Responses below and another question:
> >
> > > I guess the thought process here is that most reassignment tools want
> to
> > > know about all the reassignments that are going on.  If you don't know
> all
> > > the pending reassignments, then it's hard to say whether adding a new
> one
> > > is a good idea, or cancelling an existing one.  So I guess I can't
> think of
> > > a case where a reassignment tool would want a partial set rather than
> the
> > > full one.
> >
> > UIs often have "drill into" options. If there is a large ongoing
> > reassignment, I can see wanting to limit the scope to a specific topic.
> Any
> > downside that I'm missing?
> >
>
> We could add a mode that only lists a given set of partitions.  To be
> consistent with how we handle topics, that could be a separate "describe"
> method.  I don't think there's any downside, except some extra code to
> write.
>
> > > Good question.  It will be the current behavior.  Basically, we
> immediately
> > > try to replicate to all the targetReplicas.  As they enter the ISR,
> they
> > > leave "targetReplicas" and enter "replicas."  Making this more
> incremental
> > > would be a good follow-on improvement.
> >
> > The current behavior is to wait until all target replicas are in the ISR
> > before making a change. Are you saying that we will keep this behavior?
>
> We'd remove nodes from targetReplicas just as soon as they entered the
> ISR.  They would become regular replicas at that point.
>
> >
> > > When the /admin/reassign_partitions znode is empty, we'll listen for
> > > updates.  When an update is made, we'll treat it like a call to
> > > AlterPartitionReassignments.  Whenever we have zero pending
> reassignments,
> > > we'll delete the /admin/reassign_partitions znode.  If the znode
> already
> > > exists, we don't listen on it (same as now).
> >
> >
> > So just to be a bit more explicit, what you are saying is that we will
> keep
> > the reassignment state under /admin/reassign_partitions as we do
> currently,
> > but we will update the target_replicas field in /partition/state
> following
> > this new logic. Then as soon as the current replica set matches the
> target
> > assignment, we will remove the /admin/reassign_partitions znode. Right?
>
> One clarification: I wasn't proposing that the controller should write to
> /admin/reassign_partitions.  We will just remove the znode when we
> transition to having no ongoing reassignments.  There's no guarantee that
> what is in the znode reflects the current reassignments that are going on.
> The only thing you can know is that if the znode exists, there is at least
> one reassignment going on.  But if someone makes a new reassignment with
> the AlterPartitionReassignments API, it won't appear in the znode.
>
> Another thing to note is that if the znode exists and you overwrite it,
> your updates will be ignored.  This matches the current behavior of this
> znode.  Apparently some applications don't know about this behavior and try
> to update the znode while a reassignment is going on, but it has no
> effect-- other than making what is in ZK misleading if someone checks.
> This is, again, existing behavior :(
>
> It's not a good API by any means.  For example, what if someone else
> overwrites your znode update before the controller has a chance to read
> it?  Unfortunate, but there's no really good way to fix this without
> transitioning away from direct ZooKeeper access.  We'll transition the
> command line tools immediately, but there will be some external management
> tools that will lag a bit.
>
> >
> > Actually I'm 

Re: [DISCUSS] KIP-435: Incremental Partition Reassignment

[Viktor Somogyi-Vass]

Hey Colin,

I think there's some confusion here so I might change the name of this. So
KIP-435 is about the internal batching of reassignments (so purely a
controller change) and not about client side APIs. As per this moment these
kind of improvements are listed on KIP-455's future work section so in my
understanding KIP-455 won't touch that :).
Let me know if I'm missing any points here.

Viktor

On Tue, Jun 25, 2019 at 9:02 PM Colin McCabe  wrote:

> Hi Viktor,
>
> Now that the 2.3 release is over, we're going to be turning our attention
> back to working on KIP-455, which provides an API for partition
> reassignment, and also solves the incremental reassignment problem.  Sorry
> about the pause, but I had to focus on the stuff that was going into 2.3.
>
> I think last time we talked about this, the consensus was that KIP-455
> supersedes KIP-435, since KIP-455 supports incremental reassignment.  We
> also don't want to add more technical debt in the form of a new
> ZooKeeper-based API that we'll have to support for a while.  So let's focus
> on KIP-455 here.  We have more resources now so I think we'll be able to
> get it done soonish.
>
> best,
> Colin
>
>
> On Tue, Jun 25, 2019, at 08:09, Viktor Somogyi-Vass wrote:
> > Hi All,
> >
> > I have added another improvement to this, which is to limit the parallel
> > leader movements. I think I'll soon (maybe late this week or early next)
> > start a vote on this too if there are no additional feedback.
> >
> > Thanks,
> > Viktor
> >
> > On Mon, Apr 29, 2019 at 1:26 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> > wrote:
> >
> > > Hi Folks,
> > >
> > > I've updated the KIP with the batching which would work on both replica
> > > and partition level. To explain it briefly: for instance if the replica
> > > level is set to 2 and partition level is set to 3, then 2x3=6 replica
> > > reassignment would be in progress at the same time. In case of
> reassignment
> > > for a single partition from (0, 1, 2, 3, 4) to (5, 6, 7, 8, 9) we would
> > > form the batches (0, 1) → (5, 6); (2, 3) → (7, 8) and 4 → 9 and would
> > > execute the reassignment in this order.
> > >
> > > Let me know what you think.
> > >
> > > Best,
> > > Viktor
> > >
> > > On Mon, Apr 15, 2019 at 7:01 PM Viktor Somogyi-Vass <
> > > viktorsomo...@gmail.com> wrote:
> > >
> > >> A follow up on the batching topic to clarify my points above.
> > >>
> > >> Generally I think that batching should be a core feature as Colin said
> > >> the controller should possess all information that are related.
> > >> Also Cruise Control (or really any 3rd party admin system) might build
> > >> upon this to give more holistic approach to balance brokers. We may
> cater
> > >> them with APIs that act like building blocks to make their life
> easier like
> > >> incrementalization, batching, cancellation and rollback but I think
> the
> > >> more advanced we go we'll need more advanced control surface and
> Kafka's
> > >> basic tooling might not be suitable for that.
> > >>
> > >> Best,
> > >> Viktor
> > >>
> > >>
> > >> On Mon, 15 Apr 2019, 18:22 Viktor Somogyi-Vass, <
> viktorsomo...@gmail.com>
> > >> wrote:
> > >>
> > >>> Hey Guys,
> > >>>
> > >>> I'll reply to you all in this email:
> > >>>
> > >>> @Jun:
> > >>> 1. yes, it'd be a good idea to add this feature, I'll write this into
> > >>> the KIP. I was actually thinking about introducing a dynamic config
> called
> > >>> reassignment.parallel.partition.count and
> > >>> reassignment.parallel.replica.count. The first property would
> control how
> > >>> many partition reassignment can we do concurrently. The second would
> go one
> > >>> level in granularity and would control how many replicas do we want
> to move
> > >>> for a given partition. Also one more thing that'd be useful to fix
> is that
> > >>> a given list of partition -> replica list would be executed in the
> same
> > >>> order (from first to last) so it's overall predictable and the user
> would
> > >>> have some control over the order of reassignments should be
> specified as
> > >>> the JSON is still assembled by the user.
> > >>> 2. the /kafka/brokers/topics/{topic} znode to b

Re: [DISCUSS] KIP-435: Internal Partition Reassignment Batching

[Viktor Somogyi-Vass]

 API, but the functionality is the same.
> You're 100% right that it is a form of reassignment, but I hadn't thought
> of it like that and I some other people wouldn't have either.
> If I understand correctly, you're suggesting that we count the
> "reassignment.parallel.leader.movements" config against such preferred
> elections. I think that makes sense. If we are to do that we should
> explicitly mention that this KIP touches the preferred election logic as
> well. Would that config also bound the number of leader movements the auto
> rebalance inside the broker would do as well?
>
> Then again, the "reassignment.parallel.leader.movements" config has a bit
> of a different meaning when used in the context of a real reassignment
> (where data moves from the leader to N more replicas) versus in the
> preferred election switch (where all we need is two lightweight
> LeaderAndIsr requests), so I am not entirely convinced we may want to use
> the same config for that.
> It might be easier to limit the scope of this KIP and not place a bound on
> preferred leader election.
>
> Thanks,
> Stanislav
>
>
> On Mon, Jul 8, 2019 at 4:28 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> wrote:
>
> > Hey Stanislav,
> >
> > Thanks for the thorough look at the KIP! :)
> >
> > > Let me first explicitly confirm my understanding of the configs and the
> > > algorithm:
> > > * reassignment.parallel.replica.count - the maximum total number of
> > > replicas that we can move at once, *per partition*
> > > * reassignment.parallel.partition.count - the maximum number of
> > partitions
> > > we can move at once
> > > * reassignment.parallel.leader.movements - the maximum number of leader
> > > movements we can have at once
> >
> > Yes.
> >
> > > As far as I currently understand it, your proposed algorithm will
> > naturally
> > > prioritize leader movement first. e.g if
> > > reassignment.parallel.replica.count=1 and
> > >
> >
> >
> reassignment.parallel.partition.count==reassignment.parallel.leader.movements,
> > > we will always move one, the first possible, replica in the replica set
> > > (which will be the leader if part of the excess replica set (ER)).
> > > Am I correct in saying that?
> >
> > Yes.
> >
> > > 1. Does it make sense to add `max` somewhere in the configs' names?
> >
> > If you imply that it's not always an exact number (because the last batch
> > could be smaller) than I think it's a good idea. I don't mind this naming
> > or having max in it either.
> >
> > > 2. How does this KIP play along with KIP-455's notion of multiple
> > > rebalances - do the configs apply on a
> > > single AlterPartitionAssignmentsRequest or are they global?
> >
> > I think it is intuitive if there are on a global level and the user would
> > have the option to set KIP-435's settings to Int.MAX and thus eliminate
> any
> > batching and submit their own batches through
> > AlterPartitionAssignmentsRequest. If we applied them on every batch then
> we
> > couldn't really guarantee any limits as the user would be able to get
> > around it with submitting lots of reassignments. By default though we set
> > the batching limits to Int.MAX so effectively they're disabled.
> > Also if I understand correctly, AlterPartitionAssignmentsRequest would
> be a
> > partition level batching, isn't it? So if you submit 10 partitions at
> once
> > then they'll all be started by the controller immediately as per my
> > understanding.
> >
> > > 3. Unless I've missed it, the algorithm does not take into account
> > > `reassignment.parallel.leader.movements`
> >
> > Yea the reassignment step calculation doesn't contain that because it
> > describes only one partition's reassignment step calculation. We simply
> > fill our reassignment batch with as many leader movements as we can and
> > then fill the rest with reassignments which don't require leader movement
> > if we have space. I'll create a pseudo code block on the KIP for this.
> >
> > > 4. The KIP says that the order of the input has some control over how
> the
> > > batches are created - i.e it's deterministic. What would the batches of
> > the
> > > following reassignment look like:
> > > reassignment.parallel.replica.count=1
> > > reassignment.parallel.partition.count=MAX_INT
> > > reassignment.parallel.leader.movements=1
> > > partitionA - (0,1,2) -> (3, 4, 5)
> > > partitionB

Re: [DISCUSS] KIP-435: Internal Partition Reassignment Batching

[Viktor Somogyi-Vass]

plicit
>
> As some small nits:
> - could we have a section in the KIP where we explicitly define what each
> config does? This can be inferred from the KIP as is but requires careful
> reading, whereas some developers might want to skim through the change to
> get a quick sense. It also improves readability but that's my personal
> opinion.
> - could you better clarify how a reassignment step is different from the
> currently existing algorithm? maybe laying both algorithms out in the KIP
> would be most clear
> - the names for the OngoingPartitionReassignment and
> CurrentPartitionReassignment fields in the
> ListPartitionReassignmentsResponse are a bit confusing to me.
> Unfortunately, I don't have a better suggestion, but maybe somebody else
in
> the community has.
>
> Thanks,
> Stanislav
>
> On Thu, Jun 27, 2019 at 3:24 PM Viktor Somogyi-Vass <
viktorsomo...@gmail.com>
> wrote:
>
> > Hi All,
> >
> > I've renamed my KIP as its name was a bit confusing so we'll continue
it in
> > this thread.
> > The previous thread for record:
> >
> >
https://lists.apache.org/thread.html/0e97e30271f80540d4da1947bba94832639767e511a87bb2ba530fe7@%3Cdev.kafka.apache.org%3E
> >
> > A short summary of the KIP:
> > In case of a vast partition reassignment (thousands of partitions at
once)
> > Kafka can collapse under the increased replication traffic. This KIP
will
> > mitigate it by introducing internal batching done by the controller.
> > Besides putting a bandwidth limit on the replication it is useful to
batch
> > partition movements as fewer number of partitions will use the available
> > bandwidth for reassignment and they finish faster.
> > The main control handles are:
> > - the number of parallel leader movements,
> > - the number of parallel partition movements
> > - and the number of parallel replica movements.
> >
> > Thank you for the feedback and ideas so far in the previous thread and
I'm
> > happy to receive more.
> >
> > Regards,
> > Viktor
> >
>
>
> --
> Best,
> Stanislav

Re: [VOTE] KIP-434: Dead replica fetcher and log cleaner metrics

[Viktor Somogyi-Vass]

Hi Folks,

This thread sunk a bit but I'd like to bump it hoping to get some feedback
and/or votes.

Thanks,
Viktor

On Thu, Mar 28, 2019 at 8:47 PM Viktor Somogyi-Vass 
wrote:

> Sorry, the end of the message cut off.
>
> So I tried to be consistent with the convention in LogManager, hence the
> hyphens and in AbstractFetcherManager, hence the camel case. It would be
> nice though to decide with one convention across the whole project, however
> it requires a major refactor (especially for the components that leverage
> metrics for monitoring).
>
> Thanks,
> Viktor
>
> On Thu, Mar 28, 2019 at 8:44 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
>> Hi Dhruvil,
>>
>> Thanks for the feedback and the vote. I fixed the typo in the KIP.
>> The naming is interesting though. Unfortunately kafka overall is not
>> consistent in metric naming but at least I tried to be consistent among the
>> other metrics used in LogManager
>>
>> On Thu, Mar 28, 2019 at 7:32 PM Dhruvil Shah 
>> wrote:
>>
>>> Thanks for the KIP, Viktor! This is a useful addition. +1 overall.
>>>
>>> Minor nits:
>>> > I propose to add three gauge: DeadFetcherThreadCount for the fetcher
>>> threads, log-cleaner-dead-thread-count for the log cleaner.
>>> I think you meant two instead of three.
>>>
>>> Also, would it make sense to name these metrics consistency, something
>>> like
>>> `log-cleaner-dead-thread-count` and `replica-fetcher-dead-thread-count`?
>>>
>>> Thanks,
>>> Dhruvil
>>>
>>> On Thu, Mar 28, 2019 at 11:27 AM Viktor Somogyi-Vass <
>>> viktorsomo...@gmail.com> wrote:
>>>
>>> > Hi All,
>>> >
>>> > I'd like to start a vote on KIP-434.
>>> > This basically would add a metrics to count dead threads in
>>> > ReplicaFetcherManager and LogCleaner to allow monitoring systems to
>>> alert
>>> > based on this.
>>> >
>>> > The KIP link:
>>> >
>>> >
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics
>>> > The
>>> > PR: https://github.com/apache/kafka/pull/6514
>>> >
>>> > I'd be happy to receive any votes or additional feedback/reviews too.
>>> >
>>> > Thanks,
>>> > Viktor
>>> >
>>>
>>

Re: [ANNOUNCE] New Kafka PMC member: Sriharsh Chintalapan

[Viktor Somogyi-Vass]

Congrats Harsha!

On Tue, Apr 23, 2019 at 6:13 AM Becket Qin  wrote:

> Congrats, Harsh!
>
> On Tue, Apr 23, 2019 at 5:41 AM Colin McCabe  wrote:
>
> > Congratulations, Harsh!
> >
> > cheers,
> > Colin
> >
> >
> > On Thu, Apr 18, 2019, at 11:46, Jun Rao wrote:
> > > Hi, Everyone,
> > >
> > > Sriharsh Chintalapan has been active in the Kafka community since he
> > became
> > > a Kafka committer in 2015. I am glad to announce that Harsh is now a
> > member
> > > of Kafka PM
> > >
> > > Congratulations, Harsh!
> > >
> > > Jun
> > >
> >
>

Re: [ANNOUNCE] New Kafka PMC member: Matthias J. Sax

[Viktor Somogyi-Vass]

Congrats Matthias!

On Tue, Apr 23, 2019 at 4:24 AM Becket Qin  wrote:

> Congrats, Matthias!
>
> On Sat, Apr 20, 2019 at 10:28 AM Matthias J. Sax 
> wrote:
>
> > Thank you all!
> >
> > -Matthias
> >
> >
> > On 4/19/19 3:58 PM, Lei Chen wrote:
> > > Congratulations Matthias! Well deserved!
> > >
> > > -Lei
> > >
> > > On Fri, Apr 19, 2019 at 2:55 PM James Cheng  > > > wrote:
> > >
> > > Congrats!!
> > >
> > > -James
> > >
> > > Sent from my iPhone
> > >
> > > > On Apr 18, 2019, at 2:35 PM, Guozhang Wang  > > > wrote:
> > > >
> > > > Hello Everyone,
> > > >
> > > > I'm glad to announce that Matthias J. Sax is now a member of
> Kafka
> > > PMC.
> > > >
> > > > Matthias has been a committer since Jan. 2018, and since then he
> > > continued
> > > > to be active in the community and made significant contributions
> > the
> > > > project.
> > > >
> > > >
> > > > Congratulations to Matthias!
> > > >
> > > > -- Guozhang
> > >
> >
> >
>

Re: [VOTE] KIP-434: Dead replica fetcher and log cleaner metrics

[Viktor Somogyi-Vass]

Hi Jason,

Yea I think it could make sense. In this case I would rename the
DeadFetcherThreadCount to DeadReplicaFetcherThreadCount and introduce the
metric you're referring to as DeadLogDirFetcherThreadCount.
I'll update the KIP to reflect this.

Viktor

On Thu, Apr 25, 2019 at 8:07 PM Jason Gustafson  wrote:

> Hi Viktor,
>
> This looks good. Just one question I had is whether we may as well cover
> the log dir fetchers as well.
>
> Thanks,
> Jason
>
>
> On Thu, Apr 25, 2019 at 7:46 AM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> wrote:
>
> > Hi Folks,
> >
> > This thread sunk a bit but I'd like to bump it hoping to get some
> feedback
> > and/or votes.
> >
> > Thanks,
> > Viktor
> >
> > On Thu, Mar 28, 2019 at 8:47 PM Viktor Somogyi-Vass <
> > viktorsomo...@gmail.com>
> > wrote:
> >
> > > Sorry, the end of the message cut off.
> > >
> > > So I tried to be consistent with the convention in LogManager, hence
> the
> > > hyphens and in AbstractFetcherManager, hence the camel case. It would
> be
> > > nice though to decide with one convention across the whole project,
> > however
> > > it requires a major refactor (especially for the components that
> leverage
> > > metrics for monitoring).
> > >
> > > Thanks,
> > > Viktor
> > >
> > > On Thu, Mar 28, 2019 at 8:44 PM Viktor Somogyi-Vass <
> > > viktorsomo...@gmail.com> wrote:
> > >
> > >> Hi Dhruvil,
> > >>
> > >> Thanks for the feedback and the vote. I fixed the typo in the KIP.
> > >> The naming is interesting though. Unfortunately kafka overall is not
> > >> consistent in metric naming but at least I tried to be consistent
> among
> > the
> > >> other metrics used in LogManager
> > >>
> > >> On Thu, Mar 28, 2019 at 7:32 PM Dhruvil Shah 
> > >> wrote:
> > >>
> > >>> Thanks for the KIP, Viktor! This is a useful addition. +1 overall.
> > >>>
> > >>> Minor nits:
> > >>> > I propose to add three gauge: DeadFetcherThreadCount for the
> fetcher
> > >>> threads, log-cleaner-dead-thread-count for the log cleaner.
> > >>> I think you meant two instead of three.
> > >>>
> > >>> Also, would it make sense to name these metrics consistency,
> something
> > >>> like
> > >>> `log-cleaner-dead-thread-count` and
> > `replica-fetcher-dead-thread-count`?
> > >>>
> > >>> Thanks,
> > >>> Dhruvil
> > >>>
> > >>> On Thu, Mar 28, 2019 at 11:27 AM Viktor Somogyi-Vass <
> > >>> viktorsomo...@gmail.com> wrote:
> > >>>
> > >>> > Hi All,
> > >>> >
> > >>> > I'd like to start a vote on KIP-434.
> > >>> > This basically would add a metrics to count dead threads in
> > >>> > ReplicaFetcherManager and LogCleaner to allow monitoring systems to
> > >>> alert
> > >>> > based on this.
> > >>> >
> > >>> > The KIP link:
> > >>> >
> > >>> >
> > >>>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-434%3A+Add+Replica+Fetcher+and+Log+Cleaner+Count+Metrics
> > >>> > The
> > >>> > PR: https://github.com/apache/kafka/pull/6514
> > >>> >
> > >>> > I'd be happy to receive any votes or additional feedback/reviews
> too.
> > >>> >
> > >>> > Thanks,
> > >>> > Viktor
> > >>> >
> > >>>
> > >>
> >
>

Re: [DISCUSS] KIP-455: Create an Administrative API for Replica Reassignment

[Viktor Somogyi-Vass]

Hi Colin,

How would the changed kafka-reassign-partitions.sh command work? Would it
automatically figure out the controller node if you pass some bootstrap
nodes with --bootstrap-server or are we deferring this implementation to
the users?

Thanks,
Viktor

On Sat, Apr 20, 2019 at 1:51 AM Colin McCabe  wrote:

> On Wed, Apr 17, 2019, at 17:23, Robert Barrett wrote:
> > Thanks for the KIP, Colin. I have a couple questions:
> >
> > 1. What's the reasoning for requiring cancellation of a reassignment
> before
> > submitting a new one? It seems like overriding an existing reassignment
> > could be done with a single update to
> > /brokers/topics/[topic]/partitions/[partitionId]/state and a single
> > LeaderAndIsrRequest. Maybe we could include a flag in the request so that
> > the client can explicitly request to override an existing reassignment?
>
> Hmm, good point.  That might be more convenient than having to cancel and
> remove before creating a new assignment.
>
> > 2. I agree that supporting the old ZK API for in the long term is a bad
> > idea. However, while the number of tools that use the ZK API may be
> small,
> > this would be a non-trivial change for them. Could we temporarily support
> > both, with a config enabling the new behavior to prevent users from
> trying
> > to use both mechanisms (if the config is true, the old znode is ignored;
> if
> > the config is false, the Admin Client API returns an error indicating
> that
> > it is not enabled)? We could then remove the ZK API in a later release,
> to
> > give people time to update their tools.
>
> It seems like the big change is basically just depending on adminclient
> versus a ZK client.  The code itself for converting a JSON file into an
> adminclient call shouldn't be difficult.  Maybe we could add a helper
> method to do this, to make it easier to do the conversion.  We'll already
> need that code for the command-line tool.
>
> best,
> Colin
>
>
> >
> > Thanks,
> > Bob
> >
> > On Mon, Apr 15, 2019 at 9:33 PM Colin McCabe  wrote:
> >
> > > link:
> > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-455%3A+Create+an+Administrative+API+for+Replica+Reassignment
> > >
> > >
> > >
> > > On Mon, Apr 15, 2019, at 18:07, Colin McCabe wrote:
> > > > Hi all,
> > > >
> > > > We've been having discussions on a few different KIPs (KIP-236,
> > > > KIP-435, etc.) about what the Admin Client replica reassignment API
> > > > should look like. The current API is really hard to extend and
> > > > maintain, which is a big source of problems. I think it makes sense
> to
> > > > have a KIP that establishes a clean API that we can use and extend
> > > > going forward, so I posted KIP-455. Take a look. :)
> > > >
> > > > best,
> > > > Colin
> > > >
> >
>

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

[Viktor Somogyi-Vass]

Hi Folks,

I took over this issue from Manikumar. Recently another motivation have
been raised in Spark for this (SPARK-28173) and I think it'd be great to
continue this task.
I updated the KIP and will wait for a few days to get some feedback then
proceed for the vote.

Thanks,
Viktor

On Tue, Dec 11, 2018 at 8:29 AM Manikumar  wrote:

> Hi Harsha,
>
> Thanks for the review.
>
> With this KIP a designated superuser can create tokens without requiring
> individual user credentials.
> Any client can authenticate brokers using the created tokens. We may not
> call this as impersonation,
> since the clients API calls are executing on their own authenticated
> connections.
>
> Thanks,
> Manikumar
>
> On Fri, Dec 7, 2018 at 11:56 PM Harsha  wrote:
>
> > Hi Mani,
> >  Overall KIP looks good to me. Can we call this Impersonation
> > support, which is what the KIP is doing?
> > Also instead of using super.uses as the config which essentially giving
> > cluster-wide support to the users, we can introduce impersonation.users
> as
> > a config and users listed in the config are allowed to impersonate other
> > users.
> >
> > Thanks,
> > Harsha
> >
> >
> > On Fri, Dec 7, 2018, at 3:58 AM, Manikumar wrote:
> > > Bump up! to get some attention.
> > >
> > > BTW, recently Apache Spark added  support for Kafka delegation token.
> > > https://issues.apache.org/jira/browse/SPARK-25501
> > >
> > > On Fri, Dec 7, 2018 at 5:27 PM Manikumar 
> > wrote:
> > >
> > > > Bump up! to get some attention.
> > > >
> > > > BTW, recently Apache Spark added for Kafka delegation token support.
> > > > https://issues.apache.org/jira/browse/SPARK-25501
> > > >
> > > > On Tue, Sep 25, 2018 at 9:56 PM Manikumar  >
> > > > wrote:
> > > >
> > > >> Hi all,
> > > >>
> > > >> I have created a KIP that proposes to allow users to create
> delegation
> > > >> tokens for other users.
> > > >>
> > > >>
> > > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-373%3A+Allow+users+to+create+delegation+tokens+for+other+users
> > > >>
> > > >> Please take a look when you get a chance.
> > > >>
> > > >> Thanks,
> > > >> Manikumar
> > > >>
> > > >
> >
>

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

[Viktor Somogyi-Vass]

Sorry, reading my email the second time I probably wasn't clear.
So basically the concept is that there is a user who can add other users as
resources (such as userB and userC) prior to creating the "userA can create
delegation token for userB and userC" association with CreateTokens. To
limit who can add new users as resources I thought we can introduce a
CreateUser operation. It's true though that we could also say that a Create
operation permission on the Cluster resource would be enough to create new
users but I think from a generic security perspective it's better if we
don't extend already existing operations.
So a classic flow would be that prior to creating the delegation token for
userB, userB itself has to be added by another user who has CreateUser
permissions. After this a CreateToken permission has to be created that
says "userA can create delegation tokens for userB" and after this userA
can actually create the token.
Let me know what you think.

Viktor

On Wed, Aug 14, 2019 at 1:30 PM Manikumar  wrote:

> Hi,
>
> Why do we need  new ACL operation  "CreateUsers"?
> I think,  "CreateTokens" Operation is sufficient to create "UserA can
> create tokens for UserB, UserC" association.
>
> Thanks,
>
> On Tue, Aug 13, 2019 at 3:37 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> wrote:
>
> > Hi Manikumar,
> >
> > Yea, I just brought up superuser for the sake of simplicity :).
> > Anyway, your proposition makes sense to me, I'll modify the KIP for this.
> >
> > The changes summarized:
> > 1. We'll need a new ACL operation as well (say "CreateUsers") to create
> the
> > "UserA can create tokens for UserB, UserC" association. This can be used
> > via the createAcls API of the AdminClient.
> > 2. CreateToken will be a User level operation (instead of a Cluster level
> > as in previous drafts). So that means any user who wants to create a
> > delegation token for other users will have to have an ACL set up by a
> > higher level user to authorize this.
> > 3. DescribeToken will also be a User level operation. In this case tokenT
> > owned by userB will be described if userA has a Describe ACL on tokenT or
> > has a DescribeToken ACL on userB. Note that in the latter case userA will
> > be able to describe all other tokens belonging to userB.
> >
> > Would this work for you?
> >
> > Viktor
> >
> > On Mon, Aug 12, 2019 at 5:45 PM Colin McCabe  wrote:
> >
> > > +1 for better access control here. In general, impersonating another
> user
> > > seems like it’s equivalent to super user access.
> > >
> > > Colin
> > >
> > > On Mon, Aug 12, 2019, at 05:43, Manikumar wrote:
> > > > Hi Viktor,
> > > >
> > > > As per the KIP, It's not only superuser, any user with required
> > > permissions
> > > > (CreateTokens on Cluster Resource), can create the tokens for other
> > > users.
> > > > Current proposed permissions defined like, "UserA can create tokens
> for
> > > any
> > > > user".
> > > > I am thinking, can we change the permissions like "UserA can create
> > > tokens
> > > > for UserB, UserC"?
> > > >
> > > > Thanks,
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > On Fri, Aug 9, 2019 at 6:39 PM Viktor Somogyi-Vass <
> > > viktorsomo...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hey Manikumar,
> > > > >
> > > > > Thanks for the feedback.
> > > > > I'm not sure I fully grasp the use-case. Would this be a quota? Do
> we
> > > say
> > > > > something like "there can be 10 active delegation tokens at a time
> > > that is
> > > > > created by superuserA for other users"?
> > > > > I think such a feature could be useful to limit the responsibility
> of
> > > said
> > > > > superuser (and blast radius in case of a faulty/malicious
> superuser)
> > > and
> > > > > also to limit potential programming errors. Do you have other use
> > cases
> > > > > too?
> > > > >
> > > > > Thanks,
> > > > > Viktor
> > > > >
> > > > >
> > > > > On Tue, Aug 6, 2019 at 1:28 PM Manikumar <
> manikumar.re...@gmail.com>
> > > > > wrote:
> > > > >
> > > > >

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

[Viktor Somogyi-Vass]

Started to implement my proposition and thought about it a little bit more
and it seems like I overthought the problem and we'd actually be better off
by having only the User resource type only and not CreateUsers. The problem
with CreateUsers is that a resource apparently is created only in addAcls
(at least in SimpleAclAuthorizer). Therefore we'd need to check before
creating the token that the owner user has been created and the token
creator has the permissions. Then add the user resource and the token. This
means that we'd only use CreateUsers when creating tokens iff the token
requester principal already has CreateTokens permissions with that user
(the owner) so it's kinda duplicate.
It would work though if we require the resources to be added beforehand but
it's not the case and is the detail of the Authorizer implementation.

I'll update the KIP accordingly and apologies for the extra round :).

Thanks,
Viktor

On Wed, Aug 14, 2019 at 2:40 PM Viktor Somogyi-Vass 
wrote:

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

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

[Viktor Somogyi-Vass]

Hi Manikumar,

Yea, I just brought up superuser for the sake of simplicity :).
Anyway, your proposition makes sense to me, I'll modify the KIP for this.

The changes summarized:
1. We'll need a new ACL operation as well (say "CreateUsers") to create the
"UserA can create tokens for UserB, UserC" association. This can be used
via the createAcls API of the AdminClient.
2. CreateToken will be a User level operation (instead of a Cluster level
as in previous drafts). So that means any user who wants to create a
delegation token for other users will have to have an ACL set up by a
higher level user to authorize this.
3. DescribeToken will also be a User level operation. In this case tokenT
owned by userB will be described if userA has a Describe ACL on tokenT or
has a DescribeToken ACL on userB. Note that in the latter case userA will
be able to describe all other tokens belonging to userB.

Would this work for you?

Viktor

On Mon, Aug 12, 2019 at 5:45 PM Colin McCabe  wrote:

> +1 for better access control here. In general, impersonating another user
> seems like it’s equivalent to super user access.
>
> Colin
>
> On Mon, Aug 12, 2019, at 05:43, Manikumar wrote:
> > Hi Viktor,
> >
> > As per the KIP, It's not only superuser, any user with required
> permissions
> > (CreateTokens on Cluster Resource), can create the tokens for other
> users.
> > Current proposed permissions defined like, "UserA can create tokens for
> any
> > user".
> > I am thinking, can we change the permissions like "UserA can create
> tokens
> > for UserB, UserC"?
> >
> > Thanks,
> >
> >
> >
> >
> >
> >
> > On Fri, Aug 9, 2019 at 6:39 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com>
> > wrote:
> >
> > > Hey Manikumar,
> > >
> > > Thanks for the feedback.
> > > I'm not sure I fully grasp the use-case. Would this be a quota? Do we
> say
> > > something like "there can be 10 active delegation tokens at a time
> that is
> > > created by superuserA for other users"?
> > > I think such a feature could be useful to limit the responsibility of
> said
> > > superuser (and blast radius in case of a faulty/malicious superuser)
> and
> > > also to limit potential programming errors. Do you have other use cases
> > > too?
> > >
> > > Thanks,
> > > Viktor
> > >
> > >
> > > On Tue, Aug 6, 2019 at 1:28 PM Manikumar 
> > > wrote:
> > >
> > > > Hi Viktor,
> > > >
> > > > Thanks for taking over this KP.
> > > >
> > > > Current proposed ACL changes allows users to create tokens for any
> user.
> > > > Thinking again about this, admins may want to configure a user to
> > > > impersonate limited number of other users.
> > > > This allows us to configure fine-grained permissions. But this
> requires
> > > a
> > > > new resourceType "User". What do you think?
> > > >
> > > >
> > > > Thanks,
> > > > Manikumar
> > > >
> > > >
> > > > On Wed, Jul 31, 2019 at 2:26 PM Viktor Somogyi-Vass <
> > > > viktorsomo...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Folks,
> > > > >
> > > > > I'm starting a vote on this.
> > > > >
> > > > > Viktor
> > > > >
> > > > > On Thu, Jun 27, 2019 at 12:02 PM Viktor Somogyi-Vass <
> > > > > viktorsomo...@gmail.com> wrote:
> > > > >
> > > > > > Hi Folks,
> > > > > >
> > > > > > I took over this issue from Manikumar. Recently another
> motivation
> > > have
> > > > > > been raised in Spark for this (SPARK-28173) and I think it'd be
> great
> > > > to
> > > > > > continue this task.
> > > > > > I updated the KIP and will wait for a few days to get some
> feedback
> > > > then
> > > > > > proceed for the vote.
> > > > > >
> > > > > > Thanks,
> > > > > > Viktor
> > > > > >
> > > > > > On Tue, Dec 11, 2018 at 8:29 AM Manikumar <
> manikumar.re...@gmail.com
> > > >
> > > > > > wrote:
> > > > > >
> > > > > >> Hi Harsha,
> > > > > >>
> > > > > >> Thanks for the review.
> > > > > >>
> >

Re: [DISCUSS] KIP-435: Internal Partition Reassignment Batching

[Viktor Somogyi-Vass]

Hey Folks,

I think I'll open a vote early next week about this if there are no more
feedback.

Thanks,
Viktor

On Fri, Aug 9, 2019 at 5:25 PM Viktor Somogyi-Vass 
wrote:

> Hey Stanislav,
>
> I reiterated on the current algorithm and indeed it would change the order
> of replicas in ZK but wouldn't do a leader election, so one would need to
> run the preferred replica election tool. However still in the light of this
> I'm not sure I'd keep the existing behavior as users wouldn't win anything
> with it. Changing leadership automatically would result in a simpler,
> easier and more responsive reassign algorithm which is especially important
> if the reassignment is done to free up the broker from under heavy load.
> Automated tools (Cruise Control) would also have simpler life.
> Let me know what you think.
>
> Viktor
>
> On Thu, Jul 11, 2019 at 7:16 PM Viktor Somogyi-Vass <
> viktorsomo...@gmail.com> wrote:
>
>> Hi Stan,
>>
>> I meant the following (maybe rare) scenario - we have replicas [1, 2, 3]
>> on
>> a lot of partitions and the user runs a massive rebalance to change them
>> all to [3, 2, 1]. In the old behavior, I think that this would not do
>> anything but simply change the replica set in ZK.
>> Then, the user could run kafka-preferred-replica-election.sh on a given
>> set
>> of partitions to make sure the new leader 3 gets elected.
>>
>> I thought the old algorithm would elect 3 as the leader in this case
>> right away at the end but I have to double check this. In any case I think
>> it would make sense in the new algorithm if we elected the new preferred
>> leader right away, regardless of the new leader is chosen from the existing
>> replicas or not. If the whole reassignment is in fact just changing the
>> replica order then either way it is a simple (trivial) operation and doing
>> it batched wouldn't slow it down much as there is no data movement
>> involved. If the reassignment is mixed, meaning it contains reordering and
>> real movement as well then in fact it would help to even out the load
>> faster as data movements can get long. For instance in case of a
>> reassignment batch of two partitions concurrently: P1: (1,2,3) -> (3,2,1)
>> and P2: (4,5,6) -> (7,8,9) the P2 reassignment would elect a new leader but
>> P1 wouldn't and it wouldn't help the goal of normalizing traffic on broker
>> 1 that much.
>> Again, I'll have to check how the current algorithm works and if it has
>> any unknown drawbacks to implement what I sketched up above.
>>
>> As for generic preferred leader election when called from the admin api
>> or the auto leader balance feature I think you're right that we should
>> leave it as it is. It doesn't involve any data movement so it's fairly fast
>> and it normalizes the cluster state quickly.
>>
>> Viktor
>>
>> On Tue, Jul 9, 2019 at 9:04 PM Stanislav Kozlovski <
>> stanis...@confluent.io> wrote:
>>
>>> Hey Viktor,
>>>
>>>  I think it is intuitive if there are on a global level...If we applied
>>> > them on every batch then we
>>> > couldn't really guarantee any limits as the user would be able to get
>>> > around it with submitting lots of reassignments.
>>>
>>>
>>> Agreed. Could we mention this explicitly in the KIP?
>>>
>>> Also if I understand correctly, AlterPartitionAssignmentsRequest would
>>> be a
>>> > partition level batching, isn't it? So if you submit 10 partitions at
>>> once
>>> > then they'll all be started by the controller immediately as per my
>>> > understanding.
>>>
>>>
>>> Yep, absolutely
>>>
>>> I've raised the ordering problem on the discussion of KIP-455 in a bit
>>> > different form and as I remember the verdict there was that we
>>> shouldn't
>>> > expose ordering as an API. It might not be easy as you say and there
>>> might
>>> > be much better strategies to follow (like disk or network utilization
>>> > goals). Therefore I'll remove this section from the KIP.
>>>
>>>
>>> Sounds good to me.
>>>
>>> I'm not sure I get this scenario. So are you saying that after they
>>> > submitted a reassignment they also submit a preferred leader change?
>>> > In my mind what I would do is:
>>> > i) make auto.leader.rebalance.enable to obey the leader movement limit
>>> as
>>> > this way it will be easier to calculate the reassignment batches.
>>> >
>>>
>

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

[Viktor Somogyi-Vass]

Bumping this in the hope I can get more feedback and/or votes.

Thanks,
Viktor

On Tue, Sep 3, 2019 at 1:49 PM Gabor Somogyi 
wrote:

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