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

adapters in web workers #1215

Closed
calvinmetcalf opened this issue Jan 9, 2014 · 19 comments
Closed

adapters in web workers #1215

calvinmetcalf opened this issue Jan 9, 2014 · 19 comments

Comments

@calvinmetcalf
Copy link
Member

spin off of #169

@hongnk
Copy link

hongnk commented Jan 9, 2014

Should be spin off of #1169

Status of web storage support in web worker for browsers:

  • Chrome 31, Safari 6.1, iOS 7 supports indexeddb or websql in worker (can't find any info they support since which version)
  • Firefox doesn't support this yet
  • IE10 supports indexeddb in worker

Important: localStorage doesn't work in web worker, therefore pouchdb listening to onstorage to notify changes doesn't work.

@calvinmetcalf
Copy link
Member Author

I'm pretty sure local storage is only used in Chrome apps, based on a quick
search of the code on github
On Jan 8, 2014 7:54 PM, "hongnk" notifications@github.com wrote:

Should be spin off of #1169https://github.com/daleharvey/pouchdb/pull/1169

Status of web storage support in web worker for browsers:

  • Chrome 31, Safari 6.1, iOS 7 supports indexeddb or websql in worker
    (can't find any info they support since which version)
  • Firefox doesn't support this yet
  • IE10 supports indexeddb in worker

Important: localStorage doesn't work in web worker, therefore pouchdb
listening to onstorage to notify changes doesn't work.


Reply to this email directly or view it on GitHubhttps://github.com/daleharvey/pouchdb/issues/1215#issuecomment-31892991
.

@hongnk
Copy link

hongnk commented Jan 9, 2014

Yes you are right local storage is not the cause. Changes did not replicate to couchdb server because the replication listener is in the main thread. If I create replication listener in the worker thread, then there is a new problem: xmlhttprequest with CORS does not work in worker (known bug: http://crbug.com/69741).

@calvinmetcalf
Copy link
Member Author

@hongnk again that looks to be a Chrome apps bug

@hongnk
Copy link

hongnk commented Jan 9, 2014

It's a confirmed bug in webkit browser.When calling for example pouchdb.replicate.to('http://xyz:5984') inside worker it will returns error 405 method not allowed or cross-origin is not allowed, for both Chrome and Safari.

Work around that seems to work:

Modify pouch-nightly.js:

api.notifyLocalWindows = function (db_name) {
if(self.localStorage==undefined){ //in a web worker
postMessage('data changed')
}
}

At main thread:

pouch_worker.onmessage = function(msg){
if(msg=='data changed'){
pouchdb.replicate.to(http//remoteserver)
}
}

@hongnk
Copy link

hongnk commented Jan 9, 2014

Of course it's better if I can just add a custom changes listener in worker thread, instead of modifying the core code. However as mentioned, a custom listener seems to listen to localStorage which doesn't work in web worker. I'm not sure why use localStorage to notify changes.

@hongnk
Copy link

hongnk commented Jan 9, 2014

I think there is a problem with the above workaround (placing a replicate listener in main thread, and notify it via postMessage). There are 2 pouchdb instances in 2 threads, therefore some changes might not be in sync (I noticed conflicts that didn't occur when run in single thread). Perhaps will need to postMessage with more data to update the main thread first, before calling replicate function. But I don't understand PouchDB code fully to find what is needed.

@calvinmetcalf
Copy link
Member Author

Libraries can't really cross the worker main thread boundary meaning
Pouchdb has to exist on one side or the other, in other words adding the
ability to call Pouchdb in a parent thread for Ajax calls is pretty much
out, being able to override the Ajax function with one of your own is
probably the best solution.

That being said a 405 error does not to me smell like a CORS bug, if I
remember correctly denied cors requests end with a status code of 0, 405
sounds like the server is denying the initial option request.

Can you reproduce this in Chrome (not a an extension or Chrome app) where
the Ajax call works in the main thread but not on the worker?

On Jan 9, 2014 6:28 AM, "hongnk" notifications@github.com wrote:

Of course it's better if I can just add a custom changes listener in
worker thread, instead of modifying the core code. However as mentioned, a
custom listener seems to listen to localStorage which doesn't work in web
worker. I'm not sure why use localStorage to notify changes.


Reply to this email directly or view it on GitHub.

@hongnk
Copy link

hongnk commented Jan 9, 2014

Yes pouchdb.replicate.to has always been working in main thread. It just doesn't work in worker thread. The task now is to pass changes that happens in worker thread to it to replicate to remote server. All codes mentioned here are for web browser not chrome app or extension.

@calvinmetcalf
Copy link
Member Author

Just double checking, lemme look into this
On Jan 9, 2014 7:02 AM, "hongnk" notifications@github.com wrote:

Yes pouchdb.replicate.to has always been working in main thread. It just
doesn't work in worker thread. The task now is to pass changes that happens
in worker thread to it to replicate to remote server. All codes mentioned
here are for web browser not chrome app or extension.


Reply to this email directly or view it on GitHubhttps://github.com/daleharvey/pouchdb/issues/1215#issuecomment-31924338
.

@hongnk
Copy link

hongnk commented Jan 9, 2014

I think I should recap what has been discussed so far to be clearer. I think we are very close to achieve it.

  1. There's UI lag when perform bulkDocs on iPad, so run it in a worker thread is preferred. We can now create an additional pouchdb instance in a worker thread (both instances will access the same database).
  2. After calling bulkDocs in worker thread, local database is updated, but changes are not notified to replicate to remote server.
  3. So we attempt to add replicate function at worker thread but it causes 405 error (due to xmlhttprequest cross-origin or permission)
  4. Work-around is to add replicate function at main thread, then from worker thread just postMessage to notify it of changes.

The question is what data is needed to pass to the main thread to update it. If postMessageto with no data, and just call replicate, occasionally there are conflicts (but most times it works correctly, maybe it got updates from reading database directly).

@calvinmetcalf
Copy link
Member Author

are they both accessing the same http pouch or local pouch?

@hongnk
Copy link

hongnk commented Jan 9, 2014

Both threads accessing local storage, and in the case of iPad, they access same websql database. The local database is replicated to remote couchdb.

The main thread just read data. The worker thread is used to write data (bulkDocs) to avoid lag. After each write, we will postMessage to main thread, so it knows it should replicate to remote server.

It works quite well (I thought it should have the update since it accesses the same DB). But it seems to miss some changes occasionally.

So again, do we need to update the main thread pouchdb instance with anything (like last_sequence or something) before calling replicate?

Sseems more confusion? :)

@calvinmetcalf
Copy link
Member Author

what you want to do is put a listner on the worker object in the main thread which gets and puts a document without changing it, then in the worker post a message back to the main thread after the bulk_insert completes

@hongnk
Copy link

hongnk commented Jan 9, 2014

I've done that. What I've been trying to search is what data pouchdb
currently sends to listeners upon bulk_insert. There must be some
variables/properties/parameters that main thread needs to update to be in
sync with the worker thread. Or is it safe that I just call replicate at
the main thread, knowing that it will automatically read the latest update
from local database?

On Thu, Jan 9, 2014 at 10:42 PM, Calvin Metcalf notifications@github.comwrote:

what you want to do is put a listner on the worker object in the main
thread which gets and puts a document without changing it, then in the
worker post a message back to the main thread after the bulk_insert
completes


Reply to this email directly or view it on GitHubhttps://github.com/daleharvey/pouchdb/issues/1215#issuecomment-31937128
.

@calvinmetcalf
Copy link
Member Author

so I did some work on notifying pouch, but it's not quite as easy as I though, it'd probably a better idea to cancel the replication, have the worker do the bulk insert, and then when it's done start the replication again, we really need an api.poke() method that gets it to compare the state of the database to what it thinks it should be.

@hongnk
Copy link

hongnk commented Jan 10, 2014

Thanks for reviewing it. Continuous replication definitely doesn't work because it is not notified of changes in worker. But from what you said a manual replication start at main thread will automatically look for changes? If that is confirmed then it is sufficient. From what I tested it seemed to work fine like that. Although I did see some conflicts that I didn't see before when running in single thread, but they could be different issues.

@calvinmetcalf
Copy link
Member Author

so what I'm saying is (for now until we can fix the issue) stop the continuous replication, do the bulk docs in the worker, and AFTER you get confirmation it's done, start the continuous replication again

@calvinmetcalf
Copy link
Member Author

this has wondered a bit so I'm going to close it in favor of a more specific issue

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

2 participants