fix: HNSW schema deleting document crash #4987
Conversation
src/server/search/doc_index.cc
Outdated
| auto id = key_index_.Remove(key); | ||
| if (id) { | ||
| indices_->Remove(id.value(), *accessor); | ||
| try { |
There was a problem hiding this comment.
not sure I understand. What call exactly throws?
There was a problem hiding this comment.
now I understand, I think this should be improved.
There was a problem hiding this comment.
The exception occurs inside the library:
third_party/libs/hnswlib/include/hnswlib/hnswalg.h
There was a problem hiding this comment.
yes, but it's a legitimate flow. so we should protect ourselves and there is no reason for WARNING.
So at least I would move throw/catch into dfly::search::HnswlibAdapter::Remove and remove any logs
| auto res = Run({"HSET", "doc", "n", "0"}); | ||
| EXPECT_EQ(res, 1); | ||
|
|
||
| res = Run({"HSET", "doc", "n", "1"}); |
There was a problem hiding this comment.
Since it's during removal, I assume if we call DEL doc it would achieve similar effect? If yes, it's a clearer test, imho.
There was a problem hiding this comment.
Yes, correct. I checked it. DEL doc archives the same effect. I will change the test.
vyavdoshenko
changed the title
Signed-off-by: Volodymyr Yavdoshenko <volodymyr@dragonflydb.io>
The scenario that caused the crash:
fixes: #4979