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.

Sign up for GitHub

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

Jump to bottom

Issue/2345 #2427

Merged
merged 16 commits into from Mar 17, 2020
Merged

Issue/2345 #2427

merged 16 commits into from Mar 17, 2020

Conversation

xsrust
Copy link
Contributor

@xsrust xsrust commented Mar 12, 2020

Addresses #2345

Overview of changes:

  • Make Download Global Usage super-admin only
  • Create Download Monthly Usage button to get a CSV of:
    • Month, new plans, new users, downloads, plans shared
    • For each month since creation
  • New models/services for statistics on exported and shared plans
    • Added these to the stats rake tasks
  • Add parallel gem
    • Add parallelized tasks for rake stat:build and rake stat:build_last_month
    • Added threading to the other create_ services
    • Change DB default pooling to 16

Note:
UI with the multiple buttons looks funny at the moment, but I will refactor the layout in another feature's PR as I'd started this work there.

briri
briri approved these changes Mar 13, 2020
View reviewed changes
Copy link
Contributor

@briri briri left a comment

Choose a reason for hiding this comment

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

looks good @xsrust. It would be great at some point to go back into this stats code and clean up the models and services to make them more DRY.

Feel free to merge yourself once any requests from UAT are finished

app/services/org/create_created_plan_service.rb Outdated
@@ -9,7 +19,7 @@ class << self
def call(org = nil)
orgs = org.nil? ? Org.all : [org]

orgs.each do |org|
Parallel.each(orgs, in_threads: 2) do |org|
Copy link
Contributor

Choose a reason for hiding this comment

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

this is great. I will use it in some of the ROR/API V1 upgrade tasks that take a long time to process

Copy link
Contributor

Choose a reason for hiding this comment

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

might be able to switch to use ActiveJob once we move to Rails 5+

config/database.yml.sample
@@ -3,7 +3,7 @@ defaults: &defaults
encoding: <%= ENV['DB_ADAPTER'] == "mysql2" ? "utf8mb4" : "" %>
username: <%= ENV["DB_ADAPTER"] == "postgresql" ? 'postgres' : '' %>
database: roadmap_<%= ENV['RAILS_ENV'] %>
pool: 5
pool: 16
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this for the parallel gem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, essentially each thread/process takes an additional db connection, so it was flooding the default 5. 16 is a bit overkill for development, but it means we don't end up with connection-timeout errors

lib/tasks/stat.rake
"stat:create_last_month:shared_plan",
"stat:create_last_month:exported_plan"]

Parallel.each(tasks, progress: "Building Stats", in_processes: 4) do |task|
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 this is great

@briri
Copy link
Contributor

briri commented Mar 13, 2020

looks like you have some failing stats tests too that need to be addressed

briri
briri approved these changes Mar 16, 2020
View reviewed changes
Copy link
Contributor

@briri briri left a comment

Choose a reason for hiding this comment

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

looks good @xsrust

@xsrust
Copy link
Contributor Author

xsrust commented Mar 17, 2020

Looks like the github-workflows are getting confused when checking things out from the DCC fork.
I've updated the mySQL side of our workflows to the checkout@v2 to patch the problem.
If that works, I'll apply the same fix to the remaining workflow files.

actions/checkout#23

@xsrust
Copy link
Contributor Author

xsrust commented Mar 17, 2020

Going to merge this so I can rebase the work on #1682
They both affect the services and usage-dashboard heavily, so there's likely to be conflicts to fix on that branch.

@xsrust xsrust merged commit 66e196b into DMPRoadmap:development Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@briri briri briri approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@xsrust @briri