@noveens, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bantu, @tanghus and @butonic to be potential reviewers.

All committers have signed the CLA.

This will likely not be enough.

In past ownCloud versions, the XML file was the authority regarding database schemas.
Since master / 10.0, we should use migrations to make database changes.

Have a look how it's done here: https://github.com/owncloud/core/blob/master/core/Migrations/Version20170111103310.php#L9.
Some docs about migrations: owncloud-archive/documentation#2742 (unfinished as this is brand new).

Let me know if you need assistance.

Once this is done the next step is to adjust the background job runner to measure the execution time and put it there.

@PVince81 , thanks for the quick review, I'll look into it as soon as possible.
Thanks for the links to ducumentation as well.

@PVince81 , please check now, I completed making another column in oc_jobs table named execution time through migration.

@noveens THx - next would be to fill the columns 😉

@DeepDiver1975 , does that come in the scope of this bug?

@noveens yes, because else the column is completely useless.

Do you need pointers ?

@PVince81, pointers would be a great way to start.

The job is executed here: https://github.com/owncloud/core/blob/master/cron.php#L122

I suggest you add statements around here https://github.com/owncloud/core/blob/master/lib/private/BackgroundJob/Job.php#L52 to measure the running time. Then store the value in the database.

For storing, use something like https://github.com/owncloud/core/blob/master/lib/private/BackgroundJob/JobList.php#L307. Maybe a new method setLastRunTime().

@PVince81 , I have made the new method, but how to check the changes I have made?

Update: Pushed the changes I made, please review @DeepDiver1975

@noveens please have a look at my comment.

To test this you can run php cron.php manually.
If no job is running because of the schedule, you can reset the jobs manually with the following SQL query:

update oc_jobs set last_run=0,last_checked=0,reserved_at=0;

Then run cron.php again and see if your new column is populated.

@PVince81 , please recheck

Code looks good so far. Thanks.

Will test it later.

I tested it but after running cron.php I see the last_run being populated, but the value of "execution_duration" is always 0.

Even if my computer is too fast, I'd still expect values of at least a few millisecond.
Is that timestamp in seconds ?

Wanted to test it with LDAP (which provides ways to have slower jobs) but the app is broken at the moment: #27154

@PVince81 ,
The new column I have added is of integer type since the time() function gives seconds since epoch
But the general case of checking working time of a job has time lesser than 1 second.

Hence I chose this approach.
If you need any updation, please tell me.

• increase last digit in version.php to trigger an update

Tested with LDAP. I saw cron.php taking a minute or so, still the column "execution_duration" contains only zeroes.

How did you test it?

@PVince81 ,
To be frank, I didn't test it because I couldn't find a way to populate the database.
Also, once populated how should I check the values in the database?

It would be great if you could tell me how to test this.

Told you here: #27144 (comment)

The table is already pre-populated by default recurring jobs after setting up OC.

@bantu, @tanghus, @butonic, @DeepDiver1975 and @PVince81 ,

I managed to pass Jenkins now ( took 1h 42m :p ) please have a look and review this PR.

Revert the permission change in the config.sample.php file please.

I had to manually increase the OC version to force the trigger of the migration code (checked against daily master).

I tested the PR in the same way.

New column "execution_time" in the "jobs" table with a default value of -1
Running cron will:

set the execution time in seconds of the job in the "jobs" table. 0 is a valid value. It will help to identify what jobs have run and what jobs not. ✅ 
add new logs entries in the owncloud.log. The logs entries should be clear enough to know what jobs have run, and should match the information contained in the "jobs" table.

✅
Need to test against a "heavy" system to verify we're counting the seconds right and not just setting zeros. Probably testing with LDAP active is enough to have some slow jobs which should take more than 1 second to execute. ✅
Find a job with arguments and check the log output. ✅

👍

IMHO A small enhancement when running a job without arguments will be to not print the with arguments: text

{"reqId":"4vkT2Z8Fy\/yzBApDPlqT","remoteAddr":"","app":"cron","message":"Finished background job, the job took : 0 seconds, this job is an instance of class : OCA\\Files\\BackgroundJob\\CleanupFileLocks with arguments : ","level":0,"time":"2017-02-20T12:08:15+00:00","method":"--","url":"--","user":"--"}

@jvillafanez ,
I didn't get the part I have to change,
you're talking about which permissions?

That change in the permissions of the config.sample.php . It shouldn't appear.

@jvillafanez ,
pushed the changes.

@DeepDiver1975 ,
please re-review this

I don't have anything more to add 👍

@PVince81 ,
Can this be merged, I think it's fine now.

@DeepDiver1975 can you kick Jenkins to retest this ?

@PVince81 ,
This passed jenkins before my previous commit, which was just modifying the file permissions of config.sample.php which i suppose has nothing to do with any of the tests.

Added to new features page: https://github.com/owncloud/core/wiki/ownCloud-10.0-Features

@PVince81 @DeepDiver1975 Backport the logging part? I think it could be quite useful and not intrusive. DB changes won't be backported.

Yes, please backport the logging part.

@noveens can you do it ? Basically create a branch off "stable9.1", put your commits there then submit a PR that targets stable9.1. Only put the logging part there.

@PVince81 ,
Sure I can do that.
Just the logging part right?
And do I have to update anything in this branch?

Nothing to update here. Make a new branch.

stable9.1 logging changes backport: #27239

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.