[jira] [Resolved] (KAFKA-17430) Move RequestChannel.Metrics and RequestChannel.RequestMetrics to server module

2024-09-03 Thread Mickael Maison (Jira)


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

Mickael Maison resolved KAFKA-17430.

Fix Version/s: 4.0.0
   Resolution: Fixed

> Move RequestChannel.Metrics and RequestChannel.RequestMetrics to server module
> --
>
> Key: KAFKA-17430
> URL: https://issues.apache.org/jira/browse/KAFKA-17430
> Project: Kafka
>  Issue Type: Sub-task
>Reporter: Mickael Maison
>Assignee: Mickael Maison
>Priority: Major
> Fix For: 4.0.0
>
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Created] (KAFKA-17467) Flaky test shouldStayDeadAfterTwoCloses org.apache.kafka.streams.processor.internals.GlobalStreamThreadTest

2024-09-03 Thread Igor Soarez (Jira)
Igor Soarez created KAFKA-17467:
---

 Summary: Flaky test shouldStayDeadAfterTwoCloses 
org.apache.kafka.streams.processor.internals.GlobalStreamThreadTest
 Key: KAFKA-17467
 URL: https://issues.apache.org/jira/browse/KAFKA-17467
 Project: Kafka
  Issue Type: Test
  Components: streams
Reporter: Igor Soarez


First spotted on the Java 11 tests for 
https://github.com/apache/kafka/pull/17004

 
{code:java}
org.opentest4j.AssertionFailedError: expected:  but was: 

at 
app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at 
app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at 
app//org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
at 
app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
at 
app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
at 
app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
at 
app//org.apache.kafka.streams.processor.internals.GlobalStreamThreadTest.shouldStayDeadAfterTwoCloses(GlobalStreamThreadTest.java:234)
 {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Created] (KAFKA-17468) Move kafka.log.remote.quota to storage module

2024-09-03 Thread Mickael Maison (Jira)
Mickael Maison created KAFKA-17468:
--

 Summary: Move kafka.log.remote.quota to storage module
 Key: KAFKA-17468
 URL: https://issues.apache.org/jira/browse/KAFKA-17468
 Project: Kafka
  Issue Type: Sub-task
Reporter: Mickael Maison
Assignee: Mickael Maison






--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [DISCUSS] KAFKA-17316 and KAFKA-17423

2024-09-03 Thread Claude Warren, Jr
Colin,

I can see that it makes sense to replace the StandardAuthorizer so that
there are not 2 implementations.  However, I think the testing framework
should remain so that in future new improved implementations of
StandardAuthorizerData can be easily implemented and tested.

I will put forward a pull request to satisfy KAFKA-17423 that contains only
the new implementation.

Claude

On Mon, Sep 2, 2024 at 9:09 AM Claude Warren, Jr 
wrote:

> I have been working on implementing a Trie structure to store ACLs and
> improve the performance in the metadata/authorization code.  The upshot of
> this was that I found it very difficult to determine if the implementation
> was correctly reimplementing the current implementation.
>
> My goal was to simply change the StandardAuthorizerData implementation and
> leave the rest of the code as is.  However, there were no abstracted tests
> that would allow me to test this.
>
> KAFKA-17316 addresses this issue by creating some internal interfaces for
> the "metadata/authorizer" package.  The one change to the
> StandardAuthorizer was to implement the"authorizeByResourceType" defined in
> the "org.apache.kafka.server.authorizer.Authorizer" interface by passing
> the request down to the AuthorizerData implementation.
>
> This change allowed me to create three test implementations.  One that
> implemented "authorizeByResourceType" as it is in the released code base,
> one that verified that the StandardAuthorizerData implementation did not
> change the expected results, and one that showed the Trie implementation in
> KAFKA-17423 was also correct.
>
> I think that retaining the work in KAFKA-17316 makes sense as when the
> next faster implementation comes along we can drop in the replacement and
> verify that it works correctly.
>
> KAFKA-17423 builds on KAFKA-17316 by implementing a Trie based
> AuthorizerData implementation.  By splitting the data into a Trie format
> the search for matching ACLs is improved by an order of magnitude.  The
> trie implementation allows us to quickly locate the candidate ACLs by
> splitting them into groups based upon the similarity of the resource name.
> In addition since we are moving through the trie based on resource name we
> have several advantages:
>
>1. If we encounter a matching DENY while descending the Trie we can
>stop as it overrides anything that may be found at lower levels.
>2. We only look for LITERAL matches on the descent.  If we reach a
>matching resource name or a leaf node we know there are no LITERAL matches.
>3. If we don't have a DENY or a LITERAL match we walk back up the path
>checking the nodes from the descent looking for a PREFIX match.  The
>advantage here is that we don't have to search again but can simply retrace
>the path using the Trie structure.
>
> I believe that #1 and #2 above account for a significant portion of the
> speed increase as we do not have to reposition within a sorted list of all
> ACLs using a binary search.
>
> Finally, I think that we should prohibit the use of the java.util.stream
> classes within the authorizer due to hot path speed considerations.  The
> only existing code that uses streams within that package were test cases.
> We can prohibit the use by a simple checkstyle prohibition.  Doing so will
> short circuit any misguided potential changes.
>
> Thoughs?
> Claude
>
>


Re: [VOTE] KIP-1074: Allow the replication of user internal topics

2024-09-03 Thread Viktor Somogyi-Vass
Hi Patrik,

Thanks for working on this. +1 from me (binding).

Best,
Viktor

On Wed, Aug 28, 2024 at 1:34 PM Patrik Marton 
wrote:

> Hi all,
>
> As the proposal is finalized, and there was no new feedback in the past
> week, I would like to open voting for KIP-1074
> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1074%3A+Allow+the+replication+of+user+internal+topics
> >
> .
>
> Please vote / let me know your thoughts.
>
> Thanks,
> Patrik
>


Jenkins build is unstable: Kafka » Kafka Branch Builder » trunk #3262

2024-09-03 Thread Apache Jenkins Server
See 




[jira] [Resolved] (KAFKA-17170) Add test to ensure new consumer acks reconciled assignment even if first HB with ack lost

2024-09-03 Thread Lianet Magrans (Jira)


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

Lianet Magrans resolved KAFKA-17170.

Fix Version/s: 4.0.0
 Reviewer: Lianet Magrans
   Resolution: Fixed

> Add test to ensure new consumer acks reconciled assignment even if first HB 
> with ack lost
> -
>
> Key: KAFKA-17170
> URL: https://issues.apache.org/jira/browse/KAFKA-17170
> Project: Kafka
>  Issue Type: Task
>  Components: clients, consumer
>Reporter: Lianet Magrans
>Assignee: 黃竣陽
>Priority: Minor
>  Labels: kip-848-client-support, newbie
> Fix For: 4.0.0
>
>
> When a consumer reconciles an assignment, it transitions to ACKNOWLEDGING, so 
> that a HB is sent on the next manager poll, without waiting for the interval. 
> The consumer transitions out of this ack state as soon as it sends the 
> heartbeat, without waiting for a response. This is based on the expectation 
> that following heartbeats (sent on the interval) will act as ack, including 
> the set of partitions even in case the first ack is lost. This is the 
> expected flow:
>  # complete reconciliation and send HB1 to ack assignment tp0 
>  # HB1 times out (or fails in any way) => heartbeat request manager resets 
> the sentFields to null (HeartbeatState.reset() , triggered if the request 
> fails, or if it gets a response with an Error)
>  # following HB will include tp0 (and act as ack), because it will notice 
> that tp0 != null (last value sent)
> This seems not to be covered by any test, so we should add a unit test to the 
> HeartbeatRequestManager, to ensure that the HB generated in step 4 above 
> includes tp0 as I expect :), considering both cases of error: request fails 
> (no response) and request gets a response with an Error in it. 
> This flow is important because if failing to send the reconciled partitions 
> in a HB, the broker would remain waiting for an ack that the member would 
> considered it already sent (the broker would wait for the rebalance timeout 
> before re-assigning those partitions)



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Resolved] (KAFKA-17294) Handle retriable errors when fetching offsets in new consumer

2024-09-03 Thread Chia-Ping Tsai (Jira)


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

Chia-Ping Tsai resolved KAFKA-17294.

Fix Version/s: 4.0.0
   Resolution: Fixed

> Handle retriable errors when fetching offsets in new consumer
> -
>
> Key: KAFKA-17294
> URL: https://issues.apache.org/jira/browse/KAFKA-17294
> Project: Kafka
>  Issue Type: Bug
>  Components: clients, consumer
>Affects Versions: 3.7.0, 3.8.0
>Reporter: Lianet Magrans
>Assignee: TengYao Chi
>Priority: Major
>  Labels: kip-848-client-support
> Fix For: 4.0.0
>
>
> The new consumer CommitRequestManager fails with a fatal KafkaException 
> whenever it receives an unexpected error in the OffsetFetch response, even if 
> the error is retriable. See 
> https://github.com/apache/kafka/blob/837684a1b9b3bad244613211e90b67cf9170fb44/clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java#L1009-L1011
> This was initially implemented like this to maintain the behaviour of the 
> Classic consumer, where the ConsumerCoordinator would do the same when 
> handling the OffsetFetchResponse. That behaviour is being updated for the 
> legacy coordinator as part of KAFKA-17279, to retry on all retriable errors.
> We should review and update in the commitRequestManager to align with this, 
> and retry on all retriable errors, which seems sensible when fetching 
> offsets. 



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Possible bug in Authorize by ResourceTypeQue

2024-09-03 Thread Claude Warren
*Setup:*
Superuser = "User:superman"

ACLs added to system
new StandardAcl(TOPIC, "foo", PREFIXED, "User:alice", WILDCARD, READ, DENY)
new StandardAcl(TOPIC, "foobar", LITERAL, "User:alice", WILDCARD, READ,
ALLOW)
new StandardAcl(TOPIC, "foo", PREFIXED, "User:bob", WILDCARD, READ, ALLOW)

ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true"

AuthorizerContext requestContext = MockAuthorizableRequestContext with
principal = User:alice
host = InetAddress.getLocalHost()


*Method Call:*

authorizer.authorizeByResourceType(requestContext, READ, TOPIC)

*Question:*

Should the result be true because there is a LITERAL READ ALLOW on "foobar"
or should the result be false because there is an overriding PREFIXED READ
DENY on "foo" ?



--
LinkedIn: http://www.linkedin.com/in/claudewarren


[jira] [Created] (KAFKA-17469) Move appropriate classes and interfaces to the new share module

2024-09-03 Thread Sushant Mahajan (Jira)
Sushant Mahajan created KAFKA-17469:
---

 Summary: Move appropriate classes and interfaces to the new share 
module
 Key: KAFKA-17469
 URL: https://issues.apache.org/jira/browse/KAFKA-17469
 Project: Kafka
  Issue Type: Sub-task
Reporter: Sushant Mahajan
Assignee: Sushant Mahajan






--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: Possible bug in Authorize by ResourceTypeQue

2024-09-03 Thread Rajini Sivaram
Hi Claude,

`authorizeByResourceType` doesn't grant access to any specific topic, it
grants access to idempotent write if the user has access to write to any
topic (which may or may not exist). In this case,
ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true", so `User:alice` can
write to a topic that doesn't start with `foo` and hence
`authorizeByResourceType` should be ALLOWED. What was the behaviour you
observed?

Regards,

Rajini


On Tue, Sep 3, 2024 at 12:22 PM Claude Warren  wrote:

> *Setup:*
> Superuser = "User:superman"
>
> ACLs added to system
> new StandardAcl(TOPIC, "foo", PREFIXED, "User:alice", WILDCARD, READ, DENY)
> new StandardAcl(TOPIC, "foobar", LITERAL, "User:alice", WILDCARD, READ,
> ALLOW)
> new StandardAcl(TOPIC, "foo", PREFIXED, "User:bob", WILDCARD, READ, ALLOW)
>
> ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true"
>
> AuthorizerContext requestContext = MockAuthorizableRequestContext with
> principal = User:alice
> host = InetAddress.getLocalHost()
>
>
> *Method Call:*
>
> authorizer.authorizeByResourceType(requestContext, READ, TOPIC)
>
> *Question:*
>
> Should the result be true because there is a LITERAL READ ALLOW on "foobar"
> or should the result be false because there is an overriding PREFIXED READ
> DENY on "foo" ?
>
>
>
> --
> LinkedIn: http://www.linkedin.com/in/claudewarren
>


Re: [DISCUSS] KIP-1043: Administration of groups

2024-09-03 Thread Andrew Schofield
Hi,
I’ve spent some time working with clusters containing groups of multiple
types, fixing problems and improving error handling.

I’ve simplified the KIP so that it just adds kafka-groups.sh and improves
the error handling for describing groups of the wrong type. With the other
improvements I’ve already made, it seems to me that this is sufficient to
make working with groups of multiple types work nicely.

I’d like to ask for another round of reviews before hopefully opening up
a vote soon.

https://cwiki.apache.org/confluence/display/KAFKA/KIP-1043%3A+Administration+of+groups

Thanks,
Andrew


From: Andrew Schofield 
Sent: 02 August 2024 15:00
To: dev@kafka.apache.org 
Subject: Re: [DISCUSS] KIP-1043: Administration of groups

Hi Lianet,
Thanks for your comment.

I’ve been digging more into the situation with describing groups in a
broker with groups of multiple types. It’s a bit fiddly because of the
introduction of the modern consumer groups by KIP-848 and the
need for the admin client to cope with both kinds of consumer groups
and older brokers.

If you use `kafka-consumer-groups.sh --describe --group MYSHARE`
the output is:

  Error: Consumer group ‘MYSHARE’ does not exist.

How does it get there? AdminClient.describeConsumerGroups
is complicated.

First, it uses the ConsumerGroupDescribe RPC which responds
with GROUP_ID_NOT_FOUND (69) and an empty error message.
The broker *could* fill in the error message to help with this situation
but I don’t like that as a solution. Seems quite brittle.

Then, it uses the DescribeGroups RPC in case it’s a classic consumer
group. This responds with error code NONE (0) and makes the group
look like a Dead consumer group. There is no error message field
in that RPC at all, so we don’t have the option of using an error
message to disambiguate.

So, `kafka-consumer-groups.sh` thinks that it’s dealing with a dead
consumer group and its output makes sense.

My preferred course of action here is as you suggest to introduce
the new error code, INVALID_GROUP_TYPE. If you use any of the following
RPCs with the wrong type of group, you get this response:

* ConsumerGroupDescribe
* ShareGroupDescribe
* ConsumerGroupHeartbeat
* ShareGroupHeartbeat

The remaining RPCs for consumer groups, such as ListOffsets and
TxnOffsetCommit continue to use `GROUP_ID_NOT_FOUND`.

Does that make sense? Any further comments?

Thanks,
Andrew

> On 23 Jul 2024, at 17:26, Lianet M.  wrote:
>
> Hello Andrew,
>
> Bringing here the point I surfaced on the KIP-1071 thread:
>
> I wonder if at this point, where we're getting several new group types
>> added, each with RPCs that are supposed to include groupId of a certain
>> type, we should be more explicit about this situation. Maybe a kind of
>> INVALID_GROUP_TYPE (group exists but not with a valid type for this RPC) vs
>> a GROUP_ID_NOT_FOUND (group does not exist).  Those errors would be
>> consistently used across consumer, share, and streams RPCs whenever the
>> group id is not of the expected type.
>
>
> I noticed it on KIP-1071 but totally agree with you that it would make more
> sense to consider it here.
>
> LM9. Regarding the point of introducing a new INVALID_GROUP_TYPE vs reusing
> the existing INCONSISTENT_PROTOCOL_TYPE. My concern with reusing
> INCONSISTENT_GROUP_PROTOCOL for errors with the group ID is that it mixes
> the concepts of group type and protocol. Even though they are closely
> related, we have 2 separate concepts (internally and presented in output
> for commands), and the relationship is not 1-1 in all cases. Also, the
> INCONSISTENT_GROUP_PROTOCOL is already used not only for protocol but also
> when validating the list of assignors provided by a consumer in a
> JoinGroupRequest. Seems a bit confusing to me already, so maybe better not
> to add more to it? Just first thoughts. What do you think?
>
> Thanks,
> Lianet
>
> On Fri, Jul 19, 2024 at 5:00 AM Andrew Schofield 
> wrote:
>
>> Hi Apoorv,
>> Thanks for your comments.
>>
>> AM1: I chose to leave the majority of the administration for the different
>> types of groups in their own tools. The differences between the group
>> types are significant and I think that one uber tool that subsumes
>> kafka-consumer-groups.sh, kafka-share-groups.sh and
>> kafka-streams-application-reset.sh would be too overwhelming and
>> difficult to use. For example, the output from describing a consumer group
>> is not the same as the output from describing a share group.
>>
>> AM2: I think you’re highlighting some of the effects of the evolution
>> of groups. The classic consumer group protocol defined the idea
>> of protocol as a way of distinguishing between the various ways people
>> had extended the base protocol - “consumer", “connect", and “sr" are the
>> main ones I’ve seen, and the special “” for groups that are not using
>> member assignment.
>>
>> For the modern group protocol, each of the proposed implementations
>> brings its own use of th

Re: [DISCUSS] KIP-1034: Dead letter queue in Kafka Streams

2024-09-03 Thread Bruno Cadonna

Hi,

Thanks for the updates!

I have a couple of comments.

BC1
Do you also plan to add an argument to the StreamsResetter tool to 
delete the default DLQ topic when a Streams app is reset?



BC2
Would it make sense to add errors.deadletterqueue.topic.name to the 
ErrorHandlerContext in case that a custom exception handler wants to 
write to the configured DLQ topic?
For example if only one of the handler needs to be customized but all 
handlers should write to the configured DLQ topic.



BC3
What do you think about renaming withDeadLetterQueueRecords() to 
andAddToDeadLetterQueue()?

A customized handler would look like

public ProcessingHandlerResponse handle(
final ErrorHandlerContext context,
final Record record,
final Exception exception
) {
return ProcessingHandlerResponse.CONTINUE
.andAddToDeadLetterQueue(
Collections.singletonList(new ProducerRecord<>(
"app-dlq",
"Hello".getBytes(StandardCharsets.UTF_8),
"World".getBytes(StandardCharsets.UTF_8)
))
);
}

I think the code becomes more readable.


Best,
Bruno

On 8/30/24 3:37 PM, Damien Gasparina wrote:

Hi everyone,

We just updated KIP-1034, we changed the following:
   - We included the ProcessingExceptionHandler (KIP-1033) directly in the KIP;
   - We provided examples to clarify the new configuration, and how it
could be leveraged.

I think we can resume the conversation on this KIP.

Cheers,
Damien Sebastien and Loic


On Tue, 27 Aug 2024 at 15:37, Sebastien Viale
 wrote:



Hi Bruno,

We have planned a meeting for next friday to discuss it with Loic and Damien.

We will be able to restart discussions about it soon.

regards


De : Bruno Cadonna 
Envoyé : lundi 26 août 2024 11:32
À : dev@kafka.apache.org 
Objet : [EXT] Re: [DISCUSS] KIP-1034: Dead letter queue in Kafka Streams

Warning External sender Do not click on any links or open any attachments 
unless you trust the sender and know the content is safe.

Hi Loïc, Sebastien, and Damien,

Now that KIP-1033 is going to be released in 3.9, what is the plan to
progress with this KIP?

Is the KIP up-to-date, so that we can restart discussion?

Best,
Bruno

This email was screened for spam and malicious content but exercise caution 
anyway.




On 6/13/24 6:16 PM, Damien Gasparina wrote:

Hi Bruno,

We focused our effort (well, mostly Seb and Loic :)) on KIP-1033,
that's why not much progress has been made on this one yet.
Regarding your points:

B1: It is up to the user to specify the DLQ topic name and to
implement a potential differentiation. I tend to think that having one
DLQ per application ID is the wisest, but I encountered cases and
applications that preferred having a shared DLQ topic between multiple
applications, e.g. to reduce the number of partitions, or to ease
monitoring

B2 : Goot catch, it should be
ERRORS_DEADLETTERQUEUE_TOPIC_NAME_CONFIG, we removed the "DEFAULT"
prefix during the discussion, looks like I forgot to update all
occurrences in the KIP.

B3 :The trigger for sending to the DLQ would be if
ERRORS_DEADLETTERQUEUE_TOPIC_NAME_CONFIG is set OR if the user
implemented a custom exception handler that returns DLQ records.
ERRORS_DEADLETTERQUEUE_TOPIC_NAME_CONFIG would just override the
behavior of the default handler, thus custom exception handlers will
completely ignore this parameter.

I think it's a good trade-off between providing a production-ready
default implementation, yet providing sufficient flexibility for
complex use-cases.
This behavior definitely needs to be documented, but I guess it's safe
to push the responsibility of the DLQ records to the user if they
implement custom handlers.

Cheers,
Damien


On Tue, 11 Jun 2024 at 17:00, Bruno Cadonna  wrote:


Hi,

since there was not too much activity in this thread recently, I was
wondering what the status of this discussion is.

I cannot find the examples in the KIP Sébastien mentioned in the last
message to this thread. I can also not find the corresponding definition
of the following method call in the KIP:

FAIL.withDeadLetterQueueRecord(record, "dlq-topic")

I have also some comments:

B1
Did you consider to prefix the dead letter queue topic names with the
application ID to distinguish the topics between Streams apps? Or is the
user responsible for the differentiation? If the user is responsible, we
risk that faulty records of different Streams apps end up in the same
dead letter queue.

B2
Is the name of the dead letter queue topic config
DEFAULT_ERRORS_DEADLETTERQUEUE_TOPIC_NAME_CONFIG or
ERRORS_DEADLETTERQUEUE_TOPIC_NAME_CONFIG? In the KIP both names are used.

B3
What is exactly the trigger to send a record to the dead letter queue?
Is setting ERRORS_DEADLETTERQUEUE_TOPIC_NAME_CONFIG or is it adding a
record to the return value of the exception handler?
What happens if I set ERRORS_DEADLETTERQUEUE_TOPIC_NAME_CONFIG but do
not add a record to the return value of the han

Re: Possible bug in Authorize by ResourceTypeQue

2024-09-03 Thread Claude Warren, Jr
I am working on a replacement for the StandardAuthorizer and my
implementation DENIED while the standard implementation ALLOWED.  In
reading the specs I thought it should be DENIED.  But your statement makes
it clear that I misread.

Thank you,
Claude

On Tue, Sep 3, 2024 at 1:14 PM Rajini Sivaram 
wrote:

> Hi Claude,
>
> `authorizeByResourceType` doesn't grant access to any specific topic, it
> grants access to idempotent write if the user has access to write to any
> topic (which may or may not exist). In this case,
> ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true", so `User:alice` can
> write to a topic that doesn't start with `foo` and hence
> `authorizeByResourceType` should be ALLOWED. What was the behaviour you
> observed?
>
> Regards,
>
> Rajini
>
>
> On Tue, Sep 3, 2024 at 12:22 PM Claude Warren  wrote:
>
> > *Setup:*
> > Superuser = "User:superman"
> >
> > ACLs added to system
> > new StandardAcl(TOPIC, "foo", PREFIXED, "User:alice", WILDCARD, READ,
> DENY)
> > new StandardAcl(TOPIC, "foobar", LITERAL, "User:alice", WILDCARD, READ,
> > ALLOW)
> > new StandardAcl(TOPIC, "foo", PREFIXED, "User:bob", WILDCARD, READ,
> ALLOW)
> >
> > ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true"
> >
> > AuthorizerContext requestContext = MockAuthorizableRequestContext with
> > principal = User:alice
> > host = InetAddress.getLocalHost()
> >
> >
> > *Method Call:*
> >
> > authorizer.authorizeByResourceType(requestContext, READ, TOPIC)
> >
> > *Question:*
> >
> > Should the result be true because there is a LITERAL READ ALLOW on
> "foobar"
> > or should the result be false because there is an overriding PREFIXED
> READ
> > DENY on "foo" ?
> >
> >
> >
> > --
> > LinkedIn: http://www.linkedin.com/in/claudewarren
> >
>


Re: Possible bug in Authorize by ResourceTypeQue

2024-09-03 Thread Claude Warren, Jr
Followup:  If ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true" then
authorizeByResourceType should return true in all cases since the user
would have access for any operation on any undefined topic?


On Tue, Sep 3, 2024 at 2:08 PM Claude Warren, Jr 
wrote:

> I am working on a replacement for the StandardAuthorizer and my
> implementation DENIED while the standard implementation ALLOWED.  In
> reading the specs I thought it should be DENIED.  But your statement makes
> it clear that I misread.
>
> Thank you,
> Claude
>
> On Tue, Sep 3, 2024 at 1:14 PM Rajini Sivaram 
> wrote:
>
>> Hi Claude,
>>
>> `authorizeByResourceType` doesn't grant access to any specific topic, it
>> grants access to idempotent write if the user has access to write to any
>> topic (which may or may not exist). In this case,
>> ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true", so `User:alice` can
>> write to a topic that doesn't start with `foo` and hence
>> `authorizeByResourceType` should be ALLOWED. What was the behaviour you
>> observed?
>>
>> Regards,
>>
>> Rajini
>>
>>
>> On Tue, Sep 3, 2024 at 12:22 PM Claude Warren  wrote:
>>
>> > *Setup:*
>> > Superuser = "User:superman"
>> >
>> > ACLs added to system
>> > new StandardAcl(TOPIC, "foo", PREFIXED, "User:alice", WILDCARD, READ,
>> DENY)
>> > new StandardAcl(TOPIC, "foobar", LITERAL, "User:alice", WILDCARD, READ,
>> > ALLOW)
>> > new StandardAcl(TOPIC, "foo", PREFIXED, "User:bob", WILDCARD, READ,
>> ALLOW)
>> >
>> > ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true"
>> >
>> > AuthorizerContext requestContext = MockAuthorizableRequestContext with
>> > principal = User:alice
>> > host = InetAddress.getLocalHost()
>> >
>> >
>> > *Method Call:*
>> >
>> > authorizer.authorizeByResourceType(requestContext, READ, TOPIC)
>> >
>> > *Question:*
>> >
>> > Should the result be true because there is a LITERAL READ ALLOW on
>> "foobar"
>> > or should the result be false because there is an overriding PREFIXED
>> READ
>> > DENY on "foo" ?
>> >
>> >
>> >
>> > --
>> > LinkedIn: http://www.linkedin.com/in/claudewarren
>> >
>>
>


Re: Possible bug in Authorize by ResourceTypeQue

2024-09-03 Thread Claude Warren, Jr
Followup2: your answer speaks directly to "WRITE" access.  My example was
READ access.  So the question method is answering then is: Does the user
have access to READ any TOPIC?  And that is further restricted by the
requestContext host is it not?


On Tue, Sep 3, 2024 at 2:10 PM Claude Warren, Jr 
wrote:

> Followup:  If ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true" then
> authorizeByResourceType should return true in all cases since the user
> would have access for any operation on any undefined topic?
>
>
> On Tue, Sep 3, 2024 at 2:08 PM Claude Warren, Jr 
> wrote:
>
>> I am working on a replacement for the StandardAuthorizer and my
>> implementation DENIED while the standard implementation ALLOWED.  In
>> reading the specs I thought it should be DENIED.  But your statement makes
>> it clear that I misread.
>>
>> Thank you,
>> Claude
>>
>> On Tue, Sep 3, 2024 at 1:14 PM Rajini Sivaram 
>> wrote:
>>
>>> Hi Claude,
>>>
>>> `authorizeByResourceType` doesn't grant access to any specific topic, it
>>> grants access to idempotent write if the user has access to write to any
>>> topic (which may or may not exist). In this case,
>>> ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true", so `User:alice` can
>>> write to a topic that doesn't start with `foo` and hence
>>> `authorizeByResourceType` should be ALLOWED. What was the behaviour you
>>> observed?
>>>
>>> Regards,
>>>
>>> Rajini
>>>
>>>
>>> On Tue, Sep 3, 2024 at 12:22 PM Claude Warren  wrote:
>>>
>>> > *Setup:*
>>> > Superuser = "User:superman"
>>> >
>>> > ACLs added to system
>>> > new StandardAcl(TOPIC, "foo", PREFIXED, "User:alice", WILDCARD, READ,
>>> DENY)
>>> > new StandardAcl(TOPIC, "foobar", LITERAL, "User:alice", WILDCARD, READ,
>>> > ALLOW)
>>> > new StandardAcl(TOPIC, "foo", PREFIXED, "User:bob", WILDCARD, READ,
>>> ALLOW)
>>> >
>>> > ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true"
>>> >
>>> > AuthorizerContext requestContext = MockAuthorizableRequestContext with
>>> > principal = User:alice
>>> > host = InetAddress.getLocalHost()
>>> >
>>> >
>>> > *Method Call:*
>>> >
>>> > authorizer.authorizeByResourceType(requestContext, READ, TOPIC)
>>> >
>>> > *Question:*
>>> >
>>> > Should the result be true because there is a LITERAL READ ALLOW on
>>> "foobar"
>>> > or should the result be false because there is an overriding PREFIXED
>>> READ
>>> > DENY on "foo" ?
>>> >
>>> >
>>> >
>>> > --
>>> > LinkedIn: http://www.linkedin.com/in/claudewarren
>>> >
>>>
>>


Re: [DISCUSS] KIP-1034: Dead letter queue in Kafka Streams

2024-09-03 Thread Damien Gasparina
Hi Bruno,

Thanks a lot for your comments!

> BC1
> Do you also plan to add an argument to the StreamsResetter tool to
> delete the default DLQ topic when a Streams app is reset?
Good point, I did not think about the StreamsResetter tool. Thinking
out loud, I am not sure if it is a good idea to add an option to clean
them up. In my opinion DLQ topics should be viewed as a sink topic and
AFAIK, this tool does not clean up sink topics.

> BC2
In case of a custom exception handlers, they can get the
errors.deadletterqueue.topic.name configuration by overriding `void
configure(Map configs);`. As it is the location where all
the configuration can be accessed, I think it's the best way no? I
will think about it a bit further, it might be useful/convenient for
users.

> BC3
The "with" syntax is used in many locations in the Kafka Streams
public classes, that's why I used it, e.g. Materialized.with(),
Consumed.withKeySerde(...).withValueSerde(...), Grouped.withName(...).
I do agree that .andAddToDeadLetterQueue is more intuitive, but I
would argue that being consistent is better in this situation. What do
you think?

Cheers,
Damien

On Tue, 3 Sept 2024 at 14:59, Bruno Cadonna  wrote:
>
> Hi,
>
> Thanks for the updates!
>
> I have a couple of comments.
>
> BC1
> Do you also plan to add an argument to the StreamsResetter tool to
> delete the default DLQ topic when a Streams app is reset?
>
>
> BC2
> Would it make sense to add errors.deadletterqueue.topic.name to the
> ErrorHandlerContext in case that a custom exception handler wants to
> write to the configured DLQ topic?
> For example if only one of the handler needs to be customized but all
> handlers should write to the configured DLQ topic.
>
>
> BC3
> What do you think about renaming withDeadLetterQueueRecords() to
> andAddToDeadLetterQueue()?
> A customized handler would look like
>
> public ProcessingHandlerResponse handle(
>  final ErrorHandlerContext context,
>  final Record record,
>  final Exception exception
> ) {
>  return ProcessingHandlerResponse.CONTINUE
>  .andAddToDeadLetterQueue(
>  Collections.singletonList(new ProducerRecord<>(
>  "app-dlq",
>  "Hello".getBytes(StandardCharsets.UTF_8),
>  "World".getBytes(StandardCharsets.UTF_8)
>  ))
>  );
> }
>
> I think the code becomes more readable.
>
>
> Best,
> Bruno
>
> On 8/30/24 3:37 PM, Damien Gasparina wrote:
> > Hi everyone,
> >
> > We just updated KIP-1034, we changed the following:
> >- We included the ProcessingExceptionHandler (KIP-1033) directly in the 
> > KIP;
> >- We provided examples to clarify the new configuration, and how it
> > could be leveraged.
> >
> > I think we can resume the conversation on this KIP.
> >
> > Cheers,
> > Damien Sebastien and Loic
> >
> >
> > On Tue, 27 Aug 2024 at 15:37, Sebastien Viale
> >  wrote:
> >>
> >>
> >> Hi Bruno,
> >>
> >> We have planned a meeting for next friday to discuss it with Loic and 
> >> Damien.
> >>
> >> We will be able to restart discussions about it soon.
> >>
> >> regards
> >>
> >> 
> >> De : Bruno Cadonna 
> >> Envoyé : lundi 26 août 2024 11:32
> >> À : dev@kafka.apache.org 
> >> Objet : [EXT] Re: [DISCUSS] KIP-1034: Dead letter queue in Kafka Streams
> >>
> >> Warning External sender Do not click on any links or open any attachments 
> >> unless you trust the sender and know the content is safe.
> >>
> >> Hi Loïc, Sebastien, and Damien,
> >>
> >> Now that KIP-1033 is going to be released in 3.9, what is the plan to
> >> progress with this KIP?
> >>
> >> Is the KIP up-to-date, so that we can restart discussion?
> >>
> >> Best,
> >> Bruno
> >>
> >> This email was screened for spam and malicious content but exercise 
> >> caution anyway.
> >>
> >>
> >>
> >>
> >> On 6/13/24 6:16 PM, Damien Gasparina wrote:
> >>> Hi Bruno,
> >>>
> >>> We focused our effort (well, mostly Seb and Loic :)) on KIP-1033,
> >>> that's why not much progress has been made on this one yet.
> >>> Regarding your points:
> >>>
> >>> B1: It is up to the user to specify the DLQ topic name and to
> >>> implement a potential differentiation. I tend to think that having one
> >>> DLQ per application ID is the wisest, but I encountered cases and
> >>> applications that preferred having a shared DLQ topic between multiple
> >>> applications, e.g. to reduce the number of partitions, or to ease
> >>> monitoring
> >>>
> >>> B2 : Goot catch, it should be
> >>> ERRORS_DEADLETTERQUEUE_TOPIC_NAME_CONFIG, we removed the "DEFAULT"
> >>> prefix during the discussion, looks like I forgot to update all
> >>> occurrences in the KIP.
> >>>
> >>> B3 :The trigger for sending to the DLQ would be if
> >>> ERRORS_DEADLETTERQUEUE_TOPIC_NAME_CONFIG is set OR if the user
> >>> implemented a custom exception handler that returns DLQ records.
> >>> ERRORS_DEADLETTERQUEUE_TOPIC_NAME_CONFIG would just override the
> >>> behavior of the default handler, thus custom

[jira] [Created] (KAFKA-17470) CommitRequestManager should record failed request only once even if multiple errors in response

2024-09-03 Thread Lianet Magrans (Jira)
Lianet Magrans created KAFKA-17470:
--

 Summary: CommitRequestManager should record failed request only 
once even if multiple errors in response
 Key: KAFKA-17470
 URL: https://issues.apache.org/jira/browse/KAFKA-17470
 Project: Kafka
  Issue Type: Bug
  Components: clients, consumer
Reporter: Lianet Magrans
 Fix For: 4.0.0


The commitRequestManager calls onFailedAttempt to register a failed request, 
and this call directly affects the count of attempts, considered to determine 
the backoff to apply. Therefore, this onFailedAttempts should only be called 
once per request, but currently the CommitRequestManager may call it multiple 
times for a single request (if the response contains multiple partition errors):

- on commit: 
[https://github.com/apache/kafka/blob/2f9b23625917fed841a176a73bbbc02bfa330a2d/clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java#L726]

- on fetch: 
[https://github.com/apache/kafka/blob/2f9b23625917fed841a176a73bbbc02bfa330a2d/clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java#L1049]
 


We should ensure that onFailedAttempt is only called once per request, and add 
a test that covers both cases above, of responses with multiple partition 
errors in it.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [DISCUSS] KIP-1034: Dead letter queue in Kafka Streams

2024-09-03 Thread Bruno Cadonna

Hi Damien,

BC1

> In my opinion DLQ topics should be viewed as a sink topic and
> AFAIK, this tool does not clean up sink topics.

Maybe, but one could also argue DLQ topics are part of the runtime 
because is collects errors occurred during the runtime that might be 
specific to given execution of the Streams app. One might want want to 
reset the errors when starting from scratch.



BC2
You are right the configs are passed at creation time. My bad!


BC3
I think the `with`-syntax fits well when building objects that contain 
kind of configs like Materialized that you usually pass into an API. 
However, the handler returns the response. The response instructs 
something. It says CONTINUE processing or FAIL the processing. With your 
KIP the response gets an additional instruction, namely 
`andAddToDeadLetterQueue`. I would not sacrifice better readability for 
consistency in this case.



Best,
Bruno

On 9/3/24 3:18 PM, Damien Gasparina wrote:

Hi Bruno,

Thanks a lot for your comments!


BC1
Do you also plan to add an argument to the StreamsResetter tool to
delete the default DLQ topic when a Streams app is reset?

Good point, I did not think about the StreamsResetter tool. Thinking
out loud, I am not sure if it is a good idea to add an option to clean
them up. In my opinion DLQ topics should be viewed as a sink topic and
AFAIK, this tool does not clean up sink topics.


BC2

In case of a custom exception handlers, they can get the
errors.deadletterqueue.topic.name configuration by overriding `void
configure(Map configs);`. As it is the location where all
the configuration can be accessed, I think it's the best way no? I
will think about it a bit further, it might be useful/convenient for
users.


BC3

The "with" syntax is used in many locations in the Kafka Streams
public classes, that's why I used it, e.g. Materialized.with(),
Consumed.withKeySerde(...).withValueSerde(...), Grouped.withName(...).
I do agree that .andAddToDeadLetterQueue is more intuitive, but I
would argue that being consistent is better in this situation. What do
you think?

Cheers,
Damien

On Tue, 3 Sept 2024 at 14:59, Bruno Cadonna  wrote:


Hi,

Thanks for the updates!

I have a couple of comments.

BC1
Do you also plan to add an argument to the StreamsResetter tool to
delete the default DLQ topic when a Streams app is reset?


BC2
Would it make sense to add errors.deadletterqueue.topic.name to the
ErrorHandlerContext in case that a custom exception handler wants to
write to the configured DLQ topic?
For example if only one of the handler needs to be customized but all
handlers should write to the configured DLQ topic.


BC3
What do you think about renaming withDeadLetterQueueRecords() to
andAddToDeadLetterQueue()?
A customized handler would look like

public ProcessingHandlerResponse handle(
  final ErrorHandlerContext context,
  final Record record,
  final Exception exception
) {
  return ProcessingHandlerResponse.CONTINUE
  .andAddToDeadLetterQueue(
  Collections.singletonList(new ProducerRecord<>(
  "app-dlq",
  "Hello".getBytes(StandardCharsets.UTF_8),
  "World".getBytes(StandardCharsets.UTF_8)
  ))
  );
}

I think the code becomes more readable.


Best,
Bruno

On 8/30/24 3:37 PM, Damien Gasparina wrote:

Hi everyone,

We just updated KIP-1034, we changed the following:
- We included the ProcessingExceptionHandler (KIP-1033) directly in the KIP;
- We provided examples to clarify the new configuration, and how it
could be leveraged.

I think we can resume the conversation on this KIP.

Cheers,
Damien Sebastien and Loic


On Tue, 27 Aug 2024 at 15:37, Sebastien Viale
 wrote:



Hi Bruno,

We have planned a meeting for next friday to discuss it with Loic and Damien.

We will be able to restart discussions about it soon.

regards


De : Bruno Cadonna 
Envoyé : lundi 26 août 2024 11:32
À : dev@kafka.apache.org 
Objet : [EXT] Re: [DISCUSS] KIP-1034: Dead letter queue in Kafka Streams

Warning External sender Do not click on any links or open any attachments 
unless you trust the sender and know the content is safe.

Hi Loïc, Sebastien, and Damien,

Now that KIP-1033 is going to be released in 3.9, what is the plan to
progress with this KIP?

Is the KIP up-to-date, so that we can restart discussion?

Best,
Bruno

This email was screened for spam and malicious content but exercise caution 
anyway.




On 6/13/24 6:16 PM, Damien Gasparina wrote:

Hi Bruno,

We focused our effort (well, mostly Seb and Loic :)) on KIP-1033,
that's why not much progress has been made on this one yet.
Regarding your points:

B1: It is up to the user to specify the DLQ topic name and to
implement a potential differentiation. I tend to think that having one
DLQ per application ID is the wisest, but I encountered cases and
applications that preferred having a shared DLQ topic between multiple
applications, e.g. to

Jenkins build is still unstable: Kafka » Kafka Branch Builder » trunk #3263

2024-09-03 Thread Apache Jenkins Server
See 




Re: [DISCUSS] KIP-1076: Metrics for client applications a KIP-714 extension

2024-09-03 Thread Bill Bejeck
Thanks for commenting, Matthias.

 Why did you switch from passing in a `Collection` to a single metric?


With the changes in the implementation it became obvious that was the
necessary approach.

Since it seems the discussion has concluded, I'll start a vote now.
Thanks to all for the comments and discussion.

-Bill


On Fri, Aug 30, 2024 at 10:31 PM Matthias J. Sax  wrote:

> Thanks Bill. The KIP reads great. Overall LGTM.
>
> Just one question out of curiosity. Why did you switch from passing in a
> `Collection` to a single metric?
>
>
> -Matthias
>
>
> On 8/26/24 3:22 PM, Bill Bejeck wrote:
> > All,
> >
> > I had originally planned to call for a vote, but during some early
> > implementation work, I've discovered some changes are necessary.
> > First, we need a reciprocal method to remove a metric from subscriptions,
> > and I've updated the register method to take a single metric.
> >
> > Thanks in advance for taking another look at the updated KIP, and
> hopefully
> > we'll get to a vote this week.
> >
> > -Bill
> >
> > On Thu, Aug 22, 2024 at 3:18 PM Bill Bejeck  wrote:
> >
> >> Matthias and Andrew,
> >>
> >> I've updated the KIP.  I decided to not list the Kafka Stream metrics
> >> supported as it would create a large amount of redundant information.
> >> The types of supported metrics and behavior for unsupported metrics is
> >> clearly defined.
> >>
> >> Thanks,
> >> Bill
> >>
> >> On Thu, Aug 22, 2024 at 12:10 PM Bill Bejeck  wrote:
> >>
> >>> Hi Mathias and Andrew,
> >>>
> >>> Thanks for taking the time to read and comment on the KIP.  I'll
> address
> >>> each comment below:
> >>>
> >>> MS1 - I agree that logging a WARN here should be sufficient.  I'll
> update
> >>> the interface in KIP to the original `void` return type.
> >>> AS1 - I was considering throwing an exception, but you make a
> compelling
> >>> argument about the expected behavior and external dynamic factors.
> >>> So as stated above, the KIP will specify a `void` return type.
> >>>
> >>> MS2 - Good point. I'll update the KIP with a brief statement on
> supported
> >>> types and a table of Kafka Streams metrics displaying what's supported
> and
> >>> which ones aren't.
> >>> AS2 - I think the above addresses your second comment.
> >>>
> >>> MS3 - We could explore adding new metrics, but I wouldn't want that to
> >>> block the progress of this KIP.
> >>> So I'd prefer to defer the discussion of new metrics to a follow-on
> KIP.
> >>> Having said that, if you (or anyone else!) has an idea of
> >>> a specific metric to add, we can consider it now.
> >>>
> >>> MS4 - Unsupported metrics are silently filtered out.  Although it's an
> >>> implementation detail, I propose to log a WARN statement for each
> filtered
> >>> metric passed to `registerMetricsForSubscription`.
> >>> I also update the KIP with the behavior of unsupported metric types.
> >>>
> >>> MS5 - Setting `enable.metric.push = false` completely disables the
> >>> telemetry pipeline and any provided Kafka Streams metrics are ignored
> (I
> >>> have this in the KIP).
> >>> Although this is implementation detail, this is a valid condition to
> >>> throw a `ConfigException` on Kafka Streams startup should this
> >>> inconsistency in telemetry configs exist (at least in the main and
> admin
> >>> consumer).
> >>> I'll update the KIP to be more clear on this point as well.
> >>>
> >>> Thanks,
> >>> Bill
> >>>
> >>> On Thu, Aug 22, 2024 at 10:54 AM Andrew Schofield <
> >>> andrew_schofi...@live.com> wrote:
> >>>
>  Hi Bill (and Matthias),
>  Thanks for the KIP. This looks like a valuable extension to KIP-714.
> 
>  AS1: Personally, I think that registerMetricsForSubscription should
>  return
>  void and not throw an exception if metrics push is not enabled.
>  Otherwise,
>  you end up with an application whose behaviour is markedly different
>  depending
>  on external factors. For example, if you connect to a broker without a
>  plugin
>  that supports the ClientTelemetry interface, no metrics are going to
> be
>  pushed.
>  Similarly, if the enable.metrics.push is false, no metrics are going
> to
>  be pushed.
>  Neither of those seem to me to have implications on whether this
> method
>  executes successfully. Even if everything is enabled, the metrics will
>  only flow
>  if there’s a telemetry subscription which matches the metric names.
>  Those are
>  dynamic things which are intended to be modified by operators seeking
> to
>  diagnose problems. We certainly don’t want the
>  registerMetricsForSubscription
>  method to behave differently depending on the current state of its
>  telemetry
>  subscriptions.
> 
>  AS2: Good catch from Matthias for metrics whose type is incompatible
>  with OTLP. We need to define the behaviour here.
> 
>  Thanks,
>  Andrew
> 
> 
> > On 22 Aug 2024, at 05:35, Matthias J. Sax  wrote:
> >
> > Than

[VOTE] KIP-1076: Metrics for client applications a KIP-714 extension

2024-09-03 Thread Bill Bejeck
Hi All,

I'd like to call for a vote on KIP-1076

 (discussion can be found here
)

-Bill


Re: [VOTE] KIP-1076: Metrics for client applications a KIP-714 extension

2024-09-03 Thread Lucas Brutschy
Hi Bill,

thanks for the KIP! +1 (binding)

Lucas

On Tue, Sep 3, 2024 at 4:49 PM Bill Bejeck  wrote:
>
> Hi All,
>
> I'd like to call for a vote on KIP-1076
> 
>  (discussion can be found here
> )
>
> -Bill


Re: [DISCUSS] KIP-1034: Dead letter queue in Kafka Streams

2024-09-03 Thread Damien Gasparina
Hi Bruno,

> BC1
I see your point. I wonder if having a generic and separate tool
outside of StreamsReset to reset a topic would make sense.
Some community projects have this feature (empty topics) and that's
true that it could be quite useful, e.g. AKHQ.

> BC3
Good point, this is not really a builder class, let me update the KIP
with the proposed syntax.

On Tue, 3 Sept 2024 at 16:00, Bruno Cadonna  wrote:
>
> Hi Damien,
>
> BC1
>
>  > In my opinion DLQ topics should be viewed as a sink topic and
>  > AFAIK, this tool does not clean up sink topics.
>
> Maybe, but one could also argue DLQ topics are part of the runtime
> because is collects errors occurred during the runtime that might be
> specific to given execution of the Streams app. One might want want to
> reset the errors when starting from scratch.
>
>
> BC2
> You are right the configs are passed at creation time. My bad!
>
>
> BC3
> I think the `with`-syntax fits well when building objects that contain
> kind of configs like Materialized that you usually pass into an API.
> However, the handler returns the response. The response instructs
> something. It says CONTINUE processing or FAIL the processing. With your
> KIP the response gets an additional instruction, namely
> `andAddToDeadLetterQueue`. I would not sacrifice better readability for
> consistency in this case.
>
>
> Best,
> Bruno
>
> On 9/3/24 3:18 PM, Damien Gasparina wrote:
> > Hi Bruno,
> >
> > Thanks a lot for your comments!
> >
> >> BC1
> >> Do you also plan to add an argument to the StreamsResetter tool to
> >> delete the default DLQ topic when a Streams app is reset?
> > Good point, I did not think about the StreamsResetter tool. Thinking
> > out loud, I am not sure if it is a good idea to add an option to clean
> > them up. In my opinion DLQ topics should be viewed as a sink topic and
> > AFAIK, this tool does not clean up sink topics.
> >
> >> BC2
> > In case of a custom exception handlers, they can get the
> > errors.deadletterqueue.topic.name configuration by overriding `void
> > configure(Map configs);`. As it is the location where all
> > the configuration can be accessed, I think it's the best way no? I
> > will think about it a bit further, it might be useful/convenient for
> > users.
> >
> >> BC3
> > The "with" syntax is used in many locations in the Kafka Streams
> > public classes, that's why I used it, e.g. Materialized.with(),
> > Consumed.withKeySerde(...).withValueSerde(...), Grouped.withName(...).
> > I do agree that .andAddToDeadLetterQueue is more intuitive, but I
> > would argue that being consistent is better in this situation. What do
> > you think?
> >
> > Cheers,
> > Damien
> >
> > On Tue, 3 Sept 2024 at 14:59, Bruno Cadonna  wrote:
> >>
> >> Hi,
> >>
> >> Thanks for the updates!
> >>
> >> I have a couple of comments.
> >>
> >> BC1
> >> Do you also plan to add an argument to the StreamsResetter tool to
> >> delete the default DLQ topic when a Streams app is reset?
> >>
> >>
> >> BC2
> >> Would it make sense to add errors.deadletterqueue.topic.name to the
> >> ErrorHandlerContext in case that a custom exception handler wants to
> >> write to the configured DLQ topic?
> >> For example if only one of the handler needs to be customized but all
> >> handlers should write to the configured DLQ topic.
> >>
> >>
> >> BC3
> >> What do you think about renaming withDeadLetterQueueRecords() to
> >> andAddToDeadLetterQueue()?
> >> A customized handler would look like
> >>
> >> public ProcessingHandlerResponse handle(
> >>   final ErrorHandlerContext context,
> >>   final Record record,
> >>   final Exception exception
> >> ) {
> >>   return ProcessingHandlerResponse.CONTINUE
> >>   .andAddToDeadLetterQueue(
> >>   Collections.singletonList(new ProducerRecord<>(
> >>   "app-dlq",
> >>   "Hello".getBytes(StandardCharsets.UTF_8),
> >>   "World".getBytes(StandardCharsets.UTF_8)
> >>   ))
> >>   );
> >> }
> >>
> >> I think the code becomes more readable.
> >>
> >>
> >> Best,
> >> Bruno
> >>
> >> On 8/30/24 3:37 PM, Damien Gasparina wrote:
> >>> Hi everyone,
> >>>
> >>> We just updated KIP-1034, we changed the following:
> >>> - We included the ProcessingExceptionHandler (KIP-1033) directly in 
> >>> the KIP;
> >>> - We provided examples to clarify the new configuration, and how it
> >>> could be leveraged.
> >>>
> >>> I think we can resume the conversation on this KIP.
> >>>
> >>> Cheers,
> >>> Damien Sebastien and Loic
> >>>
> >>>
> >>> On Tue, 27 Aug 2024 at 15:37, Sebastien Viale
> >>>  wrote:
> 
> 
>  Hi Bruno,
> 
>  We have planned a meeting for next friday to discuss it with Loic and 
>  Damien.
> 
>  We will be able to restart discussions about it soon.
> 
>  regards
> 
>  
>  De : Bruno Cadonna 
>  Envoyé : lundi 26 août 2024 11:32
>  À : dev@kafka.apache.org 
>  

[jira] [Created] (KAFKA-17471) Speed Up ResetConsumerGroupOffsetTest

2024-09-03 Thread Jira
黃竣陽 created KAFKA-17471:
---

 Summary: Speed Up ResetConsumerGroupOffsetTest
 Key: KAFKA-17471
 URL: https://issues.apache.org/jira/browse/KAFKA-17471
 Project: Kafka
  Issue Type: Improvement
Reporter: 黃竣陽
Assignee: 黃竣陽


see the comments: https://github.com/apache/kafka/pull/16957



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Created] (KAFKA-17472) Speed Up DescribeConsumerGroupTest

2024-09-03 Thread Jira
黃竣陽 created KAFKA-17472:
---

 Summary: Speed Up DescribeConsumerGroupTest
 Key: KAFKA-17472
 URL: https://issues.apache.org/jira/browse/KAFKA-17472
 Project: Kafka
  Issue Type: Improvement
Reporter: 黃竣陽


see the comments: [https://github.com/apache/kafka/pull/16957]



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Created] (KAFKA-17473) Speed Up ClientTelemetryTest

2024-09-03 Thread Jira
黃竣陽 created KAFKA-17473:
---

 Summary: Speed Up ClientTelemetryTest
 Key: KAFKA-17473
 URL: https://issues.apache.org/jira/browse/KAFKA-17473
 Project: Kafka
  Issue Type: Improvement
Reporter: 黃竣陽
Assignee: 黃竣陽


see the comments: [https://github.com/apache/kafka/pull/16957]



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [VOTE] KIP-1076: Metrics for client applications a KIP-714 extension

2024-09-03 Thread Andrew Schofield
Thanks for the KIP. +1 (non-binding)

Andrew

From: Lucas Brutschy 
Sent: 03 September 2024 16:02
To: dev@kafka.apache.org 
Subject: Re: [VOTE] KIP-1076: Metrics for client applications a KIP-714 
extension

Hi Bill,

thanks for the KIP! +1 (binding)

Lucas

On Tue, Sep 3, 2024 at 4:49 PM Bill Bejeck  wrote:
>
> Hi All,
>
> I'd like to call for a vote on KIP-1076
> >
>  (discussion can be found here
> >)
>
> -Bill


[jira] [Created] (KAFKA-17474) Flaky tests in GlobalStreamThreadTest

2024-09-03 Thread Matthias J. Sax (Jira)
Matthias J. Sax created KAFKA-17474:
---

 Summary: Flaky tests in GlobalStreamThreadTest
 Key: KAFKA-17474
 URL: https://issues.apache.org/jira/browse/KAFKA-17474
 Project: Kafka
  Issue Type: Test
  Components: streams, unit tests
Reporter: Matthias J. Sax


Around Aug/24, multiple tests of GlobalStreamThreadTest started to fail with 
high failure rate.
 * 
[shouldStayDeadAfterTwoCloses()|https://ge.apache.org/scans/tests?search.names=Git%20branch&search.relativeStartTime=P28D&search.rootProjectNames=kafka&search.timeZoneId=Europe%2FVienna&search.values=trunk&tests.container=org.apache.kafka.streams.processor.internals.GlobalStreamThreadTest&tests.sortField=FLAKY&tests.test=shouldStayDeadAfterTwoCloses()]
 * 
[shouldCloseStateStoresOnClose()|https://ge.apache.org/scans/tests?search.names=Git%20branch&search.relativeStartTime=P28D&search.rootProjectNames=kafka&search.timeZoneId=Europe%2FVienna&search.values=trunk&tests.container=org.apache.kafka.streams.processor.internals.GlobalStreamThreadTest&tests.sortField=FLAKY&tests.test=shouldCloseStateStoresOnClose()]
 * 
[shouldStopRunningWhenClosedByUser()|https://ge.apache.org/scans/tests?search.names=Git%20branch&search.relativeStartTime=P28D&search.rootProjectNames=kafka&search.timeZoneId=Europe%2FVienna&search.values=trunk&tests.container=org.apache.kafka.streams.processor.internals.GlobalStreamThreadTest&tests.sortField=FLAKY&tests.test=shouldStopRunningWhenClosedByUser()]
 * 
[shouldBeRunningAfterSuccessfulStart()|https://ge.apache.org/scans/tests?search.names=Git%20branch&search.relativeStartTime=P28D&search.rootProjectNames=kafka&search.timeZoneId=Europe%2FVienna&search.values=trunk&tests.container=org.apache.kafka.streams.processor.internals.GlobalStreamThreadTest&tests.sortField=FLAKY&tests.test=shouldBeRunningAfterSuccessfulStart()]



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[VOTE] KIP-1087: Removing intermediateTopicsOption from StreamsResetter

2024-09-03 Thread Arnav Dadarya
Hello All,

I would like to call for a vote on KIP-1087:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-1087%3A+Removing+intermediateTopicsOption+from+StreamsResetter


The discussion can be found here:
https://lists.apache.org/list?dev@kafka.apache.org:lte=1M:1087

Sincerely & Thanks,
Arnav Dadarya


[jira] [Resolved] (KAFKA-17468) Move kafka.log.remote.quota to storage module

2024-09-03 Thread Chia-Ping Tsai (Jira)


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

Chia-Ping Tsai resolved KAFKA-17468.

Fix Version/s: 4.0.0
   Resolution: Fixed

> Move kafka.log.remote.quota to storage module
> -
>
> Key: KAFKA-17468
> URL: https://issues.apache.org/jira/browse/KAFKA-17468
> Project: Kafka
>  Issue Type: Sub-task
>Reporter: Mickael Maison
>Assignee: Mickael Maison
>Priority: Major
> Fix For: 4.0.0
>
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: Possible bug in Authorize by ResourceTypeQue

2024-09-03 Thread Rajini Sivaram
> If ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true" then
> authorizeByResourceType should return true in all cases since the user
> would have access for any operation on any undefined topic?


It should return false if the user is denied all access using wildcard ACL.
For example if you add an ACL to DENY the operation READ for LITERAL
*, authorizeByResourceType for READ fails. Otherwise, access is granted.

Followup2: your answer speaks directly to "WRITE" access.  My example was
> READ access.  So the question method is answering then is: Does the user
> have access to READ any TOPIC?


Yes, that is the question the method is answering. Sorry, I answered using
WRITE because we currently use `authorizeByResourceType` only for `WRITE`.
But the same logic applies for any operation from the authorizer point of
view. So in your example, READ is authorized because alice is allowed to
read a topic that doesn't start with `foo`.

And that is further restricted by the requestContext host is it not?

Yes, it is restricted by host. So the question the method is answering is:
Does the user have access to READ any TOPIC from the host in the request
context?


On Tue, Sep 3, 2024 at 2:16 PM Claude Warren, Jr
 wrote:

> Followup2: your answer speaks directly to "WRITE" access.  My example was
> READ access.  So the question method is answering then is: Does the user
> have access to READ any TOPIC?  And that is further restricted by the
> requestContext host is it not?
>
>
> On Tue, Sep 3, 2024 at 2:10 PM Claude Warren, Jr 
> wrote:
>
> > Followup:  If ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true" then
> > authorizeByResourceType should return true in all cases since the user
> > would have access for any operation on any undefined topic?
> >
> >
> > On Tue, Sep 3, 2024 at 2:08 PM Claude Warren, Jr  >
> > wrote:
> >
> >> I am working on a replacement for the StandardAuthorizer and my
> >> implementation DENIED while the standard implementation ALLOWED.  In
> >> reading the specs I thought it should be DENIED.  But your statement
> makes
> >> it clear that I misread.
> >>
> >> Thank you,
> >> Claude
> >>
> >> On Tue, Sep 3, 2024 at 1:14 PM Rajini Sivaram 
> >> wrote:
> >>
> >>> Hi Claude,
> >>>
> >>> `authorizeByResourceType` doesn't grant access to any specific topic,
> it
> >>> grants access to idempotent write if the user has access to write to
> any
> >>> topic (which may or may not exist). In this case,
> >>> ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true", so `User:alice` can
> >>> write to a topic that doesn't start with `foo` and hence
> >>> `authorizeByResourceType` should be ALLOWED. What was the behaviour you
> >>> observed?
> >>>
> >>> Regards,
> >>>
> >>> Rajini
> >>>
> >>>
> >>> On Tue, Sep 3, 2024 at 12:22 PM Claude Warren 
> wrote:
> >>>
> >>> > *Setup:*
> >>> > Superuser = "User:superman"
> >>> >
> >>> > ACLs added to system
> >>> > new StandardAcl(TOPIC, "foo", PREFIXED, "User:alice", WILDCARD, READ,
> >>> DENY)
> >>> > new StandardAcl(TOPIC, "foobar", LITERAL, "User:alice", WILDCARD,
> READ,
> >>> > ALLOW)
> >>> > new StandardAcl(TOPIC, "foo", PREFIXED, "User:bob", WILDCARD, READ,
> >>> ALLOW)
> >>> >
> >>> > ALLOW_EVERYONE_IF_NO_ACL_IS_FOUND_CONFIG = "true"
> >>> >
> >>> > AuthorizerContext requestContext = MockAuthorizableRequestContext
> with
> >>> > principal = User:alice
> >>> > host = InetAddress.getLocalHost()
> >>> >
> >>> >
> >>> > *Method Call:*
> >>> >
> >>> > authorizer.authorizeByResourceType(requestContext, READ, TOPIC)
> >>> >
> >>> > *Question:*
> >>> >
> >>> > Should the result be true because there is a LITERAL READ ALLOW on
> >>> "foobar"
> >>> > or should the result be false because there is an overriding PREFIXED
> >>> READ
> >>> > DENY on "foo" ?
> >>> >
> >>> >
> >>> >
> >>> > --
> >>> > LinkedIn: http://www.linkedin.com/in/claudewarren
> >>> >
> >>>
> >>
>


Re: [VOTE] KIP-1076: Metrics for client applications a KIP-714 extension

2024-09-03 Thread Matthias J. Sax

+1 (binding)

On 9/3/24 8:56 AM, Andrew Schofield wrote:

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

Andrew

From: Lucas Brutschy 
Sent: 03 September 2024 16:02
To: dev@kafka.apache.org 
Subject: Re: [VOTE] KIP-1076: Metrics for client applications a KIP-714 
extension

Hi Bill,

thanks for the KIP! +1 (binding)

Lucas

On Tue, Sep 3, 2024 at 4:49 PM Bill Bejeck  wrote:


Hi All,

I'd like to call for a vote on KIP-1076
>
  (discussion can be found here
>)

-Bill


Re: [DISCUSS] KIP-1071: Streams Rebalance Protocol

2024-09-03 Thread Sophie Blee-Goldman
Ah, my bad -- I thought I refreshed the page to get the latest version
which is why I was a bit confused when I couldn't find anything about the
new tools which I had previously seen in the KIP. Sorry for the confusion
and unnecessary questions

S1.

> You could imagine calling the initialize RPC
> explicitly (to implement explicit initialization), but this would
> still mean sending an event to the background thread, and the
> background thread in turn invokes the RPC. However, explicit
> initialization would require some additional public interfaces that we
> are not including in this KIP.

I'm confused -- if we do all the group management stuff in a background
thread to avoid needing public APIs, how do we pass all the
Streams-specific and app-specific info to the broker for the
StreamsGroupInitialize? I am guessing this is where we need to invoke
internal APIs and is related to our discussion about removing the
KafkaClientSupplier. However, it seems to me that no matter how you look at
it, Kafka Streams will need to pass information to and from the background
thread if the background thread is to be aware of things like tasks, offset
sums, repartition topics, etc

We don't really need to rehash everything here since it seems like the
proposal to add a StreamsConsumer/StreamsAssignment interface will address
this, which I believe we're in agreement on. I just wanted to point out
that this work to replace the KafkaClientSupplier is clearly intimately
tied up with KIP-1071, and should imho be part of this KIP and not its own
standalone thing.

By the way I'm happy to hop on a call to help move things forward on that
front (or anything really). Although I guess we're just waiting on a design
right now?

S2. I was suggesting something lower for the default upper limit on all
groups. I don't feel too strongly about it, just wanted to point out that
the tradeoff is not about the assignor runtime but rather about resource
consumption, and that too many warmups could put undue load on the cluster.
In the end if we want to trust application operators then I'd say it's fine
to use a higher cluster-wide max like 100.

S8-10 I'll get back to you on this in another followup since I'm still
thinking some things through and want to keep the discussion rolling for
now. In the meantime I have some additional questions:

S11. One of the main issues with unstable assignments today is the fact
that assignors rely on deterministic assignment and use the process Id,
which is not configurable and only persisted via local disk in a
best-effort attempt. It would be a very small change to include this in
KIP-1071 (like 5 LOC), WDYT? (Would even be willing to do the PR for this
myself so as not to add to your load). There's a ticket for it here:
https://issues.apache.org/jira/browse/KAFKA-15190

S12. What exactly is the member epoch and how is it defined? When is it
bumped? I see in the HB request field that certain values signal a member
joining/leaving, but I take it there are valid positive values besides the
0/-1/-2 codes? More on this in the next question:

S13. The KIP says the group epoch is updated in these three cases:
a. Every time the topology is updated through the StreamsGroupInitialize API
b. When a member with an assigned warm-up task reports a task changelog
offset and task changelog end offset whose difference is less that
acceptable.recovery.lag.
c. When a member updates its topology ID, rack ID, client tags or process
ID. Note: Typically, these do not change within the lifetime of a Streams
client, so this only happens when a member with static membership rejoins
with an updated configuration.

S13.i First, just to clarify, these are not the *only* times the group
epoch is bumped but rather the additional cases on top of the regular
consumer group protocol -- so it's still bumped when a new member joins or
leaves, etc, right?
S13.ii Second, does (b) imply the epoch is bumped just whenever the broker
notices a task has finished warming up, or does it first reassign the task
and then bump the group epoch as a result of this task reassignment?
S13.iii Third, (a) mentions the group epoch is bumped on each
StreamsGroupInitialize API but (c) says it gets bumped whenever a member
updates its topology ID. Does this mean that in the event of a rolling
upgrade that changes the topology, the group epoch is bumped just once or
for each member that updates its topology ID? Or does the "updating its
topology ID" somehow have something to do with static membership?
S13.iv How does the broker know when a member has changed its rack ID,
client tags, etc -- does it compare these values on every hb?? That seems
like a lot of broker load. If this is all strictly for static membership
then have we considered leaving static membership support for a followup
KIP? Not necessarily advocating for that, just wondering if this was
thought about already. Imo it would be acceptable to not support static
membership in the first version (espe

Re: [DISCUSS] KIP-1086: Add ability to specify a custom produce request parser.

2024-09-03 Thread Maxim Fortun
Based on feedback from Kirk and Colin I added configuring the parser class name 
via server.properties, added some tests, and updated the docs to reflect this.
I find the config file name by re-parsing the command line. If anyone knows a 
better way of passing KafkaConfig to static initialization, I'd appreciate a 
nudge in the right direction. It's not the most efficient way of retrieving 
configs, but it is done only once at load time, so the overhead should be 
negligible while providing a consistent location for all configs. I have also 
left the system prop and env ways of passing this config in. Hopefully this is 
ok and is not considered a code bloat.

Thanks!
Max 


> On Aug 29, 2024, at 10:25 AM, Maxim Fortun  wrote:
> 
> Hi all,
> I would like to introduce a minor code change to allow custom produce request 
> parsers. 
> 
> KIP: 
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=318606528
> JIRA: https://issues.apache.org/jira/browse/KAFKA-17348
> PR: https://github.com/apache/kafka/pull/16812
> 
> There are many potential benefits for this feature. A custom produce request 
> parser would allow to intercept all incoming messages before they get into 
> the broker and apply broker wide logic to the messages. This could be a 
> trace, a filter, a transform(such as lineage), forcing required headers 
> across all messages, compression, signing, encryption, or any other message 
> manipulation before it gets into the broker. 
> 
> Please take a look.
> Any and all feedback is greatly appreciated.
> Thanks,
> Max
> 
> 
> 
> 



[jira] [Resolved] (KAFKA-17467) Flaky test shouldStayDeadAfterTwoCloses org.apache.kafka.streams.processor.internals.GlobalStreamThreadTest

2024-09-03 Thread Matthias J. Sax (Jira)


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

Matthias J. Sax resolved KAFKA-17467.
-
Resolution: Duplicate

> Flaky test shouldStayDeadAfterTwoCloses 
> org.apache.kafka.streams.processor.internals.GlobalStreamThreadTest
> ---
>
> Key: KAFKA-17467
> URL: https://issues.apache.org/jira/browse/KAFKA-17467
> Project: Kafka
>  Issue Type: Test
>  Components: streams
>Reporter: Igor Soarez
>Priority: Minor
>
> First spotted on the Java 11 tests for 
> https://github.com/apache/kafka/pull/17004
>  
> {code:java}
> org.opentest4j.AssertionFailedError: expected:  but was: 
> 
>   at 
> app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
>   at 
> app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
>   at 
> app//org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
>   at 
> app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
>   at 
> app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
>   at 
> app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
>   at 
> app//org.apache.kafka.streams.processor.internals.GlobalStreamThreadTest.shouldStayDeadAfterTwoCloses(GlobalStreamThreadTest.java:234)
>{code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [VOTE] KIP-1076: Metrics for client applications a KIP-714 extension

2024-09-03 Thread Apoorv Mittal
Thanks for the KIP, looking forward to it.

+1 (non-binding)

Regards,
Apoorv Mittal


On Tue, Sep 3, 2024 at 9:00 PM Matthias J. Sax  wrote:

> +1 (binding)
>
> On 9/3/24 8:56 AM, Andrew Schofield wrote:
> > Thanks for the KIP. +1 (non-binding)
> >
> > Andrew
> > 
> > From: Lucas Brutschy 
> > Sent: 03 September 2024 16:02
> > To: dev@kafka.apache.org 
> > Subject: Re: [VOTE] KIP-1076: Metrics for client applications a KIP-714
> extension
> >
> > Hi Bill,
> >
> > thanks for the KIP! +1 (binding)
> >
> > Lucas
> >
> > On Tue, Sep 3, 2024 at 4:49 PM Bill Bejeck  wrote:
> >>
> >> Hi All,
> >>
> >> I'd like to call for a vote on KIP-1076
> >> <
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FKAFKA%2FKIP-1076%253A%2B%2BMetrics%2Bfor%2Bclient%2Bapplications%2BKIP-714%2Bextension&data=05%7C02%7C%7C0ba83dece6894a4126b508dccc29a7ed%7C84df9e7fe9f640afb435%7C1%7C0%7C638609726517807511%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=DqHjamMzaj4gwSvGwZv89CPCONXxNrB%2BNyhte2oJpKE%3D&reserved=0
> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1076%3A++Metrics+for+client+applications+KIP-714+extension
> >>
> >>   (discussion can be found here
> >> <
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.apache.org%2Fthread%2F9yf4w9qy0zr7qy9ftjcft0s32w32b891&data=05%7C02%7C%7C0ba83dece6894a4126b508dccc29a7ed%7C84df9e7fe9f640afb435%7C1%7C0%7C638609726517819714%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=hP3ykAMVOAb8WigLK92GDLl6pYCty4AgngZrsEDQLhQ%3D&reserved=0
> >)
> >>
> >> -Bill
>


[jira] [Reopened] (KAFKA-16502) Fix flaky EOSUncleanShutdownIntegrationTest#shouldWorkWithUncleanShutdownWipeOutStateStore

2024-09-03 Thread Matthias J. Sax (Jira)


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

Matthias J. Sax reopened KAFKA-16502:
-

> Fix flaky 
> EOSUncleanShutdownIntegrationTest#shouldWorkWithUncleanShutdownWipeOutStateStore
> --
>
> Key: KAFKA-16502
> URL: https://issues.apache.org/jira/browse/KAFKA-16502
> Project: Kafka
>  Issue Type: Test
>  Components: streams, unit tests
>Reporter: Chia-Ping Tsai
>Priority: Major
>  Labels: flaky-test
>
> org.opentest4j.AssertionFailedError: Condition not met within timeout 15000. 
> Expected ERROR state but driver is on RUNNING ==> expected:  but was: 
> 
>   at 
> app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
>   at 
> app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
>   at app//org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
>   at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
>   at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:214)
>   at 
> app//org.apache.kafka.test.TestUtils.lambda$waitForCondition$3(TestUtils.java:396)
>   at 
> app//org.apache.kafka.test.TestUtils.retryOnExceptionWithTimeout(TestUtils.java:444)
>   at 
> app//org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:393)
>   at 
> app//org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:377)
>   at 
> app//org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:350)
>   at 
> app//org.apache.kafka.streams.integration.EOSUncleanShutdownIntegrationTest.shouldWorkWithUncleanShutdownWipeOutStateStore(EOSUncleanShutdownIntegrationTest.java:169)
>   at 
> java.base@11.0.16.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
>  Method)
>   at 
> java.base@11.0.16.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>   at 
> java.base@11.0.16.1/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>   at java.base@11.0.16.1/java.lang.reflect.Method.invoke(Method.java:566)
>   at 
> app//org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
>   at 
> app//org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
>   at 
> app//org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
>   at 
> app//org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
>   at 
> app//org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:299)
>   at 
> app//org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:293)
>   at 
> java.base@11.0.16.1/java.util.concurrent.FutureTask.run(FutureTask.java:264)
>   at java.base@11.0.16.1/java.lang.Thread.run(Thread.java:829)



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Resolved] (KAFKA-16502) Fix flaky EOSUncleanShutdownIntegrationTest#shouldWorkWithUncleanShutdownWipeOutStateStore

2024-09-03 Thread Matthias J. Sax (Jira)


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

Matthias J. Sax resolved KAFKA-16502.
-
Resolution: Fixed

> Fix flaky 
> EOSUncleanShutdownIntegrationTest#shouldWorkWithUncleanShutdownWipeOutStateStore
> --
>
> Key: KAFKA-16502
> URL: https://issues.apache.org/jira/browse/KAFKA-16502
> Project: Kafka
>  Issue Type: Test
>  Components: streams, unit tests
>Reporter: Chia-Ping Tsai
>Priority: Major
>  Labels: flaky-test
>
> org.opentest4j.AssertionFailedError: Condition not met within timeout 15000. 
> Expected ERROR state but driver is on RUNNING ==> expected:  but was: 
> 
>   at 
> app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
>   at 
> app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
>   at app//org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
>   at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
>   at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:214)
>   at 
> app//org.apache.kafka.test.TestUtils.lambda$waitForCondition$3(TestUtils.java:396)
>   at 
> app//org.apache.kafka.test.TestUtils.retryOnExceptionWithTimeout(TestUtils.java:444)
>   at 
> app//org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:393)
>   at 
> app//org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:377)
>   at 
> app//org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:350)
>   at 
> app//org.apache.kafka.streams.integration.EOSUncleanShutdownIntegrationTest.shouldWorkWithUncleanShutdownWipeOutStateStore(EOSUncleanShutdownIntegrationTest.java:169)
>   at 
> java.base@11.0.16.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
>  Method)
>   at 
> java.base@11.0.16.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>   at 
> java.base@11.0.16.1/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>   at java.base@11.0.16.1/java.lang.reflect.Method.invoke(Method.java:566)
>   at 
> app//org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
>   at 
> app//org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
>   at 
> app//org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
>   at 
> app//org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
>   at 
> app//org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:299)
>   at 
> app//org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:293)
>   at 
> java.base@11.0.16.1/java.util.concurrent.FutureTask.run(FutureTask.java:264)
>   at java.base@11.0.16.1/java.lang.Thread.run(Thread.java:829)



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [EXT] Re: [VOTE] KIP-1033: Add Kafka Streams exception handler for exceptions occurring during processing

2024-09-03 Thread Colin McCabe
Hi Matthias and Sebastien,

This sounds like a very small and low-risk PR for a KIP that is new in 3.9 
(KIP-1033). I think we should be OK to cherry-pick it to 3.9, even though we're 
past code freeze. Please try to get it done this week, though :)

best,
Colin


On Fri, Aug 30, 2024, at 19:03, Matthias J. Sax wrote:
> Guess it is a question for Colin, who is the RM for 3.9.
>
>
>
> -Matthias
>
>
>
> On 8/30/24 2:02 AM, Sebastien Viale wrote:
>> Hi,
>> 
>> All PRs related to KIP-1033 have been merged.
>> 
>> However, after a last review from Matthias J. Sax and Bill Bejeck, we 
>> updated the KIP to include the timestamp in the ErrorHandlerContext 
>> interface.
>> 
>> A new PR has been initialized.
>> KAFKA-16448 Add timestamp to error handler context 
>> (https://github.com/apache/kafka/pull/17054)
>> 
>> Please let us know if we need to work on this for 3.9 or for next release.
>> 
>> thanks very much
>> 
>> Sébastien
>> 
>> 
>> De : Matthias J. Sax 
>> Envoyé : jeudi 1 août 2024 23:46
>> À : dev@kafka.apache.org 
>> Objet : [EXT] Re: [VOTE] KIP-1033: Add Kafka Streams exception handler for 
>> exceptions occurring during processing
>> 
>> Warning External sender Do not click on any links or open any attachments 
>> unless you trust the sender and know the content is safe.
>> 
>> Thanks Loïc!
>> 
>> 
>> This email was screened for spam and malicious content but exercise caution 
>> anyway.
>> 
>> 
>> On 8/1/24 6:19 AM, Loic Greffier wrote:
>>> Hi,
>>>
>>> Last PR that completed KIP-1033 has been merged.
>>> Thanks to all participants, for the guidance and the feedbacks!
>>>
>>> Just letting you know about some decisions and changes made through the 
>>> different PRs that were not initially mentioned in the KIP:
>>>
>>> *   The sourceRawKey and the sourceRawValue attributes present in the 
>>> ErrorHandlerContext have been removed from this KIP. They will be part of 
>>> KIP-1034 instead.
>>> *   The number of processing exception handler instance has been fixed 
>>> to one per StreamTask.
>>> *   The processing exception handling mechanism has been extended to 
>>> punctuations.
>>> *   The handler crash management has been unified and improved. 
>>> Exceptions thrown from a call to #handle are caught and fail the stream. 
>>> This change covers 5 use-cases:
>>>*   On processingExceptionHandler#handle to cover processing 
>>> exceptions.
>>>*   On processingExceptionHandler#handle to cover punctuation 
>>> exceptions.
>>>*   On productionExceptionHandler#handle to cover production 
>>> exceptions.
>>>*   On productionExceptionHandler#handleSerializationException to 
>>> cover serialization exceptions.
>>>*   On deserializationExceptionHandler#handle to cover 
>>> deserialization exceptions.
>>>
>>> KIP-1033 has been updated with these changes.
>>>
>>> Loïc
>>>
>> 
>>


Re: New release branch 3.9

2024-09-03 Thread Colin McCabe
On Fri, Aug 30, 2024, at 11:53, Chia-Ping Tsai wrote:
> hi Colin
>
> I open the https://issues.apache.org/jira/browse/KAFKA-17454 to be a 
> blocker for 3.9 since the e2e transactions_mixed_versions_test fail on 
> 3.9 definitely
>
> We will fix it tomorrow.
>
> Best,
> Chia-Ping

Thanks, Chia-Ping Tsai.

best,
Colin

>
> On 2024/08/30 15:38:48 Colin McCabe wrote:
>> No worries. Thanks for checking.
>> 
>> best,
>> Colin
>> 
>> On Thu, Aug 29, 2024, at 19:00, Kirk True wrote:
>> > Thanks Chia-Ping!
>> >
>> > Sorry for the noise, everyone else :)
>> >
>> >> On Aug 29, 2024, at 6:36 PM, Chia-Ping Tsai  wrote:
>> >> 
>> >> hi Kirk
>> >> 
>> >> I have updated the fix versions of KAFKA-17335 to include 3.9
>> >> 
>> >> https://issues.apache.org/jira/browse/KAFKA-17335
>> >> 
>> >> Best,
>> >> Chia-Ping
>> >> 
>> >> Chia-Ping Tsai  於 2024年8月30日 週五 上午9:32寫道:
>> >> 
>> >>> hi Kirk
>> >>> 
>> >>> KAFKA-17335 is already in 3.9 (
>> >>> https://github.com/apache/kafka/commit/a87b501a47a55a1f9038b6c52e31b71590c3a80a
>> >>> )
>> >>> 
>> >>> Best,
>> >>> Chia-Ping
>> >>> 
>> >>> Kirk True  於 2024年8月30日 週五 上午4:36寫道:
>> >>> 
>>  Hi Colin,
>>  
>>  KAFKA-17335 (Lack of default for URL encoding configuration for OAuth
>>  causes NPE) affects the internal use of the OAuth-based
>>  AuthenticateCallbackHandler. Can we back port the fix (
>>  https://github.com/apache/kafka/pull/16990) to the 3.9 branch?
>>  
>>  Thanks,
>>  Kirk
>>  
>> > On Aug 29, 2024, at 11:29 AM, Colin McCabe  wrote:
>> > 
>> > Hi all,
>> > 
>> > Code freeze has officially arrived for Apache Kafka 3.9! If you have
>>  additional fixes to get into the branch at this point, please ping me 
>>  first
>>  (unless it's already marked as a blocker in JIRA).
>> > 
>> > I did a first pass on the list of open JIRAs for 3.9, and moved out any
>>  that were not blockers for 3.9. In a few cases, I asked a question about
>>  whether the JIRA was a blocker or not.
>> > 
>> > If all goes according to plan, we should now have two more weeks of
>>  stabilization before the release.
>> > 
>> > Thanks, everyone.
>> > 
>> > Colin
>> > 
>> > 
>> > On Fri, Aug 23, 2024, at 13:30, Colin McCabe wrote:
>> >> I wanted to give a quick update on 3.9. Code freeze is coming up next
>> >> Thursday on the 29th. As usual, after that point, only critical bug
>> >> fixes will be accepted.
>> >> 
>> >> Thanks again to everyone who has helped with this release.
>> >> 
>> >> best,
>> >> Colin
>> >> 
>> >> 
>> >> On Fri, Aug 23, 2024, at 13:18, Colin McCabe wrote:
>> >>> Thanks for the heads up, Josep. I saw both emails but I didn't make
>>  the
>> >>> connection until now. It does seem likely that we'll have to modify
>> >>> this (either for 3.9... or soon.)
>> >>> 
>> >>> best,
>> >>> Colin
>> >>> 
>> >>> On Tue, Aug 13, 2024, at 05:09, Josep Prat wrote:
>>  Hi Colin,
>>  
>>  Just to make sure you (as a Release Manager) are aware of the
>>  change. ASF
>>  Infra just communicated the removal of the home.apache.org machines
>>  (in 4
>>  weeks they will be gone). This means the release.py script wouldn't
>>  work if
>>  executed after that date and would need to be modified to store RC
>>  artifacts under dist.apache.org/dev instead.
>>  
>>  Best,
>>  
>>  On Fri, Aug 9, 2024 at 11:38 PM Colin McCabe 
>>  wrote:
>>  
>> > Updates:
>> > 
>> > KIP-1007: I have removed this PR from the Apache Kafka 3.9 release
>>  plan.
>> > Since the KIP was discarded, I did not add it in the "postponed to
>> > subsequent release" section.
>> > 
>> > KIP-1025: Marked 3.9 status as Done
>> > 
>> > KIP-1023: Postponed to subsequent release
>> > 
>> > KIP-1005: Marked 3.9 status as Done
>> > 
>> > KIP-996: Postponed to subsequent release
>> > 
>> > KIP-994: Postponed to subsequent release
>> > 
>> > KIP-997: Postponed to subsequent release
>> > 
>> > KIP-966: Postponed to subsequent release
>> > 
>> > KIP-956: Postponed to subsequent release
>> > 
>> > KIP-950: Marked 3.9 status as Done
>> > 
>> > best,
>> > Colin
>> > 
>> > On Fri, Aug 9, 2024, at 14:13, Colin McCabe wrote:
>> >> Hi Chia-Ping Tsai,
>> >> 
>> >> Thank you! And thanks for your own contributions, especially the
>>  many
>> >> code reviews (not only of my PRs, but everyone's).
>> >> 
>> >> best,
>> >> Colin
>> >> 
>> >> On Thu, Aug 8, 2024, at 19:14, Chia-Ping Tsai wrote:
>>  - We have completed all the feature work for KIP

Re: New release branch 3.9

2024-09-03 Thread Colin McCabe
On Mon, Sep 2, 2024, at 20:15, Luke Chen wrote:
> Hi Colin,
>
> I've made KAFKA-17412  (
> PR ) as v3.9.0 blocker for
> documenting the unclean leader election behavior change in KRaft mode.
> This is just a document change, so I think it should be fine.
>
> Let me know if you have any concerns.
>
> Thanks.
> Luke

Thanks for the docs fix. Merged.

best,
Colin

>
> On Sat, Aug 31, 2024 at 2:53 AM Chia-Ping Tsai  wrote:
>
>> hi Colin
>>
>> I open the https://issues.apache.org/jira/browse/KAFKA-17454 to be a
>> blocker for 3.9 since the e2e transactions_mixed_versions_test fail on 3.9
>> definitely
>>
>> We will fix it tomorrow.
>>
>> Best,
>> Chia-Ping
>>
>> On 2024/08/30 15:38:48 Colin McCabe wrote:
>> > No worries. Thanks for checking.
>> >
>> > best,
>> > Colin
>> >
>> > On Thu, Aug 29, 2024, at 19:00, Kirk True wrote:
>> > > Thanks Chia-Ping!
>> > >
>> > > Sorry for the noise, everyone else :)
>> > >
>> > >> On Aug 29, 2024, at 6:36 PM, Chia-Ping Tsai 
>> wrote:
>> > >>
>> > >> hi Kirk
>> > >>
>> > >> I have updated the fix versions of KAFKA-17335 to include 3.9
>> > >>
>> > >> https://issues.apache.org/jira/browse/KAFKA-17335
>> > >>
>> > >> Best,
>> > >> Chia-Ping
>> > >>
>> > >> Chia-Ping Tsai  於 2024年8月30日 週五 上午9:32寫道:
>> > >>
>> > >>> hi Kirk
>> > >>>
>> > >>> KAFKA-17335 is already in 3.9 (
>> > >>>
>> https://github.com/apache/kafka/commit/a87b501a47a55a1f9038b6c52e31b71590c3a80a
>> > >>> )
>> > >>>
>> > >>> Best,
>> > >>> Chia-Ping
>> > >>>
>> > >>> Kirk True  於 2024年8月30日 週五 上午4:36寫道:
>> > >>>
>> >  Hi Colin,
>> > 
>> >  KAFKA-17335 (Lack of default for URL encoding configuration for
>> OAuth
>> >  causes NPE) affects the internal use of the OAuth-based
>> >  AuthenticateCallbackHandler. Can we back port the fix (
>> >  https://github.com/apache/kafka/pull/16990) to the 3.9 branch?
>> > 
>> >  Thanks,
>> >  Kirk
>> > 
>> > > On Aug 29, 2024, at 11:29 AM, Colin McCabe 
>> wrote:
>> > >
>> > > Hi all,
>> > >
>> > > Code freeze has officially arrived for Apache Kafka 3.9! If you
>> have
>> >  additional fixes to get into the branch at this point, please ping
>> me first
>> >  (unless it's already marked as a blocker in JIRA).
>> > >
>> > > I did a first pass on the list of open JIRAs for 3.9, and moved
>> out any
>> >  that were not blockers for 3.9. In a few cases, I asked a question
>> about
>> >  whether the JIRA was a blocker or not.
>> > >
>> > > If all goes according to plan, we should now have two more weeks of
>> >  stabilization before the release.
>> > >
>> > > Thanks, everyone.
>> > >
>> > > Colin
>> > >
>> > >
>> > > On Fri, Aug 23, 2024, at 13:30, Colin McCabe wrote:
>> > >> I wanted to give a quick update on 3.9. Code freeze is coming up
>> next
>> > >> Thursday on the 29th. As usual, after that point, only critical
>> bug
>> > >> fixes will be accepted.
>> > >>
>> > >> Thanks again to everyone who has helped with this release.
>> > >>
>> > >> best,
>> > >> Colin
>> > >>
>> > >>
>> > >> On Fri, Aug 23, 2024, at 13:18, Colin McCabe wrote:
>> > >>> Thanks for the heads up, Josep. I saw both emails but I didn't
>> make
>> >  the
>> > >>> connection until now. It does seem likely that we'll have to
>> modify
>> > >>> this (either for 3.9... or soon.)
>> > >>>
>> > >>> best,
>> > >>> Colin
>> > >>>
>> > >>> On Tue, Aug 13, 2024, at 05:09, Josep Prat wrote:
>> >  Hi Colin,
>> > 
>> >  Just to make sure you (as a Release Manager) are aware of the
>> >  change. ASF
>> >  Infra just communicated the removal of the home.apache.org
>> machines
>> >  (in 4
>> >  weeks they will be gone). This means the release.py script
>> wouldn't
>> >  work if
>> >  executed after that date and would need to be modified to store
>> RC
>> >  artifacts under dist.apache.org/dev instead.
>> > 
>> >  Best,
>> > 
>> >  On Fri, Aug 9, 2024 at 11:38 PM Colin McCabe <
>> cmcc...@apache.org>
>> >  wrote:
>> > 
>> > > Updates:
>> > >
>> > > KIP-1007: I have removed this PR from the Apache Kafka 3.9
>> release
>> >  plan.
>> > > Since the KIP was discarded, I did not add it in the
>> "postponed to
>> > > subsequent release" section.
>> > >
>> > > KIP-1025: Marked 3.9 status as Done
>> > >
>> > > KIP-1023: Postponed to subsequent release
>> > >
>> > > KIP-1005: Marked 3.9 status as Done
>> > >
>> > > KIP-996: Postponed to subsequent release
>> > >
>> > > KIP-994: Postponed to subsequent release
>> > >
>> > > KIP-997: Postponed to subsequent release
>> > >
>> > > KIP

Re: [DISCUSS] KAFKA-17316 and KAFKA-17423

2024-09-03 Thread Colin McCabe
On Tue, Sep 3, 2024, at 01:46, Claude Warren, Jr wrote:
> Colin,
> 
> I can see that it makes sense to replace the StandardAuthorizer so that there 
> are not 2 implementations.  However, I think the testing framework should 
> remain so that in future new improved implementations of 
> StandardAuthorizerData can be easily implemented and tested.
> 
> I will put forward a pull request to satisfy KAFKA-17423 that contains only 
> the new implementation.
> 
> Claude

Hi Claude,

Seems like we're hopping around between email threads a bit. I think your 
proposed changes are good and it sounds like we agree that they should become 
part of StandardAuthorizer rather than a separate authorizer. That was my main 
feedback... the rest is probably stuff we can discuss in a PR. (Or maybe 
whoever is reviewing can discuss...)

I am curious why StandardAuthorizerData would be easier to unit-test than 
StandardAuthorizer, but... again, probably better to discuss in a PR. (You 
might be right about this, I just can't visualize it right now without the code 
in front of me)

[More comments below]

On Mon, Sep 2, 2024 at 9:09 AM Claude Warren, Jr  wrote:
>> I have been working on implementing a Trie structure to store ACLs and 
>> improve the performance in the metadata/authorization code.  The upshot of 
>> this was that I found it very difficult to determine if the implementation 
>> was correctly reimplementing the current implementation.
>> 
>> My goal was to simply change the StandardAuthorizerData implementation and 
>> leave the rest of the code as is.  However, there were no abstracted tests 
>> that would allow me to test this.
>> 
>> KAFKA-17316 addresses this issue by creating some internal interfaces for 
>> the "metadata/authorizer" package.  The one change to the StandardAuthorizer 
>> was to implement the"authorizeByResourceType" defined in the 
>> "org.apache.kafka.server.authorizer.Authorizer" interface by passing the 
>> request down to the AuthorizerData implementation.
>> 
>> This change allowed me to create three test implementations.  One that 
>> implemented "authorizeByResourceType" as it is in the released code base, 
>> one that verified that the StandardAuthorizerData implementation did not 
>> change the expected results, and one that showed the Trie implementation in 
>> KAFKA-17423 was also correct.
>> 
>> I think that retaining the work in KAFKA-17316 makes sense as when the next 
>> faster implementation comes along we can drop in the replacement and verify 
>> that it works correctly.
>> 
>> KAFKA-17423 builds on KAFKA-17316 by implementing a Trie based 
>> AuthorizerData implementation.  By splitting the data into a Trie format the 
>> search for matching ACLs is improved by an order of magnitude.  The trie 
>> implementation allows us to quickly locate the candidate ACLs by splitting 
>> them into groups based upon the similarity of the resource name.  In 
>> addition since we are moving through the trie based on resource name we have 
>> several advantages:
>>  1. If we encounter a matching DENY while descending the Trie we can stop as 
>> it overrides anything that may be found at lower levels.  
>>  2. We only look for LITERAL matches on the descent.  If we reach a matching 
>> resource name or a leaf node we know there are no LITERAL matches.
>>  3. If we don't have a DENY or a LITERAL match we walk back up the path 
>> checking the nodes from the descent looking for a PREFIX match.  The 
>> advantage here is that we don't have to search again but can simply retrace 
>> the path using the Trie structure.
>> I believe that #1 and #2 above account for a significant portion of the 
>> speed increase as we do not have to reposition within a sorted list of all 
>> ACLs using a binary search.
>> 
>> Finally, I think that we should prohibit the use of the java.util.stream 
>> classes within the authorizer due to hot path speed considerations.  The 
>> only existing code that uses streams within that package were test cases.  
>> We can prohibit the use by a simple checkstyle prohibition.  Doing so will 
>> short circuit any misguided potential changes.
>> 
>> Thoughs?
>> Claude
>> 

You know, I've been burned by the java.util.stream classes a few times and I am 
inclined to agree with this. It just makes it too easy for people to introduce 
O(N^2) or worse behavior. If you have to write the nested "for" loop yourself, 
the O(N^2)-ness just smacks you in the face. Streams lets you sweep it under 
the rug with filter(), map(), collect(), etc.. Dangerous.

It's fine for tests, though, of course.

best,
Colin

Re: [DISCUSS] KIP-1085: Fix leaking *_DOC variables in StreamsConfig

2024-09-03 Thread Matthias J. Sax

Thanks for the KIP. I think it's a good clean.

Given that it's a simple KIP, I don't expect much more discussions; feel 
free to start a vote.



-Matthias

On 8/28/24 10:37 PM, 黃竣陽 wrote:

Hi Chia-Ping


they can be package-private, right?

Yes, your are right, I will address there variables on this KIP

Best regards,
Jiunn Yang


Chia-Ping Tsai  於 2024年8月29日 中午12:26 寫道:


These variables are used in `TopologyConfig`, so I think that they can be

public if they are presented the same meaning in both `TopologyConfig` and
`StreamConfig`.

they can be package-private, right?

Best,
Chia-Ping

黃竣陽  於 2024年8月29日 週四 上午11:53寫道:


Hi Chia-Ping,

These variables are used in `TopologyConfig`, so I think that they can be
public if they are presented the same meaning in both `TopologyConfig` and
`StreamConfig`.

Best regards,
Jiunn Yang


Chia-Ping Tsai  於 2024年8月29日 上午11:38 寫道:

hi Jiunn

The following DOC-related variables are public too. Should they be

included?


BUFFERED_RECORDS_PER_PARTITION_DOC
CACHE_MAX_BYTES_BUFFERING_DOC
DEFAULT_DESERIALIZATION_EXCEPTION_HANDLER_CLASS_DOC
DEFAULT_TIMESTAMP_EXTRACTOR_CLASS_DOC
MAX_TASK_IDLE_MS_DOC
STATESTORE_CACHE_MAX_BYTES_DOC
TASK_TIMEOUT_MS_DOC

Best,
Chia-Ping

Luke Chen  於 2024年8月29日 週四 上午11:26寫道:


Hi Jiunn,

Thanks for the KIP.

In the motivation section:
"The overly large scope of these variables can cause development

issues."


I can't see why a "doc description variable" will cause any development
issue.
Could you explain more clearly what problem we have now?

Thanks.
Luke

On Thu, Aug 29, 2024 at 9:43 AM 黃竣陽  wrote:


Hello everyone,

I would like to start a discussion about KIP-1085
<




https://cwiki.apache.org/confluence/display/KAFKA/KIP-1085%3A+Fix+leaking+*_DOC+variables+in+StreamsConfig



In this KIP, we plan to deprecate the public doc variables which only

used

in `StreamConfig`
and make them private in a future release.

Any feedback and suggestions for the KIP are welcome in this email

thread.


Thank you!
Best regards,
Jiunn Yang









Re: [DISCUSS] KIP-1081: Graduation Steps for Features

2024-09-03 Thread Colin McCabe
On Fri, Aug 30, 2024, at 16:40, Matthias J. Sax wrote:
> Great discussion. Also wanted to follow up with a few things.
>
>
> I believe the term "usable" is not well defined leading to confusion... 
> I agree with Viktor that "usable" in the context of level 2 should just 
> mean: I can enable the feature and it does something... not more, not 
> less. It might crash; it might compute the wrong result for some cases, 
> it might have terrible performance, etc... but: I can kick the tires.
>

Yeah, it would be good to clarify this, to avoid "usable" becoming too 
expansive.

>
> About the proposed "checklist" from Viktor: I think we should not have 
> anything about testing in it -- that test are required goes w/o saying 
> and is already covered in the KIP document itself. To me, it's the KIP 
> author's / community's responsivity to decide on a case-by-case basis 
> when a feature is considered ready for the next level, and what testing 
> is sufficient for each level.
>
>
> Similar for docs, even if I agree that docs should be more or less 
> complete at level 3. Otherwise, users will have a hard time to really 
> try the feature and thus kinda defeats the purpose of level 3.

+1

>
>
> Last: @Colin, yes we eventually need to pick names for the levels. But I 
> believe it's actually the right way to agree on the "what" first, and 
> just say "level X" for now, and only after we agree on the levels, we 
> enter the ring for the fun part: picking names. This should be the very 
> last step :popcorn:
>

Maybe this is just me, but using numbers instead of names makes it quite hard 
for me to get a handle on the discussion. I have opinions on what alpha / beta 
/ production-ready mean. I don't have opinions on what "Level 4" means or  what 
"manuscript" means. So I feel like we will go around and around until we can 
give a name to what we're talking about.

best,
Colin


>
>
> -Matthias
>
>
>
>
> On 8/30/24 8:57 AM, Colin McCabe wrote:
>> On Mon, Aug 26, 2024, at 10:51, Josep Prat wrote:
>>> Hi Colin,
>>>
>>> Names are in the KIP. Level 1 to 4 are never meant to be used outside of
>>> this discussion. It's my, apparently successful, attempt to focus on what
>>> the levels mean instead of their names:
>>>
 Names
>>>
>>>  "In Development"
>>>  "Early Access"
>>>  "Preview"
>>>  "Production Ready"
>> 
>> Hi Josep,
>> 
>> Thanks for the clarification. I think we should remove references to level 
>> 1, 2, 3, 4, etc. if that is not the terminology that we want to use. One of 
>> the big purposes of a KIP is to standardize on terminology. That's not 
>> achieved if different parts of the KIP use different names for the same 
>> things.
>> 
>>>
>>> Alternatively, if we want to be a bit more playful we could go with a theme
>>> borrowed from the book industry (as an homage to Franz Kafka):
>>>
>>>  "In Development"
>>>  "Manuscript"
>>>  "Pre-print"
>>>  "Published"
>>>
>>>
>> 
>> The need to standardize terminology also means that, sorry, you have to 
>> choose. :) This is actually a feedback I often give on KIPs. People like to 
>> add sections that say "maybe we'll do X, maybe we'll do Y." But to make 
>> progress on the KIP, you have to choose either X or Y and put the other one 
>> in the "rejected alternatives" section.
>> 
>> I think our purpose in choosing names should be clarity for users and 
>> developers. That's why I suggested "not implemented", "alpha", "beta", 
>> "production ready". I am curious what your thoughts are about these.
>> 
>> best,
>> Colin


Re: [DISCUSS] KIP-1085: Fix leaking *_DOC variables in StreamsConfig

2024-09-03 Thread Matthias J. Sax
One more thing: there is also `StreamsConfig#DUMMY_THREAD_INDEX` which 
is an unused variable.


Can we also deprecate it as part of this KIP as a side cleanup (it's not 
worth to do a separate KIP about it, but given it's technically public 
API, we still need to cover it with a KIP)



-Matthias

On 9/3/24 4:27 PM, Matthias J. Sax wrote:

Thanks for the KIP. I think it's a good clean.

Given that it's a simple KIP, I don't expect much more discussions; feel 
free to start a vote.



-Matthias

On 8/28/24 10:37 PM, 黃竣陽 wrote:

Hi Chia-Ping


they can be package-private, right?

Yes, your are right, I will address there variables on this KIP

Best regards,
Jiunn Yang


Chia-Ping Tsai  於 2024年8月29日 中午12:26 寫道:

These variables are used in `TopologyConfig`, so I think that they 
can be
public if they are presented the same meaning in both 
`TopologyConfig` and

`StreamConfig`.

they can be package-private, right?

Best,
Chia-Ping

黃竣陽  於 2024年8月29日 週四 上午11:53寫道:


Hi Chia-Ping,

These variables are used in `TopologyConfig`, so I think that they 
can be
public if they are presented the same meaning in both 
`TopologyConfig` and

`StreamConfig`.

Best regards,
Jiunn Yang


Chia-Ping Tsai  於 2024年8月29日 上午11:38 寫道:

hi Jiunn

The following DOC-related variables are public too. Should they be

included?


BUFFERED_RECORDS_PER_PARTITION_DOC
CACHE_MAX_BYTES_BUFFERING_DOC
DEFAULT_DESERIALIZATION_EXCEPTION_HANDLER_CLASS_DOC
DEFAULT_TIMESTAMP_EXTRACTOR_CLASS_DOC
MAX_TASK_IDLE_MS_DOC
STATESTORE_CACHE_MAX_BYTES_DOC
TASK_TIMEOUT_MS_DOC

Best,
Chia-Ping

Luke Chen  於 2024年8月29日 週四 上午11:26寫道:


Hi Jiunn,

Thanks for the KIP.

In the motivation section:
"The overly large scope of these variables can cause development

issues."


I can't see why a "doc description variable" will cause any 
development

issue.
Could you explain more clearly what problem we have now?

Thanks.
Luke

On Thu, Aug 29, 2024 at 9:43 AM 黃竣陽  wrote:


Hello everyone,

I would like to start a discussion about KIP-1085
<




https://cwiki.apache.org/confluence/display/KAFKA/KIP-1085%3A+Fix+leaking+*_DOC+variables+in+StreamsConfig


In this KIP, we plan to deprecate the public doc variables which 
only

used

in `StreamConfig`
and make them private in a future release.

Any feedback and suggestions for the KIP are welcome in this email

thread.


Thank you!
Best regards,
Jiunn Yang









Jenkins build is still unstable: Kafka » Kafka Branch Builder » 3.7 #194

2024-09-03 Thread Apache Jenkins Server
See 




Re: [DISCUSS] KIP-1085: Fix leaking *_DOC variables in StreamsConfig

2024-09-03 Thread 黃竣陽
Hi Matthias

I will add deprecated with this `StreamsConfig#DUMMY_THREAD_INDEX` variable in 
this KIP.

Best regards,
Jiunn-Yang

> Matthias J. Sax  於 2024年9月4日 清晨7:37 寫道:
> 
> One more thing: there is also `StreamsConfig#DUMMY_THREAD_INDEX` which is an 
> unused variable.
> 
> Can we also deprecate it as part of this KIP as a side cleanup (it's not 
> worth to do a separate KIP about it, but given it's technically public API, 
> we still need to cover it with a KIP)
> 
> 
> -Matthias
> 
> On 9/3/24 4:27 PM, Matthias J. Sax wrote:
>> Thanks for the KIP. I think it's a good clean.
>> Given that it's a simple KIP, I don't expect much more discussions; feel 
>> free to start a vote.
>> -Matthias
>> On 8/28/24 10:37 PM, 黃竣陽 wrote:
>>> Hi Chia-Ping
>>> 
 they can be package-private, right?
>>> Yes, your are right, I will address there variables on this KIP
>>> 
>>> Best regards,
>>> Jiunn Yang
>>> 
 Chia-Ping Tsai  於 2024年8月29日 中午12:26 寫道:
 
> These variables are used in `TopologyConfig`, so I think that they can be
 public if they are presented the same meaning in both `TopologyConfig` and
 `StreamConfig`.
 
 they can be package-private, right?
 
 Best,
 Chia-Ping
 
 黃竣陽  於 2024年8月29日 週四 上午11:53寫道:
 
> Hi Chia-Ping,
> 
> These variables are used in `TopologyConfig`, so I think that they can be
> public if they are presented the same meaning in both `TopologyConfig` and
> `StreamConfig`.
> 
> Best regards,
> Jiunn Yang
> 
>> Chia-Ping Tsai  於 2024年8月29日 上午11:38 寫道:
>> 
>> hi Jiunn
>> 
>> The following DOC-related variables are public too. Should they be
> included?
>> 
>> BUFFERED_RECORDS_PER_PARTITION_DOC
>> CACHE_MAX_BYTES_BUFFERING_DOC
>> DEFAULT_DESERIALIZATION_EXCEPTION_HANDLER_CLASS_DOC
>> DEFAULT_TIMESTAMP_EXTRACTOR_CLASS_DOC
>> MAX_TASK_IDLE_MS_DOC
>> STATESTORE_CACHE_MAX_BYTES_DOC
>> TASK_TIMEOUT_MS_DOC
>> 
>> Best,
>> Chia-Ping
>> 
>> Luke Chen  於 2024年8月29日 週四 上午11:26寫道:
>> 
>>> Hi Jiunn,
>>> 
>>> Thanks for the KIP.
>>> 
>>> In the motivation section:
>>> "The overly large scope of these variables can cause development
> issues."
>>> 
>>> I can't see why a "doc description variable" will cause any development
>>> issue.
>>> Could you explain more clearly what problem we have now?
>>> 
>>> Thanks.
>>> Luke
>>> 
>>> On Thu, Aug 29, 2024 at 9:43 AM 黃竣陽  wrote:
>>> 
 Hello everyone,
 
 I would like to start a discussion about KIP-1085
 <
 
>>> 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1085%3A+Fix+leaking+*_DOC+variables+in+StreamsConfig
> 
 In this KIP, we plan to deprecate the public doc variables which only
>>> used
 in `StreamConfig`
 and make them private in a future release.
 
 Any feedback and suggestions for the KIP are welcome in this email
>>> thread.
 
 Thank you!
 Best regards,
 Jiunn Yang
>>> 
> 
> 
>>> 



[jira] [Created] (KAFKA-17475) Change the base image of e2e from openjdk to eclipse-temurin

2024-09-03 Thread Chia-Ping Tsai (Jira)
Chia-Ping Tsai created KAFKA-17475:
--

 Summary: Change the base image of e2e from openjdk to 
eclipse-temurin
 Key: KAFKA-17475
 URL: https://issues.apache.org/jira/browse/KAFKA-17475
 Project: Kafka
  Issue Type: Improvement
Reporter: Chia-Ping Tsai
Assignee: Chia-Ping Tsai


see thread: https://lists.apache.org/thread/cjzmwysrldn61nylrq4t7x9g7r58qj3x



--
This message was sent by Atlassian Jira
(v8.20.10#820010)