This repository was archived by the owner on Oct 11, 2022. It is now read-only.
Merge to production #2570
Merged
Merge to production #2570
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**TL;DR: Rework our asyncify util and all the subscriptions once more to both plug the memory leak and stop our db connections from crashing.** Yesterday, while just sitting around and daydreaming I finally saw the light and understood why the shit that has been happening with our API has been happening. There are two phases to this story: The first phase is the phase of memory leaks and the second phase is the more recent one of database connection errors. We had those memory leaks, and I investigated them and reworked our entire subscriptions setup so that each user would get one changefeed per subscription. That worked beautifully, but unfortunately we started getting errors of db connections dropping. Thanks to @thelinuxlich I realized that those were happening because it turns out changefeeds don't scale well, especially not to hundreds at a time. This patch fixes the connection dropping by reverting back to a setup similar to our earlier version where we only have 4 changefeeds total and then filter the live results on the server per listener. Now you might be thinking "Hold on but didn't we have memory leaks then?", and we totally did and the great thing is that I realised why: The old `asyncify` util would buffer incoming live results. It would buffer all incoming results for any subscriber that came after. That means after six hours of production traffic we'd have buffered hundreds of messages and threads in memory, which led to the server running out of memory, which led to it crashing. So this time, I've reworked our asyncify util again but without that buffering, which means we'll have the stability of the connections by only having 4 changefeed open, but we also won't have memory leaks. Win win! 🎉
…-for-all Plug the memory leak and stop the db connections from crashing
This reverts commit f0204f6.
…tion-preferences Upgrade community notification preferences
Fix linkification
Oopsies
Implement DangerJS
Fix message bubbles
brianlovin
previously approved these changes
Mar 13, 2018
Generated by 🚫 dangerJS |
Contributor
Author
|
This is live. God damn tests... |
Contributor
|
Lol so uh...now what? |
brianlovin
approved these changes
Mar 13, 2018
Contributor
|
Merge anyways? |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Let's deploy the subscriptions changes to prod 🔥 I've been testing them all day and it's been working great!
Changes