mikemccand commented on a change in pull request #2052:
URL: https://github.com/apache/lucene-solr/pull/2052#discussion_r519868079
##########
File path: lucene/misc/src/java/org/apache/lucene/store/DirectIODirectory.java
##########
@@ -66,45 +66,32 @@
*
* @lucene.experimental
*/
-public class NativeUnixDirectory extends FSDirectory {
+public class DirectIODirectory extends FSDirectory {
// TODO: this is OS dependent, but likely 512 is the LCD
private final static long ALIGN = 512;
private final static long ALIGN_NOT_MASK = ~(ALIGN-1);
-
- /** Default buffer size before writing to disk (256 KB);
- * larger means less IO load but more RAM and direct
- * buffer storage space consumed during merging. */
-
- public final static int DEFAULT_MERGE_BUFFER_SIZE = 262144;
/** Default min expected merge size before direct IO is
* used (10 MB): */
public final static long DEFAULT_MIN_BYTES_DIRECT = 10*1024*1024;
- private final int mergeBufferSize;
private final long minBytesDirect;
private final Directory delegate;
/** Create a new NIOFSDirectory for the named location.
*
* @param path the path of the directory
- * @param lockFactory to use
- * @param mergeBufferSize Size of buffer to use for
- * merging. See {@link #DEFAULT_MERGE_BUFFER_SIZE}.
* @param minBytesDirect Merges, or files to be opened for
* reading, smaller than this will
* not use direct IO. See {@link
* #DEFAULT_MIN_BYTES_DIRECT}
+ * @param lockFactory to use
* @param delegate fallback Directory for non-merges
* @throws IOException If there is a low-level I/O error
*/
- public NativeUnixDirectory(Path path, int mergeBufferSize, long
minBytesDirect, LockFactory lockFactory, Directory delegate) throws IOException
{
+ public DirectIODirectory(Path path, long minBytesDirect, LockFactory
lockFactory, Directory delegate) throws IOException {
super(path, lockFactory);
- if ((mergeBufferSize & ALIGN) != 0) {
- throw new IllegalArgumentException("mergeBufferSize must be 0 mod " +
ALIGN + " (got: " + mergeBufferSize + ")");
- }
- this.mergeBufferSize = mergeBufferSize;
Review comment:
Hmm but previously it was a 256 KB buffer, by default, and caller could
change that if they wanted.
But with this change, it's now hardwired to something much smaller (512
bytes, or 1 or 4 KB; I'm not sure what "typical" filesystem block sizes are
now?).
This buffering, and its size, is really important when using direct IO
because every write will go straight to the device, so a larger buffer
amortizes the cost of such writes. I think we need to keep the option for
caller to set this buffer size, and leave it at the 256 KB default? Or at
least, let's not try to change that behavior here, and leave this change 100%
focused on moving to pure java implementation?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
