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 opaque 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) If we don't want to do that, then the hint -> ReadAdvice mapping would indeed need to be on the IOContext, or we use some sort of lambda passed in to the relevant Directory constructor to override it, which feels kinda hacky -- 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