[jira] [Resolved] (KAFKA-16595) Introduce ClusterTemplate in ClusterTests

2024-05-08 Thread Kuan Po Tseng (Jira)


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

Kuan Po Tseng resolved KAFKA-16595.
---
Resolution: Won't Do

As discussed in 
[https://github.com/apache/kafka/pull/15899#discussion_r1594890663.]

We have other ways to simplify the test cases.

> Introduce ClusterTemplate in ClusterTests
> -
>
> Key: KAFKA-16595
> URL: https://issues.apache.org/jira/browse/KAFKA-16595
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Kuan Po Tseng
>Assignee: Kuan Po Tseng
>Priority: Major
>
> discussed in https://github.com/apache/kafka/pull/15761#discussion_r1573850549
> Currently we can't apply any template in ClusterTests, thus we have to write 
> down all ClusterConfigProperty in each ClusterTest inside ClusterTests. And 
> that could leave bunch of duplicate code. We need to find a way to reduce the 
> duplicate code. Introduce template in ClusterTests could be a solution.



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


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

2024-05-08 Thread Apache Jenkins Server
See 




[jira] [Created] (KAFKA-16694) Remove rack aware code in assignors temporarily due to performance

2024-05-08 Thread Ritika Reddy (Jira)
Ritika Reddy created KAFKA-16694:


 Summary: Remove rack aware code in assignors temporarily due to 
performance
 Key: KAFKA-16694
 URL: https://issues.apache.org/jira/browse/KAFKA-16694
 Project: Kafka
  Issue Type: Sub-task
Reporter: Ritika Reddy
Assignee: Ritika Reddy






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


Re: [DISCUSS] KIP-1033: Add Kafka Streams exception handler for exceptions occuring during processing

2024-05-08 Thread Sophie Blee-Goldman
Well I definitely don't feel super strongly about it, and more importantly,
I'm not a user. So I will happily defer to the preference of anyone who
will actually be using this feature. But  I'll explain my reasoning:

There *is* a relevant distinction between these two callbacks -- because
the passed-in record will have a different type depending on whether it was
a serialization exception or something else. Even if we combined them into
a single #handle method, users will still end up implementing two distinct
branches depending on whether it was a serialization exception or not,
since that determines the type of the ProducerRecord passed in.

Not to mention they'll need to cast it to a ProducerRecord
when we could have just passed it in as this type via a dedicated callback.
And note that because of the generics, they can't do an instanceof check to
make sure that the record type is ProducerRecord and will
have to suppress the "unchecked cast" warning.

So if we combined the two callbacks, their handler will look something like
this:

@SuppressWarnings("unchecked")
public ProductionExceptionHandlerResponse handle(final ErrorHandlerContext
context,
final ProducerRecord record,
final Exception exception) {
if (exception instanceof SerializationException) {
if (exception.origin().equals(KEY)) {
log.error("Failed to serialize key", exception);
} else {
log.error("Failed to serialize value", exception);
}

} else {
final ProducerRecord serializedRecord = (ProducerRecord) record;
log.error("Failed to produce record with serialized key={} and serialized
value={}",
serializedRecord.key(), serializedRecord.value());
}
return ProductionExceptionHandlerResponse.FAIL;
}

That seems like the most basic case, and it still haswith distinct logic
even if they ultimately handle exceptions the same way. And looking forward
to KIP-1034: Dead-letter queues, it seems all the more likely that the
actual handling response might be different depending on whether it's a
serialization exception or not: a serialized record can probably be
retried/sent to a DLQ, whereas a record that can't be serialized should not
(can't, really) be forwarded to a DLQ. So if they're going to have
completely different implementations depending on whether it's a
serialization exception, why not just give them two separate callbacks?

And that's all assuming the user is perfectly aware of the different
exception types and their implications for the type of the ProducerRecord.
Many people might just miss the existence of the
RecordSerializationException altogether --
there are already so many different exception types, ESPECIALLY when it
comes to the Producer. Not to mention they'll need to understand the
nuances of how the ProducerRecord type changes depending on the type of
exception that's passed in. And on top of all that, they'll need to know
that there is metadata stored in the RecordSerializationException regarding
the origin of the error. Whereas if we just passed in the
SerializationExceptionOrigin to a #handlerSerialization callback, well,
that's pretty impossible to miss.

That all just seems like a lot for most people to have to understand to
implement a ProductionExceptionHandler, which imo is not at all an advanced
feature and should be as straightforward and easy to use as possible.

Lastly -- I don't think it's quite fair to compare this to the
RecordDeserializationException. We have a dedicated handler that's just for
deserialization exceptions specifically, hence there's no worry about users
having to be aware of the different exception types they might have to deal
with in the DeserializtionExceptionHandler. Whereas serialization
exceptions are just a subset of what might get passed in to the
ProductionExceptionHandler...

Just explaining my reasoning -- in the end I leave it up to the KIP authors
and anyone who will actually be using this feature in their applications :)



On Tue, May 7, 2024 at 8:35 PM Matthias J. Sax  wrote:

> @Loic, yes, what you describe is exactly what I had in mind.
>
>
>
> @Sophie, can you elaborate a little bit?
>
> > First of all, I agree that it makes sense to maintain the two separate
> > callbacks for the ProductionExceptionHandler, since one of them is
> > specifically for serialization exceptions while the other is used for
> > everything/anything else.
>
> What makes a serialization exception special compare to other errors
> that it's valuable to treat it differently? Why can we put "everything
> else" into a single bucket? By your train of though, should we not split
> out the "everything else" bucket into a different callback method for
> every different error? If no, why not, but only for serialization errors?
>
>  From what I believe to remember, historically, we added the
> ProductionExceptionHandler, and kinda just missed the serialization
> error case. And later, when we extended the handler we just could not
> re-use the existing callback as it was typed with `` and
> it would have been an 

Build failed in Jenkins: Kafka » Kafka Branch Builder » 3.7 #152

2024-05-08 Thread Apache Jenkins Server
See 


Changes:


--
[...truncated 411652 lines...]
[2024-05-08T17:41:38.131Z] A fine-grained performance profile is available: use 
the --scan option.
[Pipeline] junit
[2024-05-08T17:41:39.334Z] Recording test results
[2024-05-08T17:41:46.071Z] 
[2024-05-08T17:41:46.071Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testDualWriteQuotaAndScram(ClusterInstance) > 
testDualWriteQuotaAndScram [1] Type=ZK, MetadataVersion=3.5-IV2, 
Security=PLAINTEXT PASSED
[2024-05-08T17:41:46.071Z] 
[2024-05-08T17:41:46.071Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testMigrate(ClusterInstance) > testMigrate [1] 
Type=ZK, MetadataVersion=3.4-IV0, Security=PLAINTEXT STARTED
[2024-05-08T17:41:52.583Z] 
[2024-05-08T17:41:52.583Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testMigrate(ClusterInstance) > testMigrate [1] 
Type=ZK, MetadataVersion=3.4-IV0, Security=PLAINTEXT PASSED
[2024-05-08T17:41:52.583Z] 
[2024-05-08T17:41:52.583Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testMigrateAcls(ClusterInstance) > 
testMigrateAcls [1] Type=ZK, MetadataVersion=3.4-IV0, Security=PLAINTEXT STARTED
[2024-05-08T17:41:54.272Z] 
[2024-05-08T17:41:54.272Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testMigrateAcls(ClusterInstance) > 
testMigrateAcls [1] Type=ZK, MetadataVersion=3.4-IV0, Security=PLAINTEXT PASSED
[2024-05-08T17:41:54.272Z] 
[2024-05-08T17:41:54.272Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testStartZkBrokerWithAuthorizer(ClusterInstance) 
> testStartZkBrokerWithAuthorizer [1] Type=ZK, MetadataVersion=3.4-IV0, 
Security=PLAINTEXT STARTED
[2024-05-08T17:42:08.810Z] [Checks API] No suitable checks publisher found.
[Pipeline] echo
[2024-05-08T17:42:08.812Z] Skipping Kafka Streams archetype test for Java 17
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // node
[Pipeline] }
[Pipeline] // timestamps
[Pipeline] }
[Pipeline] // timeout
[Pipeline] }
[Pipeline] // stage
[Pipeline] }
[2024-05-08T17:42:12.426Z] 
[2024-05-08T17:42:12.426Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testStartZkBrokerWithAuthorizer(ClusterInstance) 
> testStartZkBrokerWithAuthorizer [1] Type=ZK, MetadataVersion=3.4-IV0, 
Security=PLAINTEXT PASSED
[2024-05-08T17:42:12.426Z] 
[2024-05-08T17:42:12.426Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testDualWrite(ClusterInstance) > testDualWrite 
[1] Type=ZK, MetadataVersion=3.4-IV0, Security=PLAINTEXT STARTED
[2024-05-08T17:42:28.009Z] 
[2024-05-08T17:42:28.009Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testDualWrite(ClusterInstance) > testDualWrite 
[1] Type=ZK, MetadataVersion=3.4-IV0, Security=PLAINTEXT PASSED
[2024-05-08T17:42:28.009Z] 
[2024-05-08T17:42:28.009Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testDualWrite(ClusterInstance) > testDualWrite 
[2] Type=ZK, MetadataVersion=3.5-IV2, Security=PLAINTEXT STARTED
[2024-05-08T17:42:52.495Z] 
[2024-05-08T17:42:52.495Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testDualWrite(ClusterInstance) > testDualWrite 
[2] Type=ZK, MetadataVersion=3.5-IV2, Security=PLAINTEXT PASSED
[2024-05-08T17:42:52.495Z] 
[2024-05-08T17:42:52.495Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testDualWrite(ClusterInstance) > testDualWrite 
[3] Type=ZK, MetadataVersion=3.6-IV2, Security=PLAINTEXT STARTED
[2024-05-08T17:43:07.955Z] 
[2024-05-08T17:43:07.955Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testDualWrite(ClusterInstance) > testDualWrite 
[3] Type=ZK, MetadataVersion=3.6-IV2, Security=PLAINTEXT PASSED
[2024-05-08T17:43:07.955Z] 
[2024-05-08T17:43:07.955Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testDualWrite(ClusterInstance) > testDualWrite 
[4] Type=ZK, MetadataVersion=3.7-IV0, Security=PLAINTEXT STARTED
[2024-05-08T17:43:28.555Z] 
[2024-05-08T17:43:28.555Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testDualWrite(ClusterInstance) > testDualWrite 
[4] Type=ZK, MetadataVersion=3.7-IV0, Security=PLAINTEXT PASSED
[2024-05-08T17:43:28.555Z] 
[2024-05-08T17:43:28.555Z] Gradle Test Run :core:test > Gradle Test Executor 99 
> ZkMigrationIntegrationTest > testDualWrite(ClusterInstance) > testDualWrite 
[5] Type=ZK, MetadataVersion=3.7-IV1, Security=PLAINTEXT STARTED
[2024-05-08T17:43:46.558Z] 
[2024-05-08T17:43:46.558Z] Gradle Test Run :core:test > Gradle 

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

2024-05-08 Thread Apache Jenkins Server
See 




Re: [VOTE] KIP-899: Allow producer and consumer clients to rebootstrap

2024-05-08 Thread Manikumar
Thanks for the KIP.

+1 (binding).

On Wed, Apr 17, 2024 at 7:50 PM Omnia Ibrahim  wrote:
>
> Hi Ivan,
> Thanks for the KIP this is a very nice feature to have.
> +1(non-binding)
> Omnia
> > On 15 Apr 2024, at 14:33, Andrew Schofield  
> > wrote:
> >
> > Thanks for the KIP
> >
> > +1 (non-binding)
> >
> > Andrew
> >
> >> On 15 Apr 2024, at 14:16, Chris Egerton  wrote:
> >>
> >> Hi Ivan,
> >>
> >> Thanks for the KIP. After the recent changes, this LGTM. +1 (binding)
> >>
> >> Cheers,
> >>
> >> Chris
> >>
> >> On Wed, Aug 2, 2023 at 12:15 AM Ivan Yurchenko 
> >> wrote:
> >>
> >>> Hello,
> >>>
> >>> The discussion [1] for KIP-899 [2] has been open for quite some time. I'd
> >>> like to put the KIP up for a vote.
> >>>
> >>> Best,
> >>> Ivan
> >>>
> >>> [1] https://lists.apache.org/thread/m0ncbmfxs5m87sszby2jbmtjx2bdpcdl
> >>> [2]
> >>>
> >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-899%3A+Allow+producer+and+consumer+clients+to+rebootstrap
> >>>
> >
>


[jira] [Created] (KAFKA-16693) Kafka Users are created with ACL entries and during performing operations allowed by ACL we see Denied Operation

2024-05-08 Thread Janardhana Gopalachar (Jira)
Janardhana Gopalachar created KAFKA-16693:
-

 Summary: Kafka Users are created with ACL entries and during 
performing operations allowed by ACL we see Denied Operation
 Key: KAFKA-16693
 URL: https://issues.apache.org/jira/browse/KAFKA-16693
 Project: Kafka
  Issue Type: Bug
  Components: core
Affects Versions: 3.5.1
Reporter: Janardhana Gopalachar


Hi

We have created 2 KafkaUsers from Strimzi operator for 2 different cluster with 
ACL entries. we have observed that the ACL entries in teh Kafka Cluster will 
not be present for approximately 4 minutes  and the ACL entries will be 
available after that. The clients which are using KafkaUser to perform 
operations not be able to perform operations till the ACL is available in 
KafkaCLuster.

We see in the Kafka Logs below are the mesages the user and cluster details in 
messages are not added since it is proprietery

 

Processing notification(s) to /config/change

Processing override for entityPath

Removing PRODUCE quota for user

Removing FETCH quota for user

Removing REQUEST quota for user

Removing CONTROLLER_MUTATION quota for user

Processing notification(s) to /kafka-acl-changes

Processing Acl change notification for

Processing notification(s) to /config/changes

Processing notification(s) to /kafka-acl-changes

Processing Acl change notification for ResourcePattern(resourceType=GROU

 

 



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


Re: [DISCUSS] KIP-1038: Add Custom Error Handler to Producer

2024-05-08 Thread Justine Olshan
My concern with respect to it being fragile: the code that ensures the
error type is internal to the producer. Someone may see it and say, I want
to add such and such error. This looks like internal code, so I don't need
a KIP, and then they can change it to whatever they want thinking it is
within the typical kafka improvement protocol.

Relying on an internal change to enforce an external API is fragile in my
opinion. That's why I sort of agreed with Artem with enforcing the error in
the method signature -- part of the public API.

Chris's comments on requiring more information to handler again makes me
wonder if we are solving a problem of lack of information at the
application level with a more powerful solution than we need. (Ie, if we
had more information, could the application close and restart the
transaction rather than having to drop records) But I am happy to
compromise with a handler that we can agree is sufficiently controlled and
documented.

Justine

On Wed, May 8, 2024 at 7:20 AM Chris Egerton 
wrote:

> Hi Alieh,
>
> Continuing prior discussions:
>
> 1) Regarding the "flexibility" discussion, my overarching point is that I
> don't see the point in allowing for this kind of pluggable logic without
> also covering more scenarios. Take example 2 in the KIP: if we're going to
> implement retries only on "important" topics when a topic partition isn't
> found, why wouldn't we also want to be able to do this for other errors?
> Again, taking authorization errors as an example, why wouldn't we want to
> be able to fail when we can't write to "important" topics because the
> producer principal lacks sufficient ACLs, and drop the record if the topic
> isn't "important"? In a security-conscious environment with
> runtime-dependent topic routing (which is a common feature of many source
> connectors, such as the Debezium connectors), this seems fairly likely.
>
> 2) As far as changing the shape of the API goes, I like Artem's idea of
> splitting out the interface based on specific exceptions. This may be a
> little laborious to expand in the future, but if we really want to
> limit the exceptions that we cover with the handler and move slowly and
> cautiously, then IMO it'd be reasonable to reflect that in the interface. I
> also acknowledge that there's no way to completely prevent people from
> shooting themselves in the foot by implementing the API incorrectly, but I
> think it's worth it to do what we can--including leveraging the Java
> language's type system--to help them, so IMO there's value to eliminating
> the implicit behavior of failing when a policy returns RETRY for a
> non-retriable error. This can take a variety of shapes and I'm not going to
> insist on anything specific, but I do want to again raise my concerns with
> the current proposal and request that we find something a little better.
>
> 3) Concerning the default implementation--actually, I meant what I wrote :)
> I don't want a "second" default, I want an implementation of this interface
> to be used as the default if no others are specified. The behavior of this
> default implementation would be identical to existing behavior (so there
> would be no backwards compatibility concerns like the ones raised by
> Matthias), but it would be possible to configure this default handler class
> to behave differently for a basic set of scenarios. This would mirror (pun
> intended) the approach we've taken with Mirror Maker 2 and its
> ReplicationPolicy interface [1]. There is a default implementation
> available [2] that recognizes a handful of basic configuration properties
> [3] for simple tweaks, but if users want, they can also implement their own
> replication policy for more fine-grained logic if those properties aren't
> flexible enough.
>
> More concretely, I'm imagining something like this for the producer
> exception handler:
>
> - Default implementation class
> of org.apache.kafka.clients.producer.DefaultProducerExceptionHandler
> - This class would recognize two properties:
>   - drop.invalid.large.records: Boolean property, defaults to false. If
> "false", then causes the handler to return FAIL whenever
> a RecordTooLargeException is encountered; if "true", then causes
> SWALLOW/SKIP/DROP to be returned instead
>   - unknown.topic.partition.retry.timeout.ms: Integer property, defaults
> to
> INT_MAX. Whenever an UnknownTopicOrPartitionException is encountered,
> causes the handler to return FAIL if that record has been pending for more
> than the retry timeout; otherwise, causes RETRY to be returned
>
> I think this is worth addressing now instead of later because it forces us
> to evaluate the usefulness of this interface and it addresses a
> long-standing issue not just with Kafka Connect, but with the Java producer
> in general. For reference, here are a few tickets I collected after briefly
> skimming our Jira showing that this is a real pain point for users:
> https://issues.apache.org/jira/browse/KAFKA-10340,
> 

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

2024-05-08 Thread Apache Jenkins Server
See 




Re: [VOTE] KIP-1041: Drop `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8)

2024-05-08 Thread Justine Olshan
+1 (binding)

Thanks,
Justine

On Wed, May 8, 2024 at 8:36 AM Federico Valeri  wrote:

> +1 non binding
>
> Thanks
>
> On Wed, May 8, 2024 at 5:27 PM Andrew Schofield
>  wrote:
> >
> > Hi,
> > Thanks for the KIP.
> >
> > +1 (non-binding)
> >
> > Thanks,
> > Andrew
> >
> > > On 8 May 2024, at 15:48, David Jacot 
> wrote:
> > >
> > > Hi folks,
> > >
> > > I'd like to start a voting thread for KIP-1041: Drop
> > > `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8).
> > >
> > > KIP: https://cwiki.apache.org/confluence/x/9YobEg
> > >
> > > Best,
> > > David
> >
>


Re: KIP-1040: Improve handling of nullable values in InsertField/ExtractField transformations

2024-05-08 Thread Chris Egerton
Wait, just one more thing--are there any other SMTs that could benefit from
this? Wouldn't ValueToKey [1] be applicable as well, for example?

[1] -
https://github.com/apache/kafka/blob/2a5efe4a334611fc7c15f76b322482bad0942db0/connect/transforms/src/main/java/org/apache/kafka/connect/transforms/ValueToKey.java#L106

On Wed, May 8, 2024 at 11:46 AM Chris Egerton  wrote:

> Hi Mario,
>
> I think we could have something like `copy` and `copyWithoutDefaults` to
> get around that, but now that you bring up compatibility, I think it's best
> to hold off on this. I'm forced to recall that anything we add to the
> Connect API may be used by plugin developers who write for the bleeding
> edge of the Connect runtime, but deployed by users who are running on
> (possibly much) older versions. In that scenario, any use of new Struct
> methods would cause issues at runtime caused by compatibility clashes
> between the newer API that the plugin was written for, and the older API
> that's provided by the runtime it's running on.
>
> Anyway, thanks for humoring me. The KIP looks good to me 
>
> Cheers,
>
> Chris
>
> On Wed, May 8, 2024 at 10:50 AM Mario Fiore Vitale 
> wrote:
>
>> Hi Chris,
>>
>> Thanks for reviewing this.
>>
>> > It seems like the pattern of "copy the contents of this Struct into
>> another
>> one for the purpose of mutation" could be fairly common in user code bases
>> in addition to the core Connect SMTs. Do you think there's a way to
>> simplify this with, e.g., a Struct.putAll(Struct destination) or
>> Struct.copy(Schema destinationSchema) method?
>>
>> The only concern that I see is backward compatibility. Suppose that you
>> are
>> not using the JsonConvert but another convert that does't support the
>> 'replace.null.with.default', when you use the current 'InsertField' smt
>> the null values will be replace by default values. If we replace the
>> "copy"
>> logic with a method in the Struct we remove this behavior.
>>
>> Isn't it?
>>
>> Mario.
>>
>> On Wed, May 8, 2024 at 2:14 PM Chris Egerton 
>> wrote:
>>
>> > Hi Mario,
>> >
>> > Thanks for the KIP! Looks good overall. One quick thought--it wasn't
>> > immediately obvious to me why we had to touch on InsertField for this.
>> It
>> > looks like the reason is that we use Struct::get [1] to create a clone
>> of
>> > the input value [2], instead of Struct::getWithoutDefault [3].
>> >
>> > It seems like the pattern of "copy the contents of this Struct into
>> another
>> > one for the purpose of mutation" could be fairly common in user code
>> bases
>> > in addition to the core Connect SMTs. Do you think there's a way to
>> > simplify this with, e.g., a Struct.putAll(Struct destination) or
>> > Struct.copy(Schema destinationSchema) method?
>> >
>> > [1] -
>> >
>> >
>> https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java#L78-L91
>> > [2] -
>> >
>> >
>> https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/transforms/src/main/java/org/apache/kafka/connect/transforms/InsertField.java#L179-L183
>> > [3] -
>> >
>> >
>> https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java#L93-L101
>> >
>> > Cheers,
>> >
>> > Chris
>> >
>> > On Wed, May 8, 2024 at 3:40 AM Mario Fiore Vitale 
>> > wrote:
>> >
>> > > Hi All,
>> > >
>> > > I have created (through Mickael Maison's account since there was an
>> issue
>> > > creating a new one for me) KIP-1040[1] to improve handling of nullable
>> > > values in InsertField/ExtractField transformations, this is required
>> > after
>> > > the introduction of KIP-581[2] that introduced the configuration
>> property
>> > > *replace.null.with.default* to *JsonConverter* to choose whether to
>> > replace
>> > > fields that have a default value and that are null to the default
>> value.
>> > >
>> > > [1]
>> > >
>> >
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=303794677
>> > > [2]
>> > >
>> > >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-581%3A+Value+of+optional+null+field+which+has+default+value
>> > >
>> > > Feedback and suggestions are welcome.
>> > >
>> > > Regards,
>> > > Mario.
>> > > --
>> > >
>> > > Mario Fiore Vitale
>> > >
>> > > Senior Software Engineer
>> > >
>> > > Red Hat 
>> > > 
>> > >
>> >
>>
>>
>> --
>>
>> Mario Fiore Vitale
>>
>> Senior Software Engineer
>>
>> Red Hat 
>> 
>>
>


Re: KIP-1040: Improve handling of nullable values in InsertField/ExtractField transformations

2024-05-08 Thread Chris Egerton
Hi Mario,

I think we could have something like `copy` and `copyWithoutDefaults` to
get around that, but now that you bring up compatibility, I think it's best
to hold off on this. I'm forced to recall that anything we add to the
Connect API may be used by plugin developers who write for the bleeding
edge of the Connect runtime, but deployed by users who are running on
(possibly much) older versions. In that scenario, any use of new Struct
methods would cause issues at runtime caused by compatibility clashes
between the newer API that the plugin was written for, and the older API
that's provided by the runtime it's running on.

Anyway, thanks for humoring me. The KIP looks good to me 

Cheers,

Chris

On Wed, May 8, 2024 at 10:50 AM Mario Fiore Vitale 
wrote:

> Hi Chris,
>
> Thanks for reviewing this.
>
> > It seems like the pattern of "copy the contents of this Struct into
> another
> one for the purpose of mutation" could be fairly common in user code bases
> in addition to the core Connect SMTs. Do you think there's a way to
> simplify this with, e.g., a Struct.putAll(Struct destination) or
> Struct.copy(Schema destinationSchema) method?
>
> The only concern that I see is backward compatibility. Suppose that you are
> not using the JsonConvert but another convert that does't support the
> 'replace.null.with.default', when you use the current 'InsertField' smt
> the null values will be replace by default values. If we replace the "copy"
> logic with a method in the Struct we remove this behavior.
>
> Isn't it?
>
> Mario.
>
> On Wed, May 8, 2024 at 2:14 PM Chris Egerton 
> wrote:
>
> > Hi Mario,
> >
> > Thanks for the KIP! Looks good overall. One quick thought--it wasn't
> > immediately obvious to me why we had to touch on InsertField for this. It
> > looks like the reason is that we use Struct::get [1] to create a clone of
> > the input value [2], instead of Struct::getWithoutDefault [3].
> >
> > It seems like the pattern of "copy the contents of this Struct into
> another
> > one for the purpose of mutation" could be fairly common in user code
> bases
> > in addition to the core Connect SMTs. Do you think there's a way to
> > simplify this with, e.g., a Struct.putAll(Struct destination) or
> > Struct.copy(Schema destinationSchema) method?
> >
> > [1] -
> >
> >
> https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java#L78-L91
> > [2] -
> >
> >
> https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/transforms/src/main/java/org/apache/kafka/connect/transforms/InsertField.java#L179-L183
> > [3] -
> >
> >
> https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java#L93-L101
> >
> > Cheers,
> >
> > Chris
> >
> > On Wed, May 8, 2024 at 3:40 AM Mario Fiore Vitale 
> > wrote:
> >
> > > Hi All,
> > >
> > > I have created (through Mickael Maison's account since there was an
> issue
> > > creating a new one for me) KIP-1040[1] to improve handling of nullable
> > > values in InsertField/ExtractField transformations, this is required
> > after
> > > the introduction of KIP-581[2] that introduced the configuration
> property
> > > *replace.null.with.default* to *JsonConverter* to choose whether to
> > replace
> > > fields that have a default value and that are null to the default
> value.
> > >
> > > [1]
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=303794677
> > > [2]
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-581%3A+Value+of+optional+null+field+which+has+default+value
> > >
> > > Feedback and suggestions are welcome.
> > >
> > > Regards,
> > > Mario.
> > > --
> > >
> > > Mario Fiore Vitale
> > >
> > > Senior Software Engineer
> > >
> > > Red Hat 
> > > 
> > >
> >
>
>
> --
>
> Mario Fiore Vitale
>
> Senior Software Engineer
>
> Red Hat 
> 
>


Re: [VOTE] KIP-1041: Drop `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8)

2024-05-08 Thread Federico Valeri
+1 non binding

Thanks

On Wed, May 8, 2024 at 5:27 PM Andrew Schofield
 wrote:
>
> Hi,
> Thanks for the KIP.
>
> +1 (non-binding)
>
> Thanks,
> Andrew
>
> > On 8 May 2024, at 15:48, David Jacot  wrote:
> >
> > Hi folks,
> >
> > I'd like to start a voting thread for KIP-1041: Drop
> > `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8).
> >
> > KIP: https://cwiki.apache.org/confluence/x/9YobEg
> >
> > Best,
> > David
>


Re: [VOTE] KIP-1041: Drop `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8)

2024-05-08 Thread Andrew Schofield
Hi,
Thanks for the KIP.

+1 (non-binding)

Thanks,
Andrew

> On 8 May 2024, at 15:48, David Jacot  wrote:
>
> Hi folks,
>
> I'd like to start a voting thread for KIP-1041: Drop
> `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8).
>
> KIP: https://cwiki.apache.org/confluence/x/9YobEg
>
> Best,
> David



Re: KIP-1040: Improve handling of nullable values in InsertField/ExtractField transformations

2024-05-08 Thread Mario Fiore Vitale
Hi Chris,

Thanks for reviewing this.

> It seems like the pattern of "copy the contents of this Struct into
another
one for the purpose of mutation" could be fairly common in user code bases
in addition to the core Connect SMTs. Do you think there's a way to
simplify this with, e.g., a Struct.putAll(Struct destination) or
Struct.copy(Schema destinationSchema) method?

The only concern that I see is backward compatibility. Suppose that you are
not using the JsonConvert but another convert that does't support the
'replace.null.with.default', when you use the current 'InsertField' smt
the null values will be replace by default values. If we replace the "copy"
logic with a method in the Struct we remove this behavior.

Isn't it?

Mario.

On Wed, May 8, 2024 at 2:14 PM Chris Egerton 
wrote:

> Hi Mario,
>
> Thanks for the KIP! Looks good overall. One quick thought--it wasn't
> immediately obvious to me why we had to touch on InsertField for this. It
> looks like the reason is that we use Struct::get [1] to create a clone of
> the input value [2], instead of Struct::getWithoutDefault [3].
>
> It seems like the pattern of "copy the contents of this Struct into another
> one for the purpose of mutation" could be fairly common in user code bases
> in addition to the core Connect SMTs. Do you think there's a way to
> simplify this with, e.g., a Struct.putAll(Struct destination) or
> Struct.copy(Schema destinationSchema) method?
>
> [1] -
>
> https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java#L78-L91
> [2] -
>
> https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/transforms/src/main/java/org/apache/kafka/connect/transforms/InsertField.java#L179-L183
> [3] -
>
> https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java#L93-L101
>
> Cheers,
>
> Chris
>
> On Wed, May 8, 2024 at 3:40 AM Mario Fiore Vitale 
> wrote:
>
> > Hi All,
> >
> > I have created (through Mickael Maison's account since there was an issue
> > creating a new one for me) KIP-1040[1] to improve handling of nullable
> > values in InsertField/ExtractField transformations, this is required
> after
> > the introduction of KIP-581[2] that introduced the configuration property
> > *replace.null.with.default* to *JsonConverter* to choose whether to
> replace
> > fields that have a default value and that are null to the default value.
> >
> > [1]
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=303794677
> > [2]
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-581%3A+Value+of+optional+null+field+which+has+default+value
> >
> > Feedback and suggestions are welcome.
> >
> > Regards,
> > Mario.
> > --
> >
> > Mario Fiore Vitale
> >
> > Senior Software Engineer
> >
> > Red Hat 
> > 
> >
>


-- 

Mario Fiore Vitale

Senior Software Engineer

Red Hat 



[VOTE] KIP-1041: Drop `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8)

2024-05-08 Thread David Jacot
Hi folks,

I'd like to start a voting thread for KIP-1041: Drop
`offsets.commit.required.acks` config in 4.0 (deprecate in 3.8).

KIP: https://cwiki.apache.org/confluence/x/9YobEg

Best,
David


Re: [DISCUSS] KIP-1038: Add Custom Error Handler to Producer

2024-05-08 Thread Chris Egerton
Hi Alieh,

Continuing prior discussions:

1) Regarding the "flexibility" discussion, my overarching point is that I
don't see the point in allowing for this kind of pluggable logic without
also covering more scenarios. Take example 2 in the KIP: if we're going to
implement retries only on "important" topics when a topic partition isn't
found, why wouldn't we also want to be able to do this for other errors?
Again, taking authorization errors as an example, why wouldn't we want to
be able to fail when we can't write to "important" topics because the
producer principal lacks sufficient ACLs, and drop the record if the topic
isn't "important"? In a security-conscious environment with
runtime-dependent topic routing (which is a common feature of many source
connectors, such as the Debezium connectors), this seems fairly likely.

2) As far as changing the shape of the API goes, I like Artem's idea of
splitting out the interface based on specific exceptions. This may be a
little laborious to expand in the future, but if we really want to
limit the exceptions that we cover with the handler and move slowly and
cautiously, then IMO it'd be reasonable to reflect that in the interface. I
also acknowledge that there's no way to completely prevent people from
shooting themselves in the foot by implementing the API incorrectly, but I
think it's worth it to do what we can--including leveraging the Java
language's type system--to help them, so IMO there's value to eliminating
the implicit behavior of failing when a policy returns RETRY for a
non-retriable error. This can take a variety of shapes and I'm not going to
insist on anything specific, but I do want to again raise my concerns with
the current proposal and request that we find something a little better.

3) Concerning the default implementation--actually, I meant what I wrote :)
I don't want a "second" default, I want an implementation of this interface
to be used as the default if no others are specified. The behavior of this
default implementation would be identical to existing behavior (so there
would be no backwards compatibility concerns like the ones raised by
Matthias), but it would be possible to configure this default handler class
to behave differently for a basic set of scenarios. This would mirror (pun
intended) the approach we've taken with Mirror Maker 2 and its
ReplicationPolicy interface [1]. There is a default implementation
available [2] that recognizes a handful of basic configuration properties
[3] for simple tweaks, but if users want, they can also implement their own
replication policy for more fine-grained logic if those properties aren't
flexible enough.

More concretely, I'm imagining something like this for the producer
exception handler:

- Default implementation class
of org.apache.kafka.clients.producer.DefaultProducerExceptionHandler
- This class would recognize two properties:
  - drop.invalid.large.records: Boolean property, defaults to false. If
"false", then causes the handler to return FAIL whenever
a RecordTooLargeException is encountered; if "true", then causes
SWALLOW/SKIP/DROP to be returned instead
  - unknown.topic.partition.retry.timeout.ms: Integer property, defaults to
INT_MAX. Whenever an UnknownTopicOrPartitionException is encountered,
causes the handler to return FAIL if that record has been pending for more
than the retry timeout; otherwise, causes RETRY to be returned

I think this is worth addressing now instead of later because it forces us
to evaluate the usefulness of this interface and it addresses a
long-standing issue not just with Kafka Connect, but with the Java producer
in general. For reference, here are a few tickets I collected after briefly
skimming our Jira showing that this is a real pain point for users:
https://issues.apache.org/jira/browse/KAFKA-10340,
https://issues.apache.org/jira/browse/KAFKA-12990,
https://issues.apache.org/jira/browse/KAFKA-13634. Although this is
frequently reported with Kafka Connect, it applies to anyone who configures
a producer to use a high retry timeout. I am aware of the max.block.ms
property, but it's painful and IMO poor behavior to require users to reduce
the value of this property just to handle the single scenario when trying
to write to topics that don't exist, since it would also limit the retry
timeout for other operations that are legitimately retriable.

Raising new points:

5) I don't see the interplay between this handler and existing
retry-related properties mentioned anywhere in the KIP. I'm assuming that
properties like "retries", "max.block.ms", and "delivery.timeout.ms" would
take precedence over the handler and once they are exhausted, the
record/batch will fail no matter what? If so, it's probably worth briefly
mentioning this (no more than a sentence or two) in the KIP, and if not,
I'm curious what you have in mind.

6) I also wonder if the API provides enough information in its current
form. Would it be possible to provide handlers with some way 

[jira] [Resolved] (KAFKA-16108) Backport fix for KAFKA-16093 to 3.7

2024-05-08 Thread Chris Egerton (Jira)


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

Chris Egerton resolved KAFKA-16108.
---
Resolution: Done

> Backport fix for KAFKA-16093 to 3.7
> ---
>
> Key: KAFKA-16108
> URL: https://issues.apache.org/jira/browse/KAFKA-16108
> Project: Kafka
>  Issue Type: Improvement
>  Components: connect
>Reporter: Chris Egerton
>Assignee: Chris Egerton
>Priority: Blocker
> Fix For: 3.7.1
>
>
> A fix for KAFKA-16093 is present on the branches trunk (the version for which 
> is currently 3.8.0-SNAPSHOT) and 3.6. We are in code freeze for the 3.7.0 
> release, and this issue is not a blocker, so it cannot be backported right 
> now.
> We should backport the fix once 3.7.0 has been released and before 3.7.1 is 
> released.



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


Re: [DISCUSS] KIP-1028: Docker Official Image for Apache Kafka

2024-05-08 Thread Krish Vora
Hi Chris. Thanks for the questions.

3. Would a separate Docker-owned repository be out of the question? I'm
> guessing there are some trademark issues that might get in the way, but
> it's worth exploring since the entire purpose of this KIP seems to be to
> provide images that are vetted and designed by Docker more than by the
> Apache Kafka contributors/committers/PMC.



   - The process for introducing a Docker Official Image involves
  - Hosting the Dockerfile in the Apache Kafka repository and
  - Providing the path to this Dockerfile to Docker Hub in Docker Hub’s
  own repo
  
  .
   - This ensures that any updates to the Dockerfile in the AK repository
   are directly applicable to the docker official images available on Docker
   Hub.


   - We also did not find any added advantage to create a separate
   repository named apache-docker within the Apache GitHub organization.

Thanks,
Krish.

On Wed, May 8, 2024 at 6:05 PM Prabha Manepalli
 wrote:

> Hi Chris,  I would like to add more context to this KIP's motivation.
> Vedarth and Krish, please weigh in with your inputs.
>
> In the motivation section it's stated that "Several other Apache projects,
> > like Flink, Spark, Solr, have already released Docker Official Images,
> with
> > download figures ranging from 50 million to over 1 billion. These numbers
> > highlight the significant demand among users." But then immediately
> > afterwards, we learn that "Also the Docker Official Images are always the
> > top 1 search result, irrespective of the number of downloads." Wouldn't a
> > high number of downloads for an image naturally follow from being the top
> > search result? It seems like we can't necessarily assume that Docker
> > Official Images are inherently more desirable for users based solely on
> > download statistics.
> >
>
> *My thoughts: *Unlike the Sponsored OSS image, the Docker Official image is
> more desirable for workloads that have stringent compliance requirements.
> More details on why official images are more trusted are documented here
> . The Docker
> Official image would also help an absolutely new Kafka beginner who might
> not know about Apache or the concept of Sponsored images. We want to make
> it easier for Kafka beginners to discover the Kafka image through
> DockerHub.
>
>
> Can you elaborate on the value that these new images would add from a
> > user's perspective? I'm hesitant to introduce another image, since it
> adds
> > to the cognitive burden of people who will inevitably have to answer the
> > question of "What are the differences between all of the available images
> > and which one is best for my use case?"
> >
>
>
> *My thoughts: *This is a valid concern to address. The response to the
> above question addresses the value-add this new Docker Official image would
> provide. I also agree we need a clear distinction between each of these
> images to be well documented. We plan to update the AK website with details
> on how, why, and when a developer would want to use each of these
> particular images(KIP-974,975,1028).
>
> Thanks,
> Prabha.
>
>
>
>
>
> On Tue, Apr 30, 2024 at 9:41 PM Chris Egerton 
> wrote:
>
> > Hi Vedarth and Krish,
> >
> > Thanks for the KIP! I have to admit I'm a little skeptical; hopefully you
> > can help me understand the need for these additional images.
> >
> > 1) In the motivation section it's stated that "Several other Apache
> > projects, like Flink, Spark, Solr, have already released Docker Official
> > Images, with download figures ranging from 50 million to over 1 billion.
> > These numbers highlight the significant demand among users." But then
> > immediately afterwards, we learn that "Also the Docker Official Images
> are
> > always the top 1 search result, irrespective of the number of downloads."
> > Wouldn't a high number of downloads for an image naturally follow from
> > being the top search result? It seems like we can't necessarily assume
> that
> > Docker Official Images are inherently more desirable for users based
> solely
> > on download statistics.
> >
> > 2) Can you elaborate on the value that these new images would add from a
> > user's perspective? I'm hesitant to introduce another image, since it
> adds
> > to the cognitive burden of people who will inevitably have to answer the
> > question of "What are the differences between all of the available images
> > and which one is best for my use case?"
> >
> > 3) Would a separate Docker-owned repository be out of the question? I'm
> > guessing there are some trademark issues that might get in the way, but
> > it's worth exploring since the entire purpose of this KIP seems to be to
> > provide images that are vetted and designed by Docker more than by the
> > Apache Kafka contributors/committers/PMC.
> >
> > I may have more questions later but wanted to get this 

[jira] [Resolved] (KAFKA-16685) RLMTask warning logs do not include parent exception trace

2024-05-08 Thread Jorge Esteban Quilcate Otoya (Jira)


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

Jorge Esteban Quilcate Otoya resolved KAFKA-16685.
--
Resolution: Fixed

Merged https://github.com/apache/kafka/pull/15880

> RLMTask warning logs do not include parent exception trace
> --
>
> Key: KAFKA-16685
> URL: https://issues.apache.org/jira/browse/KAFKA-16685
> Project: Kafka
>  Issue Type: Improvement
>  Components: Tiered-Storage
>Reporter: Jorge Esteban Quilcate Otoya
>Assignee: Jorge Esteban Quilcate Otoya
>Priority: Major
>
> When RLMTask warning exceptions happen and are logged, it only includes the 
> exception message, but we lose the stack trace.
> See 
> [https://github.com/apache/kafka/blob/70b8c5ae8e9336dbf4792e2d3cf100bbd70d6480/core/src/main/java/kafka/log/remote/RemoteLogManager.java#L821-L831]
> This makes it difficult to troubleshoot issues.



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


Re: [DISCUSS] KIP-1028: Docker Official Image for Apache Kafka

2024-05-08 Thread Prabha Manepalli
Hi Chris,  I would like to add more context to this KIP's motivation.
Vedarth and Krish, please weigh in with your inputs.

In the motivation section it's stated that "Several other Apache projects,
> like Flink, Spark, Solr, have already released Docker Official Images, with
> download figures ranging from 50 million to over 1 billion. These numbers
> highlight the significant demand among users." But then immediately
> afterwards, we learn that "Also the Docker Official Images are always the
> top 1 search result, irrespective of the number of downloads." Wouldn't a
> high number of downloads for an image naturally follow from being the top
> search result? It seems like we can't necessarily assume that Docker
> Official Images are inherently more desirable for users based solely on
> download statistics.
>

*My thoughts: *Unlike the Sponsored OSS image, the Docker Official image is
more desirable for workloads that have stringent compliance requirements.
More details on why official images are more trusted are documented here
. The Docker
Official image would also help an absolutely new Kafka beginner who might
not know about Apache or the concept of Sponsored images. We want to make
it easier for Kafka beginners to discover the Kafka image through DockerHub.


Can you elaborate on the value that these new images would add from a
> user's perspective? I'm hesitant to introduce another image, since it adds
> to the cognitive burden of people who will inevitably have to answer the
> question of "What are the differences between all of the available images
> and which one is best for my use case?"
>


*My thoughts: *This is a valid concern to address. The response to the
above question addresses the value-add this new Docker Official image would
provide. I also agree we need a clear distinction between each of these
images to be well documented. We plan to update the AK website with details
on how, why, and when a developer would want to use each of these
particular images(KIP-974,975,1028).

Thanks,
Prabha.





On Tue, Apr 30, 2024 at 9:41 PM Chris Egerton 
wrote:

> Hi Vedarth and Krish,
>
> Thanks for the KIP! I have to admit I'm a little skeptical; hopefully you
> can help me understand the need for these additional images.
>
> 1) In the motivation section it's stated that "Several other Apache
> projects, like Flink, Spark, Solr, have already released Docker Official
> Images, with download figures ranging from 50 million to over 1 billion.
> These numbers highlight the significant demand among users." But then
> immediately afterwards, we learn that "Also the Docker Official Images are
> always the top 1 search result, irrespective of the number of downloads."
> Wouldn't a high number of downloads for an image naturally follow from
> being the top search result? It seems like we can't necessarily assume that
> Docker Official Images are inherently more desirable for users based solely
> on download statistics.
>
> 2) Can you elaborate on the value that these new images would add from a
> user's perspective? I'm hesitant to introduce another image, since it adds
> to the cognitive burden of people who will inevitably have to answer the
> question of "What are the differences between all of the available images
> and which one is best for my use case?"
>
> 3) Would a separate Docker-owned repository be out of the question? I'm
> guessing there are some trademark issues that might get in the way, but
> it's worth exploring since the entire purpose of this KIP seems to be to
> provide images that are vetted and designed by Docker more than by the
> Apache Kafka contributors/committers/PMC.
>
> I may have more questions later but wanted to get this initial round out
> now without trying to list everything first.
>
> Looking forward to your thoughts!
>
> Cheers,
>
> Chris
>
> On Mon, Apr 22, 2024 at 2:14 PM Vedarth Sharma 
> wrote:
>
> > Hey folks,
> >
> > Thanks a lot for reviewing the KIP and providing feedback.
> > The discussion thread seems resolved and KIP has been updated
> accordingly.
> > We will be starting the voting thread for this KIP in the next few days.
> > Please take a look at the KIP and let us know if any further discussion
> > is needed.
> >
> > Thanks and regards,
> > Vedarth
> >
> > On Fri, Apr 19, 2024 at 1:33 PM Manikumar 
> > wrote:
> >
> > > Thanks Krish. KIP looks good to me.
> > >
> > > On Wed, Apr 17, 2024 at 1:38 PM Krish Vora 
> > wrote:
> > > >
> > > > Hi Manikumar,
> > > >
> > > > Thanks for the comments.
> > > >
> > > > Maybe as part of the release process, RM can create a JIRA for this
> > > > > task. This can be taken by RM or any comitter or any contributor
> > (with
> > > > > some help from commiters to run "Docker Image Preparation via
> GitHub
> > > > > Actions:"
> > > >
> > > > This sounds like a good idea. This step would be beneficial. By
> > creating
> > > a
> > > > JIRA ticket, it will also serve as a 

Re: KIP-1040: Improve handling of nullable values in InsertField/ExtractField transformations

2024-05-08 Thread Chris Egerton
Hi Mario,

Thanks for the KIP! Looks good overall. One quick thought--it wasn't
immediately obvious to me why we had to touch on InsertField for this. It
looks like the reason is that we use Struct::get [1] to create a clone of
the input value [2], instead of Struct::getWithoutDefault [3].

It seems like the pattern of "copy the contents of this Struct into another
one for the purpose of mutation" could be fairly common in user code bases
in addition to the core Connect SMTs. Do you think there's a way to
simplify this with, e.g., a Struct.putAll(Struct destination) or
Struct.copy(Schema destinationSchema) method?

[1] -
https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java#L78-L91
[2] -
https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/transforms/src/main/java/org/apache/kafka/connect/transforms/InsertField.java#L179-L183
[3] -
https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java#L93-L101

Cheers,

Chris

On Wed, May 8, 2024 at 3:40 AM Mario Fiore Vitale 
wrote:

> Hi All,
>
> I have created (through Mickael Maison's account since there was an issue
> creating a new one for me) KIP-1040[1] to improve handling of nullable
> values in InsertField/ExtractField transformations, this is required after
> the introduction of KIP-581[2] that introduced the configuration property
> *replace.null.with.default* to *JsonConverter* to choose whether to replace
> fields that have a default value and that are null to the default value.
>
> [1]
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=303794677
> [2]
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-581%3A+Value+of+optional+null+field+which+has+default+value
>
> Feedback and suggestions are welcome.
>
> Regards,
> Mario.
> --
>
> Mario Fiore Vitale
>
> Senior Software Engineer
>
> Red Hat 
> 
>


Re: [VOTE] KIP-932: Queues for Kafka

2024-05-08 Thread Manikumar
Hi Andrew,

Thanks for the KIP.  Great write-up!

+1 (binding)

Thanks,

On Wed, May 8, 2024 at 12:17 PM Satish Duggana  wrote:
>
> Hi Andrew,
> Thanks for the nice KIP, it will allow other messaging use cases to be
> onboarded to Kafka.
>
> +1 from me.
>
> Satish.
>
> On Tue, 7 May 2024 at 03:41, Jun Rao  wrote:
> >
> > Hi, Andrew,
> >
> > Thanks for the KIP. +1
> >
> > Jun
> >
> > On Mon, Mar 18, 2024 at 11:00 AM Edoardo Comar 
> > wrote:
> >
> > > Thanks Andrew,
> > >
> > > +1 (binding)
> > >
> > > Edo
> > >
> > > On Mon, 18 Mar 2024 at 16:32, Kenneth Eversole
> > >  wrote:
> > > >
> > > > Hi Andrew
> > > >
> > > > + 1 (Non-Binding)
> > > >
> > > > This will be great addition to Kafka
> > > >
> > > > On Mon, Mar 18, 2024 at 8:27 AM Apoorv Mittal 
> > > > wrote:
> > > >
> > > > > Hi Andrew,
> > > > > Thanks for writing the KIP. This is indeed going to be a valuable
> > > addition
> > > > > to the Kafka, excited to see the KIP.
> > > > >
> > > > > + 1 (Non-Binding)
> > > > >
> > > > > Regards,
> > > > > Apoorv Mittal
> > > > > +44 7721681581
> > > > >
> > > > >
> > > > > On Sun, Mar 17, 2024 at 11:16 PM Andrew Schofield <
> > > > > andrew_schofield_j...@outlook.com> wrote:
> > > > >
> > > > > > Hi,
> > > > > > I’ve been working to complete KIP-932 over the past few months and
> > > > > > discussions have quietened down.
> > > > > >
> > > > > > I’d like to open the voting for KIP-932:
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-932%3A+Queues+for+Kafka
> > > > > >
> > > > > > Thanks,
> > > > > > Andrew
> > > > >
> > >


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

2024-05-08 Thread Apache Jenkins Server
See 




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

2024-05-08 Thread Apache Jenkins Server
See 




Re: Request version not enabled errors upgrading from Kafka 3.5 -> 3.6

2024-05-08 Thread Johnson Okorie
Hi Justine,

Thank you for your quick reply! I created
https://issues.apache.org/jira/browse/KAFKA-16692 to describe the issue.
Happy to provide any additional information needed.

Regards,
Johnson

On Tue, 7 May 2024 at 23:18, Justine Olshan 
wrote:

> Hi Johnson,
>
> Thanks for bringing this issue to the mailing list.
>
> I'm familiar with the change you are referring to. However, during the
> upgrade you should be hitting this code path and we should not sending
> requests to older version brokers.
>
> https://github.com/apache/kafka/blob/525b9b1d7682ae2a527ceca83fedca44b1cba11a/clients/src/main/java/org/apache/kafka/clients/NodeApiVersions.java#L130
>
> https://github.com/apache/kafka/blob/525b9b1d7682ae2a527ceca83fedca44b1cba11a/core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala#L195
>
> Even if we did, we shouldn't return network exception errors.
>
> Do you mind opening a JIRA ticket with some more details so I can take a
> closer look?
>
> Justine
>
> On Tue, May 7, 2024 at 11:38 AM Johnson Okorie 
> wrote:
>
> > Hi folks,
> >
> > Awesome work you have been doing on this project!
> >
> > I was hoping I could get some help on an issue we are having in one of
> our
> > Kafka clusters. Most of the clients on this cluster use
> > exactly-once-semantics. The Kafka cluster currently runs version 3.5.2
> and
> > we were attempting an upgrade to 3.6.2. After replacing one of the
> brokers
> > with the new version we saw a bunch of the following errors on the older
> > brokers:
> >
> > ```
> > Received request api key ADD_PARTITIONS_TO_TXN with version 4 which is
> not
> > enabled
> > ```
> >
> > This manifested as 'NETWORK_EXCEPTION' errors on the clients and downtime
> > for those clients. On the new broker we saw:
> >
> > ```
> > [AddPartitionsToTxnSenderThread-1063]: AddPartitionsToTxnRequest failed
> for
> > node 1069 with a network exception.
> > ```
> >
> > Digging through the changes in 3.6, we came across some changes
> introduced
> > as part of KAFKA-14402 <
> https://issues.apache.org/jira/browse/KAFKA-14402>
> > that
> > we thought might lead to this behaviour and wanted to confirm.
> >
> > First we could see that  transaction.partition.verification.enable
> > is enabled by default and enables a new code path that culminates in we
> > sending version 4 ADD_PARTITIONS_TO_TXN requests to other brokers here
> > <
> >
> https://github.com/apache/kafka/blob/cb35ddc5ca233d5cca6f51c1c41b952a7e9fe1a0/core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala#L269
> > >
> > .
> >
> > However, we do not support  version 4 of ADD_PARTITIONS_TO_TXN requests
> as
> > of Kafka 3.5.2? If these assumptions happen to be correct, does this mean
> > that the upgrade to versions 3.6+ require
> > transaction.partition.verification.enable
> > to be set to false to allow upgrades?
> >
> > Regard,
> > Johnson
> >
>


[jira] [Created] (KAFKA-16692) InvalidRequestException: ADD_PARTITIONS_TO_TXN with version 4 which is not enabled when

2024-05-08 Thread Johnson Okorie (Jira)
Johnson Okorie created KAFKA-16692:
--

 Summary: InvalidRequestException: ADD_PARTITIONS_TO_TXN with 
version 4 which is not enabled when 
 Key: KAFKA-16692
 URL: https://issues.apache.org/jira/browse/KAFKA-16692
 Project: Kafka
  Issue Type: Bug
  Components: core
Affects Versions: 3.6.2
Reporter: Johnson Okorie


We have a kafka cluster running on version 3.5.2 that we are upgrading to 
3.6.2. This cluster has a lot of clients with exactly one semantics enabled and 
hence creating transactions. As we replaced brokers with the new binaries, we 
observed lots of clients in the cluster experiencing the following error:


{code:java}
2024-05-07T09:08:10.039Z "tid": "" -- [Producer clientId=, 
transactionalId=] Got error produce response with correlation 
id 6402937 on topic-partition , retrying (2147483512 attempts 
left). Error: NETWORK_EXCEPTION. Error Message: The server disconnected before 
a response was received.{code}
On inspecting the broker, we saw the following errors on brokers still running 
Kafka version 3.5.2:

 
{code:java}
message:     
Closing socket for  because of error
exception_exception_class:    
org.apache.kafka.common.errors.InvalidRequestException
exception_exception_message:    
Received request api key ADD_PARTITIONS_TO_TXN with version 4 which is not 
enabled
exception_stacktrace:    
org.apache.kafka.common.errors.InvalidRequestException: Received request api 
key ADD_PARTITIONS_TO_TXN with version 4 which is not enabled
{code}
On the new brokers running 3.6.2 we saw the following errors:

 
{code:java}
[AddPartitionsToTxnSenderThread-1055]: AddPartitionsToTxnRequest failed for 
node 1043 with a network exception.{code}
 

I can also see this :
{code:java}
[AddPartitionsToTxnManager broker=1055]Cancelled in-flight 
ADD_PARTITIONS_TO_TXN request with correlation id 21120 due to node 1043 being 
disconnected (elapsed time since creation: 11ms, elapsed time since send: 4ms, 
request timeout: 3ms){code}

We started investigating this issue and digging through the changes in 3.6, we 
came across some changes introduced as part of 
[KAFKA-14402|https://issues.apache.org/jira/browse/KAFKA-14402] that we thought 
might lead to this behaviour. 

First we could see that _transaction.partition.verification.enable_ is enabled 
by default and enables a new code path that culminates in we sending version 4 
ADD_PARTITIONS_TO_TXN requests to other brokers that are generated 
[here|[https://github.com/apache/kafka/blob/cb35ddc5ca233d5cca6f51c1c41b952a7e9fe1a0/core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala#L269]].

>From a 
>[discussion|https://lists.apache.org/thread/4895wrd1z92kjb708zck4s1f62xq6r8x] 
>on the mailing list, [~jolshan] pointed out that this scenario shouldn't be 
>possible as the following code paths should prevent version 4 
>ADD_PARTITIONS_TO_TXN requests being sent to other brokers:

[https://github.com/apache/kafka/blob/525b9b1d7682ae2a527ceca83fedca44b1cba11a/clients/src/main/java/org/apache/kafka/clients/NodeApiVersions.java#L130]
 
[https://github.com/apache/kafka/blob/525b9b1d7682ae2a527ceca83fedca44b1cba11a/core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala#L195]

However, this seems to be these requests are still sent to other brokers in our 
environment. 

On further inspection of the code, I am wondering if the following code path 
could lead to this issue:

[https://github.com/apache/kafka/blob/c4deed513057c94eb502e64490d6bdc23551d8b6/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java#L500]

In this scenario, we don't have any _NodeApiVersions_ available for the 
specified nodeId and potentially skipping _latestUsableVersion_ check as 
expected. I am wondering if it is possible that because 
_discoverBrokerVersions_ is set to false for the network client of the 
AddPartitionsToTxnManager, it skips fetching ApiVersions? I can see here that 
we create the network client here:

[https://github.com/apache/kafka/blob/c4deed513057c94eb502e64490d6bdc23551d8b6/core/src/main/scala/kafka/server/KafkaServer.scala#L641]

This _NetworkUtils.buildNetworkClient_ seems to create a network client that 
has _discoverBrokerVersions_ set to false. 

I was hoping I could get some assistance debugging this issue.










 

 

 



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


[jira] [Resolved] (KAFKA-16640) Replace TestUtils#resource by scala.util.Using

2024-05-08 Thread Chia-Ping Tsai (Jira)


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

Chia-Ping Tsai resolved KAFKA-16640.

Fix Version/s: 3.8.0
   Resolution: Fixed

> Replace TestUtils#resource by scala.util.Using
> --
>
> Key: KAFKA-16640
> URL: https://issues.apache.org/jira/browse/KAFKA-16640
> Project: Kafka
>  Issue Type: Improvement
>Reporter: Chia-Ping Tsai
>Assignee: TengYao Chi
>Priority: Minor
> Fix For: 3.8.0
>
>
> `scala.util.Using` is in both scala 2.13 and scala-collection-compat so we 
> don't need to have custom try-resource function.



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


KIP-1040: Improve handling of nullable values in InsertField/ExtractField transformations

2024-05-08 Thread Mario Fiore Vitale
Hi All,

I have created (through Mickael Maison's account since there was an issue
creating a new one for me) KIP-1040[1] to improve handling of nullable
values in InsertField/ExtractField transformations, this is required after
the introduction of KIP-581[2] that introduced the configuration property
*replace.null.with.default* to *JsonConverter* to choose whether to replace
fields that have a default value and that are null to the default value.

[1]
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=303794677
[2]
https://cwiki.apache.org/confluence/display/KAFKA/KIP-581%3A+Value+of+optional+null+field+which+has+default+value

Feedback and suggestions are welcome.

Regards,
Mario.
-- 

Mario Fiore Vitale

Senior Software Engineer

Red Hat 



Re: [VOTE] KIP-932: Queues for Kafka

2024-05-08 Thread Satish Duggana
Hi Andrew,
Thanks for the nice KIP, it will allow other messaging use cases to be
onboarded to Kafka.

+1 from me.

Satish.

On Tue, 7 May 2024 at 03:41, Jun Rao  wrote:
>
> Hi, Andrew,
>
> Thanks for the KIP. +1
>
> Jun
>
> On Mon, Mar 18, 2024 at 11:00 AM Edoardo Comar 
> wrote:
>
> > Thanks Andrew,
> >
> > +1 (binding)
> >
> > Edo
> >
> > On Mon, 18 Mar 2024 at 16:32, Kenneth Eversole
> >  wrote:
> > >
> > > Hi Andrew
> > >
> > > + 1 (Non-Binding)
> > >
> > > This will be great addition to Kafka
> > >
> > > On Mon, Mar 18, 2024 at 8:27 AM Apoorv Mittal 
> > > wrote:
> > >
> > > > Hi Andrew,
> > > > Thanks for writing the KIP. This is indeed going to be a valuable
> > addition
> > > > to the Kafka, excited to see the KIP.
> > > >
> > > > + 1 (Non-Binding)
> > > >
> > > > Regards,
> > > > Apoorv Mittal
> > > > +44 7721681581
> > > >
> > > >
> > > > On Sun, Mar 17, 2024 at 11:16 PM Andrew Schofield <
> > > > andrew_schofield_j...@outlook.com> wrote:
> > > >
> > > > > Hi,
> > > > > I’ve been working to complete KIP-932 over the past few months and
> > > > > discussions have quietened down.
> > > > >
> > > > > I’d like to open the voting for KIP-932:
> > > > >
> > > > >
> > > > >
> > > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-932%3A+Queues+for+Kafka
> > > > >
> > > > > Thanks,
> > > > > Andrew
> > > >
> >