-
Notifications
You must be signed in to change notification settings - Fork 26
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
Discussion about implementation #43
Comments
Reading LF project's history, It seems that the support of multiple instances is pretty recent. It could be one of the reasons to the current implementation of ELA. The fork from the ember-localstorage-adapter is certainly another one, since the latter doesn't support multiple database/store per domain. However, thanks to the current abstraction layer of LF it makes the proposed implementation now possible. |
@sebweaver Again, I am in favour of all of this. My suggestion is to change one thing at a time where possible. The tests we have, help a lot of in this case. Regarding pitfalls, I think migration should be no problem since we are dealing with pojo and arrays. |
I plan to look into this new implementation before the end of the month. |
We might want to consider running localforage in a worker. It might increase performance. I see that support for workers within Ember is premature. There are a few projects/addons on this topic. But we might not need those addons when we only localforage itself is running in a worker. There should support for that. Myself I also have never used workers before, but I think it is worth checking it out. |
Eventually I did not have enough time on December/January to look into this. Your suggestion to run LF in a worker is a good idea and I'll keep that in mind! I'll keep you posted whether and when I could reschedule all of this. |
@sebweaver No worries, totally understood. The library is in a fine shape anyway! |
I'm evaluating moving from ember-local-storage (not to be confused with ember-localstorage-adapter to localforage on a cordova backed mobile app. The goal is to increase performance, especially on mobile safari. My app's database is quite large. I'm also eyeing a sqlite driver for localforage as a possibility to increase performance. Running across this issue however has given me pause. The single-key based architecture was why I jumped ship from ember-localstorage-adapter to ember-local-storage. It is simply silly to be serializing/deserializing the entire database on every write operation. If you're interested in an alternative implementation, checkout ember-local-storage. Each model is stored in its own key of the format I didn't mean for this comment to become an advertisement for e-l-s. It clearly has its limitations (localStorage), but it has addressed some of the topics in this issue. Frankly, I'd love to switch to LF, but based on the discussion in this post, it seems it would be a step backward. |
@Ramblurr Very welcome to post your findings. Does not feel like an advertisement at all for Now we should make the next step: change the methodology. But to be honest: I think I cannot find the time to make this step right now. The app that I am using LF for is working perfectly with this adapter. However, I still hope someone can push this library a big step forward. Then I am sure I will be able help on the minor bits too. |
I wrote this issue when I worked on an application where the database was big enough to notice the performance drop. Unfortunately this application has never got beyond the POC stage. Since the single-key implementation was sufficient to make it work as intended, the new implementation progressively left my list of priorities... If I had to finally release this application on production, or another one requiring the same architecture, there is no doubt that I would be back on track towards this new implementation. In the meantime, new contributors for this issue are warmly welcomed! 😄 |
Hey! I'm the author of |
@fsmanuel At this moment, both @sebweaver and I, are not using the LF adapter. No one else has taken over development. You are free to help this package, or help your own package with the things that we have build. |
@frederikbosch I see! Thanks for the offer. I guess the tests are of great value. I'll get back to you as soon as I have a better understanding of LF. |
A I said here and there that I have some ideas to improve the whole implementation of ELA.
Purposes are:
Let me explain my POV.
I use ELA in an mobile application using Cordova. This application is designed to work offline most of the time, so ELA let me store my data locally without any concern of the underlying storage (thanks to LF). The problem is I have a lot of data with a lot of relationships.
Issue #42 already addresses a part of this issue by avoiding to preload unnecessary relationships. Which, in my case BTW, lead to load almost the whole database. However, there are still 3 remaining issues:
Memory consumption
The current implementation load all records in memory, optionally put them in cache, and copy them to the store during the loading process (without #42) or asynchronously (with #42). So in a worst case scenario, when application requires all the data, every single record of the database could exist at least twice in memory:
If the database is very big, this could be problematic depending on the available local resources.
I/O bandwidth
In the current implementation the whole database is stored in LF as a single value, under a single key. This way, every single record CRUD lead to read and/or write the whole database in one shot!
As for the previous issue, if the database is very big, this could be problematic depending on the available local resources.
Lack of granularity
Even if the database is light, the previous issue could lead to inconsistent issues if the something bad happens during the read/write process, or even worse, break the entire database.
This issue refers to the ACID principle, which is not respected here.
Proposed solution
When I used ELA to the first time I was really intrigued about the choice of implementation. I was so happy to find an adapter which solved my needs that I ended up to get used to it. I also said to myself they were certainly some pretty good reasons to implement it that way.
However, it led me to a unfortunate misinterpretation: to improve granularity, I decided to split my models into different namespaces. Big mistake, the current loading process of relationships can't see across namespaces and relationships were never satisfied. So I had to move my models under the same namespace. I concluded that namespaces were designed for other needs, may be when applications require different data models which have to be hermetic with each other.
Note: since the loading process is about to be removed by #42 and handled natively by the store, it might work now. But I did not test again and above all, the new implementation could make this issue obsolete.
Anyway, here is the thing:
This table shows that we lose at least 2 precious levels of isolation with the current implementation. The proposed usage aim to stick to original purpose of LF structure more naturally.
Note: this implementation implies there will be as many LF instances as namespaces. The latter could be referenced in ELA thanks to a container (which could be a service too) in order to dispatch operations according to a given namespace.
Benefits
JSONAPIAdapter
for example)Issues solved?
Pitfalls
The new implementation can't be compatible with existing databases. Database structure is slightly different. Serialization could even be changed too (using
JSONAPISerializer
for better compatibility in the future). There are 3 solutions to address this pitfall:What do you think about all of this? Was there any specific reason to choose this implementation in the first place? I may have missed something, if so, I'd really want to know what.
Note: as a non native english speaker, I hope my english was clear enough to explain this big task :-)
Regards,
Sébastien
The text was updated successfully, but these errors were encountered: