zeronerdzerogeekzerocool commented on code in PR #19662:
URL: https://github.com/apache/pinot/pull/19662#discussion_r4161640384
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/SegmentLocalFSDirectoryTest.java:
##########
@@ -121,6 +122,68 @@ public void testWriteAndReadBackData()
}
}
+ private static SegmentDirectoryLoaderContext prefetchLoaderContext(long
maxMmapPrefetchBytes) {
+ return new SegmentDirectoryLoaderContext.Builder()
+ .setMaxMmapPrefetchBytes(maxMmapPrefetchBytes)
+ .build();
+ }
+
+ /// Prefetching is bounded by a JVM-wide page counter that tests cannot
reset, so rather than asserting on how many
+ /// pages got faulted in, these cases pin down the config contract: the byte
limit is accepted, zero disables
+ /// prefetching, and reads stay correct either way.
+ @Test
+ public void testPrefetchLimitDisabledStillReadsData()
+ throws Exception {
+ File prefetchDir = new File(SegmentLocalFSDirectoryTest.class.getName() +
"-prefetch_disabled");
+ FileUtils.deleteQuietly(prefetchDir);
+ try {
+ FileUtils.copyDirectory(_segmentDirectory.getPath().toFile(),
prefetchDir);
+ // 0 bytes disables prefetching entirely
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(prefetchDir, _metadata,
+ ReadMode.mmap, prefetchLoaderContext(0))) {
+ try (SegmentDirectory.Writer writer = segmentDirectory.createWriter())
{
+ PinotDataBuffer buffer = writer.newIndexFor("noPrefetchColumn",
StandardIndexes.forward(), 1024);
+ loadData(buffer);
+ writer.save();
+ }
+ try (SegmentDirectory.Reader reader = segmentDirectory.createReader())
{
+ verifyData(reader.getIndexFor("noPrefetchColumn",
StandardIndexes.forward()));
+ }
+ }
+ } finally {
+ FileUtils.deleteQuietly(prefetchDir);
+ }
+ }
+
+ @Test
+ public void testSmallPrefetchLimitStillReadsData()
+ throws Exception {
+ File prefetchDir = new File(SegmentLocalFSDirectoryTest.class.getName() +
"-prefetch_small");
+ FileUtils.deleteQuietly(prefetchDir);
+ try {
+ FileUtils.copyDirectory(_segmentDirectory.getPath().toFile(),
prefetchDir);
+ // 8KB, i.e. a 2 page budget: exercises the slowdown branch that only
faults in header pages
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(prefetchDir, _metadata,
+ ReadMode.mmap, prefetchLoaderContext(8 * 1024))) {
+ try (SegmentDirectory.Writer writer = segmentDirectory.createWriter())
{
+ PinotDataBuffer buffer = writer.newIndexFor("smallPrefetchColumn",
StandardIndexes.forward(), 1024);
+ loadData(buffer);
+ writer.save();
+ }
+ try (SegmentDirectory.Reader reader = segmentDirectory.createReader())
{
Review Comment:
Fixed in f108cab
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SegmentLocalFSDirectory.java:
##########
@@ -299,6 +300,17 @@ public void close()
}
}
+ private static long toPrefetchPages(long maxMmapPrefetchBytes) {
+ Preconditions.checkArgument(maxMmapPrefetchBytes >= 0, "Max mmap prefetch
bytes must be non-negative, got: %s",
+ maxMmapPrefetchBytes);
+ return maxMmapPrefetchBytes / PAGE_SIZE_BYTES;
+ }
+
+ private static long getMaxMmapPrefetchBytes(@Nullable
SegmentDirectoryLoaderContext segmentDirectoryLoaderContext) {
Review Comment:
Fixed in f108cab
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
