mattwong949 commented on a change in pull request #10914:
URL: https://github.com/apache/kafka/pull/10914#discussion_r682018753
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -270,7 +270,8 @@ class Log(@volatile private var _dir: File,
val producerStateManager: ProducerStateManager,
logDirFailureChannel: LogDirFailureChannel,
@volatile private var _topicId: Option[Uuid],
- val keepPartitionMetadataFile: Boolean) extends Logging with
KafkaMetricsGroup {
+ val keepPartitionMetadataFile: Boolean,
+ @volatile var latestDeleteHorizon: Long = RecordBatch.NO_TIMESTAMP)
extends Logging with KafkaMetricsGroup {
Review comment:
~~hmm it seems like we only use it in a test. That goes with the return
value that was added into the `cleanInto` method in the LogCleaner. I'm going
to remove these and see if I can take another approach in the testing
edit: going through the build errors and I see other usages. I will go
through it more thoroughly~~
sorry I jumped the gun a bit w/ the earlier changes
There is an addition to the LogCleanerManager that allows the cleaner to
check for cleaning logs that have tombstones past the deleteHorizon. The logic
in the LogCleanerManager can be paraphrased to "if there are no eligible
cleanable logs, we can see if there are logs that have tombstones that can be
deleted by checking the Log's latestDeleteHorizon. We can enqueue those with
tombstones eligible for cleaning"
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -270,7 +270,8 @@ class Log(@volatile private var _dir: File,
val producerStateManager: ProducerStateManager,
logDirFailureChannel: LogDirFailureChannel,
@volatile private var _topicId: Option[Uuid],
- val keepPartitionMetadataFile: Boolean) extends Logging with
KafkaMetricsGroup {
+ val keepPartitionMetadataFile: Boolean,
+ @volatile var latestDeleteHorizon: Long = RecordBatch.NO_TIMESTAMP)
extends Logging with KafkaMetricsGroup {
Review comment:
~~hmm it seems like we only use it in a test. That goes with the return
value that was added into the `cleanInto` method in the LogCleaner. I'm going
to remove these and see if I can take another approach in the testing~~
~~edit: going through the build errors and I see other usages. I will go
through it more thoroughly~~
sorry I jumped the gun a bit w/ the earlier changes
There is an addition to the LogCleanerManager that allows the cleaner to
check for cleaning logs that have tombstones past the deleteHorizon. The logic
in the LogCleanerManager can be paraphrased to "if there are no eligible
cleanable logs, we can see if there are logs that have tombstones that can be
deleted by checking the Log's latestDeleteHorizon. We can enqueue those with
tombstones eligible for cleaning"
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
