zhaih commented on PR #12257: URL: https://github.com/apache/lucene/pull/12257#issuecomment-1546540855
> would be ok for you to try to get involved in the other contribution, trying to align your intent with the guy in there? I'll for sure trying to review that PR, but that one is trying to introduce a new data structure while this one is actually trying to add a quick workaround for `HnswGraph`'s API (which discourage thread-safe), there's no conflict from the beginning IMO. > is there any reason you believe the contribution from @zhaih to not be an improvement? In my understanding @jimczi is trying to say this change some what encourages the use of `OnHeapHnswGraph`, which originally is only used to build the graph in memory and shouldn't be used outside. I'm ok with that, but to be fair: **this PR is not trying to promote the use of that, it is trying to solve a potential issue in the current trunk.** And the real factor that encourages the use is: 1. It is a public class, even without `lucene.internal` tag 2. All related tools (Builder, Searcher) are public available 3. There's already an example of using it being recently added and no people are against it. I'm not sure what are all the solutions are, but stopping this PR won't solve any of the problems above, but will only make the thread-safety problem stays in the trunk longer. -- 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
