ostafen / clover

A lightweight document-oriented NoSQL database written in pure Golang.
MIT License
666 stars 55 forks source link

v2: db.Delete() deleting all rather than limited set from query #80

Closed axllent closed 2 years ago

axllent commented 2 years ago

Note this is regarding the v2 branch: I am experiencing very slow deletions when dealing with 1000s of documents, so I decided to try delete smaller subsets to benchmark. This is when I noticed that when trying to limit() deletions to any value, all documents appear to be deleted, for example with a collection with 1000 documents:

if err := db.Delete(clover.NewQuery("collection).Limit(100)); err != nil {
    return err
}

all documents get deleted.

ostafen commented 2 years ago

Hi, @axllent. Does this work as expected in the main branch?

Regarding the fact that deletions are slow: can you post the Delete() query you are running? You can also try to run only the query you are passing to Delete() to understand if it is the query execution to be slow, or the deletion of documents

EDIT: with the last v2 commit, Delete() should consider Limit()

axllent commented 2 years ago

Hi @ostafen . I have no idea if it was working on the main branch as I hadn't gotten to the deletion part before I switched to the v2 branch and refactored all my code ;-) It's not that easy to switch because I have to change all my queries.

Limit() is now working in v2 as expected, thanks!

The original delete query I tried was db.Delete(clover.NewQuery(mailbox)), but with about 20k documents this takes about 7.5 minutes to complete. Please note that there are actually two separate deletions (for each collection) that run the same query, and that they run after each other (so technically 40k documents split over two collections).

When I loop it to delete 1000 (x2) documents at a time (over the exact same data as above) using db.Delete(clover.NewQuery(mailbox).Limit(1000)) within a for{} loop, it takes just 20s to delete all.

(Unrelated to the issue) What I also notice (in both cases though) is after the deletion it appears all the data is still persistent on the harddrive (all the *.sst files remain), even after a db.Close(). I would have expected these to get removed once the data was deleted. They may be filled with zeros, I don't know, but it appears there is no internal tidy-up task, so if these remain on the harddrive, I suspect they also would remain in RAM when using the memory storage too, even if "empty".

Edit: I can confirm that the data files remaining after deleting all documents do in fact contain the original email data, so whilst they are being deleted from "the app" (db queries) the physical data doesn't get deleted. I think this is a separate issue though, not related to the slow delete queries (without using a loop & limit).

ostafen commented 2 years ago

Hi, @axllent, the slowdown with Delete() was due to a bug which I fixed with the last pull request. It should be way faster now.

I can confirm that the data files remaining after deleting all documents do in fact contain the original email data, so whilst they are being deleted from "the app" (db queries) the physical data doesn't get deleted.

About this, this is due to how the badger storage engine works internally. Since badger is a log-structured storage engine, each write operation causes a record to be appended to the logs. Thus, when performing a Delete() of a key, you simply append a marker record to the logs and no data is actually deleted from disk (on the contrary, disk space increases a bit, due to delete records). Disk space is reclaimed in the background, by calling the RunValueLogGC(), which by default is called every 5 minutes by clover. You can set your own reclaim interval when opening a database with the following code:

db, err := clover.Open("./db", clover.WithGCReclaimInterval(time.Minute))

Note however, that, even if you set a lower reclaim interval, garbage collection only runs if a certain space percentage can be reclaimed, so you can continue to see disk space to be the same. However, if you try to perform several insertion and deletion sequences, you will reach a point where disk space is reclaimed.

Let me know if you have further issues!

axllent commented 2 years ago

@ostafen - that's awesome, thank you. I can confirm that with the latest commit it is now deleting 20k documents in approximately 2 seconds now, without the need to delete 1000 at a time in a loop.

Re: reclaiming space - that makes perfect sense. I have noticed a RunValueLogGC(): Value log GC attempt didn't result in any cleanup from time to time in the logs, however it only happens from time to time, and after your response I did some more checking and can confirm that some storage is in fact reclaimed every now and then. I just incorrectly assumed that if I deleted all my documents the actively used storage would revert to "close to zero".