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). 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 it
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 passed in to the relevant Directory
constructor to do that mapping, 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
