New implementation

[mitar]

@kostko: Currently new tests beautifully fail.

[mitar]

OK, the new implementation draft. @kostko check it out. Few tests are still failing.

[kostko]

Ok, I'll check it out.

[mitar]

The issue is that it needs a fork of Meteor for tests: https://github.com/meteor/meteor/pull/5146 Not sure if we can go around it.

So the idea is that each fiber has its own tracker queue. And then we can use the same implementation as the client side has. Just per fiber.

I have not done anything to handle yielding (it should simply wait for yield to finish).

And for some strange reason "count on cursor with limit" and "reactive count with cached cursor" Minimongo tests are failing on the server. They should not. On the other hand, if I run those tests with official server-side Tracker, they go through (but some other fail).

Also, probably everywhere where we currently call withNoYieldsAllowed we should bind the call to the computation's fiber. But the issue is that I do not want to store a reference to the fiber into computations/tracker instances. Or can I? So instead of doing @_func = Meteor.bindEnvironment func, onException, @, we should simply always when we call function or callbacks bind to the stored fiber.

[mitar]

So if each fiber has its own queue, and if we simply wait on yield, then this is OK because it does not block other queues/autoruns for other users.

[kostko]

I cannot run any tests due to some dependency conflicts.

[mitar]

What do you mean?

[kostko]

=> Errors prevented startup:                  

   While selecting package versions:
   error: conflict: constraint random@1.0.4-anubhav.0 is not satisfied by 1.0.2.
   Constraints on random come from:
   <top level>
   local-test:peerlibrary:server-autorun@0.3.1 -> mongo-id@1.0.0-anubhav.0
   meteor-platform@1.2.1
   meteor-platform@1.2.1 -> autoupdate@1.1.5 -> webapp@1.1.6 -> boilerplate-generator@1.0.2 -> spacebars@1.0.5 -> blaze@2.0.4 ->
   observe-sequence@1.0.4 -> minimongo@1.0.6

[mitar]

You have to checkout devel Meteor branch, and then run in the package directory:

../meteor/meteor test-packages ./

[kostko]

So this PR bumps the required Meteor version to latest development version?

[mitar]

Just for testing. Because our Meteor submodule version is from devel branch. And because of this bug: https://github.com/meteor/meteor/issues/5144

[mitar]

Updated.

[kostko]

The current implementation will not work as soon as Tracker._trackerInstance() is not the same as computation._trackerInstance. In other words, it will not work correctly when a computation is being executed by a fiber which did not create the computation. And this happens in the "invalidations inside autorun with yields" test, because you yield (blocking the fiber) and another fiber will handle the deferred flush.

The issue is that in that case Tracker.currentComputation will point to the wrong computation (or none at all), which will cause dependencies to be incorrectly registered (or not registered at all).

[mitar]

Hm, because multiple fibers share the same tracker instance? But computation function is bound to the same tracker instance. I think we should only assure that also all other calls are.

So how would you address this? Feel free to add commits to this branch.

[kostko]

But computation function is bound to the same tracker instance. I think we should only assure that also all other calls are.

But the problem is with dependency registration which uses Tracker.currentComputation in case the computation is not explicitly specified. And there you need some way to get the "computation which the current fiber is executing".

I will push a proposed fix.

[mitar]

So how I see it I would make it so that you have to use autorun and flush inside the same fiber. That that would a limitation on the server side. So maybe not exactly just inside the same fiber, but at least inside the same tree of fibers. So if parent made autorun, then children fibers should reuse the same autorun, or something?

[kostko]

I've pushed a fix. All tests now pass.

[mitar]

Not on Travis CI. :-)

[kostko]

This is because of the development version issue.

[mitar]

Great stuff. Can you maybe add more fiber tests?

[mitar]

And we should check if handling of exceptions works correctly. So like an exception which happens in non-first loop.

[mitar]

Tests fixed.

[mitar]

We should remember to remove the release flag sometime in the future. It would be also great if it would be possible to run Meteor devel branch directly: https://github.com/meteor/meteor/issues/5154

[mitar]

I made initial version of server-side reactive queries. But its tests are failing. So maybe it is because of this package, or some other differences.

[mitar]

Updated, but tests still are not passing. :-(

[mitar]

Made also reactive publish endpoints, tests also failing (but they are the same as in related, so they should be OK).

[kostko]

So for server-side reactive queries, one issue may be that changes do not propagate immediately and the tests are assuming that. For example they do things like this:

  coll.insert({_id: 'D'});
  coll.insert({_id: 'E'});
  test.equal(x, "EDCBA");

So they issue a database operation and immediately expect the reactive computation to be re-run. But AFAIK on the server this may not be the case as the server and oplog watching introduce deferred behavior, which may mean that the observe callbacks have not yet fired?

[mitar]

So they issue a database operation and immediately expect the reactive computation to be re-run. But AFAIK on the server this may not be the case as the server and oplog watching introduce deferred behavior, which may mean that the observe callbacks have not yet fired?

True. So let's disable this test.

[mitar]

Or write a copy of our test which sleeps a bit or something.

[mitar]

@kostko: Why are tests failing now that I migrated them to classy tests?

[mitar]

(ServerFlushWithFibers fails for me now.)

[mitar]

I found the error. :-)

[mitar]

Only two tests are now failing for reactive mongo queries and I think it might have something with our autorun. Maybe we should always yield a bit before flush, just to allow other pending fibers to run?

[mitar]

Yea, reactive publish passes all tests! :-)

[mitar]

I think works nicely now. stop() is correctly blocking. I simply wrapped computation methods into fiber guards. But I could not wrap the invalidate method for some reason. Some tests hanged up. @kostko, do you know why?

[kostko]

It seems this causes a deadlock somewhere. Compute seems to cause invalidate to be called when remove is called on a collection. The invalidate is called from a different fiber which then blocks as compute has not yet left the guarded section.

But I am not sure why compute is blocking.

[kostko]

Oh perhaps because it is draining the observe queue immediately and this blocks until all callbacks are executed in other fibers, blocking on a future. And this causes a deadlock in our situation. And this deadlock cannot be detected from our code as it is caused by some external future.