zhaih commented on code in PR #12246:
URL: https://github.com/apache/lucene/pull/12246#discussion_r1181173636
##########
lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/Word2VecSynonymProvider.java:
##########
@@ -64,7 +65,15 @@ public Word2VecSynonymProvider(Word2VecModel model) throws
IOException {
this.hnswGraph = builder.build(word2VecModel.copy());
}
- public List<TermAndBoost> getSynonyms(
+ /**
+ * Returns the list of synonyms of a provided term. This method is
synchronized because it uses
+ * the {@link org.apache.lucene.util.hnsw.OnHeapHnswGraph} that is not
thread-safe.
+ *
+ * @param term term to search to find synonyms
+ * @param maxSynonymsPerTerm limit of synonyms returned
+ * @param minAcceptedSimilarity lower similarity threshold to consider
another term as synonym
+ */
+ public synchronized List<TermAndBoost> getSynonyms(
Review Comment:
> Maybe adding this synchronized block on the graph, may do the trick?
I don't think this will work.. since the part that the onHeapHnswGraph
violates the thread-safety is that it uses internal `cur` and `upto` to
remember the seek state
([here](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/util/hnsw/OnHeapHnswGraph.java#L126)).
So even add an sync block inside won't really help, because one thread can
still set `cur` to some node while the other set it to another value after the
first thread exit the sync block.
But if I didn't miss anything, I think make the graph thread safe to search
is not hard, the current thread-unsafe code is:
```
@Override
public void seek(int level, int targetNode) {
cur = getNeighbors(level, targetNode);
upto = -1;
}
@Override
public int nextNeighbor() {
if (++upto < cur.size()) {
return cur.node[upto];
}
return NO_MORE_DOCS;
}
```
What we need to do probably is just simply not using those two member
methods of the graph but mimic the behavior in the search process. (Basically
using some external local variable instead of `cur` and `upto`)
--
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
