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