-
Notifications
You must be signed in to change notification settings - Fork 15.4k
KAFKA-7321: Add a Maximum Log Compaction Lag (KIP-354) #6009
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -165,7 +165,7 @@ private[log] class LogCleanerManager(val logDirs: Seq[File], | |
| * each time from the full set of logs to allow logs to be dynamically added to the pool of logs | ||
| * the log manager maintains. | ||
| */ | ||
| def grabFilthiestCompactedLog(time: Time): Option[LogToClean] = { | ||
| def grabFilthiestCompactedLog(time: Time, preCleanStats: PreCleanStats = new PreCleanStats()): Option[LogToClean] = { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This works, but I think we should try and avoid passing in a new
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since grabFilthiestCompactedLog and cleaner stats are defined in two classes. Volatile global variables doesn't make our life easier. The default "new PreCleanStats()" is mainly for the purposes not to change many test cases that use this function directly.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still dislike the "step"-effect that this has. i.e., from the point in time any log is due for compaction, the |
||
| inLock(lock) { | ||
| val now = time.milliseconds | ||
| this.timeOfLastRun = now | ||
|
|
@@ -178,17 +178,24 @@ private[log] class LogCleanerManager(val logDirs: Seq[File], | |
| inProgress.contains(topicPartition) || isUncleanablePartition(log, topicPartition) | ||
|
jjkoshy marked this conversation as resolved.
Outdated
|
||
| }.map { | ||
| case (topicPartition, log) => // create a LogToClean instance for each | ||
| val (firstDirtyOffset, firstUncleanableDirtyOffset) = LogCleanerManager.cleanableOffsets(log, topicPartition, | ||
| lastClean, now) | ||
| LogToClean(topicPartition, log, firstDirtyOffset, firstUncleanableDirtyOffset) | ||
| val (firstDirtyOffset, firstUncleanableDirtyOffset) = | ||
| LogCleanerManager.cleanableOffsets(log, topicPartition, lastClean, now) | ||
|
|
||
| val compactionDelayMs = LogCleanerManager.getMaxCompactionDelay(log, firstDirtyOffset, now) | ||
| preCleanStats.updateMaxCompactionDelay(compactionDelayMs) | ||
|
|
||
| LogToClean(topicPartition, log, firstDirtyOffset, firstUncleanableDirtyOffset, compactionDelayMs > 0) | ||
| }.filter(ltc => ltc.totalBytes > 0) // skip any empty logs | ||
|
|
||
| this.dirtiestLogCleanableRatio = if (dirtyLogs.nonEmpty) dirtyLogs.max.cleanableRatio else 0 | ||
| // and must meet the minimum threshold for dirty byte ratio | ||
| val cleanableLogs = dirtyLogs.filter(ltc => ltc.cleanableRatio > ltc.log.config.minCleanableRatio) | ||
| // and must meet the minimum threshold for dirty byte ratio or have some bytes required to be compacted | ||
| val cleanableLogs = dirtyLogs.filter { ltc => | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would also be useful to log a count of how many logs are unclean and of those, how many logs are cleanable due to violating the max compaction lag constraint.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added related logs : see PreCleanStats |
||
| (ltc.needCompactionNow && ltc.cleanableBytes > 0) || ltc.cleanableRatio > ltc.log.config.minCleanableRatio | ||
| } | ||
| if(cleanableLogs.isEmpty) { | ||
| None | ||
| } else { | ||
| preCleanStats.recordCleanablePartitions(cleanableLogs.size) | ||
| val filthiest = cleanableLogs.max | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I didn't notice earlier: should we actually prioritize a log that is past its max compaction delay over a log that is more dirty?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original idea is to sort the log based on the compaction delay that passed the the max delay. But a log with a very short compaction delay may always takes priority over a very dirty log (with high dirty ratio). I think it is better not to prioritize it since the the compaction finish time is not actually guaranteed since log cleaner thread can take a long time for compaction, and it can work on other log when a log go beyond the max compaction lag.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This really depends on the interpretation of the config. For PII data for e.g., you should be able to provide some guarantee. Either way, there is the possibility of starvation. However, we do have sensors to indicate this situation, so I think we can leave it as is and revisit if people want harder guarantees. |
||
| inProgress.put(filthiest.topicPartition, LogCleaningInProgress) | ||
| Some(filthiest) | ||
|
|
@@ -476,6 +483,30 @@ private[log] object LogCleanerManager extends Logging { | |
| log.config.compact && log.config.delete | ||
| } | ||
|
|
||
| /** | ||
| * get max delay between the time when log is required to be compacted as determined | ||
| * by maxCompactionLagMs and the current time. | ||
| */ | ||
| def getMaxCompactionDelay(log: Log, firstDirtyOffset: Long, now: Long) : Long = { | ||
|
|
||
| val dirtyNonActiveSegments = log.logSegments(firstDirtyOffset, log.activeSegment.baseOffset) | ||
|
|
||
| val firstBatchTimestamps = log.getFirstBatchTimestampForSegments(dirtyNonActiveSegments).filter(_ > 0) | ||
|
|
||
| val earliestDirtySegmentTimestamp = { | ||
| if (firstBatchTimestamps.nonEmpty) | ||
| firstBatchTimestamps.min | ||
| else Long.MaxValue | ||
| } | ||
|
|
||
| val maxCompactionLagMs = math.max(log.config.maxCompactionLagMs, 0L) | ||
| val cleanUntilTime = now - maxCompactionLagMs | ||
|
|
||
| if (earliestDirtySegmentTimestamp < cleanUntilTime) | ||
| cleanUntilTime - earliestDirtySegmentTimestamp | ||
| else | ||
| 0L | ||
| } | ||
|
|
||
| /** | ||
| * Returns the range of dirty offsets that can be cleaned. | ||
|
|
@@ -505,7 +536,7 @@ private[log] object LogCleanerManager extends Logging { | |
| } | ||
| } | ||
|
|
||
| val compactionLagMs = math.max(log.config.compactionLagMs, 0L) | ||
| val minCompactionLagMs = math.max(log.config.compactionLagMs, 0L) | ||
|
|
||
| // find first segment that cannot be cleaned | ||
| // neither the active segment, nor segments with any messages closer to the head of the log than the minimum compaction lag time | ||
|
|
@@ -519,12 +550,12 @@ private[log] object LogCleanerManager extends Logging { | |
| Option(log.activeSegment.baseOffset), | ||
|
|
||
| // the first segment whose largest message timestamp is within a minimum time lag from now | ||
| if (compactionLagMs > 0) { | ||
| if (minCompactionLagMs > 0) { | ||
| // dirty log segments | ||
| val dirtyNonActiveSegments = log.logSegments(firstDirtyOffset, log.activeSegment.baseOffset) | ||
| dirtyNonActiveSegments.find { s => | ||
| val isUncleanable = s.largestTimestamp > now - compactionLagMs | ||
| debug(s"Checking if log segment may be cleaned: log='${log.name}' segment.baseOffset=${s.baseOffset} segment.largestTimestamp=${s.largestTimestamp}; now - compactionLag=${now - compactionLagMs}; is uncleanable=$isUncleanable") | ||
| val isUncleanable = s.largestTimestamp > now - minCompactionLagMs | ||
| debug(s"Checking if log segment may be cleaned: log='${log.name}' segment.baseOffset=${s.baseOffset} segment.largestTimestamp=${s.largestTimestamp}; now - compactionLag=${now - minCompactionLagMs}; is uncleanable=$isUncleanable") | ||
| isUncleanable | ||
| }.map(_.baseOffset) | ||
| } else None | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.