showuon commented on code in PR #16221:
URL: https://github.com/apache/kafka/pull/16221#discussion_r1630623662
##########
server-common/src/main/java/org/apache/kafka/server/config/ServerLogConfigs.java:
##########
@@ -188,11 +188,12 @@ public class ServerLogConfigs {
public static final String LOG_INITIAL_TASK_DELAY_MS_CONFIG = LOG_PREFIX +
"initial.task.delay.ms";
public static final long LOG_INITIAL_TASK_DELAY_MS_DEFAULT = 30 * 1000L;
+ public static final long
LOG_INITIAL_TASK_DELAY_MS_DEFAULT_INTEGRATION_TEST = 500L;
Review Comment:
This is not a good the place to put configuration value just for tests.
##########
core/src/test/scala/unit/kafka/utils/TestUtils.scala:
##########
@@ -1181,7 +1181,7 @@ object TestUtils extends Logging {
transactionVerificationEnabled: Boolean = false,
log: Option[UnifiedLog] = None,
remoteStorageSystemEnable: Boolean = false,
- initialTaskDelayMs: Long =
ServerLogConfigs.LOG_INITIAL_TASK_DELAY_MS_DEFAULT): LogManager = {
+ initialTaskDelayMs: Long =
ServerLogConfigs.LOG_INITIAL_TASK_DELAY_MS_DEFAULT_INTEGRATION_TEST):
LogManager = {
Review Comment:
We can change it here for logManager tests. But that's not a place for
integration test. The integration test will start up brokers with broker
configurations, so that we won't use `TestUtils.createLogManager` method here.
Have you checked `IntegrationTestHarness.scala` where we set broker
configurations for integration test brokers?
--
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]
