-
Notifications
You must be signed in to change notification settings - Fork 998
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
[core] Separate index cache and data cache #4438
Changes from 2 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 |
---|---|---|
|
@@ -24,25 +24,30 @@ | |
/** Key for cache manager. */ | ||
public interface CacheKey { | ||
|
||
static CacheKey forPosition(RandomAccessFile file, long position, int length) { | ||
return new PositionCacheKey(file, position, length); | ||
static CacheKey forPosition(RandomAccessFile file, long position, int length, boolean index) { | ||
return new PositionCacheKey(file, position, length, index); | ||
} | ||
|
||
static CacheKey forPageIndex(RandomAccessFile file, int pageSize, int pageIndex) { | ||
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. Can we remove this method? The invoker should always set the 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. But for the |
||
return new PageIndexCacheKey(file, pageSize, pageIndex); | ||
return new PageIndexCacheKey(file, pageSize, pageIndex, false); | ||
} | ||
|
||
/** @return Whether this cache key is for index cache. */ | ||
boolean isIndex(); | ||
|
||
/** Key for file position and length. */ | ||
class PositionCacheKey implements CacheKey { | ||
|
||
private final RandomAccessFile file; | ||
private final long position; | ||
private final int length; | ||
private final boolean index; | ||
|
||
private PositionCacheKey(RandomAccessFile file, long position, int length) { | ||
private PositionCacheKey(RandomAccessFile file, long position, int length, boolean index) { | ||
this.file = file; | ||
this.position = position; | ||
this.length = length; | ||
this.index = index; | ||
} | ||
|
||
@Override | ||
|
@@ -56,12 +61,18 @@ public boolean equals(Object o) { | |
PositionCacheKey that = (PositionCacheKey) o; | ||
return position == that.position | ||
&& length == that.length | ||
&& index == that.index | ||
&& Objects.equals(file, that.file); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(file, position, length); | ||
return Objects.hash(file, position, length, index); | ||
} | ||
|
||
@Override | ||
public boolean isIndex() { | ||
return index; | ||
} | ||
} | ||
|
||
|
@@ -71,17 +82,25 @@ class PageIndexCacheKey implements CacheKey { | |
private final RandomAccessFile file; | ||
private final int pageSize; | ||
private final int pageIndex; | ||
private final boolean index; | ||
|
||
private PageIndexCacheKey(RandomAccessFile file, int pageSize, int pageIndex) { | ||
private PageIndexCacheKey( | ||
RandomAccessFile file, int pageSize, int pageIndex, boolean index) { | ||
this.file = file; | ||
this.pageSize = pageSize; | ||
this.pageIndex = pageIndex; | ||
this.index = index; | ||
} | ||
|
||
public int pageIndex() { | ||
return pageIndex; | ||
} | ||
|
||
@Override | ||
public boolean isIndex() { | ||
return index; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
|
@@ -93,12 +112,13 @@ public boolean equals(Object o) { | |
PageIndexCacheKey that = (PageIndexCacheKey) o; | ||
return pageSize == that.pageSize | ||
&& pageIndex == that.pageIndex | ||
&& index == that.index | ||
&& Objects.equals(file, that.file); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(file, pageSize, pageIndex); | ||
return Objects.hash(file, pageSize, pageIndex, index); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,7 +49,6 @@ public BlockCache(RandomAccessFile file, CacheManager cacheManager) { | |
this.blocks = new HashMap<>(); | ||
} | ||
|
||
// TODO separate index and data cache | ||
private byte[] readFrom(long offset, int length) throws IOException { | ||
byte[] buffer = new byte[length]; | ||
int read = channel.read(ByteBuffer.wrap(buffer), offset); | ||
|
@@ -61,9 +60,9 @@ private byte[] readFrom(long offset, int length) throws IOException { | |
} | ||
|
||
public MemorySegment getBlock( | ||
long position, int length, Function<byte[], byte[]> decompressFunc) { | ||
long position, int length, Function<byte[], byte[]> decompressFunc, boolean index) { | ||
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. May be isIndex? 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. fixed |
||
|
||
CacheKey cacheKey = CacheKey.forPosition(file, position, length); | ||
CacheKey cacheKey = CacheKey.forPosition(file, position, length, index); | ||
|
||
SegmentContainer container = blocks.get(cacheKey); | ||
if (container == null || container.getAccessCount() == CacheManager.REFRESH_COUNT) { | ||
|
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.
See https://nightlies.apache.org/flink/flink-docs-release-1.20/docs/deployment/config/#rocksdb-state-backend
Can we just introduce an option like
state.backend.rocksdb.memory.high-prio-pool-ratio
?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.
Make sense.