hi Jiunn-Yang
chia05:
`task.assigned.groups` and `task.assigned.partitions` should not have
default value, right?
chia06:
`new HashSet<>(values).size()` -> `Set.copyOf(values).size()`
Best,
Chia-Ping
Jun Rao 於 2025年8月5日 週二 下午11:39寫道：
> Hi, Jiunn-Yang,
>
> Thanks for the reply and your patie
Hi, Jiunn-Yang,
Thanks for the reply and your patience. The KIP looks good to me now.
Hi, Luke, Mickael,
Could you take a look at the KIP again and see if it makes sense to you?
Thanks,
Jun
On Tue, Aug 5, 2025 at 3:04 AM 黃竣陽 wrote:
> Hello Jun,
>
> Thanks for the reply,
>
> JR46: I have cla
Hello Jun,
Thanks for the reply,
JR46: I have clarified the behavior for null and empty values, so users can
understand the reasoning behind disallowing empty lists.
JR48: I've updated the KIP accordingly.
Best Regards,
Jiunn-Yang
> Jun Rao 於 2025年8月5日 凌晨12:45 寫道：
>
> Hi, Jiunn-Yang,
>
>
Hi, Jiunn-Yang,
Thanks for the reply.
JR46. "Currently, if plugin.path is set to an empty list, no user-specified
plugin locations are processed, but plugins from the parent classloader are
still loaded." Could you add why the current behavior is bad for the users?
JR48. If we want to preserve t
Hello Jun,
Thanks for the reply.
JR42, JR45, JR46, JR47: I have updated the KIP.
JR43: If bootstrap.servers is empty for these configurations,
it will fail when creating the AdminClient. I have updated the KIP to reflect
this behavior.
JR44: Thanks for pointing this out. Kafka will indeed fa
Hi, Jiunn-Yang,
Thanks for the reply. A few more comments.
JR42. [6] [7] are not needed since currently an empty list already causes
an exception.
JR43. Could you document the current behavior when bootstrap.servers is
empty in MirrorClientConfig and WorkerConfig?
JR44. [12] seems incorrect. I
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 於 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 disal
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
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 a
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 d
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.ciph
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 `Mir
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.sui
>
> 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."
>
agre
Hello Jun,
Thanks for the reply.
JR33, I’ve updated the validator to only allow non-empty lists.
However, I have a question about the default value: It seems that in Streams,
the config uses NO_DEFAULT_VALUE and includes a comment saying “required with
no default value.”
Should we also consid
Hi, Jiunn-Yang,
Thanks for the reply.
JR31. This is fine then.
JR33. StreamsConfig: bootstrap.servers can't be empty.
bootstrap.controllers is not available in Streams. It's only available in
AdminClient.
JR34. If there are more MM instances than partitions, it seems that it's
possible for task
Hello Jun, Chia-Ping,
Thanks for the reply.
JR28:
You're right. If the user passes a null value, it will be converted to an empty
list.
I’ve updated the KIP accordingly.
JR29:
For early.start.listeners, the exception is thrown during Kafka’s internal
validation,
and it’s not caused by a null
hi Jiunn-Yang,
chia00. why could `bootstrap.servers` be null in StreamsConfig?
chia01. the empty `task.assigned.partitions` is a bit weird. That means
consumer has nothing to seek/poll.
chia02. task.assigned.groups has analogous issue.
chia03. MirrorCheckpointConfig's `groups` and `groups.exclu
Hi, Jiunn-Yang,
Thanks for the reply. A few more comments.
JR28. AdminClientConfig: Currently, bootstrap.servers doesn't throw
NullPointerException on null value. It's converted to an empty list.
JR29. The "Before exception" column: It would be useful to add which value
causes an exception. For
Hi Jun,
Thanks for the reply.
JR26: I’ve addressed this change in the KIP.
JR27:
I’ve documented the following configurations in the KIP:
ConsumerConfig
partition.assignment.strategy
StreamsConfig
bootstrap.servers
MirrorCheckpointConfig
groups
groups.exclude
task.assigned.groups
Hi, Jiunn-Yang,
Thanks for the reply.
JR 26. early.start.listeners and interceptor.classes: It seems that both
could be empty?
JR27. The following configs of type LIST are missing.
ConsumerConfig
partition.assignment.strategy
BrokerSecurityConfigs
QuorumConfig
LogConfig
leader.replication.th
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.anyNonDuplicat
> 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]
Addition
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: Currentl
Hi Chia, Jun
Thanks for the reply,
I have updated the table in the KIP.
Best Regards,
Jiunn-Yang
> Chia-Ping Tsai 於 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 valu
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 於 2025年7月11日 週五 上午6:59寫道：
> Hi, Chia-Ping,
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 se
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 e
> 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
> diff
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 cip
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"
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 K
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, th
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 ma
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
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 all
Hello all,
I am manually bumping this thread. Any feedback would be appreciated.
Best regards,
Jiunn-Yang
> 黃竣陽 於 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 o
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
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 wrote:
> Hi, Luke,
>
> Thanks for the reply.
>
> "My thoug
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
lik
Hello Mickael,
Thanks for this nice catch.
> One weirdness of cleanup.policy is that you can set it to
> "delete,delete,delete,compact,compact" for example. Setting delete or
> compact multiple times has no functional impact.
Currently, only the process.roles config checks for duplicate values
FYI while trying with an AlterConfigPolicy to stop the cleanup.policy
being emptied through a AlterConfigOp.OpType.SUBTRACT,
I stumbled in this behavior
that doesn't allow to block an empty policy in Zookeeper versions of Kafka
https://issues.apache.org/jira/browse/KAFKA-19026
On Tue, 13 May 2025
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
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
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
Hi,
Thanks for the KIP.
One weirdness of cleanup.policy is that you can set it to
"delete,delete,delete,compact,compact" for example. Setting delete or
compact multiple times has no functional impact.
I don't think it's something that has to be addressed, but while
you're looking at the configura
Hi Luke, Chia,
Thanks for your information.
>> FYI, there are indeed use cases that need to keep all history data without
>> a cleanup policy.
I agree that we should maintain backward compatibility. Therefore, I will
update the KIP as follows:
- If the user sets an empty cleanup.policy, we wil
hi Luke
We can print warnings for empty cleanup.policy in 4.x and in 5.0 we throw
exception directly to make it fail fast
Jiunn: WDYT?
Best,
Chia-Ping
> Luke Chen 於 2025年5月12日 上午10:52 寫道：
>
> Hi Jiunn-Yang,
>
> Thanks for the KIP.
>
> Comments:
> 1. "it is acceptable because an empty cl
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,
Hi, Jiunn-Yang,
Thanks for the updated KIP. It looks good to me.
Jun
On Tue, May 6, 2025 at 4:58 AM 黃竣陽 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
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-Pin
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 於 2025年5月6日 週二 上午12:28寫道：
> Hi, Jiunn-Yang,
>
> Thanks for the reply. There are still references to th
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 黃竣陽 wrote:
> Hi Jun, chia,
>
> Thanks for all the comme
Hi Jun, chia,
Thanks for all the comments. They have all been addressed in the updated KIP.
Best Regards,
Jiunn-Yang
> Chia-Ping Tsai 於 2025年5月3日 凌晨2:03 寫道：
>
> hi Jiunn-Yang
>
> in the "Compatibility, Deprecation, and Migration Plan", could you add
> description to remind readers that "clean
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 於 2025年5月3日 週六 上午1:55寫道：
> Hi, Jiunn-Yang,
>
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 黃竣陽 wrote:
> Hi Jun,
>
> I don’t think we should deprecate the ValidList#in method, as there may be
> fut
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 wou
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 黃竣陽 wrote:
> Hello Jun,
>
> Since ValidList is a public API, we need to ma
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 於 2025年5月1日 凌晨4:17 寫道：
>
> Hi, Jiunn-Yang,
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 黃竣陽 wrote:
> Hi Jun,
>
> Thank you for pointing that out.
>
> I have revised the K
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 forma
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.protoc
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 inv
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.pol
Hi Jun,
Q2:
For the group.coordinator.rebalance.protocols and process.roles configurations,
setting them to an
empty value is invalid—the broker/controller will fail to start in such cases.
Therefore, I believe it's safe
to assume that no users would configure them this way, and we can reasona
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
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
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 黃竣陽 wrote:
> Hello Jun,
>
> Thanks for review this KIP.
>
> Q1 & Q3:
> I’ve updated the method name accord
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
no
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
70 matches
Mail list logo
