Meteor-Community-Packages / ground-db

GroundDB is a thin layer providing Meteor offline database and methods
https://atmospherejs.com/ground/db
MIT License
572 stars 77 forks source link

Some data disappears/reappears #184

Closed ilan-schemoul closed 7 years ago

ilan-schemoul commented 7 years ago

Hello, I had bugs since weeks when I use grounddb that I don't have with default minimongo integrated in Meteor. I first taught it was Easysearch which was at the origin of the bug so I did create my own search engine after weeks waste trying to understand what was wrong with grounddb/Easy Search. Now I'm creating my own client search engine I have the same problem. I just understand why and it's coming from ground:db http://i.imgur.com/OWr6RAl.png This is my grounded collection of Meteor.users and as you can see after localUser (ground version Meteor.users) is ready the data I subscribed too

  Meteor.subscribe('userWorksheets');
  Meteor.subscribe('billing');

(worksheets and billing) disappears and reappears in so a short moment that blaze drives crazy and my website partially stops working. I hope @raix or anyone could help me on that. Please note that I don't have such problem with native collections and to be short I expect grounddb to be less unpredictable.

raix commented 7 years ago

what version are you using? (you can experience the same thing with regular meteor collections... might want to checkout https://atmospherejs.com/meteorhacks/subs-manager)

ilan-schemoul commented 7 years ago

I use 2.0.0-rc.5 console.log(Meteor.user(), localUser.findOne());

undefined
{emails: Array[1], _id: "46CzcyYYE6u9vJpS9", roles: Array[1], worksheets: Object, billing: Object}

{_id: "46CzcyYYE6u9vJpS9", emails: Array[1], roles: Array[1]} 
{emails: Array[1], _id: "46CzcyYYE6u9vJpS9", roles: Array[1]}

{_id: "46CzcyYYE6u9vJpS9", emails: Array[1], roles: Array[1], worksheets: Object, billing: Object}
{emails: Array[1], _id: "46CzcyYYE6u9vJpS9", roles: Array[1], worksheets: Object, billing: Object}

The problem is this moment where the regular collection has just received first bunch of data and so grounddb deletes worksheets and billing but as these fields comes right after grounddb have these field again, but as everything's fast Blaze can't follow and all my app is buggy. Maybe using the subscription cache could be a workaround, but as soon as the collection will received new subs the grounded collection will update twice in a short time making Blaze stops working.

ilan-schemoul commented 7 years ago

I open an issue on Blaze as the problem is the compatibility between Blaze and ground:db because one is updating too fast for nothing and the other cannot follow the speed of the updating https://github.com/meteor/blaze/issues/145

ilan-schemoul commented 7 years ago

@raix Please see https://github.com/meteor/blaze/issues/146 (it could be related with this issue, in any way it's related with grounddb so I really need your help on that) also and my reproduction here https://github.com/NitroBAY/ground-blaze-bug-repro (note the readme.md)

raix commented 7 years ago

I don't think it's a Blaze issue - I'm thiniing it's a dispatch:kernel issue (I should improve that package)

ilan-schemoul commented 7 years ago

hmm okay so you're telling me that all the problems that drives me to become to hate Meteor (can't use autorun(), can't use #each with grounddb collections) came all from this lovely dispatch:kernel ?? Damn then I don't know nobody ever noticed it altough a lot of ppl downloaded your package. In any way I hope you'll fix that ASAP. Thanks a lot for your work and your support in any way @raix

ilan-schemoul commented 7 years ago

You should maybe create a branch dispatch:kernel-free before working on fixing it ?

ilan-schemoul commented 7 years ago

In anyway although it can be a bit obscure exactly how the package is working to optimize Meteor can I ask you why didn't you make a PR to Meteor and Blaze ? If it's good for performance and it's backward compatible it should be merged. EDIT : in case of you didn't tried my reproduction this is the kind of message I get when I use a search engine which return a list of document to be inserted within an #each, this is the kind of error I do get when I use ground:db as a source : Error: Bad index in range.getMember: 1 at DOMRange.getMember (blaze.js?hash=a9372ce…:582) at blaze.js?hash=a9372ce…:2829 at Object.Tracker.nonreactive (tracker.js?hash=f525263…:631) at Object.changedAt (blaze.js?hash=a9372ce…:2823) at observe-sequence.js?hash=88a243b…:308 at Function..each..forEach (underscore.js?hash=27b3d66…:157) at diffArray (observe-sequence.js?hash=88a243b…:294) at observe-sequence.js?hash=88a243b…:142 at Object.Tracker.nonreactive (tracker.js?hash=f525263…:631) at observe-sequence.js?hash=88a243b…:116

Is there any clue letting you know that the problem is coming from Kernel dispatcher or do you think so because of other stuffs I dunno ? In any way dispatch:kernel has problems and has to be removed/fixed (see #101 for instance)

ilan-schemoul commented 7 years ago

@raix any ETA, you may want to create a branch kernel-free for those who really need a fix like me ?

raix commented 7 years ago

@NitroBAY try ground:db@2.0.0-rc.6

ilan-schemoul commented 7 years ago

Great, finally I'll give a try right now. Update : I don't see it on grounddb-caching-2016 branch nor on other ones. @raix did you forget to git push ;) ? BTW : you should delete some branch it's kinda mess over there

ilan-schemoul commented 7 years ago

Got it

raix commented 7 years ago

you should be able to bump the version directly in .meteor/packages

ilan-schemoul commented 7 years ago

What ? I was cloning your repo until now as I thought I had to do that because on Atmosphere it's still pre v1.

ilan-schemoul commented 7 years ago

Okay I will need some time to make checks (especially that I was making not backward compatible updates so I had to reset database). In any way thanks @raix great work. About dispatch:kernel may I know why you didn't PR instead of creating a package.

its actually overwriting Meteor behaviour

So I suppose it's backward compatible as it's overwriting Meteor's behaviour, so as I already asked before, why didn't you tried to make a PR to Meteor instead of creating a separated package @raix ?

raix commented 7 years ago

@NitroBAY lack of time - it takes time to do a pr. I might talk with @mitar about it or wait for react fibers which tries to solve the same issue

raix commented 7 years ago

@NitroBAY did it solve your issue? (I saw that you had nested blaze autoruns in your render function - was that causing the issue with the kernel? (cause that might be an issue in kernel)

mitar commented 7 years ago

Oh, I am sad to read that using kernel has bad side-effects. Because that is one of things I was planing to integrate into Blaze/Tracker at some point. It seems like a reasonable thing to do. But probably it really can have some side-effects. It would be great to learn which exactly side-effects are.

ilan-schemoul commented 7 years ago

@raix it did solves this issue (also see https://github.com/meteor/blaze/issues/146) and besides it fix #101, thanks !! @mitar maybe you and @raix can play with my reproduction https://github.com/NitroBAY/ground-blaze-bug-repro to understand what's wrong ?

raix commented 7 years ago

Thanks Ill investigate, we should have the kernel working as it unblocks the ui

mitar commented 7 years ago

What I would do is make Tracker.flush be blocking, but the normal autorun be non-blocking. And explain that if you use Tracker.flush you loose the advantages of kernel.

Also, in the latest Blaze version I released, I relaxed Blaze requirements about nested autoruns.

ilan-schemoul commented 7 years ago

My issue with autorun was that this.autorun eventually was not working whereas Tracker.autorun was always (I think) working. Maybe that this.autorun was especially not working in nested templates.

ilan-schemoul commented 7 years ago

I close, even tough you have very interesting discussion, this issue is fix in ground:db as RC6 which removed dispatch:kernel, but dispatch kernel was the one who has problems with autorun as you know : see https://github.com/DispatchMe/meteor-kernel/issues/6.

maxfi commented 7 years ago

@raix is there any chance that dispatch:kernel could be removed from v0.3.15? Unfortunately I cannot use v2 in my project as it breaks compatibility with other packages due to grounded collection being an instance of LocalCollection instead of Mongo.Collection.