Skip to content
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

New local storage implementation #181

Open
d1vanov opened this issue Feb 28, 2021 · 4 comments
Open

New local storage implementation #181

d1vanov opened this issue Feb 28, 2021 · 4 comments

Comments

@d1vanov
Copy link
Owner

d1vanov commented Feb 28, 2021

The current implementation of local storage has a fair share of shortcomings:

  1. It is single threaded. It was a deliberate design decision as it greatly simplifies the implementation but unfortunately it leads to slow response from the local storage under load: when the initial sync runs, the local storage receives a ton of write events and as all reads and writes are serialized, reads become very slow. One example is that the note editor cannot load any note until the initial sync is over.
    SQLite with WAL (which is the backend of libquentier's local storage) supports efficient way to ensure concurrent access by many readers and one writer at a time. So new local storage should be multi threaded and it should only serialize writes but not reads. A notable fact is that reads can work concurrently with a single write at a time.
  2. The current mechanism of asynchronous communication with the local storage via signals and slots is not very convenient to use - all interested parties need to listen for signals which might be meant for any entity. This is why each request request has a unique id - it is used to filter the actual response from other non-interesting events.
    Asynchronous communication via QFuture + a separate set of notification signals for parties listening for updates would be much more convenient.
  3. The code responsible for interaction with the database is located in just a single source file (LocalStorageManager_p.cpp) which is pretty large and hard to navigate. It would be better to split it into parts dealing with different tables involved - notebooks, tags, notes etc.
  4. The local storage is represented by a set of particular classes in the public API while it should really be one or more interfaces with pure virtual methods.
  5. No support for bulk writes, each data item needs to be written separately. Not sure it would be easy to implement with SQLite - it would involve several SQL queries anyway, perhaps only the transaction could be the same for all of them.
@d1vanov
Copy link
Owner Author

d1vanov commented Feb 28, 2021

One more thing: need to ensure that pragma synchronous is set to FULL to ensure the durability of content written to the database.

@d1vanov
Copy link
Owner Author

d1vanov commented Apr 19, 2023

Mostly done in branch experiment/new-synchronizer. Haven't implemented bulk writes yet and probably won't for now - the only good way to do it is through exposing transactions into the local storage interface and doing this right would take a lot of time.
Also haven't done pragma synchronous full thing yet.

@d1vanov
Copy link
Owner Author

d1vanov commented Apr 19, 2023

One thing which would be useful to improve the runtime of unit tests would be the ability to have local storage working fully in memory. Currently resource data bodies are written to files on the filesystem even if the whole SQLite database lives in memory. Need to implement handler for resource data bodies which stores them in memory only as well.

@d1vanov
Copy link
Owner Author

d1vanov commented Apr 19, 2023

New local storage relies on new Evernote data model types implementation from QEverCloud's feature/copy-on-write branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant