TestLucene95HnswVectorsFormat.testRandomWithUpdatesAndGraph fails with duplicate neighbor

[msokolov]

Description

gradlew test --tests TestLucene95HnswVectorsFormat.testRandomWithUpdatesAndGraph -Dtests.seed=D50703366970158A -Dtests.multiplier=3 -Dtests.locale=khq-Latn-ML -Dtests.timezone=Arctic/Longyearbyen -Dtests.asserts=true -Dtests.file.encoding=UTF-8

reproduces for me, giving

+org.apache.lucene.index.CheckIndex$CheckIndexException: Field "field" has repeated neighbors of node 6 with value 192
	at __randomizedtesting.SeedInfo.seed([D50703366970158A:ABDAC62BDE1BBDE9]:0)
	at app/org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.index.CheckIndex.testHnswGraph(CheckIndex.java:3035)
	at app/org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.index.CheckIndex.testHnswGraphs(CheckIndex.java:2919)
	at app/org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.index.CheckIndex.testSegment(CheckIndex.java:1126)
	at app/org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.index.CheckIndex.checkIndex(CheckIndex.java:823)
	at app/org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.index.CheckIndex.checkIndex(CheckIndex.java:593)
	at app/org.apache.lucene.test_framework@11.0.0-SNAPSHOT/org.apache.lucene.tests.util.TestUtil.checkIndex(TestUtil.java:333)
	at app/org.apache.lucene.test_framework@11.0.0-SNAPSHOT/org.apache.lucene.tests.store.MockDirectoryWrapper.close(MockDirectoryWrapper.java:917)
	at app/org.apache.lucene.test_framework@11.0.0-SNAPSHOT/org.apache.lucene.tests.index.BaseKnnVectorsFormatTestCase.testRandomWithUpdatesAndGraph(BaseKnnVectorsFormatTestCase.java:1615)

Version and environment details

No response

[msokolov]

If I revert ef10476 the test passes. I don't think we should revert, since this is a pretty benign condition, but we should figure out how to fix it

[msokolov]

It seems top be happening in the repairDisconnectNodes operation; when adding the nodes, NeighborArray.addOutOfOrder doesn't check to see if the node is already present in the neighbor list -- this is when adding the reverse link.

I wonder if in this context we should skip the adding of reverse links? We could have a version of addDiverseNeighbors that only adds the forward link.

[msokolov]

see #15470