search

nextcloud / previewgenerator

Nextcloud app to do preview generation in the background.
https://apps.nextcloud.com/apps/previewgenerator
GNU Affero General Public License v3.0
455 stars 57 forks source link

Implement command to reset the pid lock #118

Open XA21X opened 6 years ago

XA21X commented 6 years ago

Since #93, I don't believe pid is ever reset as a fail-safe?

Logs indicated that pre-generate was always returning command is already running so I was getting suspicious. I finally checked the appconfig and noticed that pid == 1, which will never not be running.

I believe this could happen if the preview generator crashes or is interrupted while executing php occ directly (via docker run/exec). If this is the case, some time-based check might still be valuable in addition to the pid check - that is, periodically update a watchdog timestamp in the appconfig and treat the previous instance as dead if it hasn't been updated for a while?

XA21X commented 6 years ago

Or it might be easier to just special-case PID1? :P

joshtrichards commented 6 months ago

I believe this could happen if the preview generator crashes or is interrupted while executing php occ directly (via docker run/exec).

Agreed, this could happen under some circumstances:

https://github.com/nextcloud/previewgenerator/blob/bf04c0afc850db10fc0a4f74234b17fc1c241462/lib/Command/PreGenerate.php#L105-L115

Or it might be easier to just special-case PID1? :P

True, it is probably most likely to happen with 1. Tempting! :)

Maybe also with Docker environments with their own PID namespaces?

If this is the case, some time-based check might still be valuable in addition to the pid check - that is, periodically update a watchdog timestamp in the appconfig and treat the previous instance as dead if it hasn't been updated for a while?

Maybe. Or just check for the other matching running processes (that look like other copies of us) outright and error out as already running in that case.

Related: #328

st3iny commented 5 months ago

That is weird. Running exec should not start the command with PID 1 inside the running container.

But yeah, having a command to reset the pid lock or a fail-safe is probably a good idea.

st3iny commented 4 months ago

In the meantime, the log can be reset by running the following raw SQL query:

DELETE FROM
  oc_appconfig
WHERE
  appid = 'previewgenerator'
  AND configkey = 'pid';