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 > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>> > > >>>>>>>> > > >>>>>> > > >>>>> > > >>>>> > > >>> > > >>> > > >> > > > > > > >