cryptoe commented on code in PR #12987:
URL: https://github.com/apache/druid/pull/12987#discussion_r957245103
##########
processing/src/main/java/org/apache/druid/frame/file/FrameFile.java:
##########
@@ -80,91 +79,173 @@ public class FrameFile implements Closeable
/**
* Delete the opened frame file when all references are closed.
*/
- DELETE_ON_CLOSE,
-
- /**
- * Map using ByteBuffer. Used only for testing.
- */
- BB_MEMORY_MAP,
-
- /**
- * Map using DataSketches Memory. Used only for testing.
- */
- DS_MEMORY_MAP
+ DELETE_ON_CLOSE
}
private final File file;
- private final Memory memory;
+ private final long fileLength;
+ private final Memory footerMemory; // Footer is everything from the final
MARKER_NO_MORE_FRAMES to EOF.
+ private final int maxMmapSize;
private final int numFrames;
private final int numPartitions;
private final ReferenceCountingCloseableObject<Closeable> referenceCounter;
private final Closeable referenceReleaser;
+ /**
+ * Mapped memory, starting from {@link #bufferOffset} in {@link #file}, up
to max of {@link #maxMmapSize}. Acts as
+ * a window on the underlying file. Remapped using {@link
#remapBuffer(long)}, freed using {@link #releaseBuffer()}.
Review Comment:
Do we require benchmarks for this ?
##########
processing/src/main/java/org/apache/druid/frame/file/FrameFile.java:
##########
@@ -63,13 +57,18 @@
*
* - 2 bytes: {@link FrameFileWriter#MAGIC}
* - NNN bytes: sequence of {@link FrameFileWriter#MARKER_FRAME} followed by
one compressed frame (see {@link Frame})
+ * - 1 byte: {@link FrameFileWriter#MARKER_NO_MORE_FRAMES}
Review Comment:
Should we also add a frameVersion byte?
We might add more things to this in the future then things like snapshotting
for fault tolerance becomes tricky
##########
core/src/main/java/org/apache/druid/java/util/common/FileUtils.java:
##########
@@ -164,13 +165,70 @@ public void addFile(File file)
*
* @return a {@link MappedByteBufferHandler}, wrapping a read-only buffer
reflecting {@code file}
*
- * @throws FileNotFoundException if the {@code file} does not exist
- * @throws IOException if an I/O error occurs
+ * @throws FileNotFoundException if the {@code file} does not exist
+ * @throws IOException if an I/O error occurs
+ * @throws IllegalArgumentException if length is greater than {@link
Integer#MAX_VALUE}
* @see FileChannel#map(FileChannel.MapMode, long, long)
*/
public static MappedByteBufferHandler map(File file) throws IOException
{
- MappedByteBuffer mappedByteBuffer = com.google.common.io.Files.map(file);
+ return map(file, 0, file.length());
+ }
+
+ /**
+ * Fully maps a file read-only in to memory as per
+ * {@link FileChannel#map(FileChannel.MapMode, long, long)}.
+ *
+ * @param file the file to map
+ * @param offset starting offset for the mmap
+ * @param length length for the mmap
+ *
+ * @return a {@link MappedByteBufferHandler}, wrapping a read-only buffer
reflecting {@code file}
+ *
+ * @throws FileNotFoundException if the {@code file} does not exist
+ * @throws IOException if an I/O error occurs
+ * @throws IllegalArgumentException if length is greater than {@link
Integer#MAX_VALUE}
+ * @see FileChannel#map(FileChannel.MapMode, long, long)
+ */
+ public static MappedByteBufferHandler map(File file, long offset, long
length) throws IOException
+ {
+ if (length > Integer.MAX_VALUE) {
+ throw new IAE("Cannot map region larger than %,d bytes",
Integer.MAX_VALUE);
+ }
+
+ try (final RandomAccessFile randomAccessFile = new RandomAccessFile(file,
"r");
+ final FileChannel channel = randomAccessFile.getChannel()) {
+ final MappedByteBuffer mappedByteBuffer =
channel.map(FileChannel.MapMode.READ_ONLY, offset, length);
Review Comment:
Nit should we use method at line 220 here ?
--
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]
