Skip to content

Compact the leveldb database on startup. #1494

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

Closed
wants to merge 1 commit into from
Closed

Compact the leveldb database on startup. #1494

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Firestore/Source/Local/FSTLevelDB.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ - (nullable DB *)createDBWithDirectory:(NSString *)directory error:(NSError **)e
return nullptr;
}

database->CompactRange(nullptr, nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.


return database;
}

Expand Down