Pulkitg64 commented on PR #15003: URL: https://github.com/apache/lucene/pull/15003#issuecomment-3188381701
> I am confused! This PR suddenly got so much simpler, which is great, Yeah, the ```initGraph``` implementation in `InitializedHnswGraphBuilder.java` simplifies lot of things for us as it is already providing support of creating OnHeapHnswGraph by passing OffHeapGraph from the older segment. > we are no longer checking the largest graph to see if its delete % is below a threshold? Yes, in the first revision I added the arbitrary percentage without doing any testing. But this time, I wanted to see the impact of mergePolicy that kicks in when delete % is higher than certain threshold. I thought we may not need to add explicit check of checking delete % of largest graph because merge policy will automatically take care of this. > Also I think we are now ignoring the various edge cases around upper-level graph layers possibly becoming empty? ```initGraph``` implementation takes care of it. In the implementation we start with top level and if there is no live node in that level the new entry node is never set and when we will iterate to next level with some live nodes there we will set the new entry node. Hence in this way we remove the risk of empty upper layer in the graph. But on the other there is still risk of completely deleting middle layer which we need to take care I believe. -- 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
