aweisberg commented on code in PR #4606:
URL: https://github.com/apache/cassandra/pull/4606#discussion_r2897530598
##########
src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java:
##########
@@ -1417,44 +1419,60 @@ public StatsMetadata getSSTableMetadata()
return sstableMetadata;
}
+ public RandomAccessReader openDataReader()
+ {
+ return openDataReaderInternal(null, null, false);
+ }
+
public RandomAccessReader openDataReader(RateLimiter limiter)
{
assert limiter != null;
- return dfile.createReader(limiter);
+ return openDataReaderInternal(null, limiter, false);
}
- public RandomAccessReader openDataReader()
+ public RandomAccessReader openDataReader(DiskAccessMode diskAccessMode)
{
- return dfile.createReader();
+ return openDataReaderInternal(diskAccessMode, null, false);
}
public RandomAccessReader openDataReaderForScan()
{
- return openDataReaderForScan(dfile.diskAccessMode());
+ return openDataReaderInternal(null, null, true);
}
public RandomAccessReader openDataReaderForScan(DiskAccessMode
diskAccessMode)
{
- boolean isSameDiskAccessMode = diskAccessMode ==
dfile.diskAccessMode();
- boolean isDirectIONotSupported = diskAccessMode ==
DiskAccessMode.direct && !dfile.supportsDirectIO();
+ return openDataReaderInternal(diskAccessMode, null, true);
+ }
- if (isSameDiskAccessMode || isDirectIONotSupported)
- return dfile.createReaderForScan(OnReaderClose.RETAIN_FILE_OPEN);
+ private RandomAccessReader openDataReaderInternal(@Nullable DiskAccessMode
diskAccessMode,
+ @Nullable RateLimiter
limiter,
+ boolean forScan)
+ {
+ if (canReuseDfile(diskAccessMode))
+ return dfile.createReader(limiter, forScan,
OnReaderClose.RETAIN_FILE_OPEN);
- FileHandle dataFile = dfile.toBuilder()
- .withDiskAccessMode(diskAccessMode)
- .complete();
+ FileHandle handle = dfile.toBuilder()
+ .withDiskAccessMode(diskAccessMode)
+ .complete();
try
{
- return dataFile.createReaderForScan(OnReaderClose.CLOSE_FILE);
+ return handle.createReader(limiter, forScan,
OnReaderClose.CLOSE_FILE);
}
catch (Throwable t)
{
- dataFile.close();
+ handle.close();
throw t;
}
}
+ private boolean canReuseDfile(@Nullable DiskAccessMode diskAccessMode)
+ {
+ return diskAccessMode == null
+ || diskAccessMode == dfile.diskAccessMode()
+ || (diskAccessMode == DiskAccessMode.direct &&
!dfile.supportsDirectIO());
Review Comment:
Rate limiting should make the noise palatable. If they configure direct IO
enabled and we don't actually do it then I think people would want to know.
There is some plumbing to make doing this easy.
If they enable direct IO and the sstables are uncompressed then it seems
like we should log in that case because they don't know their config doesn't
support direct IO even though their filesystem does. In that case we would even
want a log message that tells them that this is the exact issue.
That said there is a config mismatch between compression and direct IO since
one is table level and the other is process wide. So... I don't think we should
log in that case, but in other cases where we expect it to work and it doesn't
it should be logged.
--
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]
