From 69538274c1aceb40374124063bb902cf32da76e4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 26 May 2017 08:46:48 -0700 Subject: [PATCH] Garbage collect old daemon records based on modification date, not creation date Summary: Fixes T12720. Currently, old daemon records are collected based on creation date. By default, the GC collects them after 7 days. After T12298, this can incorrectly collect hibernating daemons which are in state "wait". In all cases, this could fail to collect daemons which are stuck in "running" for a long time for some reason. This doesn't seem to be causing any problems right now, but it makes me hesitant to do "dateCreated + not running or waiting" since that might make this become a problem, or make an existing problem with this that we just haven't bumped into worse. Daemons always heartbeat periodically and update their rows, so `dateModified` is always fresh, so collect rows based only on modification date. Test Plan: - Ran daemons (`bin/phd start`). - Waited a few minutes. - Verified that hibernating daemons in the "wait" state had fresh timestamps. - Verified that very old daemons still got GC'd properly. ``` mysql> select id, daemon, status, FROM_UNIXTIME(dateCreated), FROM_UNIXTIME(dateModified) from daemon_log; +-------+--------------------------------------+--------+----------------------------+-----------------------------+ | id | daemon | status | FROM_UNIXTIME(dateCreated) | FROM_UNIXTIME(dateModified) | +-------+--------------------------------------+--------+----------------------------+-----------------------------+ | 73377 | PhabricatorTaskmasterDaemon | exit | 2017-05-19 10:53:03 | 2017-05-19 12:38:54 | ... | 73388 | PhabricatorRepositoryPullLocalDaemon | run | 2017-05-26 08:43:29 | 2017-05-26 08:45:30 | | 73389 | PhabricatorTriggerDaemon | run | 2017-05-26 08:43:29 | 2017-05-26 08:46:35 | | 73390 | PhabricatorTaskmasterDaemon | wait | 2017-05-26 08:43:29 | 2017-05-26 08:46:35 | | 73391 | PhabricatorTaskmasterDaemon | wait | 2017-05-26 08:43:33 | 2017-05-26 08:46:33 | | 73392 | PhabricatorTaskmasterDaemon | wait | 2017-05-26 08:43:37 | 2017-05-26 08:46:31 | | 73393 | PhabricatorTaskmasterDaemon | wait | 2017-05-26 08:43:40 | 2017-05-26 08:46:33 | +-------+--------------------------------------+--------+----------------------------+-----------------------------+ 17 rows in set (0.00 sec) ``` Note that: - The oldest daemon is <7 days old -- I had some other older rows but they got GC'd properly. - The hibernating taskmasters (at the bottom, in state "wait") have recent/more-current `dateModified` dates than their `dateCreated` dates. Reviewers: chad, amckinley Reviewed By: chad Maniphest Tasks: T12720 Differential Revision: https://secure.phabricator.com/D18024 --- .../PhabricatorDaemonLogGarbageCollector.php | 5 ++--- src/applications/daemon/storage/PhabricatorDaemonLog.php | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/applications/daemon/garbagecollector/PhabricatorDaemonLogGarbageCollector.php b/src/applications/daemon/garbagecollector/PhabricatorDaemonLogGarbageCollector.php index 3ff26e3db7..fd7de9dd33 100644 --- a/src/applications/daemon/garbagecollector/PhabricatorDaemonLogGarbageCollector.php +++ b/src/applications/daemon/garbagecollector/PhabricatorDaemonLogGarbageCollector.php @@ -19,10 +19,9 @@ final class PhabricatorDaemonLogGarbageCollector queryfx( $conn_w, - 'DELETE FROM %T WHERE dateCreated < %d AND status != %s LIMIT 100', + 'DELETE FROM %T WHERE dateModified < %d LIMIT 100', $table->getTableName(), - $this->getGarbageEpoch(), - PhabricatorDaemonLog::STATUS_RUNNING); + $this->getGarbageEpoch()); return ($conn_w->getAffectedRows() == 100); } diff --git a/src/applications/daemon/storage/PhabricatorDaemonLog.php b/src/applications/daemon/storage/PhabricatorDaemonLog.php index 2e61da3872..d3ac485ea2 100644 --- a/src/applications/daemon/storage/PhabricatorDaemonLog.php +++ b/src/applications/daemon/storage/PhabricatorDaemonLog.php @@ -37,13 +37,13 @@ final class PhabricatorDaemonLog extends PhabricatorDaemonDAO 'status' => array( 'columns' => array('status'), ), - 'dateCreated' => array( - 'columns' => array('dateCreated'), - ), 'key_daemonID' => array( 'columns' => array('daemonID'), 'unique' => true, ), + 'key_modified' => array( + 'columns' => array('dateModified'), + ), ), ) + parent::getConfiguration(); }