Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-05-17 Thread 黃竣陽
Hi Jun, chia, >> 1. What's the desired way for returning a map when some of the values are >> not present? Users can use the input value to identify which values are missing from the map. To me, this is more intuitive in terms of semantics. For now, it’s reasonable to keep the original design. W

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-05-15 Thread Chia-Ping Tsai
hi Jun > For 1, is it better to skip the missing value in the return map or include it? To me, including every item passed in from the input seems more intuitive and less error prone for the users. Yes, I agree that using null for now is suitable. Especially considering the growing number of offs

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-05-15 Thread Jun Rao
Hi, Chia-Ping, Yes, there are two things to consider. 1. What's the desired way for returning a map when some of the values are not present? 2. Once we agreed on a desired way, how to make that consistent across all APIs. For 1, is it better to skip the missing value in the return map or includ

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-05-15 Thread Chia-Ping Tsai
hi Jun I understand the use case where the "Result" is expected to represent both "exist" and "non-exist." However, I'm concerned about consistency across our APIs. Not all of Admin APIs follow this pattern; for example, listPartitionReassignments and describeProducers do not. Designing a single A

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-05-14 Thread Jun Rao
Hi, Jiunn-Yang, Thanks for the reply. If we omit the non-existing value in the return map, for applications that care about the missing value, is it convenient for them to write the code? They need to save the input to find this out, right? Jun On Fri, May 9, 2025 at 4:39 AM 黃竣陽 wrote: > Hi

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-05-09 Thread 黃竣陽
Hi Jun, chia Thanks for great examples. I still believe that when a map contains a key, its value should not be null. Now that we've introduced the allow.null.offsets.entries config as a bridge between the two behaviors, I trust that most users who rely on null values will have a reasonable p

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-05-05 Thread Chia-Ping Tsai
hi Jun Thanks for the great example. Whether null has value depends on user expectations regarding the "map structure." If users expect it to represent both existence and non-existence, then null is meaningful. Otherwise, for users expecting the map structure to represent only existence, null mig

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-05-05 Thread Jun Rao
Hi, Jiunn-Yang, Looking at the code again, it seems that the assumption "For a real-world example, we can see that developers typically do not check for null when using this." is not true. For example, the following is how Streams uses committed(). // those which do not have a committ

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-05-02 Thread 黃竣陽
Hi chia, Jun, > It would be useful to think through the consistency with > adminClient.listOffset() While the returned map entries are never null, a value of -1 is used to indicate the absence of an offset. Given this approach, I believe we could apply a similar strategy by introducing a new

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-30 Thread Chia-Ping Tsai
hi Jun > It seems that consumer.committed() has the same behavior. If you pass in a non-existing partition, it returns a map with a null value. So, if we want to make a change, it would be useful to make it consistent too. yes, both consumers have same behavior. For another, `listStreamsGroupOff

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-30 Thread Jun Rao
Hi, Chia-Ping, It seems that consumer.committed() has the same behavior. If you pass in a non-existing partition, it returns a map with a null value. So, if we want to make a change, it would be useful to make it consistent too. Thanks, Jun On Wed, Apr 30, 2025 at 9:05 AM Chia-Ping Tsai wrote:

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-30 Thread Chia-Ping Tsai
hi Ken, there is another inconsistent case for the offset-related APIs. see https://github.com/apache/kafka/pull/19578#discussion_r2068913749 Best, Chia-Ping Chia-Ping Tsai 於 2025年4月30日 週三 下午7:03寫道: > hi all, > > >Also, it would be useful to think through the behavior > of ListOffsetsResult.pa

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-30 Thread Chia-Ping Tsai
hi all, >Also, it would be useful to think through the behavior of ListOffsetsResult.partitionResult(final TopicPartition partition) too. It seems there are three options: 1. Keep the current behavior - it's inconsistent, as other result classes, such as DeleteRecordsResult, don't have a

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-29 Thread Jun Rao
Hi, Jiunn-Yang, Thanks for the KIP. It would be useful to think through the consistency with adminClient.listOffset(). Currently, if the offset for a timestamp doesn't exist, consumer.offsetsForTimes() returns a null value for that partition while adminClient.listOffset().all().get() returns a Li

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-23 Thread 黃竣陽
Hi Kirk, Thanks for your comment. I agree that ‘allow.null.offsets.entries' is a better name. I've updated the KIP accordingly. Best Regards, Jiunn-Yang > Kirk True 於 2025年4月23日 上午8:20 寫道: > > Hi Jiunn-Yang, > > Thanks for the updates! > > IMO, "allow.null.entries" might be a little too va

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-22 Thread Kirk True
Hi Jiunn-Yang, Thanks for the updates! IMO, "allow.null.entries" might be a little too vague. Is "allow.null.offsets.entries" better? Thanks, Kirk On Tue, Apr 22, 2025, at 6:03 AM, 黃竣陽 wrote: > Hello everyone, > > I’ve submitted an updated version of this KIP based on recent discussions. > I

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-22 Thread 黃竣陽
Hello everyone, I’ve submitted an updated version of this KIP based on recent discussions. I'm happy to hear any further concerns or suggestions you might have. Best Regards, Jiunn-Yang > 黃竣陽 於 2025年4月22日 下午6:35 寫道: > > Hello, > > Thank for th

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-22 Thread 黃竣陽
Hello, Thank for the feedback. I will address this new/deprecated mechanism in the updated version of the KIP. Best Regards, Jiunn-Yang > Chia-Ping Tsai 於 2025年4月22日 中午12:58 寫道: > > hi Matthias > > Thanks for offering this approach. I had considered proposing it before. My > concern was the

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-21 Thread Chia-Ping Tsai
hi Matthias Thanks for offering this approach. I had considered proposing it before. My concern was the new/deprecated config is used by client, so it could cause a bunch of warning messages by default. Also, users have to add the new/deprecated config to each client properties if they want to

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-21 Thread Ismael Juma
That's right. Similar approach: https://lists.apache.org/thread/3fxqdsosm3z7xp4rr8db2qmyg5vd0v1b Ismael On Mon, Apr 21, 2025, 7:43 PM Matthias J. Sax wrote: > I don't think we would need a second deprecation cycle. > > Instead, we could add this new config, and deprecate it right away. This > w

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-21 Thread Matthias J. Sax
I don't think we would need a second deprecation cycle. Instead, we could add this new config, and deprecate it right away. This way, we can change the default behavior and remove the config both with 5.0. I guess the core question is really, do we want to enable the old or new behavior by de

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-17 Thread Kirk True
Hi Colin, If so, is 5.0 the earliest this 'allow.nulls.in.consumer' configuration can be changed and marked as deprecated? And if that holds, is 6.0 the earliest it can be removed? Thanks, Kirk On Mon, Apr 14, 2025, at 1:10 PM, Colin McCabe wrote: > I would suggest adding a configuration key w

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-17 Thread Chia-Ping Tsai
hi Kirk “coarsely grained” is a good point. We need to list all behaviors impacted by the config - no matter which new config we adopted. Maybe we can add the flag to xxxOption? The benefit is users can explicitly see “which” APIs are impacted. The downside is the number of deprecated methods i

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-17 Thread Kirk True
Hi Chia-Ping, An "enable unreleased behavior" flag is an interesting idea; basically a feature flag. The main benefit (coarsely grained) is also its biggest weakness. I'm not really sure the target audience that would enable it. Thanks, Kirk On Mon, Apr 14, 2025, at 9:39 PM, Chia-Ping Tsai wro

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-14 Thread Chia-Ping Tsai
hi Colin Adding the config is a cool idea. However, the config will force us to maintain both behaviors in 5.0. Additionally, we need a complete deprecation cycle to remove the config. Maybe another alternative is to introduce a flag called “enable.unrelased.behavior”, allowing users to “test”

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-04-14 Thread Colin McCabe
I would suggest adding a configuration key which controls whether the null values are added. That configuration key can default to true in 4.x and false in 5.x. This will give people a chance to test the new behavior before 5.x. best, Colin On Fri, Mar 14, 2025, at 04:30, 黃竣陽 wrote: > Hello eve

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-03-31 Thread 黃竣陽
Hi Kirk, > it would be good to see a PR with some of these null edge cases clearly > spelled out in the JavaDoc. Then, once the KIP is accepted, those comments > could be updated with the intended future changes. It a good idea, I open a Jira

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-03-27 Thread Ismael Juma
Hi, Thanks for the KIP. A few comments: 1. Incompatible changes are generally not allowed in minor releases. Are you proposing a change for a minor release (eg 4.1) or for a major release (5.0)? 2. Given that clients in 4.0 support brokers with version 2.1 or higher, `offsetsForTimes` should neve

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-03-24 Thread Kirk True
Hi Jiunn-Yang, Regardless of what form the KIP takes and when it's implemented, near term it would be good to see a PR with some of these null edge cases clearly spelled out in the JavaDoc. Then, once the KIP is accepted, those comments could be updated with the intended future changes. Thanks

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-03-22 Thread 黃竣陽
Hello Ismael, Thanks for your comments. **Q1:** I believe this change should be introduced in Kafka 5.0. I will include a note about this in the KIP. **Q2, Q3:** Although these methods are not explicitly mentioned, they currently return null. While tracing the code, I found that null is retu

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-03-19 Thread Ismael Juma
> > 2. Given that clients in 4.0 support brokers with version 2.1 or higher, > `offsetsForTimes` should never return maps with `null` values (check the > javadoc for the details of when this happens). We can perhaps consider this > a documentation fix (versus a behavior change). > Thinking about t

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-03-19 Thread Kirk True
Hi Jiunn-Yang, Response inline... On Wed, Mar 19, 2025, at 5:28 AM, 黃竣陽 wrote: > Hello Kirk True, > > Thanks for your comments. > > KT1: In fact, while reviewing projects on GitHub, I haven't found any that > would be affected by this change. Most developers simply retrieve the Long > value f

Re: [DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-03-17 Thread Kirk True
Hi Jiunn-Yang, Thanks for the KIP! I agree that this oddity should be fixed. It's a bit of a funny case, KIP-wise, in that the API signature itself isn't changing, but the documentation and behavior are. Some questions: KT1: The KIP specifically calls out "The scenarios that will be impacted"

[DISCUSS] KIP-1140: Avoid to return null value in Map from public api of consumer

2025-03-14 Thread 黃竣陽
Hello everyone, I would like to start a discussion on KIP-1140: Avoid to return null value in Map from public api of consumer This proposal aims to improve the Kafka consumer API by ensuring that the Map it returns contains only non-null values,