Hi Jun, Chia >> Also, it would be useful to add a compatibility column in the table. If a >> value is formally disallowed, it would be useful to compare the old and the >> new behavior (e.g. a different exception is thrown, etc). I’ve updated the table to show which exceptions will be thrown for each behavior.
>>> I don't understand the point of "5.0". Could you please share more details? >>> It seems to me changing the type or default value should be fine in minor >>> release if we don't break the existing property files. I don't think this requires any compatibility considerations, so I’ve removed the Kafka 5.0 changes section. >>> `max.connections.per.ip.overrides` is parsed as "map" type, so using `LIST` >>> instead of `String` does not seems to make sense. I have removed the config from KIP Best Regards, Jiunn-Yang > Chia-Ping Tsai <chia7...@apache.org> 於 2025年7月9日 晚上10:26 寫道： > >> For ssl.cipher.suites, changing the default value from null to empty is >> actually a breaking change. According to the doc, null means that all the >> available cipher suites are supported. I am thinking that we should >> continue to allow null as the default since it could represent a state >> different from empty. > > "Empty list" seems to be a weird case. It appears to signify that no cipher > suites are accepted. However, in the codebase, an empty list is handled as if > all available cipher suites are supported [0]. We should not support the case > of "no supported cipher suite," as it doesn't make sense. > > In short, changing the default value from null to empty does not break the > behavior. > > [0] > https://github.com/apache/kafka/blob/dabde76ebf105aaa945db60b7753331c83a8c989/clients/src/main/java/org/apache/kafka/common/security/ssl/DefaultSslEngineFactory.java#L139 > > On 2025/07/08 17:01:07 Jun Rao wrote: >> Hi, Jiunn-Yang, >> >> I agree with Chia-Ping. If we allow cleanup.policy to be empty, we will >> just continue to allow it in all future releases. >> >> For ssl.cipher.suites, changing the default value from null to empty is >> actually a breaking change. According to the doc, null means that all the >> available cipher suites are supported. I am thinking that we should >> continue to allow null as the default since it could represent a state >> different from empty. >> >> Also, it would be useful to add a compatibility column in the table. If a >> value is formally disallowed, it would be useful to compare the old and the >> new behavior (e.g. a different exception is thrown, etc). >> >> Thanks, >> >> Jun >> >> On Tue, Jul 8, 2025 at 8:36 AM Chia-Ping Tsai <chia7...@gmail.com> wrote: >> >>> hi Jiunn >>> >>> Thanks for the explanation. I have updated the KIP accordingly: in Kafka >>>> 4.x, cleanup.policy >>>> can be set to an empty list with a warning message emitted; however, in >>>> Kafka 5.0, setting an >>>> empty list for cleanup.policy will no longer be allowed. >>> >>> >>> If we agree to support "empty list", the approach of "none" could be >>> eliminated, right? >>> >>> I don't understand the point of "5.0". Could you please share more details? >>> It seems to me changing the type or default value should be fine in minor >>> release if we don't break the existing property files. >>> >>> `max.connections.per.ip.overrides` is parsed as "map" type, so using `LIST` >>> instead of `String` does not seems to make sense. >>> >>> Best, >>> Chia-Ping >>> >>> >>> >>> >>> 黃竣陽 <s7133...@gmail.com> 於 2025年7月8日 週二 下午11:25寫道： >>> >>>> Hi all, >>>> >>>>>> Null could make sense as the default value, but it just means that the >>>>>> config key is not set explicitly. >>>>>> Given that, it's probably simpler to just allow >>>>>> log.cleanup.policy to be empty and properly support it? >>>> Thanks for the explanation. I have updated the KIP accordingly: in Kafka >>>> 4.x, cleanup.policy >>>> can be set to an empty list with a warning message emitted; however, in >>>> Kafka 5.0, setting an >>>> empty list for cleanup.policy will no longer be allowed. >>>> >>>>> For another, there is an additional issue which could be handled by >>> this >>>>> KIP. Some configs have "string" type, but they are handled as a LIST. >>> For >>>>> example, `early.start.listeners`, `security.providers`. Could we change >>>>> their type to LIST to benefit from this KIP? I mean to make them >>>>> non-nullable. >>>> I have also updated the KIP to include the tables detailing the changes >>>> for LIST-type configuration >>>> default values and the migration from STRING-type to LIST-type >>>> configurations. >>>> >>>> Best Regards, >>>> Jiunn-Yang >>>> >>>>> Chia-Ping Tsai <chia7...@gmail.com> 於 2025年7月8日 清晨7:06 寫道： >>>>> >>>>> hi all, >>>>> >>>>> JR11. There is an existing config interceptor.classes that allows an >>>> empty >>>>>> list and it makes intuitive sense. So, it seems that it's ok to >>> support >>>>>> empty lists in general. Given that, it's probably simpler to just >>> allow >>>>>> log.cleanup.policy to be empty and properly support it? >>>>> >>>>> >>>>> yes, that is what I meant before. Additionally, the `LIST` type should >>>> not >>>>> accept "null". Instead, we should use an empty list as the default >>> value. >>>>> For example, `sasl.oauthbearer.expected.audience` should never has >>> "null" >>>>> value. That could eliminate the null check. >>>>> >>>>> For another, there is an additional issue which could be handled by >>> this >>>>> KIP. Some configs have "string" type, but they are handled as a LIST. >>> For >>>>> example, `early.start.listeners`, `security.providers`. Could we change >>>>> their type to LIST to benefit from this KIP? I mean to make them >>>>> non-nullable. >>>>> >>>>> Best, >>>>> Chia-Ping >>>>> >>>>> >>>>> >>>>> Jun Rao <j...@confluent.io.invalid> 於 2025年7月8日 週二 上午5:56寫道： >>>>> >>>>>> Hi, Jiunn-Yang, >>>>>> >>>>>> Thanks for the reply. >>>>>> >>>>>> JR10. Regarding supporting the null value, I am not sure how one could >>>> set >>>>>> a null value in a config file. For example, if you set the following >>> in >>>> a >>>>>> config file. >>>>>> configkey= >>>>>> The value of configkey will be an empty list, instead of null, right? >>>>>> >>>>>> Null could make sense as the default value, but it just means that the >>>>>> config key is not set explicitly. >>>>>> >>>>>> JR11. There is an existing config interceptor.classes that allows an >>>> empty >>>>>> list and it makes intuitive sense. So, it seems that it's ok to >>> support >>>>>> empty lists in general. Given that, it's probably simpler to just >>> allow >>>>>> log.cleanup.policy to be empty and properly support it? >>>>>> >>>>>> Jun >>>>>> >>>>>> >>>>>> >>>>>> On Fri, Jul 4, 2025 at 5:13 AM 黃竣陽 <s7133...@gmail.com> wrote: >>>>>> >>>>>>> Hello Jun, >>>>>>> >>>>>>> JR10: The sasl.oauthbearer.expected.audience is optional, that is >>>> right, >>>>>>> so I think >>>>>>> that we can allow null value, but not allow the empty >>>>>>> >>>>>>> JR11: In this KIP, the goal is to address configurations that should >>>> not >>>>>>> accept empty arrays. >>>>>>> We want to prevent users from passing in an empty list when it's not >>>>>>> valid. I believe there are two >>>>>>> main scenarios to consider: >>>>>>> >>>>>>> 1. Nullable but not empty: >>>>>>> The parameter allows either a null value or a non-empty list. In this >>>>>>> case, >>>>>>> if the user provides an empty list, we should throw a >>> ConfigException. >>>>>>> >>>>>>> 2. Non-null and non-empty only: The parameter must always contain a >>>>>>> non-empty list — null is also >>>>>>> invalid. In this case, if the user provides either a null or an empty >>>>>>> list, we should throw a ConfigException. >>>>>>> >>>>>>> Therefore, I want to prevent scenarios where users can pass in an >>> empty >>>>>>> list. >>>>>>> >>>>>>> JR12: I’ve updated NonEmptyListValidator to include an allowNull >>>>>> property. >>>>>>> This allows us to specify >>>>>>> whether a configuration permits null values or not. >>>>>>> >>>>>>> JR13: I have updated the table >>>>>>> >>>>>>> >>>>>>>> Jun Rao <j...@confluent.io.INVALID> 於 2025年7月1日 清晨6:30 寫道： >>>>>>>> >>>>>>>> Hi, Jiunn-Yang, >>>>>>>> >>>>>>>> Sorry for the late reply. A few more comments. >>>>>>>> >>>>>>>> JR10. The doc for sasl.oauthbearer.expected.audience says that it's >>>>>>>> optional. So an empty list seems to be valid. >>>>>>>> >>>>>>>> JR11. Thinking a bit more. If we are going to support empty lists in >>>>>>>> general, it's probably more consistent to just allow >>>> log.cleanup.policy >>>>>>> to >>>>>>>> be empty as Luke suggested, instead of introducing a new option >>> None. >>>>>>>> >>>>>>>> JR12. NonEmptyListValidator allows the input to be null. It seems >>> that >>>>>>>> only ssl.cipher.suites allows null. The remaining ones shouldn't be >>>>>> null. >>>>>>>> >>>>>>>> JR13. The table is very helpful. Could you also describe the old/new >>>>>>>> behavior for each config with an empty list or duplicate entry? >>>>>>>> >>>>>>>> Thanks, >>>>>>>> >>>>>>>> Jun >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Mon, Jun 30, 2025 at 6:02 AM 黃竣陽 <s7133...@gmail.com> wrote: >>>>>>>> >>>>>>>>> Hello all, >>>>>>>>> >>>>>>>>> I am manually bumping this thread. Any feedback would be >>> appreciated. >>>>>>>>> >>>>>>>>> Best regards, >>>>>>>>> Jiunn-Yang >>>>>>>>> >>>>>>>>>> 黃竣陽 <s7133...@gmail.com> 於 2025年5月28日 晚上11:21 寫道： >>>>>>>>>> >>>>>>>>>> Hello Jun, >>>>>>>>>> >>>>>>>>>> I have updated the KIP and introduced a new validator, >>>>>>>>> NonEmptyListValidator, which ensures that the >>>>>>>>>> provided list is either null or a non-empty list without duplicate >>>>>>>>> entries. I’ve also listed the configurations that will >>>>>>>>>> be validated using this logic. Please feel free to share any >>>> feedback >>>>>>> or >>>>>>>>> suggestions. >>>>>>>>>> >>>>>>>>>> Best Regards, >>>>>>>>>> Jiunn-Yang >>>>>>>>>> >>>>>>>>>>> Jun Rao <j...@confluent.io.INVALID> 於 2025年5月23日 凌晨2:29 寫道： >>>>>>>>>>> >>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>> >>>>>>>>>>> It seems there are quite a few other configs of type list (e.g. >>>>>>> several >>>>>>>>> SSL >>>>>>>>>>> related ones). It would be useful to understand if empty lists >>> are >>>>>>> valid >>>>>>>>>>> for them or not. >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> >>>>>>>>>>> Jun >>>>>>>>>>> >>>>>>>>>>> On Tue, May 13, 2025 at 10:12 AM Jun Rao <j...@confluent.io> >>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi, Luke, >>>>>>>>>>>> >>>>>>>>>>>> Thanks for the reply. >>>>>>>>>>>> >>>>>>>>>>>> "My thought is that this behavior has been existed >>>>>>>>>>>> for years (maybe after "cleanup.policy" was introduced?), there >>>>>> could >>>>>>>>> be >>>>>>>>>>>> users relying on the empty cleanup.policy for a long time. " >>>>>>>>>>>> >>>>>>>>>>>> If you do a google search on "kafka infinite retention", you >>> will >>>>>> get >>>>>>>>>>>> links like >>>>>>>>>>>> >>>>>>>>> >>>>>>> >>>>>> >>>> >>> https://stackoverflow.com/questions/39735036/make-kafka-topic-log-retention-permanent >>>>>>>>>>>> and >>>>>>>>>>>> >>>>>>>>> >>>>>>> >>>>>> >>>> >>> https://www.reddit.com/r/apachekafka/comments/mpz4bp/kafka_retention_question/ >>>>>>>>> , >>>>>>>>>>>> both pointing to setting the retention time and retention size >>> to >>>>>> -1 >>>>>>> to >>>>>>>>>>>> achieve this goal. So, it seems that if a user intentionally >>> wants >>>>>>>>> infinite >>>>>>>>>>>> retention, it's more likely they will use that approach instead >>> of >>>>>>>>> setting >>>>>>>>>>>> cleanup.policy to empty. On the other hand, our API/tools make >>> it >>>>>>>>> possible >>>>>>>>>>>> for users to make a mistake by inadvertently leaving the config >>>>>> value >>>>>>>>> empty. >>>>>>>>>>>> >>>>>>>>>>>> Thanks, >>>>>>>>>>>> >>>>>>>>>>>> Jun >>>>>>>>>>>> >>>>>>>>>>>> On Mon, May 12, 2025 at 11:38 PM Luke Chen <show...@gmail.com> >>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hi Jun, >>>>>>>>>>>>> >>>>>>>>>>>>>> Regarding the motivation, currently, we never documented that >>> an >>>>>>>>> empty >>>>>>>>>>>>> cleanup.policy implies infinite retention. In fact, if one >>> leaves >>>>>>>>>>>>> cleanup.policy empty, it actually breaks remote storage since >>> it >>>>>>> will >>>>>>>>> stop >>>>>>>>>>>>> the cleaning based on local retention size and time too. So, >>>>>> leaving >>>>>>>>>>>>> cleanup.policy empty is probably more like a user mistake than >>> an >>>>>>>>>>>>> intentional choice. Based on this assumption, the idea behind >>> the >>>>>>> KIP >>>>>>>>> is >>>>>>>>>>>>> to >>>>>>>>>>>>> fail an empty cleanup.policy so that the user is aware of this >>>>>>> likely >>>>>>>>>>>>> mistake and provide a new option None for users wanting >>> infinite >>>>>>>>> retention >>>>>>>>>>>>> to opt in explicitly. >>>>>>>>>>>>> >>>>>>>>>>>>> While we never documented the empty cleanup.policy implies >>>>>> infinite >>>>>>>>>>>>> retention, we also never documented (or failed) the empty >>>>>>>>> cleanup.policy >>>>>>>>>>>>> is >>>>>>>>>>>>> an invalid configuration. My thought is that this behavior has >>>>>> been >>>>>>>>>>>>> existed >>>>>>>>>>>>> for years (maybe after "cleanup.policy" was introduced?), there >>>>>>> could >>>>>>>>> be >>>>>>>>>>>>> users relying on the empty cleanup.policy for a long time. It >>> is >>>>>> not >>>>>>>>> good >>>>>>>>>>>>> to break the backward compatibility in a minor release version >>>>>> (ex: >>>>>>>>>>>>> v4.1.0). Even though we think we are fixing a long existing >>> bug, >>>>>> it >>>>>>>>> could >>>>>>>>>>>>> be a surprise to users. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>> We could also consider supporting an empty cleanup.policy by >>>>>> fixing >>>>>>>>> the >>>>>>>>>>>>> issue in remote storage. But then the user may never realize >>> this >>>>>> if >>>>>>>>> it's >>>>>>>>>>>>> a >>>>>>>>>>>>> mistake. >>>>>>>>>>>>> >>>>>>>>>>>>> Good point! I agree we need to fix it! >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Hi Chia-Ping, >>>>>>>>>>>>> >>>>>>>>>>>>>> We can print warnings for empty cleanup.policy in 4.x and in >>> 5.0 >>>>>> we >>>>>>>>>>>>> throw >>>>>>>>>>>>> exception directly to make it fail fast >>>>>>>>>>>>> >>>>>>>>>>>>> This suggestion looks good to me! >>>>>>>>>>>>> >>>>>>>>>>>>> Thank you. >>>>>>>>>>>>> Luke >>>>>>>>>>>>> >>>>>>>>>>>>> On Tue, May 13, 2025 at 2:14 PM Chia-Ping Tsai < >>>>>> chia7...@apache.org >>>>>>>> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> hi all, >>>>>>>>>>>>>> >>>>>>>>>>>>>> Given Luke mentioned the existence of use cases, it is worth >>>>>>>>> considering >>>>>>>>>>>>>> the compatibility issue. In fact, I had previously encountered >>>>>> this >>>>>>>>> use >>>>>>>>>>>>>> case but advised customers to utilize retention configurations >>>>>>>>> instead. >>>>>>>>>>>>>> >>>>>>>>>>>>>>> We could also consider supporting an empty cleanup.policy by >>>>>>> fixing >>>>>>>>>>>>> the >>>>>>>>>>>>>>> issue in remote storage. But then the user may never realize >>>>>> this >>>>>>> if >>>>>>>>>>>>>> it's a >>>>>>>>>>>>>>> mistake. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Good catch! The "empty" or "none" wouldn't make sense for >>> remote >>>>>>>>>>>>> storage. >>>>>>>>>>>>>> I've opened KAFKA-19273 to ensure topics with tiered storage >>> use >>>>>> a >>>>>>>>> valid >>>>>>>>>>>>>> delete policy. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Best, >>>>>>>>>>>>>> Chia-Ping >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> On 2025/05/12 19:06:10 Jun Rao wrote: >>>>>>>>>>>>>>> Hi, Luke, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Regarding the motivation, currently, we never documented that >>>> an >>>>>>>>> empty >>>>>>>>>>>>>>> cleanup.policy implies infinite retention. In fact, if one >>>>>> leaves >>>>>>>>>>>>>>> cleanup.policy empty, it actually breaks remote storage since >>>> it >>>>>>>>> will >>>>>>>>>>>>>> stop >>>>>>>>>>>>>>> the cleaning based on local retention size and time too. So, >>>>>>> leaving >>>>>>>>>>>>>>> cleanup.policy empty is probably more like a user mistake >>> than >>>>>> an >>>>>>>>>>>>>>> intentional choice. Based on this assumption, the idea behind >>>>>> the >>>>>>>>> KIP >>>>>>>>>>>>> is >>>>>>>>>>>>>> to >>>>>>>>>>>>>>> fail an empty cleanup.policy so that the user is aware of >>> this >>>>>>>>> likely >>>>>>>>>>>>>>> mistake and provide a new option None for users wanting >>>> infinite >>>>>>>>>>>>>> retention >>>>>>>>>>>>>>> to opt in explicitly. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> We could also consider supporting an empty cleanup.policy by >>>>>>> fixing >>>>>>>>>>>>> the >>>>>>>>>>>>>>> issue in remote storage. But then the user may never realize >>>>>> this >>>>>>> if >>>>>>>>>>>>>> it's a >>>>>>>>>>>>>>> mistake. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Sun, May 11, 2025 at 7:53 PM Luke Chen <show...@gmail.com >>>> >>>>>>>>> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Hi Jiunn-Yang, >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Thanks for the KIP. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Comments: >>>>>>>>>>>>>>>> 1. "it is acceptable because an empty cleanup.policy is >>>>>>> considered >>>>>>>>>>>>>> invalid >>>>>>>>>>>>>>>> in Kafka. Therefore, this trade-off is justified." >>>>>>>>>>>>>>>> Could you explain more about this? Why is this trade-off >>>>>>> justified? >>>>>>>>>>>>>>>> If we think the empty cleanup.policy is invalid in kafka, >>> why >>>>>> do >>>>>>> we >>>>>>>>>>>>>> provide >>>>>>>>>>>>>>>> an additional "none" option to users in this KIP? >>>>>>>>>>>>>>>> FYI, there are indeed use cases that need to keep all >>> history >>>>>>> data >>>>>>>>>>>>>> without >>>>>>>>>>>>>>>> a cleanup policy. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> 2. Following (1), I agree we should fail fast for empty >>>>>>>>>>>>>>>> "group.coordinator.rebalance.protocols" and "process.roles" >>>>>>> configs >>>>>>>>>>>>>> since >>>>>>>>>>>>>>>> they are invalid. >>>>>>>>>>>>>>>> But about "cleanup.policy", I don't see a persuasive reason >>>> why >>>>>>> we >>>>>>>>>>>>>> should >>>>>>>>>>>>>>>> break backward compatibility to change it. >>>>>>>>>>>>>>>> "This provides a clear and direct way to configure Kafka to >>>>>>> retain >>>>>>>>>>>>> all >>>>>>>>>>>>>> log >>>>>>>>>>>>>>>> segments without any automatic cleanup." >>>>>>>>>>>>>>>> This is the motivation we mentioned in the KIP, but to me, >>>>>>> backward >>>>>>>>>>>>>>>> compatibility is much more important than "a clear and >>> direct >>>>>> way >>>>>>>>> to >>>>>>>>>>>>>> config >>>>>>>>>>>>>>>> kafka". >>>>>>>>>>>>>>>> Do we really have to change the "cleanup.policy" config? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Thanks. >>>>>>>>>>>>>>>> Luke >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Wed, May 7, 2025 at 2:17 AM Jun Rao >>>>>> <j...@confluent.io.invalid >>>>>>>> >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Thanks for the updated KIP. It looks good to me. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On Tue, May 6, 2025 at 4:58 AM 黃竣陽 <s7133...@gmail.com> >>>>>> wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Hi Jun, chia >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Thanks for all the comments. They have all been addressed >>> in >>>>>>> the >>>>>>>>>>>>>>>> updated >>>>>>>>>>>>>>>>>> KIP. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> I've removed all deprecated-related sections. >>> Additionally, >>>>>> an >>>>>>>>>>>>>>>> exception >>>>>>>>>>>>>>>>>> will now be thrown if a >>>>>>>>>>>>>>>>>> developer passes an empty validStrings list to the >>>> inNonEmpty >>>>>>>>>>>>>> method. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Best Regards, >>>>>>>>>>>>>>>>>> Jiunn-Yang >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Chia-Ping Tsai <chia7...@gmail.com> 於 2025年5月6日 凌晨12:46 >>>> 寫道： >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> hi Jiunn-Yang >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> The `inNonEmpty` is used to prevent users pass empty >>>> config, >>>>>>>>>>>>> so >>>>>>>>>>>>>>>> should >>>>>>>>>>>>>>>>> it >>>>>>>>>>>>>>>>>>> be non-empty too? >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> For example, `inNonEmpty()` should throw exception >>>> directly. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Best, >>>>>>>>>>>>>>>>>>> Chia-Ping >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Jun Rao <j...@confluent.io.invalid> 於 2025年5月6日 週二 >>>>>> 上午12:28寫道： >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> Thanks for the reply. There are still references to the >>>>>>>>>>>>>> deprecated >>>>>>>>>>>>>>>>>> method. >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> "stop relying on the deprecated methods" >>>>>>>>>>>>>>>>>>>> "Finally, the deprecated method will be removed in Kafka >>>>>> 5.0" >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> On Sat, May 3, 2025 at 7:42 AM 黃竣陽 <s7133...@gmail.com> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Hi Jun, chia, >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Thanks for all the comments. They have all been >>> addressed >>>>>> in >>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>> updated >>>>>>>>>>>>>>>>>>>>> KIP. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Best Regards, >>>>>>>>>>>>>>>>>>>>> Jiunn-Yang >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Chia-Ping Tsai <chia7...@gmail.com> 於 2025年5月3日 >>> 凌晨2:03 >>>>>> 寫道： >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> hi Jiunn-Yang >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> in the "Compatibility, Deprecation, and Migration >>> Plan", >>>>>>>>>>>>>> could you >>>>>>>>>>>>>>>>> add >>>>>>>>>>>>>>>>>>>>>> description to remind readers that "clean.policy=" >>>> should >>>>>>>>>>>>> be >>>>>>>>>>>>>>>>> replaced >>>>>>>>>>>>>>>>>>>> by >>>>>>>>>>>>>>>>>>>>>> "clean.policy=none" if they do want to disable delete >>>> and >>>>>>>>>>>>>> compact. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Best, >>>>>>>>>>>>>>>>>>>>>> Chia-Ping >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Jun Rao <j...@confluent.io.invalid> 於 2025年5月3日 週六 >>>>>>>>>>>>> 上午1:55寫道： >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> It's fine not to deprecate ValidList#in for now. >>> Could >>>>>> you >>>>>>>>>>>>>> update >>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>> KIP >>>>>>>>>>>>>>>>>>>>>>> completely? There are still references to >>> deprecation. >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> On Fri, May 2, 2025 at 4:59 AM 黃竣陽 < >>> s7133...@gmail.com >>>>> >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Hi Jun, >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> I don’t think we should deprecate the ValidList#in >>>>>>>>>>>>> method, >>>>>>>>>>>>>> as >>>>>>>>>>>>>>>>> there >>>>>>>>>>>>>>>>>>>> may >>>>>>>>>>>>>>>>>>>>>>> be >>>>>>>>>>>>>>>>>>>>>>>> future scenarios where we want >>>>>>>>>>>>>>>>>>>>>>>> to allow list configs to be empty. It’s useful to >>> have >>>>>>>>>>>>> both >>>>>>>>>>>>>>>>> methods: >>>>>>>>>>>>>>>>>>>> in >>>>>>>>>>>>>>>>>>>>>>>> (which allows empty lists) >>>>>>>>>>>>>>>>>>>>>>>> and inNonEmpty (which enforces non-empty lists). >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Just a minor comment. It would be useful to >>> document >>>>>>>>>>>>> that >>>>>>>>>>>>>>>> during >>>>>>>>>>>>>>>>>>>>>>> upgrade, >>>>>>>>>>>>>>>>>>>>>>>>> if cleanup.policy is empty, the broker will fail to >>>>>>>>>>>>> start. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> I’ve updated the KIP accordingly. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Best Regards, >>>>>>>>>>>>>>>>>>>>>>>> Jiunn-Yang >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Jun Rao <j...@confluent.io.INVALID> 於 2025年5月2日 >>>>>> 凌晨12:50 >>>>>>>>>>>>> 寫道： >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the reply. Sounds good. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Just a minor comment. It would be useful to >>> document >>>>>>>>>>>>> that >>>>>>>>>>>>>>>> during >>>>>>>>>>>>>>>>>>>>>>> upgrade, >>>>>>>>>>>>>>>>>>>>>>>>> if cleanup.policy is empty, the broker will fail to >>>>>>>>>>>>> start. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> On Thu, May 1, 2025 at 8:40 AM 黃竣陽 < >>>>>> s7133...@gmail.com> >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Hello Jun, >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Since ValidList is a public API, we need to >>> maintain >>>>>>>>>>>>>> backward >>>>>>>>>>>>>>>>>>>>>>>>>> compatibility. Therefore, the isEmptyAllowed flag >>> is >>>>>>>>>>>>>>>> necessary. >>>>>>>>>>>>>>>>>>>>>>>>>> We can update the signature of isNonEmpty() to >>>> remove >>>>>>>>>>>>> the >>>>>>>>>>>>>>>>> boolean >>>>>>>>>>>>>>>>>>>>>>>>>> parameter. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Best Regards, >>>>>>>>>>>>>>>>>>>>>>>>>> Jiunn-Yang >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> Jun Rao <j...@confluent.io.INVALID> 於 2025年5月1日 >>>>>> 凌晨4:17 >>>>>>>>>>>>>> 寫道： >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the reply. >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> Do we really need isEmptyAllowed? It's awkward >>>> since >>>>>>>>>>>>> the >>>>>>>>>>>>>>>> method >>>>>>>>>>>>>>>>>>>> name >>>>>>>>>>>>>>>>>>>>>>> is >>>>>>>>>>>>>>>>>>>>>>>>>>> inNonEmpty. Also, it's not clear when it's set to >>>>>>>>>>>>> true. >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> On Fri, Apr 25, 2025 at 6:26 AM 黃竣陽 < >>>>>>>>>>>>> s7133...@gmail.com> >>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Jun, >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> Thank you for pointing that out. >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> I have revised the KIP as follows: >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> “In this case, the change does not introduce new >>>>>>>>>>>>> invalid >>>>>>>>>>>>>>>>>>>>>>>> configurations >>>>>>>>>>>>>>>>>>>>>>>>>> or >>>>>>>>>>>>>>>>>>>>>>>>>>>> prevent any currently valid setup. >>>>>>>>>>>>>>>>>>>>>>>>>>>> The main behavioral difference is that we now >>>>>>>>>>>>> explicitly >>>>>>>>>>>>>>>>> throw a >>>>>>>>>>>>>>>>>>>>>>>>>>>> ConfigException during the storage format >>>>>> validation >>>>>>>>>>>>>> phase >>>>>>>>>>>>>>>>>>>>>>>>>>>> instead of relying on the current behavior.” >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> This reflects the correct behavior. >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> Best Regards, >>>>>>>>>>>>>>>>>>>>>>>>>>>> Jiunn-Yang >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jun Rao <j...@confluent.io.INVALID> 於 >>> 2025年4月25日 >>>>>>>>>>>>>> 凌晨1:17 寫道： >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> "The main behavioral difference introduced by >>>> this >>>>>>>>>>>>>> change >>>>>>>>>>>>>>>> is >>>>>>>>>>>>>>>>>>>> that >>>>>>>>>>>>>>>>>>>>> a >>>>>>>>>>>>>>>>>>>>>>>>>>>>> ConfigException will now be thrown during the >>>>>>>>>>>>> storage >>>>>>>>>>>>>>>> format >>>>>>>>>>>>>>>>>>>>>>>>>> validation >>>>>>>>>>>>>>>>>>>>>>>>>>>>> phase, rather than during server startup." >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> It seems that currently formating the storage >>>>>>>>>>>>> already >>>>>>>>>>>>>> fails >>>>>>>>>>>>>>>>> if >>>>>>>>>>>>>>>>>>>>>>>>>>>>> group.coordinator.rebalance.protocols and >>>>>>>>>>>>> process.roles >>>>>>>>>>>>>>>> are >>>>>>>>>>>>>>>>>>>>> empty. >>>>>>>>>>>>>>>>>>>>>>>> It >>>>>>>>>>>>>>>>>>>>>>>>>>>> just >>>>>>>>>>>>>>>>>>>>>>>>>>>>> gets a different exception. >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Apr 24, 2025 at 5:32 AM 黃竣陽 < >>>>>>>>>>>>>> s7133...@gmail.com> >>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Jun, chia, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for your feedback. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I add a new section for this change: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> - Validation for >>>>>>>>>>>>>> group.coordinator.rebalance.protocols and >>>>>>>>>>>>>>>>>>>>>>>>>> process.roles >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> will be >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> moved from the server startup phase to the >>>>>> storage >>>>>>>>>>>>>> format >>>>>>>>>>>>>>>>>>>> phase. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> - For cleanup.policy, setting an empty list >>> will >>>>>>>>>>>>> now >>>>>>>>>>>>>> be >>>>>>>>>>>>>>>>>>>>> considered >>>>>>>>>>>>>>>>>>>>>>>>>>>> invalid >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> and will >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> result in a ConfigException during storage >>>> format >>>>>>>>>>>>>> phase. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Best Regards, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jiunn-Yang >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jun Rao <j...@confluent.io.INVALID> 於 >>>> 2025年4月24日 >>>>>>>>>>>>>> 凌晨4:44 >>>>>>>>>>>>>>>> 寫道： >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the reply. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Q2. It's true that >>>>>>>>>>>>>> group.coordinator.rebalance.protocols >>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> process.roles >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> configurations can't be empty right now. With >>>>>> this >>>>>>>>>>>>>> KIP, >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>> user >>>>>>>>>>>>>>>>>>>>>>>> will >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> probably get a different (and more accurate) >>>>>>>>>>>>>> exception. >>>>>>>>>>>>>>>> It >>>>>>>>>>>>>>>>>>>> will >>>>>>>>>>>>>>>>>>>>>>> be >>>>>>>>>>>>>>>>>>>>>>>>>>>> useful >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to document that. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Regarding cleanup.policy, I kind of agree >>> with >>>>>>>>>>>>>> Chia-Ping. >>>>>>>>>>>>>>>>> If >>>>>>>>>>>>>>>>>> a >>>>>>>>>>>>>>>>>>>>>>> user >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> leaves >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> cleanup.policy empty, it's more likely to be >>> a >>>>>>>>>>>>>> mistake >>>>>>>>>>>>>>>> than >>>>>>>>>>>>>>>>>> an >>>>>>>>>>>>>>>>>>>>>>>>>>>> intention. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> If we automatically treat it as None, the >>> user >>>>>>>>>>>>> will >>>>>>>>>>>>>> never >>>>>>>>>>>>>>>>>>>>> realize >>>>>>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mistake. Throwing an exception will let the >>>> user >>>>>>>>>>>>>> realize >>>>>>>>>>>>>>>>>> there >>>>>>>>>>>>>>>>>>>>>>> is a >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mistake. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Apr 23, 2025 at 8:26 AM Chia-Ping >>> Tsai >>>> < >>>>>>>>>>>>>>>>>>>>>>> chia7...@gmail.com >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> hi Jiunn-Yang, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the KIP, and I understand that >>>>>>>>>>>>>> maintaining >>>>>>>>>>>>>>>>>>>>>>>> compatibility >>>>>>>>>>>>>>>>>>>>>>>>>> is >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> important. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> However, according to the documentation, we >>>>>> don't >>>>>>>>>>>>>> allow >>>>>>>>>>>>>>>> an >>>>>>>>>>>>>>>>>>>>> empty >>>>>>>>>>>>>>>>>>>>>>>>>> value >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> for >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the cleanup.policy. Hence, should we >>> consider >>>>>>>>>>>>>> throwing >>>>>>>>>>>>>>>> an >>>>>>>>>>>>>>>>>>>>>>>> exception >>>>>>>>>>>>>>>>>>>>>>>>>>>> for >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> an >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> empty value in version 4.x? This could >>>>>> streamline >>>>>>>>>>>>>> the >>>>>>>>>>>>>>>> code >>>>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>>> avoid >>>>>>>>>>>>>>>>>>>>>>>>>>>> an >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> extra deprecation cycle. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Best, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Chia-Ping >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jun Rao <j...@confluent.io.invalid> 於 >>>>>> 2025年4月23日 >>>>>>>>>>>>> 週三 >>>>>>>>>>>>>>>>>> 上午6:33寫道： >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Regarding "The none policy will not delete >>> or >>>>>>>>>>>>>> compact >>>>>>>>>>>>>>>> any >>>>>>>>>>>>>>>>>>>>>>>>>> segments", >>>>>>>>>>>>>>>>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> should be more accurate. We won't delete >>>>>>>>>>>>> segments >>>>>>>>>>>>>> based >>>>>>>>>>>>>>>>> on >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> log.retention.bytes/log.retention.ms, but >>> we >>>>>>>>>>>>>> should >>>>>>>>>>>>>>>>>>>> continue >>>>>>>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>>>>>>> delete >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> segments based on >>> log.local.retention.bytes/ >>>>>>>>>>>>>>>>>>>> log.retention.ms. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Otherwise, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> risk running out of local disk space when >>>>>> remote >>>>>>>>>>>>>>>> storage >>>>>>>>>>>>>>>>> is >>>>>>>>>>>>>>>>>>>>>>>>>> enabled. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, Apr 22, 2025 at 9:45 AM Jun Rao < >>>>>>>>>>>>>>>>> j...@confluent.io> >>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the reply. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Q2. What about existing empty values for >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> group.coordinator.rebalance.protocols and >>>>>>>>>>>>>>>> process.roles >>>>>>>>>>>>>>>>>>>>> during >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> upgrade? >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, Apr 22, 2025 at 7:29 AM 黃竣陽 < >>>>>>>>>>>>>>>> s7133...@gmail.com >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hello Jun, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for review this KIP. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Q1 & Q3: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I’ve updated the method name accordingly >>>> and >>>>>>>>>>>>>> revised >>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> cleanup.policy >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> documentation >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to clarify that the none policy cannot be >>>>>> used >>>>>>>>>>>>>> with >>>>>>>>>>>>>>>> any >>>>>>>>>>>>>>>>>>>>> other >>>>>>>>>>>>>>>>>>>>>>>>>>>> policy. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Q2: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> For users currently using an empty >>>>>>>>>>>>>> cleanup.policy, >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>> approach >>>>>>>>>>>>>>>>>>>>>>>>>> is >>>>>>>>>>>>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> apply the none policy >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> during the preProcessParsedConfig step. >>>>>>>>>>>>>>>> Additionally, a >>>>>>>>>>>>>>>>>>>>>>> warning >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> message >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> will be emitted to inform users >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> of the upcoming change. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Best Regards, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jiunn-Yang >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jun Rao <j...@confluent.io.INVALID> 於 >>>>>>>>>>>>> 2025年4月22日 >>>>>>>>>>>>>>>>> 凌晨4:52 >>>>>>>>>>>>>>>>>>>> 寫道： >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, Jiunn-Yang, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the KIP. A few comments. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 1. It's fine to introduce a new value >>> None >>>>>>>>>>>>> for >>>>>>>>>>>>>>>>>>>>>>> cleanup.policy. >>>>>>>>>>>>>>>>>>>>>>>>>> But >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> now >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> not >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> all value combinations are valid. For >>>>>>>>>>>>> example, >>>>>>>>>>>>>> None >>>>>>>>>>>>>>>>>> can't >>>>>>>>>>>>>>>>>>>>> be >>>>>>>>>>>>>>>>>>>>>>>>>> used >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> with >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Delete or Compact. It would be useful to >>>>>>>>>>>>>> document >>>>>>>>>>>>>>>>> that. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2. What's the behavior during upgrade >>> when >>>>>> an >>>>>>>>>>>>>>>> existing >>>>>>>>>>>>>>>>>>>>>>> config >>>>>>>>>>>>>>>>>>>>>>>>>> has >>>>>>>>>>>>>>>>>>>>>>>>>>>> an >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> empty >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> list. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 3. inWithEmptyCheck: It's not clear what >>>>>> the >>>>>>>>>>>>>> empty >>>>>>>>>>>>>>>>> check >>>>>>>>>>>>>>>>>>>>>>> does. >>>>>>>>>>>>>>>>>>>>>>>>>> How >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> about >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sth like inNonEmpty ? >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jun >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, Apr 15, 2025 at 8:25 AM 黃竣陽 < >>>>>>>>>>>>>>>>> s7133...@gmail.com >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hello everyone, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I would like to start a discussion on >>>>>>>>>>>>> KIP-1161: >>>>>>>>>>>>>>>>>>>>>>>> cleanup.policy >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> shouldn't >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> be empty >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> < >>>>>>>>>>>>> https://cwiki.apache.org/confluence/x/HArXF> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> This proposal aims to improve the >>>>>>>>>>>>>> cleanup.policy >>>>>>>>>>>>>>>>>>>>>>>> configuration. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Currently, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> this setting should not be left empty. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Therefore, there are two proposed >>>>>>>>>>>>> improvements: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 1. Update ValidList to validate whether >>>> an >>>>>>>>>>>>>> empty >>>>>>>>>>>>>>>> list >>>>>>>>>>>>>>>>>> is >>>>>>>>>>>>>>>>>>>>>>>>>> allowed. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2. Introduce a new 'none' value for >>>>>>>>>>>>>> cleanup.policy. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Best Regards, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jiunn-Yang >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>> >>>> >>>> >>> >>
