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 _for a
particular IOContext for each slice_. 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.
Additionally, 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]
