Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not assume node is out of bounds in deserializer #3044

Merged
merged 1 commit into from May 24, 2023

Conversation

trengrj
Copy link
Member

@trengrj trengrj commented May 19, 2023

What's being changed:

The HNSW deserializer assumed if id > len(res.Nodes) then the node was out of bounds. However len(res.Nodes) is just set initially to the initialSize of the HNSW index. With the reduction of initialSize in #3005 this triggered Test_NoRace_ManySmallCommitlogs to become flaky with too many links. This PR changes the deserializer to run growIndexToAccomodateNode in these conditions.

Review checklist

  • Documentation has been updated, if necessary. Link to changed documentation:
  • Chaos pipeline run or not necessary. Link to pipeline:
  • All new code is covered by tests where it is reasonable.
  • Performance tests have been run or not necessary.

@trengrj trengrj requested a review from etiennedi May 19, 2023 10:44
@trengrj
Copy link
Member Author

trengrj commented May 19, 2023

Note the best way to trigger the flaky test Test_NoRace_ManySmallCommitlogs is to set n := 100000. I have left at 10000 for performance reasons but we may want a chaos pipeline where we check links after many inserts and deletes.

@trengrj trengrj force-pushed the hnsw-deserialization-links branch from 0d4c8e0 to 261e5a4 Compare May 22, 2023 02:10
@trengrj trengrj changed the base branch from master to stable/v1.19 May 22, 2023 02:10
@trengrj
Copy link
Member Author

trengrj commented May 22, 2023

Updated to target stable/v1.19

@trengrj trengrj merged commit 16c4ce9 into stable/v1.19 May 24, 2023
12 checks passed
@trengrj trengrj deleted the hnsw-deserialization-links branch December 6, 2023 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants