Skip to content

fix(jobs): resolve portability issues with background job cleanup - #61830

Merged
susnux merged 2 commits into
nextcloud:masterfrom
stklcode:fix/61635-ps
Jul 17, 2026
Merged

fix(jobs): resolve portability issues with background job cleanup#61830
susnux merged 2 commits into
nextcloud:masterfrom
stklcode:fix/61635-ps

Conversation

@stklcode

@stklcode stklcode commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR resolves two issues with background job cleanup introduced recently:

Portability of cleanup on Alpine, BSD and macOS

The previous implementation executes "ps" with flags -p and -o to check whether a job process was still running. This approach has portability issues regarding e.g.

  • BusyBox ps (Alpine Linux) where ps -p fails
ps: unrecognized option: p
BusyBox v1.37.0 (2026-01-10 15:38:28 UTC) multi-call binary.

Usage: ps [-o COL1,COL2=HEADER] [-T]

Show list of processes

	-o COL1,COL2=HEADER	Select columns for display
	-T			Show threads

(this affects the official "fpm-alpine" Docker image)

  • BSD/macOS where ps -o fails
ps: cmd: keyword not found
ps: no valid keywords; valid keywords:

Replace with posix_kill($pid, 0), the standard POSIX way to probe process existence without delivering a signal. EPERM is treated as "process exists but not owned by current user". This should be available on all supported platforms.

Hardcoded snowflake ID prevents background_jobs_expiration_days

Snowflake ID in JobRuns:deleteBefore() is computed from given timestamp, but immediately overwritten by a literal value making the expiration setting have no effect.

This looks like a leftover artifact from development that should be cleaned up. Remove the hardcoded ID to restore the desired behavior.

Both fixes suggested by @mmdevl in the linked issue.

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@stklcode
stklcode requested a review from a team as a code owner July 5, 2026 08:42
@stklcode
stklcode requested review from come-nc, icewind1991, leftybournes and salmart-dev and removed request for a team July 5, 2026 08:42
@SystemKeeper
SystemKeeper requested a review from Altahrim July 5, 2026 16:24
@susnux susnux added the community pull requests from community label Jul 6, 2026

@Altahrim Altahrim left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice improvement, thank you!

(and yes… remaining test thank you for catching it!)

@come-nc come-nc added the 3. to review Waiting for reviews label Jul 6, 2026
@come-nc come-nc added this to the Nextcloud 35 milestone Jul 6, 2026
@voidzero

Copy link
Copy Markdown

Glad the cause for the error was found but atm the pull request is still open, is someone able to fix the tests so we can get this merged please? I'm receiving 21 emails from cron a day and disabling those emails is not optimal because that would suppress other issues, and manually applying the fix causes checksum failures. Thanks.

@stklcode

Copy link
Copy Markdown
Contributor Author

Rebased the branch. Previously failing CI seems to be unrelated (some database test and failing permissions on a workflow), looks like the rebase might fix at least the latter one.

stklcode added 2 commits July 17, 2026 17:46
The previous implementation executes "ps" with flags -p and -o to check
whether a job process was still running. This approach has portability
issues regarding e.g. BusyBox ps (Alpine Linux) where `ps -p` fails or
BSD/macOS where `ps -o` fails.

Replace with `posix_kill($pid, 0)`, the standard POSIX way to probe
process existence without delivering a signal. EPERM is treated as
"process exists but not owned by current user". This should be available
on all supported platforms.

Fixes: 60ce92a
Suggested-by: Michele Marcionelli
Signed-off-by: Stefan Kalscheuer <stefan@stklcode.de>
Snowflake ID in `JobRuns:deleteBefore()` is computed from given
timestamp, but immediately overwritten by a literal value making the
expiration setting have no effect.

This looks like a leftover artifact from development that should be
cleaned up. Remove the hardcoded ID to restore the desired behavior.

Fixes: dc5499a
Suggested-by: Michele Marcionelli
Signed-off-by: Stefan Kalscheuer <stefan@stklcode.de>
@susnux
susnux merged commit 188d7f6 into nextcloud:master Jul 17, 2026
211 of 215 checks passed
@welcome

welcome Bot commented Jul 17, 2026

Copy link
Copy Markdown

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@susnux susnux added the bug label Jul 17, 2026
@susnux

susnux commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

/backport to stable34

@susnux

susnux commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

/backport to stable33

@backportbot

backportbot Bot commented Jul 17, 2026

Copy link
Copy Markdown

The backport to stable33 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable33
git pull origin stable33

# Create the new backport branch
git checkout -b backport/61830/stable33

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick f3874e0f 5a4737e6

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/61830/stable33

Error: Failed to check for changes with origin/stable33: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@voidzero

Copy link
Copy Markdown

Thanks @stklcode for merging this.

@stklcode
stklcode deleted the fix/61635-ps branch July 17, 2026 17:06
@github-actions

Copy link
Copy Markdown
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

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

Labels

3. to review Waiting for reviews bug community pull requests from community feedback-requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Background job cleanup fails on BSD/macOS; also background_jobs_expiration_days is ignored

5 participants