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.

Sign up for GitHub

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

Jump to bottom

Improved cleaning of tombstones #4003

Merged
merged 48 commits into from
Feb 15, 2024
Merged

Improved cleaning of tombstones #4003

merged 48 commits into from
Feb 15, 2024

Conversation

abdelr
Copy link
Contributor

@abdelr abdelr commented Jan 11, 2024 •
edited

What's being changed:

Currently, we iterate over all nodes to find those connected to the removed nodes. Additionally, we search for the closest neighbours by performing a complete search on the graph.

This PR introduces two main changes. First, it rebuilds the section of the graph by only inferring the connections in the neighbourhood of the removed nodes based on the information available in the graph, rather than performing a search.

Second, it runs the repair of the nodes concurrently since no search is need any longer.

Review checklist

@abdelr abdelr self-assigned this Jan 11, 2024
@abdelr abdelr marked this pull request as draft January 11, 2024 11:36
@abdelr
fix infinite recursion
800f79a
@abdelr
fixing deadlock
32668f7
@abdelr
fixing a bug in recursive processor
8e91427
@abdelr
no elements make it to the list if they are farer from the search point
4b9b678
@abdelr
Avoid panic
fab1102
@abdelr
Some extra fixes
10ef7d1
@abdelr
Merge branch 'fix/sharded-locks' into improved_cleaning_of_tombstones
2963dc4
@abdelr
last reverts
99917e0
@abdelr
Revert "Merge branch 'fix/sharded-locks' into improved_cleaning_of_to…
42ed5ed 
…mbstones"

This reverts commit 2963dc4, reversing
changes made to 10ef7d1.
@abdelr
remove unnecessary check
bbc4b8a
@abdelr
some extra cleaning
fd41f08
@abdelr
Merge branch 'master' into improved_cleaning_of_tombstones
51cb43a
@abdelr
pooling visited for processRecursively
658fef7
@abdelr
fixing double iteration
2eba1d9
@abdelr
naive multithreading
ba55dbf
@abdelr
Merge branch 'master' into improved_cleaning_of_tombstones
75ed62a
@abdelr
Actually multithreading
f5f5868
@abdelr
Fixing recall regression
7b4552f
@abdelr
Adding a test for maximumConnectionsLayerZero
fd57c48
@abdelr
Fixing tests
93a9710
@abdelr
Merge pull request #4079 from weaviate/algorithmic_change_cleaning_of…
9fba622 
…_tombstones

Fixing recall regression
trengrj
trengrj reviewed Jan 29, 2024
View reviewed changes
adapters/repos/db/vector/hnsw/neighbor_connections.go Outdated Show resolved Hide resolved
adapters/repos/db/vector/hnsw/neighbor_connections.go Outdated Show resolved Hide resolved
adapters/repos/db/vector/hnsw/search.go Show resolved Hide resolved
adapters/repos/db/vector/hnsw/delete.go Outdated Show resolved Hide resolved
asdine and others added 23 commits February 6, 2024 14:01
@asdine
configurable concurrency
1fabfb2
@asdine
add cancelable context to hnsw
3418b3f
@asdine
pass context to neighborFinderConnector
ce9ac17
@asdine
fix concurrency logic
4a88cf2
@asdine
reduce commit-log lock contention
5ef32b6
@asdine
gofumpt file
27b6ac4
@asdine
remove custom bufio writer implementation
b642153
@asdine
close the logger after background activity is done
7f25668
@asdine
remove leftover debug lines
a484795
@asdine
Merge branch 'improved_cleaning_of_tombstones' into commit-log-lock-c…
471deca 
…ontention
@abdelr
using sharded locks instead of full locking the index
60dd4ae
@abdelr
Merge pull request #4149 from weaviate/commit-log-lock-contention
a386e31 
reduce commit log lock contention
@asdine
Merge remote-tracking branch 'origin/master' into improved_cleaning_o…
9312d79 
…f_tombstones
@abdelr
renaming function for better code readability
52bcc26
@abdelr
Renaming parameter for better code quality
0c48601
@abdelr
fixing bug introduced
d9a8979
@abdelr
restoring also the nodeVec
e722d6e
@abdelr
Fixing test
17d4351
@asdine
Merge remote-tracking branch 'origin/master' into improved_cleaning_o…
7a13543 
…f_tombstones
@asdine
use write-only sharded locks
5b16d57
@asdine
Revert "reduce commit-log lock contention"
bcdce09 
This reverts commit 5ef32b6.
@asdine
Revert "remove custom bufio writer implementation"
709103b 
This reverts commit b642153.
@asdine
Revert "close the logger after background activity is done"
b95d5dd 
This reverts commit 7f25668.
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Feb 14, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
14.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@trengrj trengrj marked this pull request as ready for review February 15, 2024 02:09
@parkerduckworth parkerduckworth merged commit a001ce5 into master Feb 15, 2024
31 of 33 checks passed
@parkerduckworth parkerduckworth deleted the improved_cleaning_of_tombstones branch February 15, 2024 18:54
This was referenced Feb 21, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trengrj trengrj trengrj approved these changes

@etiennedi etiennedi Awaiting requested review from etiennedi

@aliszka aliszka Awaiting requested review from aliszka

@asdine asdine Awaiting requested review from asdine

@CShorten CShorten Awaiting requested review from CShorten

Assignees

@abdelr abdelr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@abdelr @trengrj @parkerduckworth @asdine