Hi Chia, Jun Thanks for the reply, I have updated the table in the KIP.
Best Regards, Jiunn-Yang > Chia-Ping Tsai <chia7...@gmail.com> 於 2025年7月11日 清晨7:38 寫道： > > Hi, Jun > > Thanks for the reply. > > I think the following configs could use null as the default value and treat > an "empty list" as an illegal value. > > `sasl.oauthbearer.expected.audience`, `ssl.cipher.suites`, > `ssl.enabled.protocols`, > and `log.dirs` > > Jun Rao <j...@confluent.io.invalid> 於 2025年7月11日 週五 上午6:59寫道： > >> Hi, Chia-Ping, Jiunn-Yang, >> >> Thanks for the reply. >> >> As you said, if ssl.cipher.suites is empty, intuitively this means that no >> cipher is allowed. So, this should be an illegal config. It just happens >> that our implementation treats it as if the config is not set (meaning the >> value is null). So, it seems that a more intuitive convention is: if the >> config is not set, it defaults to all available cipher suites. If it's >> explicitly set, it can't be empty. >> >> Jun >> >> >> >> On Wed, Jul 9, 2025 at 8:27 AM 黃竣陽 <s7133...@gmail.com> wrote: >> >>> 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 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>> >>> >>> >>
