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

Improve job runner configuration/execution #9823

Closed
jonasraoni opened this issue Mar 22, 2024 · 13 comments
Closed

Improve job runner configuration/execution #9823

jonasraoni opened this issue Mar 22, 2024 · 13 comments
Assignees
Labels
Enhancement:1:Minor A new feature or improvement that can be implemented in less than 3 days.
Milestone

Comments

@jonasraoni
Copy link
Contributor

jonasraoni commented Mar 22, 2024

Describe the bug
The documentation states that jobs can be executed by:

  • Scheduled tasks
  • At the end of requests
  • Specialized worker

But in fact, once the job_runner is enabled, both the scheduled task and the request based execution will execute the jobs.

Solution
Adding a setting to control whether jobs should be executed by a scheduled task/request should be enough.

What application are you using?
OJS 3.4

PRs (main branch)
pkp-lib --> #10196
ojs --> pkp/ojs#4364
omp --> pkp/omp#1662
ops --> pkp/ops#743

Additional Information
This need to be merged to main (upcoming 3.5) after the merge of #9678
For implementation details see release note and #9823 (comment)

@jonasraoni jonasraoni added the Enhancement:1:Minor A new feature or improvement that can be implemented in less than 3 days. label Mar 22, 2024
@jonasraoni jonasraoni added this to the 3.4.0-x milestone Mar 22, 2024
@bozana
Copy link
Collaborator

bozana commented Mar 22, 2024

@jonasraoni, what documentation you mean, this one https://docs.pkp.sfu.ca/admin-guide/en/deploy-jobs?
Is CLI tool the same as scheduled task? -- I thought not...
For CLI tool (run via Cron job) and Worker solution one needs to disable the build-in option job_runner.
You maybe mean when a user uses Cron job for scheduled tasks, then the jobs are also run there? But isn't it in that case that the user needs to deactivate AcronPlugin and then the jobs would also not run two times, or is it here the problem that even if the AcronPlugin is not used the jobs would run?

@jonasraoni
Copy link
Contributor Author

There was this problem before, but I've already fixed 😁

I've mixed the CLI tool with the scheduled task execution, so the documentation is ok, I've updated the description :)

@bozana
Copy link
Collaborator

bozana commented Mar 22, 2024

So just to be sure I understand:
What happens if job_runner = off and the user is running scheduled tasks by a Cron job? Will jobs be run however by the scheduled task?

@jonasraoni
Copy link
Contributor Author

At this moment:
job_runner=On = OJS will execute jobs at the end of requests and also when the scheduled tasks are running (either via Acron or a CRON task).
job_runner=Off = OJS will not execute jobs by itself.

@bozana
Copy link
Collaborator

bozana commented Mar 25, 2024

Hmm...
So if scheduled tasks are run using a Cron job, the Acron plugin needs to be disabled.
If the jobs are run via Cron + CLI or Workder (as in documentation) then the job_runner has to be Off.
Then there are no problems, correct?

@touhidurabir
Copy link
Member

@jonasraoni I also think we need to have another config option to handle it . However along with this I also like to have some sort of limiter to introduce when jobs are running via cron and that can also be configured (perhaps from the config file).

Right now if a job runs via cron, it will run or try to run all the pending jobs in the list at a time . This is not bad when there is only few jobs but not an ideal situation when there is a lot of pending jobs (which has been accumulated for quite some time for some unforeseen reason) or few jobs with long run time . This may cause memory run out issue and also not very efficient .

Solution 1 :

we can :-

  1. introduce another config option along with config option to control the job execution via cron (probably not a good idea as we are starting to have too many config options)
  2. we can use the existing config option job_runner_max_jobs which also used by internal job runner (seems a reasonable one to me).

Only issue using the existing job_runner_max_jobs is when an installation want to use both the job runner and corn with different config setting but then will be forced to use to same one for both option (probably such use case will be too low or non existence).

Solution 2:

Another way we can go about this is to use the job runner mechanism for the cron also . and it's not much different as both use pretty much same approach but job runner have some additional constrains (such as max jobs, time, memory and possible estimation of next job to run ), underneath after applying the constrains, it runs jobs one by one .

@touhidurabir touhidurabir modified the milestones: 3.4.0-x, 3.4.0-6 Apr 1, 2024
@touhidurabir
Copy link
Member

touhidurabir commented Jul 11, 2024

@asmecher @jonasraoni I like to propose the following changes

we do need a new config setting for job section . Maybe named as schedule_job_runner which will control the job processing via scheduler . But we also need to consider if job is getting processed via other means e.g. worker/job_runner . so how about :-

  1. if only the process_jobs_at_task_scheduler in job section set to On jobs will get to processed via schedule tasks .
  2. When the scheduler run in CLI, the schedule task will process jobs but not more than a specific number . we can use the value of job_runner_max_jobs for this purpose. The reason for this is not to allow schedule tasks process a huge number of jobs at a single time as scheduler also need to process other schedule tasks . Basically we need to consider the performance impact for scheduler .
  3. if the scheduler running in web mode, it will utilize Job Runner feature but only if job_runner set to Off. The reason for this is as job runner process jobs in web request mode, we don't want scheduler to process jobs in same way when it's also running in web request mode .

One important decision to make , should we prioritize it for 3.4.0-x for push it for upcoming 3.5.0 as we will have some breaking changes with the merge of #9678 ?

@touhidurabir touhidurabir modified the milestones: 3.4.0-6, 3.5.0 LTS Jul 11, 2024
@asmecher
Copy link
Member

^ @touhidurabir and I discussed this and I'd prefer leaving this kind of change for 3.5.0. We're not making heavy use of jobs/tasks that have much overhead or turnaround time, and aren't worried about concurrent e.g. execution of the same task, as we have decent enough locking. We should be fine for 3.4.0 as-is IMO.

@touhidurabir
Copy link
Member

@jonasraoni can you review the linked PRs at #9823 (comment) ? However this need to wait before we finalise and merge the #9678 .

@jonasraoni
Copy link
Contributor Author

FYI I'll wait the other issue to get merged before reviewing this one.

touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 1, 2024
@touhidurabir
Copy link
Member

@jonasraoni I have updated and rebased the PR. As the #9678 got merged, it's ready for review .

@jonasraoni
Copy link
Contributor Author

I'll take a look now, I just read the comments, and I think it's aligned with what I wrote in the description (to have a more granular control).

touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/omp that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/omp that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/ops that referenced this issue Aug 2, 2024
touhidurabir added a commit to touhidurabir/ops that referenced this issue Aug 2, 2024
touhidurabir added a commit that referenced this issue Aug 2, 2024
#9823 update the crontrol job processing via scheduler
touhidurabir added a commit to pkp/ojs that referenced this issue Aug 2, 2024
pkp/pkp-lib#9823 update the crontrol job processing via scheduler
touhidurabir added a commit to pkp/omp that referenced this issue Aug 2, 2024
pkp/pkp-lib#9823 update the crontrol job processing via scheduler
touhidurabir added a commit to pkp/ops that referenced this issue Aug 2, 2024
pkp/pkp-lib#9823 update the crontrol job processing via scheduler
@touhidurabir
Copy link
Member

All merged, PR and implementation details #9823 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement:1:Minor A new feature or improvement that can be implemented in less than 3 days.
Projects
None yet
Development

No branches or pull requests

4 participants