thecoop commented on code in PR #16883:
URL: https://github.com/apache/lucene/pull/16883#discussion_r4358701170
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90CompoundReader.java:
##########
@@ -176,28 +182,50 @@ public IndexInput openInput(String name, IOContext
context) throws IOException {
+ entries.keySet()
+ ")");
}
- IndexInput data = context.context() == IOContext.Context.MERGE ?
mergeHandle() : handle;
- return data.slice(name, entry.offset, entry.length, context);
+ synchronized (this) {
+ Mapping mapping = mappingFor(id, context);
+ IndexInput slice = mapping.data.slice(name, entry.offset, entry.length,
context);
+ mapping.firstOpens.putIfAbsent(id, slice);
Review Comment:
Right, after many minutes staring at this, I think I've got it.
Each mapping stores the 'parent' handle and any open slices _each with a
separate IOContext_. The issue is that the `IOContext` each mapping is for is
_implicit_, due to the opaqueness of `isEfficientFor`. It's additionally
confusing because `Mapping` is a `record`, which implies that the `IndexInput`
is somehow part of its identity, but it's actually got nothing to do with the
`IOContext` that that specific mapping is for, it's just stored for re-use for
creating more slices, because the same pages of the same `IndexInput` can't be
re-mapped with a different context.
On top of this, each `Mapping` can have slices for _different_ inner files
open with _different_ contexts, because it just depends on in which order the
contexts are opened for a specific inner file.
To say this is confusing is an understatement.
Firstly, it shouldn't be a record. It doesn't depend on record semantics,
and it implies that the IndexInput matters for the 'identity' of the Mapping.
Secondly, this needs to be keyed, somehow, on what is actually being
separated here - the inner files. I suggest the following:
There's a `Map<String, List<Slice>>` mapping each inner file to the
collection of *slices* for that file. `Slice` is a tuple of the slice
IndexInput and the 'parent' `IndexInput` it was created from. When opening, the
slices are searched for one that `isEfficientFor` the context.
If one is not found, then a separate *pool* of top-level `IndexInput`s are
searched to find an `IndexInput` that has not already been used to open a slice
for that inner file (existing ones can be recorded during first iteration of
the `List<Slice>`), opening a new one if necessary, and that is used to create
a `Slice` for that file, and recorded in that file's list.
This way, the key is the thing we're actually trying to open, and the
relationship between the source IndexInputs is not embedded into the search
structure, but is kept separate from the slices.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
