Hello Jun, Sorry for the late reply,
JR40, JR41: I have updated the KIP based on your comments. Please take a look. Best Regards, Jiunn-Yang > Jun Rao <j...@confluent.io.INVALID> 於 2025年7月24日 凌晨2:56 寫道： > > Hi, Jiunn-Yang, > > Thanks for the reply. > > JR40. In the table, there are a few configs for which we now disallow empty > lists. It would be useful to document the current behavior for each of them > more accurately. It seems that they should all lead to some bad behavior. > For example, if log.dirs is empty, it seems that we can't create topics? > If group.coordinator.rebalance.protocols is empty, the group rebalance > protocol will fail? > > JR41. It would be useful to clean up the "Compatibility, Deprecation, and > Migration Plan" section a bit. > JR41.1 The migration plan has "Introduce a new properties isEmptyAllowed , > isNullAllowed", which needs to be changed. > JR41.2 It started with "For users who configure an empty list", but not all > the bullet points are about empty lists. > JR41.3 To me, it would be useful to summarize why the behavior changes are > justified. We made 3 types of changes. (1) Disallow null. This is ok since > one can't really set a null value through a config file. (2) Disallow > duplicates. This is ok since it's rare and confusing. (3) Disallow empty. > Ideally, we want to claim that for those cases, they all lead to some other > errors or had behavior currently. > > Jun > > On Wed, Jul 23, 2025 at 4:42 AM 黃竣陽 <s7133...@gmail.com> wrote: > >> Hello Jun, >> >> Thanks for the reply. >> >> JR34 ,JR36, JR39: I’ve updated the KIP to reflect these changes. >> >> JR38: I clarified the following configurations: >> early.start.listeners: This config is optional and allows both null and >> empty lists. >> log.dirs and plugin.path: These are also optional, but only allow >> null—empty lists are not permitted. >> >> Best Regards, >> Jiunn-Yang >> >>> Jun Rao <j...@confluent.io.INVALID> 於 2025年7月23日 凌晨2:28 寫道： >>> >>> Hi, Jiunn-Yang and Chia-Ping, >>> >>> Thanks for the reply. >>> >>> JR34. Sounds good. >>> >>> JR36. Could you document that the default value for >>> sasl.oauthbearer.expected.audience and ssl.cipher.suites have been >> changed? >>> >>> JR38. My concern wasn't quite addressed. "If the configuration is >> optional, >>> we will reject any duplicate values in the list." This implies that if a >>> config is optional, we allow empty lists. However, for plugin.path, we >>> reject empty lists. >>> >>> JR39. We can remove the fields isEmptyAllowed and isNullAllowed since >> they >>> are not public facing. >>> >>> Jun >>> >>> >>> On Tue, Jul 22, 2025 at 4:26 AM 黃竣陽 <s7133...@gmail.com> wrote: >>> >>>> Hello Jun, Chia, >>>> >>>> Thanks for the reply, >>>> >>>> JR36: I’ve updated the following configurations: >>>> - sasl.oauthbearer.expected.audience >>>> - ssl.cipher.suites >>>> - ssl.enabled.protocols >>>> These now allow both empty and non-empty lists. Since the default values >>>> of >>>> sasl.oauthbearer.expected.audience and ssl.cipher.suites were previously >>>> null, >>>> I’ve updated their defaults to List.of() for consistency. >>>> >>>> JR37: A ClassCastException does not occur in MirrorClientConfig and it >>>> should be none. >>>> I’ve updated this in the KIP. >>>> >>>> JR38 & JR40: I’ve added a note below the table to indicate which >>>> configurations require special attention. >>>> >>>> JR39: Yes, these properties are package-private. >>>> >>>> Best Regards, >>>> Jiunn-Yang >>>> >>>>> Chia-Ping Tsai <chia7...@gmail.com> 於 2025年7月22日 下午6:04 寫道： >>>>> >>>>> JR34. Hmm, it seems that the code in taskConfigs() could return an >> empty >>>>> list for a task if knownSourceTopicPartitions is less than maxTasks, >>>>> Chia-Ping? >>>>> >>>>> >>>>> Yes, `MirrorSourceConnector#taskConfigs` could return an empty list, >>>> which >>>>> means no `MirrorSourceTask` can be created. >>>>> >>>>> By contrast, if any `MirrorSourceTask` is created, the configs used by >>>>> `MirrorSourceTask` must contain `task.assigned.partitions` with a >>>> non-empty >>>>> value. >>>>> >>>>> Jun Rao <j...@confluent.io.invalid> 於 2025年7月22日 週二 上午5:14寫道： >>>>> >>>>>> Hi, Jiunn-Yang and Chia-Ping, >>>>>> >>>>>> Thanks for the reply. >>>>>> >>>>>> JR34. Hmm, it seems that the code in taskConfigs() could return an >> empty >>>>>> list for a task if knownSourceTopicPartitions is less than maxTasks, >>>>>> Chia-Ping? >>>>>> >>>>>> JR36. Sounds good. Could you update the KIP? Also, should we do the >> same >>>>>> for ssl.cipher.suites and sasl.oauthbearer.expected.audience? >>>>>> >>>>>> JR37. Is it true that MirrorClientConfig.bootstrap.servers returns >>>>>> ClassCastException for null? It seems that null could be casted to any >>>>>> class. >>>>>> >>>>>> JR38. "If the configuration is optional, we will reject any duplicate >>>>>> values in the list." Could you clarify this? For example, plugin.path >> is >>>>>> optional, but with a default value. We reject duplicates as well as >>>> empty >>>>>> lists. >>>>>> >>>>>> JR39. isEmptyAllowed and isNullAllowed are not public fields, right? >>>> They >>>>>> are only public in the anyNonDuplicateValues() method. >>>>>> >>>>>> JR40. It would be useful to mention that if cleanup.policy is empty >> and >>>>>> remote.storage.enable is true, the local log segments will be cleaned >>>> based >>>>>> on log.local.retention.bytes and log.local.retention.ms. >>>>>> >>>>>> Jun >>>>>> >>>>>> On Fri, Jul 18, 2025 at 9:52 AM Chia-Ping Tsai <chia7...@gmail.com> >>>> wrote: >>>>>> >>>>>>>> >>>>>>>> JR34: For these two configurations, setting an empty list feels a >> bit >>>>>>>> unintuitive. If an empty list is >>>>>>>> provided, the consumer will call the unsubscribe method, which >> doesn't >>>>>>>> seem appropriate given >>>>>>>> the documentation states: "Topic-partitions assigned to this task to >>>>>>>> replicate." >>>>>>>> >>>>>>> >>>>>>> agreed. Additionally, the source code [0] shows that it is not >> intended >>>>>> to >>>>>>> generate empty tps for the source task >>>>>>> >>>>>>> [0] >>>>>>> >>>>>>> >>>>>> >>>> >> https://github.com/apache/kafka/blob/9b542b6ea21e84677a9292f250fc25f8b4162e6f/connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorSourceConnector.java#L202 >>>>>>> >>>>>> >>>> >>>> >> >>
