futtta / ao_critcss_aas

Autoptimize power-up to integrate with criticalcss.com
9 stars 1 forks source link

stale lockfile #50

Closed futtta closed 6 years ago

futtta commented 6 years ago

i was working on a site and cron was running, but apperantly there was a stale queue.lock file lingering about (probably because the PHP-process that was processing the queue was killed on the server). (how) can we detect & fix this?

futtta commented 6 years ago

maybe we can call filemtime() on the lockfile and if older then x hours (where 1<x<4, to be choosen) then hard-remove it (as one instance of a job-processsing process cannot take more then x hours)? or, if we want to be super-careful, not remove the lockfile but tell the user with a warning on the dashboard?

futtta commented 6 years ago

@denydias have been looking at the code and it seems the lockfile is already getting hard-removed in the twice-daily running ao_ccss_maintenance job, cfr. https://github.com/futtta/ao_critcss_aas/blob/master/inc/cron.php#L802-L818, correct? so a stale lock-file could stick around for approx. 12h, but would automatically get removed unconditionally?

denydias commented 6 years ago

Yes, @futtta. Every queue should have a lock mechanism to avoid race conditions. As such, there should be also a mechanism to clean up the locking feature to avoid a stale queue.

ao_ccss_cleaning() is bind to the ao_ccss_maintenance action, which in turn is activated by WP-Cron's twicedaily default schedule. This schedule runs every 12h to remove a locked queue unconditionally.

I do not recommend your 1<x<4 formula embed to the queue processing. A locking mechanism clean up should not depend on the same code of the queue it locks. This is a best practice for queues, but nothing stops you to brake it.

Of course, the clean up depends on a properly running cron.

futtta commented 6 years ago

clear, thanks Deny!

On Tue, Jul 24, 2018 at 5:09 PM, Deny Dias notifications@github.com wrote:

Yes, @futtta https://github.com/futtta. Ever queue should have a lock mechanism to avoid race conditions. As such, there should be also a mechanism to clean up the locking feature to avoid a stale queue.

ao_ccss_cleaning() is bind to the ao_ccss_maintenance action, which in turn is activated by WP-Cron's twicedaily default schedule. This schedule runs every 12h to remove a locked queue.

I do not recommend your 1<x<4 formula embed to the queue processing. A locking mechanism clean up should not depend on the same code of the queue it locks. This is a best practice for queues, but nothing stops you to brake it.

Of course, the clean up depends on a properly running cron.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/futtta/ao_critcss_aas/issues/50#issuecomment-407441937, or mute the thread https://github.com/notifications/unsubscribe-auth/AALEMVyfXUZLgT6pfhkHwb2fCvXpjUtwks5uJzjCgaJpZM4VaF17 .

futtta commented 6 years ago

as there already is logic to remove stale lockfiles, this is closed as "invalid" :)