Compact the leveldb database on startup. #1494
Conversation
|
It's not at all clear to me that this is the correct thing to do. It does seem to drastically speed up certain operations, but also note (from leveldb's docs): "This operation should typically only be invoked by users who understand the underlying implementation." That set of users does not include me. This PR is being proposed as a discussion point, rather than the final suggested change. |
| @@ -209,6 +209,8 @@ - (nullable DB *)createDBWithDirectory:(NSString *)directory error:(NSError **)e | |||
| return nullptr; | |||
| } | |||
|
|
|||
| database->CompactRange(nullptr, nullptr); | |||
There was a problem hiding this comment.
If we're going to do this we should add log entries around this so that we get some idea of how long this is taking in the wild. As I said in chat I think this pessimizes the first query which is our hardest to make fast. We should schedule this later, maybe after the first snapshot has been delivered?
There was a problem hiding this comment.
I suspect (especially given @var-const s recent performance improvements) we can likely abandon this PR and not (manually) compact the database; leveldb should be taking care of this for us automatically. There's certainly some cases where that doesn't immediately happen, but I suspect they're corner cases. They may be worth pursing anyway; but I'd prefer to do so only if we still observe slowness (due to an uncompacted db) after Costa's changes hit.
And yes, if we do decide to pursue this, doing so before the first query seems sub-optimal. I suspect if we could schedule this after all (large?) queries complete when the db is otherwise idle, we'd get the same improvement without penalizing first query.
|
OK. We can reopen this later or do something similar at a later date if it's warranted. |
|
The change in this branch is trivial, so you could reasonably delete this branch too, though it's not required. |
No description provided.