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