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

[FEATURE] - Change limit at runtime #768

Open
pcfreak30 opened this issue Aug 12, 2024 · 7 comments
Open

[FEATURE] - Change limit at runtime #768

pcfreak30 opened this issue Aug 12, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@pcfreak30
Copy link
Contributor

I have run into the need to set the runtime limit for the cron and want input on what I did before I make a PR.

v2...LumeWeb:gocron:a0bc9ad53805a261cf60a87414e8d3ee734c6632

Since the re-schedule mode uses a chan and they cannot be resized, and trying to "move" entries between chans might be messy? I just added a check and new error object for it.

Kudos.

@pcfreak30 pcfreak30 added the enhancement New feature or request label Aug 12, 2024
@pcfreak30
Copy link
Contributor Author

Update:

Also had to expose limitModeConfig

LumeWeb@2ecb4bd

@JohnRoesler
Copy link
Contributor

This is altering the limit mode on the scheduler after it's been set up, right?

To support this, I'd want to see done in a manner similar to how the job has an Update method. The scheduler could have an update method, that would allow then passing options similar to initial creation. If possible, it could just be all the same options. If there are too many complications with that, there could be a lesser set of update-able options that could be passed in, e.g. WithLimitConcurrentJobs

@pcfreak30
Copy link
Contributor Author

This is altering the limit mode on the scheduler after it's been set up, right?

To support this, I'd want to see done in a manner similar to how the job has an Update method. The scheduler could have an update method, that would allow then passing options similar to initial creation. If possible, it could just be all the same options. If there are too many complications with that, there could be a lesser set of update-able options that could be passed in, e.g. WithLimitConcurrentJobs

Yes. though im not sure what your referring to. Would appreciate if you can clarify or give an example of what your thinking?

(also ill be submitting a PR with all changes im doing outside this too, just many scattered priorities ATM)

@JohnRoesler
Copy link
Contributor

func NewScheduler(options ...SchedulerOption) (Scheduler, error) {}

type scheduler struct {
    ...
}

func (s *scheduler) Update(options ...SchedulerOption) error {}

@pcfreak30
Copy link
Contributor Author

func NewScheduler(options ...SchedulerOption) (Scheduler, error) {}

type scheduler struct {
    ...
}

func (s *scheduler) Update(options ...SchedulerOption) error {}

Only concern is if that would break anything because some things cannot be edited at runtime safely in all cases? Which might add complexity in trying to do that.

I additionally still need a getter for the Limit mode so i can reference it for task coordination, so want to know if you have an issue with that API change?

@pcfreak30
Copy link
Contributor Author

func NewScheduler(options ...SchedulerOption) (Scheduler, error) {}

type scheduler struct {
    ...
}

func (s *scheduler) Update(options ...SchedulerOption) error {}

Only concern is if that would break anything because some things cannot be edited at runtime safely in all cases? Which might add complexity in trying to do that.

I additionally still need a getter for the Limit mode so i can reference it for task coordination, so want to know if you have an issue with that API change?

@JohnRoesler I am spending time on this again in my project, would like to know your response. Thanks.

@JohnRoesler
Copy link
Contributor

Yes, I am ok with adding a Getter for limit mode.

I'd still like to continue exploring the Update method on the Scheduler path. I do agree there are some complications and that not all methods should be available for Update.

Perhaps rather than having it be ...SchedulerOption, it could be a new set, ...SchedulerUpdateOption and then we could have that be a wrapper basically around schedulerOption and only expose the ones that are allowed to be updated

pcfreak30 added a commit to LumeWeb/gocron that referenced this issue Oct 21, 2024
- Add GetLimitMode method to retrieve current limit mode configuration
- Implement UpdateScheduler method to modify scheduler settings at runtime
- Create WithUpdateLimitMode option for updating limit mode and concurrency limit
- Add corresponding unit tests for new functionality
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants