From 2a1c82cf055ce3abce85eccbc44ea9883bff1b3b Mon Sep 17 00:00:00 2001 From: Michael Vogel Date: Sat, 21 Nov 2020 13:28:06 +0100 Subject: [PATCH 1/4] Avoid multiple database update mails, changed log level --- src/Core/Update.php | 47 ++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/src/Core/Update.php b/src/Core/Update.php index 0fca9744a9..4d65f549a0 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -111,12 +111,12 @@ class Update if ($stored < $current || $force) { DI::config()->load('database'); - Logger::notice('Update starting.', ['from' => $stored, 'to' => $current]); - // Compare the current structure with the defined structure // If the Lock is acquired, never release it automatically to avoid double updates if (DI::lock()->acquire('dbupdate', 120, Cache\Duration::INFINITE)) { + Logger::notice('Update starting.', ['from' => $stored, 'to' => $current]); + // Checks if the build changed during Lock acquiring (so no double update occurs) $retryBuild = DI::config()->get('system', 'build', null, true); if ($retryBuild !== $build) { @@ -127,16 +127,20 @@ class Update // run the pre_update_nnnn functions in update.php for ($x = $stored + 1; $x <= $current; $x++) { + Logger::notice('Execute pre update.', ['version' => $x]); $r = self::runUpdateFunction($x, 'pre_update', $sendMail); if (!$r) { Logger::warning('Pre update failed', ['version' => $x]); DI::config()->set('system', 'update', Update::FAILED); DI::lock()->release('dbupdate'); return $r; + } else { + Logger::notice('Pre update executed.', ['version' => $x]); } } // update the structure in one call + Logger::notice('Execute structure update'); $retval = DBStructure::update($basePath, $verbose, true); if (!empty($retval)) { if ($sendMail) { @@ -152,27 +156,32 @@ class Update } else { DI::config()->set('database', 'last_successful_update', $current); DI::config()->set('database', 'last_successful_update_time', time()); - Logger::info('Update finished.', ['from' => $stored, 'to' => $current]); + Logger::notice('Database structure update finished.', ['from' => $stored, 'to' => $current]); } // run the update_nnnn functions in update.php for ($x = $stored + 1; $x <= $current; $x++) { + Logger::notice('Execute post update.', ['version' => $x]); $r = self::runUpdateFunction($x, 'update', $sendMail); if (!$r) { Logger::warning('Post update failed', ['version' => $x]); DI::config()->set('system', 'update', Update::FAILED); DI::lock()->release('dbupdate'); return $r; + } else { + DI::config()->set('system', 'build', $x); + Logger::notice('Post update executed.', ['version' => $x]); } } + DI::config()->set('system', 'build', $current); + DI::config()->set('system', 'update', Update::SUCCESS); + DI::lock()->release('dbupdate'); + Logger::notice('Update success.', ['from' => $stored, 'to' => $current]); if ($sendMail) { self::updateSuccessfull($stored, $current); } - - DI::config()->set('system', 'update', Update::SUCCESS); - DI::lock()->release('dbupdate'); } } } @@ -194,7 +203,7 @@ class Update { $funcname = $prefix . '_' . $x; - Logger::info('Update function start.', ['function' => $funcname]); + Logger::notice('Update function start.', ['function' => $funcname]); if (function_exists($funcname)) { // There could be a lot of processes running or about to run. @@ -207,9 +216,9 @@ class Update if (DI::lock()->acquire('dbupdate_function', 120, Cache\Duration::INFINITE)) { // call the specific update - Logger::info('Pre update function start.', ['function' => $funcname]); + Logger::notice('Pre update function start.', ['function' => $funcname]); $retval = $funcname(); - Logger::info('Update function done.', ['function' => $funcname]); + Logger::notice('Update function done.', ['function' => $funcname]); if ($retval) { if ($sendMail) { @@ -223,30 +232,16 @@ class Update DI::lock()->release('dbupdate_function'); return false; } else { - DI::config()->set('database', 'last_successful_update_function', $funcname); - DI::config()->set('database', 'last_successful_update_function_time', time()); - - if ($prefix == 'update') { - DI::config()->set('system', 'build', $x); - } - DI::lock()->release('dbupdate_function'); - Logger::info('Update function finished.', ['function' => $funcname]); + Logger::notice('Update function finished.', ['function' => $funcname]); return true; } } else { Logger::error('Locking failed.', ['function' => $funcname]); + return false; } } else { - Logger::info('Update function skipped.', ['function' => $funcname]); - - DI::config()->set('database', 'last_successful_update_function', $funcname); - DI::config()->set('database', 'last_successful_update_function_time', time()); - - if ($prefix == 'update') { - DI::config()->set('system', 'build', $x); - } - + Logger::notice('Update function skipped.', ['function' => $funcname]); return true; } } From b40218eb0baab6526449a76b1c3fcafa68aea3d0 Mon Sep 17 00:00:00 2001 From: Michael Vogel Date: Sat, 21 Nov 2020 14:17:14 +0100 Subject: [PATCH 2/4] Immediately fail when lock hadn't been acquired to prevent stocked updates --- 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 4d65f549a0..a640d5ec2f 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -113,7 +113,7 @@ class Update // Compare the current structure with the defined structure // If the Lock is acquired, never release it automatically to avoid double updates - if (DI::lock()->acquire('dbupdate', 120, Cache\Duration::INFINITE)) { + if (DI::lock()->acquire('dbupdate', 0, Cache\Duration::INFINITE)) { Logger::notice('Update starting.', ['from' => $stored, 'to' => $current]); From 1de6251627674ce1f508662952bc9deb512454e1 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 21 Nov 2020 14:58:48 +0000 Subject: [PATCH 3/4] Code cleaning --- src/Core/Update.php | 71 ++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/src/Core/Update.php b/src/Core/Update.php index a640d5ec2f..b2dc1a383a 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -41,7 +41,7 @@ class Update * * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function check($basePath, $via_worker, App\Mode $mode) + public static function check(string $basePath, bool $via_worker, App\Mode $mode) { if (!DBA::connected()) { return; @@ -80,15 +80,15 @@ class Update * Automatic database updates * * @param string $basePath The base path of this application - * @param bool $force Force the Update-Check even if the database version doesn't match - * @param bool $override Overrides any running/stuck updates - * @param bool $verbose Run the Update-Check verbose - * @param bool $sendMail Sends a Mail to the administrator in case of success/failure + * @param bool $force Force the Update-Check even if the database version doesn't match + * @param bool $override Overrides any running/stuck updates + * @param bool $verbose Run the Update-Check verbose + * @param bool $sendMail Sends a Mail to the administrator in case of success/failure * * @return string Empty string if the update is successful, error messages otherwise * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function run($basePath, $force = false, $override = false, $verbose = false, $sendMail = true) + public static function run(string $basePath, bool $force = false, bool $override = false, bool $verbose = false, bool $sendMail = true) { // In force mode, we release the dbupdate lock first // Necessary in case of an stuck update @@ -126,16 +126,16 @@ class Update } // run the pre_update_nnnn functions in update.php - for ($x = $stored + 1; $x <= $current; $x++) { - Logger::notice('Execute pre update.', ['version' => $x]); - $r = self::runUpdateFunction($x, 'pre_update', $sendMail); + for ($version = $stored + 1; $version <= $current; $version++) { + Logger::notice('Execute pre update.', ['version' => $version]); + $r = self::runUpdateFunction($version, 'pre_update', $sendMail); if (!$r) { - Logger::warning('Pre update failed', ['version' => $x]); + Logger::warning('Pre update failed', ['version' => $version]); DI::config()->set('system', 'update', Update::FAILED); DI::lock()->release('dbupdate'); return $r; } else { - Logger::notice('Pre update executed.', ['version' => $x]); + Logger::notice('Pre update executed.', ['version' => $version]); } } @@ -160,17 +160,17 @@ class Update } // run the update_nnnn functions in update.php - for ($x = $stored + 1; $x <= $current; $x++) { - Logger::notice('Execute post update.', ['version' => $x]); - $r = self::runUpdateFunction($x, 'update', $sendMail); + for ($version = $stored + 1; $version <= $current; $version++) { + Logger::notice('Execute post update.', ['version' => $version]); + $r = self::runUpdateFunction($version, 'update', $sendMail); if (!$r) { - Logger::warning('Post update failed', ['version' => $x]); + Logger::warning('Post update failed', ['version' => $version]); DI::config()->set('system', 'update', Update::FAILED); DI::lock()->release('dbupdate'); return $r; } else { - DI::config()->set('system', 'build', $x); - Logger::notice('Post update executed.', ['version' => $x]); + DI::config()->set('system', 'build', $version); + Logger::notice('Post update executed.', ['version' => $version]); } } @@ -180,7 +180,7 @@ class Update Logger::notice('Update success.', ['from' => $stored, 'to' => $current]); if ($sendMail) { - self::updateSuccessfull($stored, $current); + self::updateSuccessful($stored, $current); } } } @@ -192,16 +192,16 @@ class Update /** * Executes a specific update function * - * @param int $x the DB version number of the function + * @param int $version the DB version number of the function * @param string $prefix the prefix of the function (update, pre_update) * @param bool $sendMail whether to send emails on success/failure * @return bool true, if the update function worked * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function runUpdateFunction($x, $prefix, bool $sendMail = true) + public static function runUpdateFunction(int $version, string $prefix, bool $sendMail = true) { - $funcname = $prefix . '_' . $x; + $funcname = $prefix . '_' . $version; Logger::notice('Update function start.', ['function' => $funcname]); @@ -224,8 +224,8 @@ class Update if ($sendMail) { //send the administrator an e-mail self::updateFailed( - $x, - DI::l10n()->t('Update %s failed. See error logs.', $x) + $version, + DI::l10n()->t('Update %s failed. See error logs.', $version) ); } Logger::error('Update function ERROR.', ['function' => $funcname, 'retval' => $retval]); @@ -253,9 +253,9 @@ class Update * @param string $error_message error message * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - private static function updateFailed($update_id, $error_message) { + private static function updateFailed(int $update_id, string $error_message) { //send the administrators an e-mail - $condition = ['email' => explode(",", str_replace(" ", "", DI::config()->get('config', 'admin_email'))), 'parent-uid' => 0]; + $condition = ['email' => explode(',', str_replace(' ', '', DI::config()->get('config', 'admin_email'))), 'parent-uid' => 0]; $adminlist = DBA::select('user', ['uid', 'language', 'email'], $condition, ['order' => ['uid']]); // No valid result? @@ -284,7 +284,7 @@ class Update This needs to be fixed soon and I can't do it alone. Please contact a friendica developer if you can not help me on your own. My database might be invalid.", $update_id)); - $body = $l10n->t("The error message is\n[pre]%s[/pre]", $error_message); + $body = $l10n->t('The error message is\n[pre]%s[/pre]', $error_message); $email = DI::emailer() ->newSystemMail() @@ -295,14 +295,20 @@ class Update DI::emailer()->send($email); } - //try the logger - Logger::alert('Database structure update FAILED.', ['error' => $error_message]); + Logger::alert('Database structure update failed.', ['error' => $error_message]); } - private static function updateSuccessfull($from_build, $to_build) + /** + * Send a mail to the administrator about the successful update + * + * @param integer $from_build + * @param integer $to_build + * @return void + */ + private static function updateSuccessful(int $from_build, int $to_build) { //send the administrators an e-mail - $condition = ['email' => explode(",", str_replace(" ", "", DI::config()->get('config', 'admin_email'))), 'parent-uid' => 0]; + $condition = ['email' => explode(',', str_replace(' ', '', DI::config()->get('config', 'admin_email'))), 'parent-uid' => 0]; $adminlist = DBA::select('user', ['uid', 'language', 'email'], $condition, ['order' => ['uid']]); if (DBA::isResult($adminlist)) { @@ -318,8 +324,8 @@ class Update $lang = (($admin['language']) ? $admin['language'] : 'en'); $l10n = DI::l10n()->withLang($lang); - $preamble = Strings::deindent($l10n->t(" - The friendica database was successfully updated from %s to %s.", + $preamble = Strings::deindent($l10n->t(' + The friendica database was successfully updated from %s to %s.', $from_build, $to_build)); $email = DI::emailer() @@ -332,7 +338,6 @@ class Update } } - //try the logger Logger::debug('Database structure update successful.'); } } From aec9f1ebf043baa2c23cf2cbcba94220a5e2d031 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 21 Nov 2020 15:15:58 +0000 Subject: [PATCH 4/4] Remove unused config variable --- src/Core/Update.php | 2 -- src/Module/Admin/DBSync.php | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/Core/Update.php b/src/Core/Update.php index b2dc1a383a..1d8f88d012 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -154,8 +154,6 @@ class Update DI::lock()->release('dbupdate'); return $retval; } else { - DI::config()->set('database', 'last_successful_update', $current); - DI::config()->set('database', 'last_successful_update_time', time()); Logger::notice('Database structure update finished.', ['from' => $stored, 'to' => $current]); } diff --git a/src/Module/Admin/DBSync.php b/src/Module/Admin/DBSync.php index 36f683e087..662fe08e27 100644 --- a/src/Module/Admin/DBSync.php +++ b/src/Module/Admin/DBSync.php @@ -57,8 +57,6 @@ class DBSync extends BaseAdmin $retval = DBStructure::update($a->getBasePath(), false, true); if ($retval === '') { $o = DI::l10n()->t("Database structure update %s was successfully applied.", DB_UPDATE_VERSION) . "
"; - DI::config()->set('database', 'last_successful_update', DB_UPDATE_VERSION); - DI::config()->set('database', 'last_successful_update_time', time()); } else { $o = DI::l10n()->t("Executing of database structure update %s failed with error: %s", DB_UPDATE_VERSION, $retval) . "
"; }