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

Better workload splitting #381

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Better workload splitting #381

wants to merge 1 commit into from

Conversation

mkldon
Copy link

@mkldon mkldon commented Feb 5, 2019

Hi.

The current approach to handling differnt workloads have few issues:

  • it's hard to maintain configuration file, which requires you to list every worker class you have (especially when you have many workers and big team on a project)
  • no way to configure each worker group
  • no way to run individual groups

This patch tries to address these issues.

First of all, it adds .workgroup class method to Worker, so you can assign workers to workgroups when you define them:

class SomeWorker
  include Sneakers::Worker
  workgroup :transactions
end

If workgroup is not specified, worker is assigned to:default group.

Then in sneakers.rb file you can configure each workgroup separately:

Sneakers::Cluster.configure_workrgoups(
  default: {
    workers: 10,
    share_threads: true,
    threads: 10
  },
  transactions: {
    workers: 2,
    share_threads: false
  }
)

Workgroup configuration will be merged in your default configuration after workgroups will be forked.

You should also add Sneakers::Cluster.apply_workgroup_config! to the end of your config if you use hot reload.

Then you can run all workgroups with rake sneakers:cluster or run individual groups with rake sneakers:cluster[<comma-separated-workgroups-list>]

I can supply a more detailed description of this feature to the wiki if you are interested in merging this.

@michaelklishin
Copy link
Collaborator

Thank you but such substantial features will always be slow to review unless you provide a lot of details .e.g. on backwards compatibility.

This looks reasonable on the surface. What are the risks if we decide to adopt this?

@mkldon
Copy link
Author

mkldon commented Feb 12, 2019

Thanks for quick reply.

I don't see any risk of adoption. The only change visible for existing user code is the addition of workgroup class method to Worker, which will be shadowed by a user-supplied method in case a user already had one.

Cluster code is not invoked unless you start sneakers with sneakers:cluster command.

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.

2 participants