-
Notifications
You must be signed in to change notification settings - Fork 6
Batch Coordinate cache #438
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
16d3d83
Batch Coordinate cache
giuseppelillo c69749c
Address review comments
giuseppelillo 24a54ef
Use Time in place of Clock
giuseppelillo 4b20ce5
Atomic put and exception refactor
giuseppelillo 8934c3c
Remove unused methods
giuseppelillo f9c0d92
Add CacheHitsWithoutData metric
giuseppelillo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
30 changes: 30 additions & 0 deletions
30
storage/inkless/src/main/java/io/aiven/inkless/cache/BatchCoordinateCache.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /* | ||
| * Inkless | ||
| * Copyright (C) 2024 - 2025 Aiven OY | ||
| * | ||
| * This program is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU Affero General Public License as published by | ||
| * the Free Software Foundation, either version 3 of the License, or | ||
| * (at your option) any later version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU Affero General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License | ||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
| package io.aiven.inkless.cache; | ||
|
|
||
| import org.apache.kafka.common.TopicIdPartition; | ||
|
|
||
| import java.io.Closeable; | ||
|
|
||
| public interface BatchCoordinateCache extends Closeable { | ||
|
|
||
| LogFragment get(TopicIdPartition topicIdPartition, long offset); | ||
|
|
||
| void put(TopicIdPartition topicIdPartition, CacheBatchCoordinate cacheBatchCoordinate); | ||
|
|
||
| } |
72 changes: 72 additions & 0 deletions
72
storage/inkless/src/main/java/io/aiven/inkless/cache/BatchCoordinateCacheMetrics.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package io.aiven.inkless.cache; | ||
|
|
||
| import org.apache.kafka.server.metrics.KafkaMetricsGroup; | ||
|
|
||
| import java.io.Closeable; | ||
| import java.util.concurrent.atomic.LongAdder; | ||
|
|
||
|
|
||
| public final class BatchCoordinateCacheMetrics implements Closeable { | ||
| static final String CACHE_HITS = "CacheHits"; | ||
| static final String CACHE_HITS_WITHOUT_DATA = "CacheHitsWithoutData"; | ||
| static final String CACHE_MISSES = "CacheMisses"; | ||
| static final String CACHE_INVALIDATIONS = "CacheInvalidations"; | ||
| static final String CACHE_EVICTIONS = "CacheEvictions"; | ||
| static final String CACHE_SIZE = "CacheSize"; | ||
|
|
||
| private final KafkaMetricsGroup metricsGroup = new KafkaMetricsGroup(BatchCoordinateCache.class); | ||
| private final LongAdder cacheHits = new LongAdder(); | ||
| private final LongAdder cacheHitsWithoutData = new LongAdder(); | ||
| private final LongAdder cacheMisses = new LongAdder(); | ||
| private final LongAdder cacheInvalidations = new LongAdder(); | ||
| private final LongAdder cacheEvictions = new LongAdder(); | ||
| private final LongAdder cacheSize = new LongAdder(); | ||
|
|
||
| public BatchCoordinateCacheMetrics() { | ||
| metricsGroup.newGauge(CACHE_HITS, cacheHits::intValue); | ||
| metricsGroup.newGauge(CACHE_HITS_WITHOUT_DATA, cacheHitsWithoutData::intValue); | ||
| metricsGroup.newGauge(CACHE_MISSES, cacheMisses::intValue); | ||
| metricsGroup.newGauge(CACHE_INVALIDATIONS, cacheInvalidations::intValue); | ||
| metricsGroup.newGauge(CACHE_EVICTIONS, cacheEvictions::intValue); | ||
| metricsGroup.newGauge(CACHE_SIZE, cacheSize::intValue); | ||
| } | ||
|
|
||
| public void recordCacheHit() { | ||
| cacheHits.increment(); | ||
| } | ||
|
|
||
| public void recordCacheHitWithoutData() { | ||
| cacheHitsWithoutData.increment(); | ||
| } | ||
|
|
||
|
|
||
| public void recordCacheMiss() { | ||
| cacheMisses.increment(); | ||
| } | ||
|
|
||
| public void recordCacheInvalidation() { | ||
| cacheInvalidations.increment(); | ||
| } | ||
|
|
||
| public void recordCacheEviction() { | ||
| cacheEvictions.increment(); | ||
| } | ||
|
|
||
| public void incrementCacheSize() { | ||
| cacheSize.increment(); | ||
| } | ||
|
|
||
| public void decreaseCacheSize(int removedEntries) { | ||
| cacheSize.add(-removedEntries); | ||
| } | ||
|
|
||
| @Override | ||
| public void close() { | ||
| metricsGroup.removeMetric(CACHE_HITS); | ||
| metricsGroup.removeMetric(CACHE_HITS_WITHOUT_DATA); | ||
| metricsGroup.removeMetric(CACHE_MISSES); | ||
| metricsGroup.removeMetric(CACHE_INVALIDATIONS); | ||
| metricsGroup.removeMetric(CACHE_EVICTIONS); | ||
| metricsGroup.removeMetric(CACHE_SIZE); | ||
| } | ||
| } |
90 changes: 90 additions & 0 deletions
90
storage/inkless/src/main/java/io/aiven/inkless/cache/CacheBatchCoordinate.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| /* | ||
| * Inkless | ||
| * Copyright (C) 2024 - 2025 Aiven OY | ||
| * | ||
| * This program is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU Affero General Public License as published by | ||
| * the Free Software Foundation, either version 3 of the License, or | ||
| * (at your option) any later version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU Affero General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License | ||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
| package io.aiven.inkless.cache; | ||
|
|
||
| import org.apache.kafka.common.TopicIdPartition; | ||
| import org.apache.kafka.common.record.TimestampType; | ||
|
|
||
| import io.aiven.inkless.control_plane.BatchInfo; | ||
| import io.aiven.inkless.control_plane.BatchMetadata; | ||
|
|
||
| /** | ||
| * Represents the coordinates of a batch that is handled by the Batch Coordinate cache | ||
| */ | ||
| public record CacheBatchCoordinate( | ||
| String objectKey, | ||
| long byteOffset, | ||
| long byteSize, | ||
| long baseOffset, | ||
| long lastOffset, | ||
| TimestampType timestampType, | ||
| long logAppendTimestamp, | ||
| byte magic, | ||
| long logStartOffset | ||
| ) { | ||
|
|
||
| public CacheBatchCoordinate { | ||
| if (lastOffset < baseOffset) { | ||
| throw new IllegalArgumentException( | ||
| String.format( | ||
| "lastOffset must be greater than or equal to baseOffset, but got lastOffset=%d and baseOffset=%d", | ||
| lastOffset, | ||
| baseOffset | ||
| ) | ||
| ); | ||
| } | ||
| if (byteSize <= 0) { | ||
| throw new IllegalArgumentException( | ||
| String.format("byteSize must be positive, but got %d", byteSize) | ||
| ); | ||
| } | ||
| if (byteOffset < 0) { | ||
| throw new IllegalArgumentException( | ||
| String.format("byteOffset must be non-negative, but got %d", byteOffset) | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| public BatchInfo batchInfo(TopicIdPartition topicIdPartition, long batchId) { | ||
| return new BatchInfo( | ||
| batchId, | ||
| objectKey, | ||
| new BatchMetadata( | ||
| magic, | ||
| topicIdPartition, | ||
| byteOffset, | ||
| byteSize, | ||
| baseOffset, | ||
| lastOffset, | ||
| logAppendTimestamp, | ||
| -1, | ||
| timestampType | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| // To be used when batchId is not used by the caller | ||
| public BatchInfo batchInfo(TopicIdPartition topicIdPartition) { | ||
| return batchInfo(topicIdPartition, -1L); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "BatchCoordinate[" + objectKey + " -> (" + baseOffset + ", " + lastOffset + ")]"; | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't go to the control plane in case of misses, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, requests to the Control Plane are done only when the DelayedFetch completes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some other broker can append lots of batches without the cache in the local broker being aware of this. It becomes aware only when the local broker produces something to this partition and has to invalidate the cache due to offset mismatch. -- Do I understand this correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes exactly. This means that in theory you can have stale entries up to the TTL (e.g. the last 5 seconds of the log are not visible to a broker because no append was done through it).
This is fine as a best effort for checking if the DelayedFetch can be completed, but when the fetch is actually done the "real" batches will be retrieved from the control plane.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense. I need to stop seeing this cache as very consistent with PG and long-living :)