> JR20. controller.listener.names : It's required on the broker. Is it required on the controller?
there is a small "sugar" in the controller, which we will create listener.security.protocol.map based on controller.listener.names if listener.security.protocol.map is not explicitly set [0] Additionally, `controller.listener.names` is also used to filter out the advertised controller listeners [1] > JR22. group.coordinator.rebalance.protocols: Currently, it can be set to null, right? I don't think so. There is no null check after calling `getList(GroupCoordinatorConfig.GROUP_COORDINATOR_REBALANCE_PROTOCOLS_CONFIG)`[2], which causes an NPE if getList returns null." [0] https://github.com/apache/kafka/blob/trunk/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java#L133 [1] https://github.com/apache/kafka/blob/7ea32a0e938c22119f11908aa419aaf0ffd9b6d8/core/src/main/scala/kafka/server/KafkaConfig.scala#L459 [2] https://github.com/apache/kafka/blob/7ea32a0e938c22119f11908aa419aaf0ffd9b6d8/core/src/main/scala/kafka/server/KafkaConfig.scala#L373 Jun Rao <j...@confluent.io.invalid> 於 2025年7月15日 週二 上午6:20寫道： > Hi, Jiunn-Yang, > > Thanks for the updated KIP. > > JR20. controller.listener.names : It's required on the broker. Is it > required on the controller? > > JR21. bootstrap.servers: Some of them (e.g. in producer/consumer) already > require it to be non-null. > > JR22. group.coordinator.rebalance.protocols: Currently, it can be set to > null, right? > > JR23. early.start.listeners: It allows default value of null, which means > that all listeners included in controller.listener.names will also be early > start listeners. > > JR24. log.dirs: It currently has a default value of null. > > JR25. It seems there are quite a few more configs of type list. Do they > need to be changed too? > > AdminClientConfig: 4 > ConsumerConfig: 5 > ProducerConfig: 4 > SaslConfigs: 1 > SslConfigs: 2 > BrokerSecurityConfigs: 5 > MirrorClientConfig: 1 > org.apache.kafka.connect.mirror : 16 > org.apache.kafka.connect.runtime : 21 > GroupCoordinatorConfig: 3 > QuorumConfig: 2 > ServerConfigs: 1 > KRaftConfigs: 1 > ClientMetricsConfigs: 2 > MetricConfigs: 2 > LogConfig: 4 > StreamsConfig: 4 > > Jun > > On Fri, Jul 11, 2025 at 8:13 PM 黃竣陽 <s7133...@gmail.com> wrote: > > > Hi Chia, Jun > > > > Thanks for the reply, > > I have updated the table in the KIP. > > > > Best Regards, > > Jiunn-Yang > > > > > Chia-Ping Tsai <chia7...@gmail.com> 於 2025年7月11日 清晨7:38 寫道： > > > > > > Hi, Jun > > > > > > Thanks for the reply. > > > > > > I think the following configs could use null as the default value and > > treat > > > an "empty list" as an illegal value. > > > > > > `sasl.oauthbearer.expected.audience`, `ssl.cipher.suites`, > > > `ssl.enabled.protocols`, > > > and `log.dirs` > > > > > > Jun Rao <j...@confluent.io.invalid> 於 2025年7月11日 週五 上午6:59寫道： > > > > > >> 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 > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>> > > >>>>> > > >>> > > >>> > > >>> > > >> > > > > > > >
