-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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
Sync improvements #37
Conversation
da87461
to
e063c2d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commits need better commit messages. There are some commits rewriting 100s lines of code and it's difficult to understand what they do.
dbcli/dbcli.cpp
Outdated
|
||
int main(int argc, char** argv) { | ||
auto lmdbenv = std::make_shared<bxt::Utilities::LMDB::Environment>( | ||
std::make_shared<coro::io_scheduler>()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these shared pointer necessary? Value types are when possible preferable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LMDB's Enviroment is indeed intended to be used with a shared global io_scheduler (to manage transactions and concurrency) and also be shared with DBs itself for the same purpose, that's why it's needed.
In this particular case this doesn't really make sense but I just reused the daemon's components. I can rewrite using plain LMDB(++) if preferable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Will do, boss. |
e063c2d
to
a363d5a
Compare
Implements a basic database CLI for looking through the LMDB
Drastically improves sync service, including: - The service now retries failed downloads and has a connection timeout - Improved error handling and logging on all steps - Use scope guard for event dispatching on errors
- Templated hash functions - More verbose variables
Async synchronization needs more testing.
a363d5a
to
7189886
Compare
This PR improves overall previously half-baked sync service. It also includes
dbcli
dev utility to look through the LMDB packages database+
in package version)Those all are somewhat connected to the sync process but other subsystems will benefit as well.