Skip to content

Remove dictionary rows when no document is referring to it#1314

Open
tanzhang0504 wants to merge 3 commits intoblevesearch:masterfrom
tanzhang0504:leak_fix
Open

Remove dictionary rows when no document is referring to it#1314
tanzhang0504 wants to merge 3 commits intoblevesearch:masterfrom
tanzhang0504:leak_fix

Conversation

@tanzhang0504
Copy link
Copy Markdown

Hi - we have a project that uses bleve in-memory index based on gtreap implementation. We noticed a similar behavior that dictionary rows keep grow even after all the documents referring to them are deleted. I found #374, and seems that someone made a temporary only to leveldb, which does not apply to bleve in-memory index use-case.

So I've made this PR to systematically fix #374 for all different index store implementations. Looking forward to your feedback!

doc := document.NewDocument("1")
doc.AddField(document.NewTextField("name", []uint64{}, []byte("test")))
err = idx.Update(doc)
doc.AddField(document.NewTextField("name1", []uint64{}, []byte("test1")))
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I enhanced the test to include one index that's shared by two documents, and the other is not. I'm also happy to revert to the original test setup if you think this hurts readability.

@tmm1
Copy link
Copy Markdown

tmm1 commented Nov 26, 2019

Similar issue was discussed in blevesearch/blevex#30 and blevesearch/blevex#44

@tanzhang0504
Copy link
Copy Markdown
Author

Similar issue was discussed in blevesearch/blevex#30 and blevesearch/blevex#44

Hi Aman, thanks for the pointers. I'm also wondering is it possible to not generate dictionary rows altogether? Seems like it's needed by bleve to look up the terms. I did an experiment to not insert dictionary rows (by removing this line -

wb.Merge(buf[:dictRowKeyLen], buf[dictRowKeyLen:dictRowKeyLen+DictionaryRowMaxValueSize])
). Then my inserted index can't be found when looking up with "+field:val" query.

@tanzhang0504
Copy link
Copy Markdown
Author

@mschoch Just wondering any comment on my PR? I saw the past discussions in blevesearch/blevex#30 as pointed out by Aman, and seems like the suggested solution is to add a Compact() function for each DB store and rely on callers to invoke that function. Just wondering will that add too much burden on the caller instead of fixing the leak at the store level? Any feedback is greatly appreciated!

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.

Data leak with goleveldb backend?

2 participants