Re: [VOTE] KIP-953: partition method to be overloaded to accept headers as well.

[Sagar]

Hi Ismael,

Thanks for pointing us towards the direction of a DTO based approach. The
AdminClient examples seem very neat and extensible in that sense.
Personally, I was trying to think only along the lines of how the current
Partitioner interface has been designed, i.e having all requisite
parameters as separate arguments (Topic, Key, Value etc).

Regarding this question of yours:

A more concrete question: did we consider having the method `partition`
> take `ProduceRecord` as one of its parameters and `Cluster` as the other?


No, I don't think in the discussion thread it was brought up and as I said
it appears that could be due to an attempt to keep the new method's
signature similar to the existing one within Partitioner. If I understood
the intent of the question correctly, are you trying to hint here that
`ProducerRecord` already contains all the arguments that the `partition`
method accepts and also has a `headers` field within it. So, instead of
adding another method for the `headers` field, why not create a new method
taking ProducerRecord directly?

If my understanding is correct, then it seems like a very clean way of
adding support for `headers`. Anyways, the partition method within
KafkaProducer already takes a ProducerRecord as an argument so that makes
it easier. Keeping that in mind, should this new method's (which takes a
ProducerRecord as an input) default implementation invoke the existing
method ? One challenge I see there is that the existing partition method
expects serialized keys and values while ProducerRecord doesn't have access
to those (It directly operates on K, V).

Thanks!
Sagar.


On Sun, Aug 27, 2023 at 8:51 AM Ismael Juma  wrote:

> A more concrete question: did we consider having the method `partition`
> take `ProduceRecord` as one of its parameters and `Cluster` as the other?
>
> Ismael
>
> On Sat, Aug 26, 2023 at 12:50 PM Greg Harris  >
> wrote:
>
> > Hey Ismael,
> >
> > > The mention of "runtime" is specific to Connect. When it comes to
> > clients,
> > one typically compiles and runs with the same version or runs with a
> newer
> > version than the one used for compilation. This is standard practice in
> > Java and not something specific to Kafka.
> >
> > When I said "older runtimes" I was being lazy, and should have said
> > "older versions of clients at runtime," thank you for figuring out
> > what I meant.
> >
> > I don't know how common it is to compile a partitioner against one
> > version of clients, and then distribute and run the partitioner with
> > older versions of clients and expect graceful degradation of features.
> > If you say that it is very uncommon and not something that we should
> > optimize for, then I won't suggest otherwise.
> >
> > > With regards to the Admin APIs, they have been extended several times
> > since introduction (naturally). One of them is:
> > >
> >
> https://github.com/apache/kafka/commit/1d22b0d70686aef5689b775ea2ea7610a37f3e8c
> >
> > Thanks for the example. I also see that includes a migration from
> > regular arguments to the DTO style, consistent with your
> > recommendation here.
> >
> > I think the DTO style and the proposed additional argument style are
> > both reasonable.
> >
> > Thanks,
> > Greg
> >
> > On Sat, Aug 26, 2023 at 9:46 AM Ismael Juma  wrote:
> > >
> > > Hi Greg,
> > >
> > > The mention of "runtime" is specific to Connect. When it comes to
> > clients,
> > > one typically compiles and runs with the same version or runs with a
> > newer
> > > version than the one used for compilation. This is standard practice in
> > > Java and not something specific to Kafka.
> > >
> > > With regards to the Admin APIs, they have been extended several times
> > since
> > > introduction (naturally). One of them is:
> > >
> > >
> >
> https://github.com/apache/kafka/commit/1d22b0d70686aef5689b775ea2ea7610a37f3e8c
> > >
> > > Ismael
> > >
> > > On Sat, Aug 26, 2023 at 8:29 AM Greg Harris
>  > >
> > > wrote:
> > >
> > > > Hey Ismael,
> > > >
> > > > Thank you for clarifying where the DTO pattern is used already, I did
> > > > not have the admin methods in mind.
> > > >
> > > > > With the DTO approach, you don't create a new DTO, you simply add a
> > new
> > > > overloaded constructor and accessor to the DTO.
> > > >
> > > > With this variant, partitioner implementations would receive a
> > > > `NoSuchMethodException` when trying to access newer methods in older
> > > > runtimes. Do we expect the interface implementers will maintain the
> > > > try-catch to support backwards-compatibility?
> > > > Fortunately here the Headers type already exists, but in the future
> if
> > > > a new subtype is added at the same time as the change to the DTO is
> > > > made, interface implementers will need to be careful to avoid
> > > > NoClassDefFoundErrors.
> > > > We used this "add a new method" style extension in
> > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-610%3A+Error+Reporting+in+Sink+Connectors
> > > > and 

Build failed in Jenkins: Kafka » Kafka Branch Builder » trunk #2146

[Apache Jenkins Server]

See 


Changes:


--
[...truncated 305858 lines...]

Gradle Test Run :streams:test > Gradle Test Executor 84 > TasksTest > 
shouldDrainPendingTasksToCreate() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > TasksTest > 
shouldDrainPendingTasksToCreate() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > TasksTest > 
onlyRemovePendingTaskToRecycleShouldRemoveTaskFromPendingUpdateActions() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > TasksTest > 
onlyRemovePendingTaskToRecycleShouldRemoveTaskFromPendingUpdateActions() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > TasksTest > 
shouldAddAndRemovePendingTaskToCloseClean() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > TasksTest > 
shouldAddAndRemovePendingTaskToCloseClean() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > TasksTest > 
shouldAddAndRemovePendingTaskToCloseDirty() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > TasksTest > 
shouldAddAndRemovePendingTaskToCloseDirty() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > TasksTest > 
shouldKeepAddedTasks() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > TasksTest > 
shouldKeepAddedTasks() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldAssignTasksThatCanBeSystemTimePunctuated() 
STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldAssignTasksThatCanBeSystemTimePunctuated() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotUnassignNotOwnedTask() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotUnassignNotOwnedTask() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotSetUncaughtExceptionsTwice() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotSetUncaughtExceptionsTwice() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > 
shouldNotAssignTasksForPunctuationIfPunctuationDisabled() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > 
shouldNotAssignTasksForPunctuationIfPunctuationDisabled() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldAddTask() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldAddTask() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotAssignAnyLockedTask() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotAssignAnyLockedTask() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldRemoveTask() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldRemoveTask() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotRemoveAssignedTask() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotRemoveAssignedTask() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldAssignTaskThatCanBeProcessed() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldAssignTaskThatCanBeProcessed() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotRemoveUnlockedTask() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotRemoveUnlockedTask() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldReturnAndClearExceptionsOnDrainExceptions() 
STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldReturnAndClearExceptionsOnDrainExceptions() 
PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldUnassignTask() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldUnassignTask() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > 
shouldNotAssignTasksForProcessingIfProcessingDisabled() STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > 
shouldNotAssignTasksForProcessingIfProcessingDisabled() PASSED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotSetUncaughtExceptionsForUnassignedTasks() 
STARTED

Gradle Test Run :streams:test > Gradle Test Executor 84 > 
DefaultTaskManagerTest > shouldNotSetUncaug

RE: [VOTE] KIP-942: Add Power(ppc64le) support

[Vaibhav Nazare]

Hi Colin,

As discussed with Divij according the point 2 mentioned in the last comment I 
updated the Test Plan to run job for every merge to trunk. 

Thanks,
Vaibhav Nazare 


-Original Message-
From: Divij Vaidya  
Sent: Monday, July 24, 2023 6:14 PM
To: dev@kafka.apache.org
Subject: [EXTERNAL] Re: [DISCUSS] KIP-942: Add Power(ppc64le) support

Hi Vaibhav

1. Could you please add the details that we discussed here in this thread to 
the KIP (e.g. why is it safe to add this infra now and which other projects are 
using it). This would help to have KIP as a source of record if someone wants 
to read about this decision in future.

2. Given that we merge ~5-6 commits a day to trunk, I would be inclined towards 
running this infra on every merge to trunk (instead of a daily run as I 
suggested earlier). It won't be run for every PR though.

These are the last two thoughts from my side. Otherwise, it looks good.

--
Divij Vaidya


On Mon, Jul 10, 2023 at 7:09 PM Vaibhav Nazare  
wrote:
>
> Hi Colin and Mickael,
>
> Yes I agree we don't have to run on every build , may be as mentioned we can 
> run the job once/twice a day at specific time.
> According the test plan the tests which I mentioned are not specific to 
> ppc64le , I was running tests on the trunk branch which were common across 
> platforms on that specific commit and I have removed the table and mentioned 
> the test plan accordingly when to run the jobs.
>
>
>
> -Original Message-
> From: Mickael Maison 
> Sent: Tuesday, July 4, 2023 3:12 PM
> To: dev@kafka.apache.org
> Subject: [EXTERNAL] Re: [DISCUSS] KIP-942: Add Power(ppc64le) support
>
> Hi,
>
> Thanks for the KIP!
> In the Test Plan section you mentioned a few unit and integration tests are 
> failing. Are these flaky tests or did you find issues related to ppc64le? If 
> they are just flaky tests, I think we can remove that table and instead 
> describe how often we intend to run the tests in the CI.
>
> Thanks,
> Mickael
>
> On Tue, Jul 4, 2023 at 12:04 AM Colin McCabe  wrote:
> >
> > I agree with Divij. A nightly Apache Kafka build for PowerPC would be 
> > welcome. But it shouldn't run on every build, since the extra time and 
> > complexity would not be worth it.
> >
> > By the way, are there any features or plugins we don't intend to support on 
> > PPC? If there are, this KIP would be a good place to spell them out.
> >
> > Naively, I would think all of our Java and Scala code should work on 
> > PPC without changes. However, there may be library dependencies that 
> > don't exist on PPC. (We have to remember that the last desktop 
> > PowerPC chip that an average user could buy shipped in 2005)
> >
> > best,
> > Colin
> >
> >
> > On Mon, Jun 19, 2023, at 23:12, Vaibhav Nazare wrote:
> > > Thank you for response Divij.
> > >
> > > 1. We are going to use ASF infra provided nodes for better 
> > > availability and stability as there are 3 power9 nodes managed 
> > > officially by ASF infra team themselves.
> > > Ref:
> > > INVALID URI REMOVED
> > > rg_jira_browse_INFRA-2D24663&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=s
> > > 9I
> > > 3h_d72lHAurpHrTUoOkX8ByFHVUGD0XU1PTKfCiw&m=wok2fkRzJGACiUg-o6hyaJP
> > > 11 
> > > WLbfqYbH78J2VuRdvseBKrVLuXW9PtUJqs9BWO4&s=EeWMMaaUqDLlqUjJmw3_ouI9
> > > lt
> > > 4B_EUGyH8OZ_VTIRE&e=
> > > INVALID URI REMOVED.
> > > apache.org_view_Shared-2520-2D-2520ppc64le-2520nodes_&d=DwIFaQ&c=j
> > > f_ 
> > > iaSHvJObTbx-siA1ZOg&r=s9I3h_d72lHAurpHrTUoOkX8ByFHVUGD0XU1PTKfCiw&
> > > m= 
> > > wok2fkRzJGACiUg-o6hyaJP11WLbfqYbH78J2VuRdvseBKrVLuXW9PtUJqs9BWO4&s
> > > =O R-kg7uZdehAVxU7TdCUruDpGb0yOAEa0wW2dC3P3yU&e=
> > > previously used power node details for apache/kafka CI:
> > > RAM- 16GB
> > > VCPUs- 8 VCPU
> > > Disk- 160GB
> > > for shared VMs we need to check with ASF infra team to provide 
> > > details
> > >
> > > 2. We can run nightly builds once or twice in a day on specific 
> > > period of time instead of every commit 3. apache/camel INVALID URI 
> > > REMOVED 
> > > rg_job_Camel_job_el_&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=s9I3h_d72lHAurpHrTUoOkX8ByFHVUGD0XU1PTKfCiw&m=wok2fkRzJGACiUg-o6hyaJP11WLbfqYbH78J2VuRdvseBKrVLuXW9PtUJqs9BWO4&s=J35xYPMn5tCJRn91aARKgyiwas_cu4RGeSsh50tV4GI&e=
> > >   has already enabled CI for power platform they are using same H/W 
> > > resources as
> > > RAM- 16GB
> > > VCPUs- 8 VCPU
> > > Disk- 160GB
> > >
> > > -Original Message-
> > > From: Divij Vaidya 
> > > Sent: Monday, June 19, 2023 10:20 PM
> > > To: dev@kafka.apache.org
> > > Subject: [EXTERNAL] Re: [DISCUSS] KIP-942: Add Power(ppc64le) 
> > > support
> > >
> > > Thank you for the KIP Vaibhav.
> > >
> > > 1. Builds for power architecture were intentionally disabled in 
> > > the past since the infrastructure was flaky [1]. Could you please 
> > > add to the KIP on what has changed since then?
> > > 2. What do you think about an alternative solution where we run a 
> > > nightly build for this platform instead of running the CI