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

Restore local SCP speed via Sprockets 4, fix Dockerized segfault (SCP-3531) #1109

Merged
merged 2 commits into from
Jul 30, 2021

Conversation

eweitz
Copy link
Member

@eweitz eweitz commented Jul 28, 2021

This fixes very long iteration times in local SCP instances, which affect almost all SCP engineers. Local page load times are now back to 3-5 seconds, down from 30-180 seconds.

Background

In a first, Mixpanel performance tracking for development helped diagnose this productivity bug. git bisect was used to isolate the causal commit. That commit downgraded from Sprockets 4 to Sprockets 3.7.2 to fix segfault errors on Ubuntu. The problematic change fixed the segfault when running assets:precompile [1] on Dockerized environments -- CI, staging, and production -- but caused severe slowness for non-Dockerized environments, i.e. development. Oddly, the slowness affected all engineers except one, who had made the change. An affected engineer verified they shared the same Ruby environment and general local setup as the unaffected engineer.

Changes

These changes restore local speed while avoiding segfaults in Dockerized environments. The fix was copied from anecdotal solutions noted in the upstream issue, specifically here. The mechanism is unclear but seems effective. Over 5 runs of the triggering command (leveraging ways to rapid test here and here) have been successful.

The original segfault only occurred on 10-15% of deployments. So it's possible that these changes do not fix the segfault. If that proves so, then team chats have decided that the cost of very slow development outweighs the cost of an extra occasional few minutes of downtime and need to manually run the full command [1] in the production VM.

This satisfies SCP-3531.


  1. Full command that had triggered segfault:
    sudo -E -u app -H bundle exec rake NODE_ENV=production RAILS_ENV=$PASSENGER_APP_ENV SECRET_KEY_BASE=$SECRET_KEY_BASE assets:precompile

@bistline
Copy link
Contributor

bistline commented Jul 28, 2021

@eweitz if you want these to run even faster, you could temporarily comment out lines 116-122 and 137-153 in bin/run_tests.sh. That way it will only build the Docker container, compile the static assets, and run yarn ui-test. One we've shown that we can get many green builds, then we can uncomment everything to run the full builds.

@codecov
Copy link

codecov bot commented Jul 28, 2021

Codecov Report

Merging #1109 (231c24c) into development (6b22109) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 231c24c differs from pull request most recent head 52bfb9f. Consider uploading reports for the commit 52bfb9f to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           development    #1109      +/-   ##
===============================================
+ Coverage        66.43%   66.44%   +0.01%     
===============================================
  Files              192      193       +1     
  Lines            16956    17016      +60     
  Branches           662      662              
===============================================
+ Hits             11265    11307      +42     
- Misses            5531     5549      +18     
  Partials           160      160              
Impacted Files Coverage Δ
lib/delayed_job_accessor.rb 88.88% <0.00%> (-5.56%) ⬇️
app/controllers/api/v1/search_controller.rb 84.68% <0.00%> (-0.73%) ⬇️
app/models/user_annotation.rb 38.57% <0.00%> (-0.38%) ⬇️
app/models/data_repo_client.rb 76.22% <0.00%> (ø)
lib/service_account_manager.rb 100.00% <0.00%> (ø)
app/models/hca_azul_client.rb 59.61% <0.00%> (ø)
app/controllers/application_controller.rb 69.38% <0.00%> (+0.42%) ⬆️
app/models/study.rb 83.22% <0.00%> (+0.46%) ⬆️
app/models/admin_configuration.rb 43.95% <0.00%> (+1.25%) ⬆️
app/lib/summary_stats_utils.rb 95.55% <0.00%> (+8.88%) ⬆️

@eweitz eweitz marked this pull request as ready for review July 28, 2021 20:25
@eweitz eweitz changed the title Restore local SCP speed via Sprockets 4, fix Dockerized segfault Restore local SCP speed via Sprockets 4, fix Dockerized segfault (SCP-3531) Jul 29, 2021
@eweitz eweitz marked this pull request as draft July 29, 2021 13:47
@eweitz eweitz force-pushed the ew-fix-local-slowness branch from 3098212 to 231c24c Compare July 29, 2021 13:50
@eweitz eweitz marked this pull request as ready for review July 29, 2021 13:50
@eweitz eweitz requested review from bistline and ehanna4 July 29, 2021 13:50
Copy link
Contributor

@bistline bistline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad this fixes the local problem. I hope it addresses the segfaults too!

Copy link
Contributor

@ehanna4 ehanna4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the background/context, excited to try this out!

@eweitz eweitz merged commit 06be69d into development Jul 30, 2021
@bistline bistline deleted the ew-fix-local-slowness branch August 9, 2021 17:17
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.

None yet

3 participants