thecoop commented on code in PR #14482: URL: https://github.com/apache/lucene/pull/14482#discussion_r2057921646
########## lucene/core/src/java/org/apache/lucene/store/Directory.java: ########## @@ -79,6 +83,31 @@ public abstract class Directory implements Closeable { */ public abstract long fileLength(String name) throws IOException; + protected void validateIOContext(IOContext context) { + Map<Class<? extends IOContext.FileOpenHint>, List<IOContext.FileOpenHint>> hintClasses = + context.hints().stream().collect(Collectors.groupingBy(IOContext.FileOpenHint::getClass)); + + // there should only be one of FileType, FileData, DataAccess + List<IOContext.FileOpenHint> fileTypes = + hintClasses.getOrDefault(FileTypeHint.class, List.of()); + if (fileTypes.size() > 1) { + throw new IllegalArgumentException("Multiple file type hints specified: " + fileTypes); + } + List<IOContext.FileOpenHint> fileData = hintClasses.getOrDefault(FileDataHint.class, List.of()); + if (fileData.size() > 1) { + throw new IllegalArgumentException("Multiple file data hints specified: " + fileData); + } + List<IOContext.FileOpenHint> dataAccess = + hintClasses.getOrDefault(DataAccessHint.class, List.of()); + if (dataAccess.size() > 1) { + throw new IllegalArgumentException("Multiple data access hints specified: " + dataAccess); + } + } + + protected ReadAdvice toReadAdvice(IOContext context) { Review Comment: The `MMapDirectory` wouldn't call the overridden `toReadAdvice`, as thats on a different class, not in the same type hierarchy. Hmm... One way to think of it might be that trying to set a specific ReadAdvice is wrong. You should instead be setting the correct hints _such that_ the directory chooses the behaviour you want. So rather than setting `ReadAdvice.NORMAL` directly, you would specify hints to make MMapDirectory select the `ReadAdvice` that you want - or whatever else MMapDirectory chooses to do to follow the hints you've specified. The `ReadAdvice` is entirely hidden within the Directory implementation, and you can _only_ set hints. After all, you're specifying a specific read advice for a reason - if you encode the reason (as hints), rather than the specific behaviour that you want, then the directory can choose the best way to do that according to the directory implementation, which may or may not be using certain ReadAdvices (or any other setting it chooses). So the code would be: ``` Directory dir = new FilterDirectory(mmapDirectory) { @Override public IndexInput openInput(String name, IOContext context) { if (someCondition()) { // be more specific in what this file is being opened for, // MMapDirectory chooses what to do with this info context = context.withHints(FileDataHint.VECTORS, DataAccessHint.RANDOM); } return in.openInput(name, context); } } ``` If we don't want to do that, then the hint -> ReadAdvice mapping would indeed need to be on the IOContext, and _not_ the responsibility of the `Directory`; or we use some sort of lambda override similar to `setPreload` -- 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: issues-unsubscr...@lucene.apache.org 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