clolov commented on code in PR #21943:
URL: https://github.com/apache/kafka/pull/21943#discussion_r3949922686
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -245,101 +203,6 @@ class KafkaConfig private(doLog: Boolean, val props:
util.Map[_, _], enforceProv
}
}
- /***************** rack configuration **************/
- val replicaSelectorClassName =
Option(getString(ReplicationConfigs.REPLICA_SELECTOR_CLASS_CONFIG))
-
- /** ********* Replication configuration ***********/
- val replicaLagTimeMaxMs =
getLong(ReplicationConfigs.REPLICA_LAG_TIME_MAX_MS_CONFIG)
Review Comment:
Good point! I tried moving all of these in the constructor, but one can't
use getInt, getLong etc. before KafkaConfig.currentConfig is initialised. I
think I can see a way to do it, but that would be a separate PR on which this
one will build upon. The other approach I can think of is to wrap everything in
code similar to the quotaConfig i.e. creating it on a first usage and from then
on returning the reference. I traced the usages of a few of the configs and
they appear to be used during startup or/and only once and do not appear to be
called on every request. As such, if we leave them as this PR suggests right
now there may be a regression, but it doesn't seem to affect traffic.
What would you feel more comfortable with proceeding, the current version, a
wrapper similar to what we do today with quotaConfig or trying to refactor
KafkaConfig.currentConfig in AbstractKafkaConfig in order to be able to use
getInt, getLong etc. in the constructor? Or maybe you can see another way?
##########
core/src/main/scala/kafka/server/ControllerServer.scala:
##########
@@ -235,7 +235,7 @@ class ControllerServer(
OptionalLong.empty()
}
- val maxIdleIntervalNs =
config.metadataMaxIdleIntervalNs.fold(OptionalLong.empty)(OptionalLong.of)
+ val maxIdleIntervalNs =
config.metadataMaxIdleIntervalNs.map[OptionalLong](v =>
OptionalLong.of(v)).orElse(OptionalLong.empty)
Review Comment:
Great spot!
--
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]
