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