divijvaidya commented on code in PR #12331:
URL: https://github.com/apache/kafka/pull/12331#discussion_r912790203
##########
core/src/main/scala/kafka/log/AbstractIndex.scala:
##########
@@ -108,32 +107,42 @@ abstract class AbstractIndex(@volatile private var _file:
File, val baseOffset:
@volatile
protected var mmap: MappedByteBuffer = {
val newlyCreated = file.createNewFile()
- val raf = if (writable) new RandomAccessFile(file, "rw") else new
RandomAccessFile(file, "r")
+
+ val options = if (writable) List(StandardOpenOption.READ,
StandardOpenOption.WRITE, StandardOpenOption.SPARSE) else
List(StandardOpenOption.READ, StandardOpenOption.SPARSE)
Review Comment:
You don't need StandardOpenOption.SPARSE for read-only mode since that
option is only applicable as a hint to operating system when creating new
files. see:
https://docs.oracle.com/javase/7/docs/api/java/nio/file/StandardOpenOption.html#SPARSE
##########
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##########
@@ -1432,4 +1425,21 @@ public static String[] enumOptions(Class<? extends
Enum<?>> enumClass) {
.toArray(String[]::new);
}
+ /**
+ * Creates a preallocated file
Review Comment:
Please add a note that the responsibility to free up resource i.e. close the
FileChannel is transferred to the caller of this function.
##########
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##########
@@ -1432,4 +1425,21 @@ public static String[] enumOptions(Class<? extends
Enum<?>> enumClass) {
.toArray(String[]::new);
}
+ /**
+ * Creates a preallocated file
+ * @param path File path
+ * @param size The size used for pre allocate file, for example 512 * 1025
*1024
+ */
+ public static FileChannel createPreallocatedFile(Path path, int size)
throws IOException {
Review Comment:
My suggestion:
1. Create another function preallocatedFile() which will not create a new
file but instead it will just preallocate the file i.e. it won't have
`StandardOpenOption.CREATE_NEW`
2. Refactor the code such that there is no code duplication between
`createPreallocatedFile` and `preallocatedFile`.
3. You can now use this new `preallocatedFile` function to replace
AbstractIndex.scala, lines 121-126 and lines 195-200.
--
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]
