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

Tipsy stops logging when local storage gets full #100

Open
karger opened this issue Feb 12, 2017 · 14 comments
Open

Tipsy stops logging when local storage gets full #100

karger opened this issue Feb 12, 2017 · 14 comments

Comments

@karger
Copy link
Member

karger commented Feb 12, 2017

My installation gave up logging after about a year of usage; it turns out this was enough to fill up chrome.storage.local which proceeds to reject any further storage. Potential fixes include a move to indexDB or some kind of garbage collection (which we may need even if we move to indexDB). For a given site, we could keep only the last few urls along with a total time spent (paid for and not-yet paid for).

@Stebalien
Copy link
Member

move to indexDB

I'm not sure you can from an addon.

Anyways, for future reference, the restrictions are here: https://developer.chrome.com/extensions/storage#property-local. TL;DR: You can store 5MiB. If you want to store more, you can request the unlimitedStorage permission but that's not a good fix (especially considering the fact that tipsy currently reads/writes the entire database from/to storage as one big blob).

@tbranyen
Copy link
Member

I have two suggestions varying in robustness:

  1. Easiest: Warn the user the storage is full and offer to export to JSON (and purging internally). I don't think we need to worry too much about loading 5MB of JSON (if we enabled unlimited, I would image that would be an issue per @Stebalien's comment). I haven't looked into how the app runs under max load (w/ 5MB JSON). @karger do you find it that it loads and scrolls suitably?

  2. More involved solution: Switch to a more suitable client side database like indexeddb via an abstraction library paired with virtual scrolling to show all results. I've written such a library, HyperList that can reliably handle hundreds of thousands of rows.

Additional thoughts: We could load a reasonable chunk up front from the DB and then using the browser's idle callback to load more into memory. Then if they scroll to the bottom they can be presented an option to load all or something.

Short-term I'd say we implement option 1, and then in a separate branch flesh out ideas for migrating to a more scalable approach. Thoughts @karger @Stebalien?

@karger
Copy link
Member Author

karger commented Feb 12, 2017 via email

@tbranyen
Copy link
Member

tbranyen commented Feb 12, 2017

Ah shoot, missed the part about not being able to use indexeddb (woosh), I'll investigate alternative solutions.

@Stebalien
Copy link
Member

Actually, scratch that. You may be able to use indexedDB. I assumed that addons would have to use the chrome addon storage API but it appears that they can use normal HTML5 storage mechanisms as well.

@Stebalien
Copy link
Member

Stebalien commented Feb 12, 2017

FYI, I'm using localForage in list.it. It supports multiple backends including localStorage and indexedDB and exposes an API similar to Chrome's storage API.

@tbranyen
Copy link
Member

@Stebalien nice, localForage is exactly the kind of abstraction I was hoping would work! Although we need to do more than just switch to indexeddb per my reply, otherwise it'd achieve the same effect as just enabling unlimited quota w/ chrome.storage (loading all data upfront at once).

@karger
Copy link
Member Author

karger commented Feb 13, 2017

I've dug through my storage and, assuming tipsy records its accessTime field in milliseconds, it looks like it took tipsy 139 days to fill up. I haven't cleared storage yet in case we want to look through it, but I'd like to in order to resume using/testing/demoing tipsy. So let me know if there's a reason to keep it.

@da2x
Copy link
Contributor

da2x commented Feb 15, 2017

Until a better solution is found, it seems requesting unlimitedStorage will prevent the extension from breaking for your most dedicated users.

@tbranyen
Copy link
Member

I remember now why we used chrome.storage. It syncs across your browser sessions and devices, which I think is worthwhile to keep.

@Stebalien
Copy link
Member

@tbranyen Only chrome.storage.sync does that and tipsy currently doesn't use it (except in some test cases). Also note that the synced storage isn't affected by the unlimitedStorage permission and has a very small quota (https://developer.chrome.com/apps/storage#property-sync).

@tbranyen
Copy link
Member

tbranyen commented Feb 15, 2017

@Stebalien I don't know now, but at the time I wrote the storage adapter I did account for sync:

engine = chrome.storage[this.sync ? 'sync' : 'local'];

Do you mean that this code path is only ever hit in tests and not in actual usage? It does look like sync has a significantly smaller quota so probably not all that useful for this case.

@Stebalien
Copy link
Member

Do you mean that this code path is only ever hit in tests and not in actual usage?

Exactly.

@rht
Copy link
Contributor

rht commented Feb 16, 2017

+1 for localforage: it defaults to indexedDB when available, http://caniuse.com/#search=indexeddb, has quirks from various browsers already taken care of, and with api that is almost like localstorage, but async. This library was implemented a long time after @tbranyen wrote the storage adapter, I think.

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

No branches or pull requests

5 participants