mayya-sharipova commented on code in PR #16794:
URL: https://github.com/apache/lucene/pull/16794#discussion_r4261590247
##########
lucene/core/src/java/org/apache/lucene/codecs/hnsw/HnswGraphProvider.java:
##########
@@ -29,6 +29,9 @@ public interface HnswGraphProvider {
/**
* Return the stored HnswGraph for the given field.
*
+ * <p>Each call returns a new instance. An instance is not thread-safe, but
separate instances can
+ * be read concurrently, so callers that read the graph from several threads
get one per thread.
+ *
Review Comment:
This is a new requirement that is true for the Lucene codebase.
I wonder if we should assert this somehow:
may be: HnswConcurrentMergeBuilder:joinGraphs
```
for (ConcurrentMergeWorker worker : workers) {
worker.joinedGraph = graphs.get(g).get();
}
The assert
for (int w = 0; w < workers.length; w++) {
HnswGraph graph = graphs.get(g).get();
assert isNewInstance(graph, w)
: "HnswGraphProvider#getGraph must return a new instance on every
call";
workers[w].joinedGraph = graph;
}
/** Whether no worker before {@code upTo} already reads {@code graph}. */
private boolean isNewInstance(HnswGraph graph, int upTo) {
for (int i = 0; i < upTo; i++) {
if (workers[i].joinedGraph == graph) { // reference comparison
return false;
}
}
return true;
}
```
Or any other cheap assertion you can think of?
--
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]
