Reduce Puma memory usage by fixing N+1s and Ruby-side data processing#764
Merged
Reduce Puma memory usage by fixing N+1s and Ruby-side data processing#764
Conversation
…ssing - Memoize Host.find_by_domain with a class-level domain map, invalidated on commit. Was loading all 2K hosts and parsing each URL with Addressable on every API lookup request. - AuthorsController#show: replace .empty? with .exists?, consolidate duplicate query scopes, move label counting from pluck/flatten/group_by in Ruby to SQL unnest(), replace group(:repository) with a join that avoids loading AR objects as hash keys. - RepositoriesController: replace issues.includes(:owner).map(&:owner) with a direct Owner subquery to find hidden users. Was loading every issue record for the repository into memory. - Repository model: rewrite label counting methods to use SQL unnest() instead of plucking all label arrays into Ruby. - IssuesController#index: add includes(:host, :repository) to fix N+1 from issue.html_url in the partial.
…x to domain map Move labels_with_counts to Issue model as a class method instead of duplicating it in AuthorsController and Repository. Extract hidden_users_for helper in RepositoriesController to replace three identical blocks. Add Mutex around Host.domain_map for thread safety under multi-threaded Puma.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Puma web process is using ~25GB RSS. AppSignal metrics show
AuthorsController#showconsuming 64% of total request time, withRepositoriesController#showat 18.5%.Root causes and fixes:
Host.find_by_domainwas loading all 2,038 hosts and parsing each URL with Addressable on every request. Now memoized as a class-level hash, invalidated viaafter_commit.AuthorsController#showhad several problems:pluck(:labels).flatten.compact.group_bymaterialized every label array into Ruby -- replaced with SQLunnest()group(:repository).countloaded AR objects as hash keys for 35M repos -- replaced withjoins(:repository).group("repositories.full_name").empty?instead of.exists?for existence checkRepositoriesController#show,charts,chart_dataloaded all issues withincludes(:owner)then.map(&:owner)to find 3 hidden users. Replaced with a directOwner.hidden.where(login: subquery).Repositorymodel*_labels_countmethods plucked all label arrays into Ruby. Replaced with SQLunnest().IssuesController#indexhad N+1 fromissue.html_urlloading host and repository per row. Addedincludes(:host, :repository).