junrao commented on a change in pull request #9364:
URL: https://github.com/apache/kafka/pull/9364#discussion_r499051238
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -244,7 +244,8 @@ class Log(@volatile private var _dir: File,
val producerIdExpirationCheckIntervalMs: Int,
val topicPartition: TopicPartition,
val producerStateManager: ProducerStateManager,
- logDirFailureChannel: LogDirFailureChannel) extends Logging with
KafkaMetricsGroup {
+ logDirFailureChannel: LogDirFailureChannel,
+ val hadCleanShutdown: Boolean = true) extends Logging with
KafkaMetricsGroup {
Review comment:
Could we add the new param to the javadoc above?
##########
File path: core/src/test/scala/unit/kafka/log/LogTest.scala
##########
@@ -4447,9 +4504,10 @@ class LogTest {
private def recoverAndCheck(config: LogConfig,
expectedKeys: Iterable[Long],
- expectDeletedFiles: Boolean = true): Log = {
+ expectDeletedFiles: Boolean = true,
Review comment:
It seems that all callers set expectDeletedFiles to false. So, do we
need this param?
##########
File path: core/src/test/scala/unit/kafka/log/LogTest.scala
##########
@@ -3073,9 +3139,8 @@ class LogTest {
// check if recovery was attempted. Even if the recovery point is 0L,
recovery should not be attempted as the
// clean shutdown file exists.
recoveryPoint = log.logEndOffset
- log = createLog(logDir, logConfig)
+ log = createLog(logDir, logConfig, lastShutdownClean = true)
Review comment:
The change is unnecessary.
##########
File path: core/src/test/scala/unit/kafka/log/LogTest.scala
##########
@@ -2882,11 +2953,8 @@ class LogTest {
records.foreach(segment.append _)
segment.close()
- // Create clean shutdown file so that we do not split during the load
- createCleanShutdownFile()
-
val logConfig = LogTest.createLogConfig(indexIntervalBytes = 1,
fileDeleteDelayMs = 1000)
- val log = createLog(logDir, logConfig, recoveryPoint = Long.MaxValue)
+ val log = createLog(logDir, logConfig, recoveryPoint = Long.MaxValue,
lastShutdownClean = true)
Review comment:
The change is unnecessary.
##########
File path: core/src/test/scala/unit/kafka/log/LogTest.scala
##########
@@ -4429,9 +4485,10 @@ class LogTest {
scheduler: Scheduler = mockTime.scheduler,
time: Time = mockTime,
maxProducerIdExpirationMs: Int = 60 * 60 * 1000,
- producerIdExpirationCheckIntervalMs: Int =
LogManager.ProducerIdExpirationCheckIntervalMs): Log = {
+ producerIdExpirationCheckIntervalMs: Int =
LogManager.ProducerIdExpirationCheckIntervalMs,
+ lastShutdownClean: Boolean = true): Log = {
Review comment:
The callers of createLog() in line 652 and 2205 seem to need
lastShutdownClean to be false.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
