Hello Jun Chia, Thanks for the reply,
JR20: This configuration should remain unchanged. I have updated the KIP. JR21: For both Consumer and Producer, the default value is an empty list, and the validator used is NonNullValidator. I think we can change the validator to ValidList.anyNonDuplicateValues with isNullAllowed = false and isEmptyAllowed = false. JR23: I’ve addressed this change in the KIP. Although the default value of early.start.listeners remains unchanged, the validator now disallows empty lists. JR24: Since log.dirs does not change its default value, I used a dash (-) to indicate that the field remains unchanged. JR25: I’ve updated the table and aligned similar validators as much as possible for consistency. Best Regards, Jiunn-Yang > Chia-Ping Tsai <chia7...@gmail.com> 於 2025年7月15日 下午2:12 寫道： > >> 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 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>> >>> >>> >>
