kowshik commented on a change in pull request #11110:
URL: https://github.com/apache/kafka/pull/11110#discussion_r683184053
##########
File path:
storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerConfig.java
##########
@@ -253,9 +253,9 @@ public RemoteLogManagerConfig(AbstractConfig config) {
config.getInt(REMOTE_LOG_READER_THREADS_PROP),
config.getInt(REMOTE_LOG_READER_MAX_PENDING_TASKS_PROP),
config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP),
-
config.originalsWithPrefix(config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP)),
+
config.originalsWithPrefix(config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP)
== null ? "" : config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP)),
Review comment:
Hmm, why is this change needed? It doesn't seem like the PR is altering
behavior such as these but maybe I'm missing something.
##########
File path: core/src/main/scala/kafka/log/LogConfig.scala
##########
@@ -107,46 +107,52 @@ case class LogConfig(props: java.util.Map[_, _],
overriddenConfigs: Set[String]
val LeaderReplicationThrottledReplicas =
getList(LogConfig.LeaderReplicationThrottledReplicasProp)
val FollowerReplicationThrottledReplicas =
getList(LogConfig.FollowerReplicationThrottledReplicasProp)
val messageDownConversionEnable =
getBoolean(LogConfig.MessageDownConversionEnableProp)
- val remoteStorageEnable = getBoolean(LogConfig.RemoteLogStorageEnableProp)
- val localRetentionMs: Long = {
- val localLogRetentionMs = getLong(LogConfig.LocalLogRetentionMsProp)
+ class RemoteLogConfig {
+ val remoteStorageEnable = getBoolean(LogConfig.RemoteLogStorageEnableProp)
- // -2 indicates to derive value from retentionMs property.
- if(localLogRetentionMs == -2) retentionMs
- else {
- // Added validation here to check the effective value should not be more
than RetentionMs.
- if(localLogRetentionMs == -1 && retentionMs != -1) {
- throw new ConfigException(LogConfig.LocalLogRetentionMsProp,
localLogRetentionMs, s"Value must not be -1 as ${LogConfig.RetentionMsProp}
value is set as $retentionMs.")
- }
+ val localRetentionMs: Long = {
+ val localLogRetentionMs = getLong(LogConfig.LocalLogRetentionMsProp)
- if (localLogRetentionMs > retentionMs) {
- throw new ConfigException(LogConfig.LocalLogRetentionMsProp,
localLogRetentionMs, s"Value must not be more than property:
${LogConfig.RetentionMsProp} value.")
- }
+ // -2 indicates to derive value from retentionMs property.
+ if(localLogRetentionMs == -2) retentionMs
+ else {
+ // Added validation here to check the effective value should not be
more than RetentionMs.
+ if(localLogRetentionMs == -1 && retentionMs != -1) {
+ throw new ConfigException(LogConfig.LocalLogRetentionMsProp,
localLogRetentionMs, s"Value must not be -1 as ${LogConfig.RetentionMsProp}
value is set as $retentionMs.")
+ }
+
+ if (localLogRetentionMs > retentionMs) {
+ throw new ConfigException(LogConfig.LocalLogRetentionMsProp,
localLogRetentionMs, s"Value must not be more than property:
${LogConfig.RetentionMsProp} value.")
+ }
- localLogRetentionMs
+ localLogRetentionMs
+ }
}
- }
- val localRetentionBytes: Long = {
- val localLogRetentionBytes = getLong(LogConfig.LocalLogRetentionBytesProp)
+ val localRetentionBytes: Long = {
+ val localLogRetentionBytes =
getLong(LogConfig.LocalLogRetentionBytesProp)
- // -2 indicates to derive value from retentionSize property.
- if(localLogRetentionBytes == -2) retentionSize;
- else {
- // Added validation here to check the effective value should not be more
than RetentionBytes.
- if(localLogRetentionBytes == -1 && retentionSize != -1) {
- throw new ConfigException(LogConfig.LocalLogRetentionBytesProp,
localLogRetentionBytes, s"Value must not be -1 as
${LogConfig.RetentionBytesProp} value is set as $retentionSize.")
- }
+ // -2 indicates to derive value from retentionSize property.
+ if(localLogRetentionBytes == -2) retentionSize;
Review comment:
nit: it seems like we could remove the semicolon at the end.
--
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]
