ChrisHegarty commented on code in PR #16749:
URL: https://github.com/apache/lucene/pull/16749#discussion_r4144167001
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99FlatVectorsReader.java:
##########
@@ -312,16 +366,30 @@ public RandomVectorScorer getRandomVectorScorer(String
field, short[] target) th
target);
}
+ /**
+ * Closes the mapping a merge used, once no merge instance holds it. A later
merge maps the file
+ * again.
+ */
@Override
public void finishMerge() throws IOException {
- // This makes sure that the access pattern hint is reverted back since
HNSW implementation
- // needs it
- vectorData.updateIOContext(dataContext);
+ original.releaseMergeVectorData();
+ }
+
+ private synchronized void releaseMergeVectorData() throws IOException {
+ assert original == this;
+ if (--mergeInstances > 0) {
+ return;
+ }
+ if (mergeVectorData != null && mergeVectorData != vectorData) {
+ mergeVectorData.close();
+ }
+ mergeVectorData = null;
}
@Override
public void close() throws IOException {
- IOUtils.close(vectorData);
+ IOUtils.close(
+ vectorData, original == this && mergeVectorData != vectorData ?
mergeVectorData : null);
Review Comment:
`close()` reads `mergeVectorData` (and implicitly relies on original)
without synchronizing, but `mergeVectorData()`/`releaseMergeVectorData()` both
mutate it under synchronized (on `original`). There's no happens-before edge
guaranteeing `close()` sees the latest value if it races with a merge finishing.
Rather than making all of close() synchronized, maybe pulling just the read
into a small synchronized accessor:
```java
private synchronized IndexInput mergeVectorDataToClose() {
return original == this && mergeVectorData != vectorData ?
mergeVectorData : null;
}
@Override
public void close() throws IOException {
IOUtils.close(vectorData, mergeVectorDataToClose());
}
```
That keeps the actual close I/O outside the lock while making the field read
safe.
--
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]
