Hi, Jiunn-Yang, Thanks for the reply. The changes look good to me and we can follow up on the 0.0.0.0 issue separately in KIP-1202.
Jun On Thu, Aug 21, 2025 at 6:55 AM 黃竣陽 <s7133...@gmail.com> wrote: > Hello Jun, chia, > > > (By the way, the table in KIP-1202 has an incorrect value — null is > acceptable for both cases.) > > KIP-1202 covers the scenario where the listeners configuration is set to > the special > value PLAINTEXT://0.0.0.0:9092. In this case, if advertised.listeners is > configured as null, > an IllegalArgumentException will be thrown: requirement failed: > advertised.listeners cannot > use the non-routable meta-address 0.0.0.0. Use a routable IP address. > > > If we want to address `advertised.listeners` in this KIP, it would be > better to also have the controller reject an `empty` value > > Yes, I fully agree that we should disallow an empty list for this > configuration, since specifying an > empty list is odd, it would mean that the node does not advertise any > listener addresses, > making it unreachable by clients. I will updated KIP according by this. > > Best Regards, > Jiunn-Yang > > > Chia-Ping Tsai <chia7...@apache.org> 於 2025年8月21日 晚上8:48 寫道： > > > > 2. It seems that advertised.listeners should default to null and can't be > > > > That is an inconsistency between the broker and controller, and it will > be addressed by KIP-1202. `null` is valid for both, while `empty` is valid > only for the controller. As Jun mentioned, the broker encounters an error > in this case. > > (By the way, the table in KIP-1202 has an incorrect value — null is > acceptable for both cases.) > > > > If we want to address `advertised.listeners` in this KIP, it would be > better to also have the controller reject an `empty` value > > > > Best, > > Chia-Ping > > > > On 2025/08/20 18:01:35 Jun Rao wrote: > >> Hi, Jiunn-Yang, > >> > >> Thanks for the update. > >> > >> 1. Sounds good. > >> > >> 2. It seems that advertised.listeners should default to null and can't > be > >> empty. Currently, if it's set to empty, it fails with the following. > >> > >> java.lang.IllegalArgumentException: requirement failed: There must be at > >> least one broker advertised listener. Perhaps all listeners appear in > >> controller.listener.names? > >> at scala.Predef$.require(Predef.scala:337) > ~[scala-library-2.13.16.jar:?] > >> at > >> > kafka.server.KafkaConfig.validateAdvertisedBrokerListenersNonEmptyForBroker$1(KafkaConfig.scala:545) > >> ~[kafka_2.13-4.2.0-SNAPSHOT.jar:?] > >> > >> Jun > >> > >> On Wed, Aug 20, 2025 at 4:58 AM 黃竣陽 <s7133...@gmail.com> wrote: > >> > >>> Hello Jun, > >>> > >>> I have updated the KIP with the following changes. > >>> > >>> 1. The type of controller.listener.names should be changed from string > to > >>> list. > >>> Its default value should be updated from null to NO_DEFAULT_VALUE, and > its > >>> validator should be updated to anyNonDuplicateValues(isNullAllowed = > >>> false, isEmptyAllowed = false). > >>> > >>> 2. The type of advertised.listeners should be changed from string to > list. > >>> As for its validator, > >>> I think we can continue the discussion in KIP-1202. > >>> > >>> Best Regards, > >>> Jiunn-Yang > >>> > >>>> Jun Rao <j...@confluent.io.INVALID> 於 2025年8月14日 凌晨12:37 寫道： > >>>> > >>>> Hi, Jiunn-Yang, > >>>> > >>>> Thanks for the updated KIP. Looks good to me. > >>>> > >>>> Jun > >>>> > >>>> On Wed, Aug 13, 2025 at 3:08 AM 黃竣陽 <s7133...@gmail.com> wrote: > >>>> > >>>>> Hello Jun, > >>>>> > >>>>> Thanks for the reply. > >>>>> > >>>>> I have updated the KIP according there comments. > >>>>> > >>>>> Best Regards, > >>>>> Jiunn-Yang > >>>>> > >>>>>> Jun Rao <j...@confluent.io.INVALID> 於 2025年8月13日 凌晨1:57 寫道： > >>>>>> > >>>>>> Hi, Jiunn-Yang, > >>>>>> > >>>>>> Thanks for the reply. A few more comments. > >>>>>> > >>>>>> JR50. It seems that you changed the default value for > config.providers > >>>>>> incorrectly. The change is meant for bootstrap.servers. > >>>>>> > >>>>>> JR51. Could you document the current behavior if bootstrap.servers > is > >>>>> empty > >>>>>> in ConsumerConfig, WorkerConfig, ProducerConfig and StreamsConfig? > >>>>>> > >>>>>> JR52. Could you document the justification for changing the default > >>> value > >>>>>> for bootstrap.servers in WorkerConfig? > >>>>>> > >>>>>> JR53. It seems that WorkerConfig is not public facing. Only classes > in > >>>>>> connect api are public. So, there is no need to document the > >>> deprecation > >>>>>> of BOOTSTRAP_SERVERS_DEFAULT in WorkerConfig. > >>>>>> > >>>>>> Jun > >>>>>> > >>>>>> > >>>>>> On Tue, Aug 12, 2025 at 8:14 AM 黃竣陽 <s7133...@gmail.com> wrote: > >>>>>> > >>>>>>> Hi chia, > >>>>>>> > >>>>>>> I have updated the KIP with these changes. > >>>>>>> > >>>>>>> Best Regards, > >>>>>>> Jiunn-Yang > >>>>>>> > >>>>>>>> Chia-Ping Tsai <chia7...@gmail.com> 於 2025年8月12日 晚上9:54 寫道： > >>>>>>>> > >>>>>>>> hi Jiunn > >>>>>>>> > >>>>>>>>> 黃竣陽 <s7133...@gmail.com> 於 2025年8月12日 下午6:18 寫道： > >>>>>>>>> > >>>>>>>>> 3. WorkerConfig – Change the default value of bootstrap.servers > from > >>>>>>> "localhost:9092" to NO_DEFAULT_VALUE > >>>>>>>>> and deprecate the constant BOOTSTRAP_SERVERS_DEFAULT. > >>>>>>>> > >>>>>>>> > >>>>>>>> I agree that the default value of “localhost:9092” is strange. > >>> However, > >>>>>>> it is still a breaking change, so please highlight this change in > the > >>>>> KIP. > >>>>>>>> > >>>>>>>> Best, > >>>>>>>> Chia-Ping > >>>>>>> > >>>>>>> > >>>>> > >>>>> > >>> > >>> > >> > >
