[GraphQL Replication] Only create a new push sequence if necessary

[hdwatts]

This PR contains:

• A bugfix

Describe the problem you have without this PR

I was working on a project using GraphQL replication without subscriptions and I left a browser tab open for an ungodly amount of time. To my surprise, my local storage had gotten bloated to the point where I couldn't access it anymore. While investigating I noticed that thousands of push-checkpoints were being created.

Indeed, I was able to confirm that a new push-checkpoint was made on every sync operation. @pubkey confirmed in Gitter that this is indeed a bug.

My solution is to surround the setLastPushSequence with the same check that is in the catch block above. This works well in my testing. I'll note that this doesn't technically solve the memory leak and is only kicking it down the road to when thousands of pushes have been required. I would think there should be a cleanup operation on successful pulls that remove old and irrelevant push checkpoints - but this is early in my tooling with rxdb and I'm very unfamiliar with both this codebase as well the intricacies of pouchdb.

In my testing, this is still working great and the records are no longer being created unnecessarily.

@pubkey wouldn't mind your assistance on if this is the right solution and what the best practice for the test should be

Todos

• Testing
• Changelog

[pubkey]

Change looks good.
We need at least one unit test.
Maybe you could subscribe to collection.$ and count the writes to local documents. Then in the test you can make sure to not do more writes to local documents then needed.

Also we should apply the same change to the replication primitives https://github.com/pubkey/rxdb/blob/master/src/plugins/replication/index.ts#L397

This will not solve the problem of pouchdb filling up the disc space with previous revisions of documents. That can be solved by running a compaction https://pouchdb.com/guides/compact-and-destroy.html#compacting-a-database or by setting auto_compaction: true

[hdwatts]

Thanks @pubkey - I've added tests that check to make sure the lastPushSequence isn't getting updated in both the replication primitives and graphql test suites. I've also implemented a potential fix in the replication primitives codebase.

Separately - thank youfor the compacting docs! This is exactly what I was looking for. As stated in my PR I'm still dipping my toes into PouchDB in general, so this is very helpful insight.

Let me know how else I can assist and I hope you have a nice Sunday

[pubkey]

Code looks good.
Somehow the CI is failing now at another test, I am an not sure why.

[hdwatts]

I'm investigating - it does appear to be failing for a legitimate reason caused by my code change.

[hdwatts]

@pubkey I've adjusted the logic and now all tests are passing locally

[pubkey]

Multiple calls to run() are automatically de-duplicated so that run() does never run in parallel.
Instead of using Promise.all() you should await each call before starting the next one.

for(var i = 0; i < 3; i++){
    await replicationState.run();
}

[hdwatts]

Thanks - made the change and will push it once the current CI is finished

[pubkey]

Great work, I merged it. Thank you.