remotestorage.js
remotestorage.js copied to clipboard
remotestorage
deleteChildPathsFromTasks looks wrong
looks like https://github.com/remotestorage/remotestorage.js/blob/master/src/sync.js#L287 will lead to queued get requests not getting resolved. e.g., a request for foo/bar.txt goes to remote, and then a task for foo/ is scheduled. simply deleting the foo/bar.txt task would lose track of the unfulfilled promise for its retrieval
How does this affect library behaviour? Is it an edge case or did we miss a bug with a common case?
when get requests are not getting resolved, it affects library behavior quite badly - you would for instance do:
remoteStorage.scope('/myfavoritedrinks/').getObject('aervoqirjvsergergs', 0)
.then(function(obj) {
console.log('get request for /myfavoritedrinks/aervoqirjvsergergs was resolved');
console.log('result:', obj);
});
you would be waiting forever, those console.log statements would never be executed. I think @silverbucket is looking into this one, because we thought it may be what is happening to one of his apps in production (don't remember which one)
Thanks, but that doesn't answer the question really. The important bit is in which cases this can happen exactly. What do you have to do to trigger this behaviour?
The example you posted just does a single get request, and I'm fairly certain that this code alone will never result in the request not being resolved at all.
It could, if you have caching strategy 'ALL' for /myfavoritedrinks/.
@silverbucket what is the ticket where you have data not showing up in your app? Did you try out what happens to that if you comment out deleteChildPathsFromTasks?
@michielbdejong That's not enough it seems. I have that in Webmarks and I never encountered this bug. I'm using it every day since months.
Also, if this is a bug, of which you know when it happens, you could write a failing test for it.
It seems you already thought about when this could happen from the original description. I'm just asking for a clarification of when this is the case and what it means exactly.
looks like https://github.com/remotestorage/remotestorage.js/blob/master/src/sync.js#L271 will lead to queued get requests not getting resolved. e.g., a request for foo/bar.txt goes to remote, and then a task for foo/ is scheduled. simply deleting the foo/bar.txt task would lose track of the unfulfilled promise for its retrieval
In this context, what does "simply deleting the foo/bar.txt task" mean? Queuing a delete request? Deleting it from the remote? Deleting it at what point exactly? Etc etc.
If you know when it might fail, you (or anyone) can then write a test and simply emulate what you think is the case it might fail in. No?
what does "simply deleting the foo/bar.txt task" mean?
it's now line 287. updated the description.
Queuing a delete request? Deleting it from the remote? Deleting it at what point exactly? Etc etc.
No, not detelete requests. There is an object this._tasks in remoteStorage.sync, and the deleteChildPathsFromTasks deletes child paths from it.
Are you saying tasks are not synchronous? Wouldn't that create a whole lot more problems just by design?
That means if the remote responds slower to one request (e.g. because an app server instance is slower than another one) the earlier action could win, meaning the sync is broken by design (with race conditions).
I think we need to address this asap. Anyone else sees the issue with this?
The thing is, it must be asynchronous for peak performance, but shouldn't be if there's an earlier task for the same path. Right?
the earlier action could win [...] if there's an earlier task for the same path.
Ah, sorry, I should have explained that more clearly; if it's for the same path (the same document), then it's a totally different story for two reasons.
The architecture is as follows:
app <-> module <-> local store <-> asynchronous synchronization <-> remote store
First, from the app/module's point of view, actions on the same document get applied locally first, and even unpushed changes become visible immediately. So a canonical order of events is already forced onto the versioning of each document in the local store.
Second, from sync's point of view, there can only be one concurrent task for each document. There can be up to remoteStorage.sync.numThreads concurrent tasks, but the same path can never occur twice in that list, because that would be indeed dangerous, but also useless.
This ticket is about what happens when both an item and its parent or an ancestor further up the tree are in the tasks list.
Ok, thanks for clarifying. This obviously only applies when caching is active, but for apps where it isn't, the app will usually wait for successful requests itself, before allowing more changes. So it's not an issue I think.
This ticket is about what happens when both an item and its parent or an ancestor further up the tree are in the tasks list.
Ok, and when can this happen? What is the order of tasks that would fail?
I don't know when, if at all, this can happen, I haven't tried it out, I just created this ticket because it looks wrong. Maybe assigning the 'bug' label was therefore confusing, sorry. Removed it.
It would still be useful to look into this, does someone want to pair?