Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/bulk redis payload processing #368

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

emaciel10
Copy link
Contributor

@emaciel10 emaciel10 commented Nov 19, 2020

Ability to process redis payloads in bulk to reduce the number of requeries that large updates on a server can cause

Findings in production

Bulk updates that send many redis payloads to our meteor servers causes expensive database requeries for limit-sort publications and can spike cpu for other publications which attempt to process these payloads one at a time.

Goal

Reduce the large number of database lookups and requeries that happen when a large number of redis payloads are processed in bulk

Instead of doing document lookups and requeries one by one as the are needed, we process events in bulk, perform these database lookups in bulk and store them in a temporary documentMap and then iterate over the payloads triggering only a single requery when necessary.

This also gives us the ability to set maxRedisEventsToProcess threshold which prevents us from spiking server cpu for extended periods of time in the case where we have received too many redis payloads to process effectively.

Test Fixes

Looks like mocha tests had started failing due to some updates to the underlying mocha libraries in meteortesting:mocha-core. I pinned down the versions so that the issues with the test suite are resolved without needing to modify all the tests to no longer make use of async and done

@theodorDiaconu
Copy link
Contributor

Please respect the indentation as it's very hard for me to spot the differences. While I do agree that we should use 2, that's for another PR. For now we keep the standard.

@emaciel10
Copy link
Contributor Author

@theodorDiaconu pushed up a fix for the formatting.

@etyp
Copy link

etyp commented Dec 2, 2020

@theodorDiaconu we've been using this in production for a few days now and are seeing great results. Great stuff @emaciel10

@evolross
Copy link
Contributor

evolross commented Jan 9, 2021

Findings in production

Bulk updates that send many redis payloads to our meteor servers causes expensive database requeries for limit-sort publications and can spike cpu for other publications which attempt to process these payloads one at a time.

What exactly do you mean by bulk updates? Like a Mongo initializeUnorderedBulkOp? I'm assuming it's not that as that can only be done on the raw collection beneath the scope of redis-oplog.

So if this for when doing a regular collection.update(query, update, {multi: true}) that would target a "bulk" amount of documents?

@emaciel10
Copy link
Contributor Author

@evolross Yeah by "bulk updates" I am just referring to mongo updates that update a large number of documents. In some cases we use the raw mongo driver to trigger these updates and publish to redis after if really necessary but that is generally the exception. More often these are just regular collection multi updates when that update targets a very large number of documents

collection.update(query, update, {multi: true})

@StorytellerCZ StorytellerCZ added this to the V2.2 milestone Oct 7, 2022
@StorytellerCZ
Copy link
Collaborator

Hi @emaciel10 could you please pull the latest master into this PR, so that we can run the tests here on GitHub? Thank you!

@StorytellerCZ
Copy link
Collaborator

@emaciel10 the tests are failing, please see here: Meteor-Community-Packages#9

@StorytellerCZ StorytellerCZ modified the milestones: V2.2, V2.3 Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants