redis-oplog
redis-oplog copied to clipboard
Support addedAt/changedAt/removedAt/movedAt
Redis oplog doesn't seem to support the addedAt, changedAt, removedAt, or movedAt observe callbacks.
These callbacks are described in the Meteor docs https://docs.meteor.com/api/collections.html#Mongo-Cursor-observe
This could break many use cases, but in particular it breaks Easy Search: https://github.com/matteodem/meteor-easy-search/blob/master/packages/easysearch:core/lib/core/search-collection.js#L217
This can be replicated by running the following code on the server from the Meteor shell:
Meteor.users.find({}).observe({ addedAt(){ console.log('zzz'); } });
Note that zzz is not printed to the logs when redis oplog is enabled.
Edit: Looks like a workaround is falling back to the polling observe driver for observes that require these callbacks, as described in https://github.com/cult-of-coders/redis-oplog/blob/master/docs/finetuning.md.
The oplog observe driver falls back to the polling observe driver on ordered callbacks. Perhaps redis oplog should just do this as well.
It works with the new branch that is in PR, we're only overriding the observerDriver, hooking directly into the core. I still have some small things to do in there, mostly related to sending optimistic ui events async so it doesn't lock.
Oh nice, this one https://github.com/cult-of-coders/redis-oplog/pull/283 ?
@veered yes. The guys from hive.com are using it successfully for over 2 weeks already and they see great improvements, so I would mark it as stable, but I feel that merging it without solving async optimistic ui pushes is like a step-back. I will do my best to fit it in the next weeks to solve it.
@theodorDiaconu #283 has better performance?
Havent tested performance. I think that thos is how it should have been coded since the beginning.
Regards, Theodor
On 22 Aug 2018, at 1:33 AM, Lucas Hansen [email protected] wrote:
@theodorDiaconu #283 has better performance?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
@veered I believe this is working now. That PR got merged and released. Did you have a chance to try it out?