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