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

occ commands to manage the job list #24551

Closed
wants to merge 1 commit into from

Conversation

DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented May 11, 2016

ToDo:

  • introduce select for update table lock in query builder
  • use select for update to lock selected job
  • add state update call to the job queue to mark job as selected by executor
  • add occ command to remove jobs from the queue

@DeepDiver1975 DeepDiver1975 added this to the 9.1-current milestone May 11, 2016
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @nickvergessen, @icewind1991 and @blizzz to be potential reviewers

@nickvergessen
Copy link
Contributor

introduce select for update in query builder

I looked a bit into SELECT ... FOR UPDATE. I assume you want to use it in lib/private/BackgroundJob/JobList.php to get the new job and lock the row until you wrote the reserved flag to the database?

@DeepDiver1975
Copy link
Member Author

I assume you want to use it in lib/private/BackgroundJob/JobList.php to get the new job and lock the row until you wrote the reserved flag to the database?

exactly

@PVince81
Copy link
Contributor

@nickvergessen please update status, PR was merged. Raise doc / QA tickets if applicable.
Thanks

@nickvergessen
Copy link
Contributor

Done, didn't update this PR, the worker still looks good, but the table change of course is in conflict now.

@DeepDiver1975 DeepDiver1975 force-pushed the stand-alone-job-executer branch 2 times, most recently from d639831 to 4dbf074 Compare May 29, 2016 19:44
@DeepDiver1975 DeepDiver1975 changed the title [WIP] stand alone job executor process stand alone job executor process May 30, 2016
@PVince81
Copy link
Contributor

We're past feature freeze, move to 9.2 ? @DeepDiver1975 @nickvergessen

@DeepDiver1975 DeepDiver1975 modified the milestones: 9.2-next, 9.1-current May 31, 2016
@DeepDiver1975
Copy link
Member Author

We're past feature freeze, move to 9.2 ? @DeepDiver1975 @nickvergessen

agreed

@PVince81
Copy link
Contributor

Is there still interest in getting this feature finished ?

Not sure how this fits in with the latest changes with bkg jobs

@PVince81
Copy link
Contributor

PVince81 commented Apr 6, 2017

moving to backlog for now unless there is interest and time in reviving this soon and getting it completed

@PVince81 PVince81 modified the milestones: backlog, 10.0 Apr 6, 2017
@DeepDiver1975 DeepDiver1975 changed the title stand alone job executor process occ commands to manage the job list Jul 19, 2017
@DeepDiver1975 DeepDiver1975 self-assigned this Jul 19, 2017
$this->logger = new CommandLogger($output);

$waitTime = $input->getOption('sleep');
while (true) {
Copy link
Member

Choose a reason for hiding this comment

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

Any idea how this is expected to be executed? I'm not fan of infinite loops without any escape route, and if this is going to be run as a service the only way to stop the loop would be kill it with fire (aka, sigkill signal).

I think it would be good to capture signals if possible in order to try to make a smooth exit if possible. While this is possible in between the jobs during the sleep time, the challenge is to send some kind of notice to the background job in order to stop it nicely, otherwise the execution thread will likely be running inside the background job without any chance to process the incomming signal, which will lead to the admin to kill the process forcefully.

Copy link
Member Author

Choose a reason for hiding this comment

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

signal handling has need added

protected function execute(InputInterface $input, OutputInterface $output) {
$this->logger = new CommandLogger($output);

$waitTime = $input->getOption('sleep');
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should force a minimum value. I'm not sure if setting a value of 0 will keep the CPU running near 100%

Copy link
Member Author

Choose a reason for hiding this comment

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

addressed - there is a minimum of 1 now

@DeepDiver1975
Copy link
Member Author

@ownclouders rebase

@ownclouders
Copy link
Contributor

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

@ownclouders
Copy link
Contributor

Automated rebase with GitMate.io was successful! 🎉

@DeepDiver1975
Copy link
Member Author

@ownclouders rebase

@ownclouders
Copy link
Contributor

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

@ownclouders
Copy link
Contributor

Automated rebase with GitMate.io was successful! 🎉

@DeepDiver1975
Copy link
Member Author

use owncloud/market#86 which introduces a command to download the latest revocation list from the marketplace.

@mmattel
Copy link
Contributor

mmattel commented Jun 12, 2018

@DeepDiver1975 Is thisPR still valid?

Linking:

Merged: #31630 (Adding background:queue commands: status and delete)
and
Open: #31617 (Add background:execute occ command for running cron jobs manually)

@tomneedham
Copy link
Member

No longer relevant, obsolete

@tomneedham tomneedham closed this Jun 12, 2018
@DeepDiver1975 DeepDiver1975 deleted the stand-alone-job-executer branch June 12, 2018 11:02
@lock
Copy link

lock bot commented Jul 31, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants