see-quick commented on code in PR #21384:
URL: https://github.com/apache/kafka/pull/21384#discussion_r2772996211
##########
server-common/src/main/java/org/apache/kafka/server/config/QuotaConfig.java:
##########
@@ -212,17 +224,7 @@ public int controllerQuotaWindowSizeSeconds() {
}
public static ConfigDef brokerQuotaConfigs() {
- return new ConfigDef()
- // Round minimum value down, to make it easier for users.
- .define(QuotaConfig.LEADER_REPLICATION_THROTTLED_RATE_CONFIG,
ConfigDef.Type.LONG,
- QuotaConfig.QUOTA_BYTES_PER_SECOND_DEFAULT,
ConfigDef.Range.atLeast(0),
- ConfigDef.Importance.MEDIUM,
QuotaConfig.LEADER_REPLICATION_THROTTLED_RATE_DOC)
-
.define(QuotaConfig.FOLLOWER_REPLICATION_THROTTLED_RATE_CONFIG,
ConfigDef.Type.LONG,
- QuotaConfig.QUOTA_BYTES_PER_SECOND_DEFAULT,
ConfigDef.Range.atLeast(0),
- ConfigDef.Importance.MEDIUM,
QuotaConfig.FOLLOWER_REPLICATION_THROTTLED_RATE_DOC)
-
.define(QuotaConfig.REPLICA_ALTER_LOG_DIRS_IO_MAX_BYTES_PER_SECOND_CONFIG,
ConfigDef.Type.LONG,
- QuotaConfig.QUOTA_BYTES_PER_SECOND_DEFAULT,
ConfigDef.Range.atLeast(0),
- ConfigDef.Importance.MEDIUM,
QuotaConfig.REPLICA_ALTER_LOG_DIRS_IO_MAX_BYTES_PER_SECOND_DOC);
+ return BROKER_QUOTA_CONFIG_DEF;
Review Comment:
> You might want to check out the
[KIP-1051](https://cwiki.apache.org/confluence/x/C4xyEg), which address the
exact issue
So it's still under-discussion? I am bit a lost in the JIRAs like there is
always at least one JIRA for issue I found :D . Because that KIP just wants to
move such configuration from DynamicConfig to KafkaConfig which is not a case.
> Actually, the comment is already there. see
https://github.com/apache/kafka/blob/trunk/server/src/main/java/org/apache/kafka/server/config/DynamicConfig.java#L27
_Class used to hold dynamic configs. These are configs which have no
physical manifestation in the server.properties and can only be set
dynamically._
used to (past)? It's still holding `dynamic-only` configs (i.e.,
QuotaConfig.brokerQuotaConfigs();)? And then you have also dual-mode configs
which are filtered from the `AbstractKafkaConfig` i.e., :
```java
// Filter and define all dynamic configurations
AbstractKafkaConfig.CONFIG_DEF.configKeys().forEach((name,
value) -> {
if (DynamicBrokerConfig.ALL_DYNAMIC_CONFIGS.contains(name)) {
configs.define(value);
}
});
```
I would probably fix that doc something like:
```
Holds dynamic configs, including both dynamic-only configs (e.g., quotas)
and dual-mode configs that can be set statically or dynamically.
```
I think it would help with overall understanding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
