Support for Oplog on cursors using skip

[SimonSimCity]

I've lately dived into the thought of using oplog-redis where I discovered that the default implementation of the oplog-observer does not work when using skip in your db-query:

meteor/packages/mongo/oplog_observe_driver.js

Lines 970 to 974 in 61960a0

	// skip is not supported: to support it we would need to keep track of all
	// "skipped" documents or at least their ids.
	// limit w/o a sort specifier is not supported: current implementation needs a
	// deterministic way to order documents.
	if (options.skip || (options.limit && !options.sort)) return false;

I've talked a bit to the owner of the package and found out that the oplog-redis actually supports this well.

https://github.com/cult-of-coders/redis-oplog/blob/9487742b1aa1ce8942c202ebf5da454607de2983/lib/processors/limit-sort.js#L42-L82

Would it not be an option to take this into the oplog implementation of the main package here?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[SimonSimCity]

ping ...

[filipenevola]

Hi @SimonSimCity this would be definitely a good improvement. I just added pull-requests-encouraged here.

[sebakerckhof]

@SimonSimCity The code you refer to in oplog-redis seems to show this is not supported in oplog-redis, since as soon as there are any sort fields, it falls back to a requery (basically what Meteor will also do in case there is a skip). And the code comment explains why supporting skip in an actual oplog fashion is a bad idea memory-wise.

[sebakerckhof]

Didn't mean to remove the label though. In fact, I think it's possible to do without keeping all skipped ones in memory. The published set needs to be min/max heap instead of a max heap and some other changes. But can't think of why it wouldn't work.

[SimonSimCity]

@sebakerckhof thanks for the feedback. Could you please compare it to the changes I did here? cult-of-coders/redis-oplog#331 I've also written all my insight down at the linked issue (cult-of-coders/redis-oplog#330).

[sebakerckhof]

If you just remove the check which makes meteor use the polling driver for queries with skip, it will work for changes to the objects in the original result set, but it will not update the result set correctly when objects no longer match or if there are new matches.

Meteor's oplog system makes some tradeoffs. One of them is to prefer correctness over potential performance improvements. Another one is that it prefers sending less data over the wire at the cost of memory usage (mergebox). I think these are the correct tradeoffs for Meteor. But there are alternatives available like redis oplog which make different trade offs.

The way oplog works for the normal limit case is quite complex. I'm planning to make a video about the oplog internals soon, but the short version is that it puts the results in a max heap to efficiently compare new results with the current set. If the set becomes too large, it will drop elements. If the set becomes too small, Meteor has too fetch just enough new elements. Then there's a bit of caching and a couple of tricks too improve performance.

Now I don't see a reason why this can't be expanded to support skip, but maybe I'm missing something.

The reason you get immediate updates in the case of the polling driver, is likely because you have just 1 meteor instance. If you connect 2 browsers to 2 instances of your app, you would see delays between the 2.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[SimonSimCity]

ping ...