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