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