Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add support for instance-level throttles#1411

Open
tessereth wants to merge 1 commit into
Shopify:main from
tessereth:instance-throttle-on
Open

Add support for instance-level throttles #1411
tessereth wants to merge 1 commit into
Shopify:main from
tessereth:instance-throttle-on

Conversation

@tessereth

@tessereth tessereth commented Mar 11, 2026

Copy link
Copy Markdown

I've been trying to set up throttles for my maintenance tasks based on configurable values and it's pretty tricky to do in the current code. For instance, I want to be able to configure a database load threshold and tweak the backoff using an attribute. I'm also running the same maintenance task multiple times against different databases and I need different throttle parameters for each. Currently, throttles are set at the class level, so they can't vary per instance.

This PR adds support for per-instance throttles, using basically the same API as the current class-level throttles. Usage looks like:

class MyMaintenanceTask < MaintenanceTasks::Task
 attribute :database, , inclusion: { in: DatabaseShards.all }
 attribute :throttle_db_load_limit, :integer, default: 16
 attribute :throttle_db_load_backoff, :integer, default: 3600
 def initialize(*)
 super
 throttle_on(backoff: throttle_db_load_backoff) do
 ApplicationRecord.connected_to(shard: database) do
 current_load >= throttle_db_load_limit
 end
 end
 end
end

The throttle conditions array is only accessed here via the instance, so I've just overridden this method:

@task.throttle_conditions.reduce(collection_enum) do |enum, condition|

Please let me know what you think. I'm happy to change things if you can think of a better way to achieve the outcomes I'm after.

Copy link
Copy Markdown
Author

The CLA is now signed, I hope you get a chance to take a look at this.

@adrianna-chang-shopify adrianna-chang-shopify left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @tessereth , thanks for your contribution! I think the feature makes sense, but I don't love registering the throttle conditions inside #initialize with the same class-level DSL.

My vote would probably be to change the class-level throttle_on block to take the task instance:

 class UpdatePostsThrottledTask < MaintenanceTasks::Task
 attribute :throttle_backoff_seconds, :integer
 throttle_on(backoff: ->(task) { task.throttle_backoff_seconds }) do
 DatabaseStatus.unhealthy?
 end
 end

It does mean adding an arity check to make things backwards compatible and deprecating backoff procs that don't take an argument. We did something similar for the error handler: #321

Does that sound like it would meet your use case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adrianna-chang-shopify adrianna-chang-shopify adrianna-chang-shopify left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /