From e46a7cdf80e4b6b51adc7df2dd77918f7eb5b1da Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sat, 9 Mar 2019 20:25:39 +0100 Subject: [PATCH 1/4] Avoid double update runs --- src/Core/Update.php | 119 ++++++++++++++++++++++++-------------------- 1 file changed, 65 insertions(+), 54 deletions(-) diff --git a/src/Core/Update.php b/src/Core/Update.php index e0a7c4a266..b849b4c151 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -68,68 +68,67 @@ class Update Lock::release('dbupdate', true); } - $build = Config::get('system', 'build'); + $current = intval(DB_UPDATE_VERSION); + $stored = self::getBuild(); - if (empty($build) || ($build > DB_UPDATE_VERSION)) { - $build = DB_UPDATE_VERSION - 1; - Config::set('system', 'build', $build); - } - - if ($build != DB_UPDATE_VERSION || $force) { + if ($stored < $current || $force) { require_once 'update.php'; - $stored = intval($build); - $current = intval(DB_UPDATE_VERSION); - if ($stored < $current || $force) { - Config::load('database'); + Config::load('database'); - Logger::log('Update from \'' . $stored . '\' to \'' . $current . '\' - starting', Logger::DEBUG); + Logger::log('Update from \'' . $stored . '\' to \'' . $current . '\' - starting', Logger::DEBUG); - // Compare the current structure with the defined structure - // If the Lock is acquired, never release it automatically to avoid double updates - if (Lock::acquire('dbupdate', 120, Cache::INFINITE)) { - - // run the pre_update_nnnn functions in update.php - for ($x = $stored + 1; $x <= $current; $x++) { - $r = self::runUpdateFunction($x, 'pre_update'); - if (!$r) { - break; - } - } - - // update the structure in one call - $retval = DBStructure::update($basePath, $verbose, true); - if (!empty($retval)) { - if ($sendMail) { - self::updateFailed( - DB_UPDATE_VERSION, - $retval - ); - } - Logger::log('ERROR: Update from \'' . $stored . '\' to \'' . $current . '\' - failed: ' - $retval, Logger::ALL); - Lock::release('dbupdate'); - return $retval; - } else { - Config::set('database', 'last_successful_update', $current); - Config::set('database', 'last_successful_update_time', time()); - Logger::log('Update from \'' . $stored . '\' to \'' . $current . '\' - finished', Logger::DEBUG); - } - - // run the update_nnnn functions in update.php - for ($x = $stored + 1; $x <= $current; $x++) { - $r = self::runUpdateFunction($x, 'update'); - if (!$r) { - break; - } - } - - Logger::log('Update from \'' . $stored . '\' to \'' . $current . '\' - successful', Logger::DEBUG); - if ($sendMail) { - self::updateSuccessfull($stored, $current); - } + // Compare the current structure with the defined structure + // If the Lock is acquired, never release it automatically to avoid double updates + if (Lock::acquire('dbupdate', 0, Cache::INFINITE)) { + // recheck again in case we accidentally spawned multiple updates + $stored = intval(self::getBuild()); + if ($stored >= $current) { Lock::release('dbupdate'); + return ''; } + + // run the pre_update_nnnn functions in update.php + for ($x = $stored + 1; $x <= $current; $x++) { + $r = self::runUpdateFunction($x, 'pre_update'); + if (!$r) { + break; + } + } + + // update the structure in one call + $retval = DBStructure::update($basePath, $verbose, true); + if (!empty($retval)) { + if ($sendMail) { + self::updateFailed( + DB_UPDATE_VERSION, + $retval + ); + } + Logger::log('ERROR: Update from \'' . $stored . '\' to \'' . $current . '\' - failed: ' - $retval, Logger::ALL); + Lock::release('dbupdate'); + return $retval; + } else { + Config::set('database', 'last_successful_update', $current); + Config::set('database', 'last_successful_update_time', time()); + Logger::log('Update from \'' . $stored . '\' to \'' . $current . '\' - finished', Logger::DEBUG); + } + + // run the update_nnnn functions in update.php + for ($x = $stored + 1; $x <= $current; $x++) { + $r = self::runUpdateFunction($x, 'update'); + if (!$r) { + break; + } + } + + Logger::log('Update from \'' . $stored . '\' to \'' . $current . '\' - successful', Logger::DEBUG); + if ($sendMail) { + self::updateSuccessfull($stored, $current); + } + + Lock::release('dbupdate'); } } @@ -293,4 +292,16 @@ class Update //try the logger Logger::log("Database structure update successful.", Logger::TRACE); } + + private static function getBuild() + { + $build = Config::get('system', 'build'); + + if (empty($build) || ($build > DB_UPDATE_VERSION)) { + $build = DB_UPDATE_VERSION - 1; + Config::set('system', 'build', $build); + } + + return (is_int($build) ? intval($build) : DB_UPDATE_VERSION - 1); + } } From 7047a13ffa264d3fca66be2c259dde2706e4e2a5 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sat, 9 Mar 2019 20:31:49 +0100 Subject: [PATCH 2/4] added phpdoc --- src/Core/Update.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Core/Update.php b/src/Core/Update.php index b849b4c151..d65adfebea 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -293,6 +293,11 @@ class Update Logger::log("Database structure update successful.", Logger::TRACE); } + /** + * Returns the current build number of the instance + * + * @return int the build number + */ private static function getBuild() { $build = Config::get('system', 'build'); From 20498d1d40ada34aba549beb62567200495bc170 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sat, 9 Mar 2019 20:41:27 +0100 Subject: [PATCH 3/4] replaced duplicate code with method --- src/Core/Update.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Core/Update.php b/src/Core/Update.php index d65adfebea..025397fde0 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -24,19 +24,15 @@ class Update return; } - $build = Config::get('system', 'build'); - - if (empty($build)) { - Config::set('system', 'build', DB_UPDATE_VERSION - 1); - $build = DB_UPDATE_VERSION - 1; - } + $build = self::getBuild(); + $current = intval(DB_UPDATE_VERSION); // We don't support upgrading from very old versions anymore if ($build < NEW_UPDATE_ROUTINE_VERSION) { die('You try to update from a version prior to database version 1170. The direct upgrade path is not supported. Please update to version 3.5.4 before updating to this version.'); } - if ($build < DB_UPDATE_VERSION) { + if ($build < $current ) { if ($via_worker) { // Calling the database update directly via the worker enables us to perform database changes to the workerqueue table itself. // This is a fallback, since normally the database update will be performed by a worker job. From f7c7851c2d576b41cb3ab03a6b610ca4dc4eb87d Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sat, 9 Mar 2019 20:50:06 +0100 Subject: [PATCH 4/4] replaced duplicate code with method --- src/Core/Update.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Update.php b/src/Core/Update.php index 025397fde0..523753a1b1 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -79,7 +79,7 @@ class Update if (Lock::acquire('dbupdate', 0, Cache::INFINITE)) { // recheck again in case we accidentally spawned multiple updates - $stored = intval(self::getBuild()); + $stored = self::getBuild(); if ($stored >= $current) { Lock::release('dbupdate'); return '';