satishd commented on code in PR #14114:
URL: https://github.com/apache/kafka/pull/14114#discussion_r1278245511
##########
core/src/test/scala/unit/kafka/log/LogConfigTest.scala:
##########
@@ -62,13 +63,17 @@ class LogConfigTest {
kafkaProps.put(KafkaConfig.LogRollTimeJitterHoursProp, "2")
kafkaProps.put(KafkaConfig.LogRetentionTimeHoursProp, "2")
kafkaProps.put(KafkaConfig.LogMessageFormatVersionProp, "0.11.0")
+ kafkaProps.put(RemoteLogManagerConfig.LOG_LOCAL_RETENTION_MS_PROP,
"300000")
Review Comment:
As these values are long types. Can we have a value more than Integer.MAX?
##########
core/src/test/scala/unit/kafka/server/DynamicBrokerConfigTest.scala:
##########
@@ -713,6 +714,99 @@ class DynamicBrokerConfigTest {
config.updateCurrentConfig(new KafkaConfig(props))
assertFalse(config.nonInternalValues.containsKey(KafkaConfig.MetadataLogSegmentMinBytesProp))
}
+
+ @Test
+ def testDynamicLogLocalRetentionMsConfig(): Unit = {
+ val props = TestUtils.createBrokerConfig(0, TestUtils.MockZkConnect, port
= 8181)
+ props.put(KafkaConfig.LogRetentionTimeMillisProp, "1000")
Review Comment:
As these values are long types. Can we have a value more than Integer.MAX?
Same comment in this class for other retention properties.
##########
core/src/test/scala/unit/kafka/log/LogConfigTest.scala:
##########
@@ -62,13 +63,17 @@ class LogConfigTest {
kafkaProps.put(KafkaConfig.LogRollTimeJitterHoursProp, "2")
kafkaProps.put(KafkaConfig.LogRetentionTimeHoursProp, "2")
kafkaProps.put(KafkaConfig.LogMessageFormatVersionProp, "0.11.0")
+ kafkaProps.put(RemoteLogManagerConfig.LOG_LOCAL_RETENTION_MS_PROP,
"300000")
+ kafkaProps.put(RemoteLogManagerConfig.LOG_LOCAL_RETENTION_BYTES_PROP,
"1024")
Review Comment:
As these values are long types. Can we have a value more than Integer.MAX?
--
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]
