search

feedhenry / fh-sync

Node.js implementation of the FeedHenry Data Synchronisation Server. To be used in conjunction with the FeedHenry Data Synchronisation Client.
http://feedhenry.org
Apache License 2.0
5 stars 17 forks source link

Use a version of mongodb-queue with built in ttl support for messages #22

Closed david-martin closed 7 years ago

david-martin commented 7 years ago

@wei-lee @aidenkeating Would you mind reviewing this change? The mongodb-queue dependency has been updated to include a change for ttl support https://github.com/chilts/mongodb-queue/pull/22

david-martin commented 7 years ago

@aidenkeating I'm going to merge this now and revisit to reference the upstream version if/when the change is merged.

wtrocki commented 7 years ago

@david-martin It looks like this change broke community version. After migrating from 1.0.3 to 1.0.4 sync calls stopped responding and I'm getting timeouts. Do I need to change something in client to support this?

david-martin commented 7 years ago

@wtrocki Apologies for breaking something with this change. That was certainly not the intent. The change is intended to be 100% backwards compatible with existing sync clients and servers. Looking at the change, it's hard to see how it would make sync requests timeout. Perhaps the startup flow where it sets up indexes is stalling in some scenarios. A greenfield scenario seems to work OK given the test suites here and in the acceptance test repo. Also it is working in a producing scenario.

I wonder if existing indexes or records causes the problem when migrating? Could you get a reproducable test case or example?

wtrocki commented 7 years ago

@david-martin Things like that happening all the time so no worries, but I really stuck on debugging this so just wanted to point it out here. Code is breaking for Raincatcher but basically it will be possible to see the same problem when running example application in this repository. I'm going to use unreleased version for the moment and we can have chat about how this can be fixed without removing this change.

wtrocki commented 7 years ago

What I noticed is that initially sync works for 2-3 requests but then entire processing stops. I add a lot of console logs for this but still cannot find place that is responsible for this behavior.

aidenkeating commented 7 years ago

@david-martin @wtrocki I think this is because of this section here https://github.com/feedhenry/fh-sync/blob/76ef90b98dc5d3c5098e64db656b1d2132477df6/lib/mongodbQueue.js#L89 , it doesn't always invoke the callback. Will try to get around to a PR tonight.

wtrocki commented 7 years ago

Mentioned PR: https://github.com/feedhenry/fh-sync/pull/29