Last Comment Bug 893276 - sdk/simple-storage is writing to disk every 5 mins !
: sdk/simple-storage is writing to disk every 5 mins !
Status: REOPENED
:
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: P2 normal with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on: 895223 996200 1001074
Blocks: sdk/simple-storage 973750
  Show dependency treegraph
 
Reported: 2013-07-12 16:36 PDT by Erik Vold [:erikvold] (please needinfo? me)
Modified: 2014-11-04 11:14 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Pull request (358 bytes, text/html)
2013-07-18 11:26 PDT, Dave Townsend [:mossop]
rFobic: feedback+
Details
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1134 (358 bytes, text/html)
2013-07-25 17:20 PDT, Dave Townsend [:mossop]
rFobic: review+
Details
fixes tests (46 bytes, text/x-github-pull-request)
2014-04-15 18:22 PDT, Dave Townsend [:mossop]
rFobic: review+
Details | Review

Description Erik Vold [:erikvold] (please needinfo? me) 2013-07-12 16:36:37 PDT
This is insane.
Comment 1 Erik Vold [:erikvold] (please needinfo? me) 2013-07-12 16:42:01 PDT
Frankly I think we should deprecate this..
Comment 2 Erik Vold [:erikvold] (please needinfo? me) 2013-07-12 16:42:49 PDT
I'd be happy to write a JEP for something that offers a `write` method.
Comment 3 Erik Vold [:erikvold] (please needinfo? me) 2013-07-12 16:52:17 PDT
Another option is to only write after setting (with a timer so that setting multiple times at once doesn't cause multiple saves, and instead only one save occurs)
Comment 4 Erik Vold [:erikvold] (please needinfo? me) 2013-07-12 16:53:02 PDT
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #3)
> Another option is to only write after setting (with a timer so that setting
> multiple times at once doesn't cause multiple saves, and instead only one
> save occurs)

This is what I do with Scriptish, and it this seems to be how most old school add-ons behave..
Comment 5 Erik Vold [:erikvold] (please needinfo? me) 2013-07-12 16:55:01 PDT
If we do keep this 5min interval then there should be no write operation if the content hasn't changed, at the very least..
Comment 6 Jeff Griffiths (:canuckistani) (:⚡︎) 2013-07-14 19:58:29 PDT
Can we discuss this at the next team meeting? I don't have a good feel for what the trade-offs are.
Comment 7 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2013-07-15 10:49:06 PDT
Here are my thoughts on this subject:


- Lot's of IO is unfortunate, but since it happens off the main UI thread I don't consider it to be that critical.
- I don't think changing API to manual IO is a viable direction. It would feel as step backwards.
- Simple storage is no different from `localStorage` if we wanna fix this I think we should just pursue exposure of `localStorage` per add-on. There are some platform issues to be addressed to make it possible, but they're worth fixing & maybe workarounding in someways until then.
- Writes on changes would obviously be a better option that what we're currently doing. I believe it's not being done originally since there were no proxies available to make that possible. So changing implementation to use proxies and do
IO on mutations is an option, although I'd prefer localStorage option.
- I would also consider doing some benchmarking before investing a lot of time into this to be sure that these changes would have any visible impact justifying efforts put into.
Comment 8 Dave Townsend [:mossop] 2013-07-15 14:34:00 PDT
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #1)
> Frankly I think we should deprecate this..

This is silly and we should not be causing disk I/O every 5 minutes, however it seems like there are ways to avoid that (only writing on change, after a delay maybe when user is idle). What would be the reasons for deprecating this module entirely?
Comment 9 Gregg Lind (User Advocacy - Heartbeat - Test Pilot) 2013-07-15 14:36:28 PDT
There is an additional possible problem that some other addons might 'police' the profile directory, and clean out the persisted JSON file.  Right now, we have a hunch that this is happening, but need to verify it.
Comment 10 Gregg Lind (User Advocacy - Heartbeat - Test Pilot) 2013-07-15 14:38:48 PDT
(Our (UX Prototyping, Test Pilot) needs around this are for storing the starts of long-running timers, experiment phase settings, and other things that in the Bad Old Days, would have been stored in settings.  A small 'persist-across-restart' store is probably enough.)
Comment 11 Erik Vold [:erikvold] (please needinfo? me) 2013-07-15 15:07:13 PDT
(In reply to Gregg Lind (User Research - Test Pilot) from comment #9)
> There is an additional possible problem that some other addons might
> 'police' the profile directory, and clean out the persisted JSON file. 
> Right now, we have a hunch that this is happening, but need to verify it.

What issue are you experiencing? and do you know the file is gone? or is it just the simple-storage that appears to be missing?
Comment 12 Dave Townsend [:mossop] 2013-07-15 19:08:53 PDT
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #11)
> (In reply to Gregg Lind (User Research - Test Pilot) from comment #9)
> > There is an additional possible problem that some other addons might
> > 'police' the profile directory, and clean out the persisted JSON file. 
> > Right now, we have a hunch that this is happening, but need to verify it.
> 
> What issue are you experiencing? and do you know the file is gone? or is it
> just the simple-storage that appears to be missing?

This sounds unrelated to the disk I/O issue, can we spin it off into a separate bug?
Comment 13 :Gavin Sharp [email: gavin@gavinsharp.com] 2013-07-16 16:56:20 PDT
(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #7)
> - Lot's of IO is unfortunate, but since it happens off the main UI thread I
> don't consider it to be that critical.

Forgive me for not being familiar with addon-sdk stuff, but what ensures that the IO happens off the main thread? The code in io/text-streams.js / io/file.js seem to use sync I/O APIs, is there something that enforces that those run in a separate process/worker? 

> - Simple storage is no different from `localStorage` if we wanna fix this I
> think we should just pursue exposure of `localStorage` per add-on. There are
> some platform issues to be addressed to make it possible, but they're worth
> fixing & maybe workarounding in someways until then.

We're trying to discourage and deprecate localStorage in general, so exposing it more widely doesn't seem like a good idea.
Comment 14 Dave Townsend [:mossop] 2013-07-17 17:31:40 PDT
Switching to localStorage is a non-starter, for reasons gavin mentioned. (In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #13)
> (In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from
> comment #7)
> > - Lot's of IO is unfortunate, but since it happens off the main UI thread I
> > don't consider it to be that critical.
> 
> Forgive me for not being familiar with addon-sdk stuff, but what ensures
> that the IO happens off the main thread? The code in io/text-streams.js /
> io/file.js seem to use sync I/O APIs, is there something that enforces that
> those run in a separate process/worker?

We write the data with NetUtil.asyncCopy. Opening and closing the file are still sync I think though.

> > - Simple storage is no different from `localStorage` if we wanna fix this I
> > think we should just pursue exposure of `localStorage` per add-on. There are
> > some platform issues to be addressed to make it possible, but they're worth
> > fixing & maybe workarounding in someways until then.
> 
> We're trying to discourage and deprecate localStorage in general, so
> exposing it more widely doesn't seem like a good idea.

Yeah localStorage isn't the right solution here. We should either deprecate this module (but I haven't heard any reasons to do so yet) or clean up the redundant disk I/O.
Comment 15 Dave Townsend [:mossop] 2013-07-17 18:29:18 PDT
I have a mostly working patch that switches to only writing on change unfortunately it runs afoul of bug 895223
Comment 16 Dave Townsend [:mossop] 2013-07-18 11:26:27 PDT
Created attachment 777955 [details]
Pull request

This makes us only write after data is modified.
Comment 17 Erik Vold [:erikvold] (please needinfo? me) 2013-07-19 19:06:35 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #13)
> (In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from
> comment #7)
> > - Simple storage is no different from `localStorage` if we wanna fix this I
> > think we should just pursue exposure of `localStorage` per add-on. There are
> > some platform issues to be addressed to make it possible, but they're worth
> > fixing & maybe workarounding in someways until then.
> 
> We're trying to discourage and deprecate localStorage in general, so
> exposing it more widely doesn't seem like a good idea.

Interesting, why is this?
Comment 18 Erik Vold [:erikvold] (please needinfo? me) 2013-07-19 19:18:52 PDT
(In reply to Dave Townsend (:Mossop) from comment #8)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #1)
> > Frankly I think we should deprecate this..
> 
> This is silly and we should not be causing disk I/O every 5 minutes, however
> it seems like there are ways to avoid that (only writing on change, after a
> delay maybe when user is idle). What would be the reasons for deprecating
> this module entirely?

It's not necessary, but I think this module is far less useful than an alternate could be, here are some issues with it:

* Must use JSON, no other format choice.
* I cannot choose when to do a save, thus it is impossible to be optimal, and I hate the idea of useless disk writing.
Comment 19 Dave Townsend [:mossop] 2013-07-20 11:23:03 PDT
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #18)
> (In reply to Dave Townsend (:Mossop) from comment #8)
> > (In reply to Erik Vold [:erikvold] [:ztatic] from comment #1)
> > > Frankly I think we should deprecate this..
> > 
> > This is silly and we should not be causing disk I/O every 5 minutes, however
> > it seems like there are ways to avoid that (only writing on change, after a
> > delay maybe when user is idle). What would be the reasons for deprecating
> > this module entirely?
> 
> It's not necessary, but I think this module is far less useful than an
> alternate could be, here are some issues with it:
> 
> * Must use JSON, no other format choice.

Well JS objects, arrays and primitives, you don't need to worry about JSON. What other kinds of data would you want to store from JS?

> * I cannot choose when to do a save, thus it is impossible to be optimal,
> and I hate the idea of useless disk writing.

The patch I have gets rid of any useless disk writing, we just write whenever data is changed. We could go even further and only write on shutdown, though that loses us some protection from crashes.

I'm not against creating a better settings storage module, though we already have indexeddb too, but assuming we can solve the problems with simple-storage I don't see a need to deprecate it and force all users to switch unnecessarily right now.
Comment 20 :Gavin Sharp [email: gavin@gavinsharp.com] 2013-07-22 15:33:28 PDT
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #17)
> > We're trying to discourage and deprecate localStorage in general, so
> > exposing it more widely doesn't seem like a good idea.
> 
> Interesting, why is this?

A synchronous API dependent on disk IO is just generally a bad idea. We've made some improvements recently (bug 600307, bug 807021) but the fundamental problem of page script/responsiveness being blocked on IO remains.
Comment 21 Erik Vold [:erikvold] (please needinfo? me) 2013-07-24 08:54:40 PDT
Ah I just noticed that simple-storage does a sync io when reading from disk.. yet another reason to deprecate it.
Comment 22 Erik Vold [:erikvold] (please needinfo? me) 2013-07-24 08:55:49 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #20)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #17)
> > > We're trying to discourage and deprecate localStorage in general, so
> > > exposing it more widely doesn't seem like a good idea.
> > 
> > Interesting, why is this?
> 
> A synchronous API dependent on disk IO is just generally a bad idea. We've
> made some improvements recently (bug 600307, bug 807021) but the fundamental
> problem of page script/responsiveness being blocked on IO remains.

The sync API for simple-storage is regrettable.
Comment 23 Erik Vold [:erikvold] (please needinfo? me) 2013-07-24 09:10:59 PDT
Comment on attachment 777955 [details]
Pull request

Irakli I'm going to wait for your a+ before reviewing this optimization.  If you'd like to do the review too then be my guest, I'd rather not.
Comment 24 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2013-07-24 10:37:17 PDT
Comment on attachment 777955 [details]
Pull request

Erik: I don't mind taking over this review.

This approach is fine but does has a flaw of backwards incompatibility. Which maybe alright but I'd like to ensure we understand consequences.

For example:

```js
var point = { x: 0, y: 0 };
storage.point = point;
point.x = 1;
```

This is very simple example, but you can imagine more complicated one. Problem is attempts to save changes to point will only be made at shutdown (assuming it was clean one). But then since we write async there's no guarantees data will end up
saved. That being said current implementation has "maybe saves on shutdown" flow
too but I don't know which one would be more error prone in practice.

I think we could preserve behavior backwards compatible if instead of doing proxies we just state snapshot via `JSON.stringify` after delays and compared it to previous one if there is a diff we can do actual writes to disk.
Comment 25 Erik Vold [:erikvold] (please needinfo? me) 2013-07-24 10:51:23 PDT
(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #24)
> Comment on attachment 777955 [details]
> Pull request
> 
> Erik: I don't mind taking over this review.
> 
> This approach is fine but does has a flaw of backwards incompatibility.
> Which maybe alright but I'd like to ensure we understand consequences.
> 
> For example:
> 
> ```js
> var point = { x: 0, y: 0 };
> storage.point = point;
> point.x = 1;
> ```
> 
> This is very simple example, but you can imagine more complicated one.
> Problem is attempts to save changes to point will only be made at shutdown
> (assuming it was clean one). But then since we write async there's no
> guarantees data will end up
> saved. That being said current implementation has "maybe saves on shutdown"
> flow
> too but I don't know which one would be more error prone in practice.
> 
> I think we could preserve behavior backwards compatible if instead of doing
> proxies we just state snapshot via `JSON.stringify` after delays and
> compared it to previous one if there is a diff we can do actual writes to
> disk.

Thanks Irakli.  I probably would've missed that.
Comment 26 Dave Townsend [:mossop] 2013-07-24 12:05:18 PDT
(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #24)
> Comment on attachment 777955 [details]
> Pull request
> 
> Erik: I don't mind taking over this review.
> 
> This approach is fine but does has a flaw of backwards incompatibility.
> Which maybe alright but I'd like to ensure we understand consequences.
> 
> For example:
> 
> ```js
> var point = { x: 0, y: 0 };
> storage.point = point;
> point.x = 1;
> ```

Ah yes, that is problematic.

> This is very simple example, but you can imagine more complicated one.
> Problem is attempts to save changes to point will only be made at shutdown
> (assuming it was clean one). But then since we write async there's no
> guarantees data will end up
> saved. That being said current implementation has "maybe saves on shutdown"
> flow
> too but I don't know which one would be more error prone in practice.

I think it should always save on shutdown from manager.unload

> I think we could preserve behavior backwards compatible if instead of doing
> proxies we just state snapshot via `JSON.stringify` after delays and
> compared it to previous one if there is a diff we can do actual writes to
> disk.

I considered that but after seeing that the quota was 5MB that means potentially holding onto 5MB for the lifetime of the app and I'd hoped to avoid that. Maybe it's a better solution though.
Comment 27 Erik Vold [:erikvold] (please needinfo? me) 2013-07-25 02:19:14 PDT
(In reply to Dave Townsend (:Mossop) from comment #26)
> (In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from
> comment #24)
> > I think we could preserve behavior backwards compatible if instead of doing
> > proxies we just state snapshot via `JSON.stringify` after delays and
> > compared it to previous one if there is a diff we can do actual writes to
> > disk.
> 
> I considered that but after seeing that the quota was 5MB that means
> potentially holding onto 5MB for the lifetime of the app and I'd hoped to
> avoid that. Maybe it's a better solution though.

We could hash the snapshots.
Comment 28 Dave Townsend [:mossop] 2013-07-25 09:15:13 PDT
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #27)
> (In reply to Dave Townsend (:Mossop) from comment #26)
> > (In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from
> > comment #24)
> > > I think we could preserve behavior backwards compatible if instead of doing
> > > proxies we just state snapshot via `JSON.stringify` after delays and
> > > compared it to previous one if there is a diff we can do actual writes to
> > > disk.
> > 
> > I considered that but after seeing that the quota was 5MB that means
> > potentially holding onto 5MB for the lifetime of the app and I'd hoped to
> > avoid that. Maybe it's a better solution though.
> 
> We could hash the snapshots.

I like it, I'll see how costly that is for a large data set, I can probably do it asynchronously.
Comment 29 Dave Townsend [:mossop] 2013-07-25 17:20:28 PDT
Created attachment 781351 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1134

Pointer to Github pull-request
Comment 30 Dave Townsend [:mossop] 2013-07-25 17:21:43 PDT
Comment on attachment 781351 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1134

This uses the same tests from the previous pull request but instead uses hashing to check if the data has changed. I had to do an asynchronous hash which uses up around 30ms of main thread time for the full 5MB of data, a synchronous hash of 5MB was taking around 1.2s which is too much.
Comment 31 Dave Townsend [:mossop] 2013-07-26 09:08:39 PDT
Irakli thinks we can make simple-storage reads async, let's talk about that issue in bug 898474
Comment 32 (dormant account) 2013-07-30 17:22:01 PDT
(In reply to Dave Townsend (:Mossop) from comment #30)
> Comment on attachment 781351 [details]
> Pointer to Github pull request:
> https://github.com/mozilla/addon-sdk/pull/1134
> 
> This uses the same tests from the previous pull request but instead uses
> hashing to check if the data has changed. I had to do an asynchronous hash
> which uses up around 30ms of main thread time for the full 5MB of data, a
> synchronous hash of 5MB was taking around 1.2s which is too much.

30ms on what hardware? Are we making a change to poll for changes every 5seconds and keeping 5mb of data per jetpack in memory?
Comment 33 Dave Townsend [:mossop] 2013-07-30 22:16:56 PDT
(In reply to Taras Glek (:taras) from comment #32)
> (In reply to Dave Townsend (:Mossop) from comment #30)
> > Comment on attachment 781351 [details]
> > Pointer to Github pull request:
> > https://github.com/mozilla/addon-sdk/pull/1134
> > 
> > This uses the same tests from the previous pull request but instead uses
> > hashing to check if the data has changed. I had to do an asynchronous hash
> > which uses up around 30ms of main thread time for the full 5MB of data, a
> > synchronous hash of 5MB was taking around 1.2s which is too much.
> 
> 30ms on what hardware?

On my Sony Vaio Z

> Are we making a change to poll for changes every
> 5seconds and keeping 5mb of data per jetpack in memory?

No, we're making a change so that instead of writing any data in simple-storage (anywhere from 1 byte to 5MB) out to disk every 5 minutes we instead hash the data and only write it to disk if the hash has changed.
Comment 34 (dormant account) 2013-07-31 18:11:25 PDT
30ms jank on a fast machine is likely 60-100ms on common hardware(eg our amd E350 slow reference machines). Sounds better than doing IO every 5s, but still terrible. Should just get rid of such apis.
Comment 35 Dave Townsend [:mossop] 2013-07-31 19:23:03 PDT
(In reply to Taras Glek (:taras) from comment #34)
> 30ms jank on a fast machine is likely 60-100ms on common hardware(eg our amd
> E350 slow reference machines). Sounds better than doing IO every 5s, but
> still terrible. Should just get rid of such apis.

I don't think it was a solid 30ms, that was just the total time spent on the main thread, I'll have to test more to find out how many uses of the event loop that was
Comment 36 Dave Townsend [:mossop] 2013-07-31 19:33:56 PDT
Also I don't know where the 5s came from
Comment 37 Dave Townsend [:mossop] 2013-11-08 13:37:50 PST
Comment on attachment 781351 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1134

Updated to master and switch to use the fs API, warrants another review pass I think then we can land this.
Comment 38 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2013-11-14 17:08:47 PST
Comment on attachment 781351 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1134

Still made few comments, but nothing critical, I'll leave it up to you weather you wanna address them before landing.
Comment 39 urishmorgun 2013-11-18 04:09:08 PST
Is there any chance to force simple-storage to save to disk? Or make it persist after every write, and not wait for event in the future?
Comment 40 Gregg Lind (User Advocacy - Heartbeat - Test Pilot) 2013-11-18 08:52:47 PST
(For all those who are suggesting deprecating the api, some in the wild stories for what I am using this for:

- JSONable addon specific configuration and prefs that would otherwise go to 'prefs'
- small amounts of transitory data storage

The alternative (for my use cases) would be to stick things in prefs, which is gross because of stringify, integer overflow on timestamps, etc.  

A 'proxy a pref' solution (myprefs.storage) with 'auto json', that exposes a 'proxy' (that looks like the `simplestore.storage` object) would be fine.
)
Comment 41 Dave Townsend [:mossop] 2013-11-18 10:41:59 PST
(In reply to urishmorgun from comment #39)
> Is there any chance to force simple-storage to save to disk? Or make it
> persist after every write, and not wait for event in the future?

Persisting after every write isn't something we can do without breaking the API, and if we're going to do that then we'd be more likely to switch to something other than simple storage.
Comment 42 urishmorgun 2013-11-18 10:46:45 PST
(In reply to Dave Townsend (:Mossop) from comment #41)
> Persisting after every write isn't something we can do without breaking the
> API, and if we're going to do that then we'd be more likely to switch to
> something other than simple storage.

Well, then a write that user initiates would be very useful
Comment 43 [github robot] 2013-11-21 16:45:00 PST
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/08cf8c66717bcf8992286aee702abc4c8daebbf3
Bug 893276: Only write simple-storage to disk after it has been changed.

https://github.com/mozilla/addon-sdk/commit/4f3ef6ced9a1377afd7c3dc3bf232c0183fd47c6
Merge pull request #1134 from Mossop/bug893276_2

Bug 893276: Only write simple-storage to disk after it has been changed. r=@gozala
Comment 44 Wes Kocher (:KWierso) 2013-11-21 18:52:27 PST
https://tbpl.mozilla.org/php/getParsedLog.php?id=30928521&tree=Jetpack 

One of the test runs failed with this failure. Not sure how important that is.
Comment 45 [github robot] 2013-11-27 14:10:07 PST
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/11344b27cc0a1632ab757fda3b43b320e8c6830d
Revert bug 893276 due to permanent failures.

This reverts commit 4f3ef6ced9a1377afd7c3dc3bf232c0183fd47c6, reversing
changes made to 7b5f80a40e79506293c58ba1c337c93be45423b2.
Comment 46 [github robot] 2014-04-14 09:29:53 PDT
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/4b6a467c3ce61fbe96083f1de2885a06a9b853b5
Bug 893276: sdk/simple-storage is writing to disk every 5 mins. r=gozala
Comment 47 Dave Townsend [:mossop] 2014-04-14 09:30:37 PDT
After unbitrotting this and running in through try I couldn't see any failures, so let's see what happens if I land it again.
Comment 48 [github robot] 2014-04-14 13:42:19 PDT
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/bb87af0616a451c89cb2349235d20b160963a952
Revert "Bug 893276: sdk/simple-storage is writing to disk every 5 mins. r=gozala"

This reverts commit 4b6a467c3ce61fbe96083f1de2885a06a9b853b5.
Comment 49 Dave Townsend [:mossop] 2014-04-15 18:22:57 PDT
Created attachment 8407231 [details]
fixes tests
Comment 50 [github robot] 2014-04-17 11:43:39 PDT
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/7df29f8d057645ab8428ed7eacac73eeaa26fc88
Bug 893276: Only write simple-storage json file if the data has changed.

https://github.com/mozilla/addon-sdk/commit/9a82bbc2ec7d8df9023aa11acac5d5f7e8c2ece1
Merge pull request #1462 from Mossop/bug893276

Bug 893276: Only write simple-storage json file if the data has changed. r=gozala
Comment 51 Erik Vold [:erikvold] (please needinfo? me) 2014-09-16 09:50:08 PDT
Irakli, can you revert this patch and do the uplifts?

This patch caused a regression see bug 1058840, we should also write a test for this.
Comment 54 Erik Vold [:erikvold] (please needinfo? me) 2014-10-08 11:55:31 PDT
Someone should probably work on this.
Comment 55 Erik Vold [:erikvold] (please needinfo? me) 2014-10-08 11:55:56 PDT
(In reply to Erik Vold [:erikvold] (work week -> pto) from comment #54)
> Someone should probably work on this.

Adding it to the triage list.

Note You need to log in before you can comment on or make changes to this bug.