zacharymorn commented on a change in pull request #2052: URL: https://github.com/apache/lucene-solr/pull/2052#discussion_r519135013
########## 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: This section was removed since `mergeBufferSize` was no longer being used in the constructors for `DirectIODirectory$DirectIOIndexInput` and `DirectIODirectory$DirectIOIndexOutput` (previously passed in to fill the `bufferSize` constructor parameter), as those are now replaced by the dynamic block size allocation: ``` blockSize = Math.toIntExact(Files.getFileStore(path).getBlockSize()); bufferSize = Math.addExact(blockSize, blockSize - 1); ``` I assume the new bufferSize being 2 x dynamic blockSize will still provide that buffering? ---------------------------------------------------------------- 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