]> source.dussan.org Git - nextcloud-server.git/commitdiff
Revert "Wait for cron to finish before running upgrade command" 12188/head
authorMorris Jobke <hey@morrisjobke.de>
Thu, 1 Nov 2018 11:52:09 +0000 (12:52 +0100)
committerMorris Jobke <hey@morrisjobke.de>
Thu, 1 Nov 2018 14:23:40 +0000 (15:23 +0100)
This reverts commit 18e9631810ad1d3d72c2b4bbee330169808108ad.

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
core/Command/Upgrade.php
core/ajax/update.php
lib/private/BackgroundJob/JobList.php
lib/private/Updater.php
tests/lib/UpdaterTest.php

index 86f049d9f2aae4bae6d5daa9a9a6ec750ec21bab..5a2deea0b6cbb0fa13047218e59e4cf6589c22f7 100644 (file)
@@ -99,8 +99,7 @@ class Upgrade extends Command {
                                        $this->config,
                                        \OC::$server->getIntegrityCodeChecker(),
                                        $this->logger,
-                                       $this->installer,
-                                       \OC::$server->getJobList()
+                                       $this->installer
                        );
 
                        $dispatcher = \OC::$server->getEventDispatcher();
@@ -192,9 +191,6 @@ class Upgrade extends Command {
                        $updater->listen('\OC\Updater', 'maintenanceActive', function () use($output) {
                                $output->writeln('<info>Maintenance mode is kept active</info>');
                        });
-                       $updater->listen('\OC\Updater', 'waitForCronToFinish', function () use($output) {
-                               $output->writeln('<info>Waiting for cron to finish (checks again in 5 seconds) …</info>');
-                       });
                        $updater->listen('\OC\Updater', 'updateEnd',
                                function ($success) use($output, $self) {
                                        if ($success) {
index 6def2c6797b1fe5624d63ac0a2d5682edebe442c..7dead22b9ddefc96067a9680a5a897e4aaffde02 100644 (file)
@@ -119,8 +119,7 @@ if (\OCP\Util::needUpgrade()) {
                        $config,
                        \OC::$server->getIntegrityCodeChecker(),
                        $logger,
-                       \OC::$server->query(\OC\Installer::class),
-                       \OC::$server->getJobList()
+                       \OC::$server->query(\OC\Installer::class)
        );
        $incompatibleApps = [];
 
@@ -153,9 +152,6 @@ if (\OCP\Util::needUpgrade()) {
        $updater->listen('\OC\Updater', 'maintenanceActive', function () use ($eventSource, $l) {
                $eventSource->send('success', (string)$l->t('Maintenance mode is kept active'));
        });
-       $updater->listen('\OC\Updater', 'waitForCronToFinish', function () use ($eventSource, $l) {
-               $eventSource->send('success', (string)$l->t('Waiting for cron to finish (checks again in 5 seconds) …'));
-       });
        $updater->listen('\OC\Updater', 'dbUpgradeBefore', function () use($eventSource, $l) {
                $eventSource->send('success', (string)$l->t('Updating database schema'));
        });
index fab608cf16496363a6ae7a009a17a648d1f39527..e890c35868b81223576b30856cbdaeaa9a83fcc2 100644 (file)
@@ -48,9 +48,6 @@ class JobList implements IJobList {
        /**@var ITimeFactory */
        protected $timeFactory;
 
-       /** @var int - 12 hours * 3600 seconds*/
-       private $jobTimeOut = 43200;
-
        /**
         * @param IDBConnection $connection
         * @param IConfig $config
@@ -186,7 +183,7 @@ class JobList implements IJobList {
                $query = $this->connection->getQueryBuilder();
                $query->select('*')
                        ->from('jobs')
-                       ->where($query->expr()->lte('reserved_at', $query->createNamedParameter($this->timeFactory->getTime() - $this->jobTimeOut, IQueryBuilder::PARAM_INT)))
+                       ->where($query->expr()->lte('reserved_at', $query->createNamedParameter($this->timeFactory->getTime() - 12 * 3600, IQueryBuilder::PARAM_INT)))
                        ->andWhere($query->expr()->lte('last_checked', $query->createNamedParameter($this->timeFactory->getTime(), IQueryBuilder::PARAM_INT)))
                        ->orderBy('last_checked', 'ASC')
                        ->setMaxResults(1);
@@ -346,39 +343,4 @@ class JobList implements IJobList {
                        ->where($query->expr()->eq('id', $query->createNamedParameter($job->getId(), IQueryBuilder::PARAM_INT)));
                $query->execute();
        }
-
-       /**
-        * checks if a job is still running (reserved_at time is smaller than 12 hours ago)
-        *
-        * Background information:
-        *
-        * The 12 hours is the same timeout that is also used to re-schedule an non-terminated
-        * job (see getNext()). The idea here is to give a job enough time to run very
-        * long but still be able to recognize that it maybe crashed and re-schedule it
-        * after the timeout. It's more likely to be crashed at that time than it ran
-        * that long.
-        *
-        * In theory it could lead to an nearly endless loop (as in - at most 12 hours).
-        * The cron command will not start new jobs when maintenance mode is active and
-        * this method is only executed in maintenance mode (see where it is called in
-        * the upgrader class. So this means in the worst case we wait 12 hours when a
-        * job has crashed. On the other hand: then the instance should be fixed anyways.
-        *
-        * @return bool
-        */
-       public function isAnyJobRunning(): bool {
-               $query = $this->connection->getQueryBuilder();
-               $query->select('*')
-                       ->from('jobs')
-                       ->where($query->expr()->gt('reserved_at', $query->createNamedParameter($this->timeFactory->getTime() - $this->jobTimeOut, IQueryBuilder::PARAM_INT)))
-                       ->setMaxResults(1);
-               $result = $query->execute();
-               $row = $result->fetch();
-               $result->closeCursor();
-
-               if ($row) {
-                       return true;
-               }
-               return false;
-       }
 }
index 8a2b1cd188c412f514c4ce51c7eee28a6db25578..4b4723be94fd8c25d36a966c1f515a6495ac0caf 100644 (file)
@@ -38,7 +38,6 @@ use OC\DB\MigrationService;
 use OC\Hooks\BasicEmitter;
 use OC\IntegrityCheck\Checker;
 use OC_App;
-use OCP\BackgroundJob\IJobList;
 use OCP\IConfig;
 use OCP\ILogger;
 use OCP\Util;
@@ -67,9 +66,6 @@ class Updater extends BasicEmitter {
        /** @var Installer */
        private $installer;
 
-       /** @var IJobList */
-       private $jobList;
-
        private $logLevelNames = [
                0 => 'Debug',
                1 => 'Info',
@@ -78,16 +74,20 @@ class Updater extends BasicEmitter {
                4 => 'Fatal',
        ];
 
+       /**
+        * @param IConfig $config
+        * @param Checker $checker
+        * @param ILogger $log
+        * @param Installer $installer
+        */
        public function __construct(IConfig $config,
                                                                Checker $checker,
-                                                               ILogger $log,
-                                                               Installer $installer,
-                                                               IJobList $jobList) {
+                                                               ILogger $log = null,
+                                                               Installer $installer) {
                $this->log = $log;
                $this->config = $config;
                $this->checker = $checker;
                $this->installer = $installer;
-               $this->jobList = $jobList;
        }
 
        /**
@@ -114,11 +114,6 @@ class Updater extends BasicEmitter {
                $installedVersion = $this->config->getSystemValue('version', '0.0.0');
                $currentVersion = implode('.', \OCP\Util::getVersion());
 
-               // see https://github.com/nextcloud/server/issues/9992 for potential problem
-               if (version_compare($installedVersion, '14.0.0.9', '>=')) {
-                       $this->waitForCronToFinish();
-               }
-
                $this->log->debug('starting upgrade from ' . $installedVersion . ' to ' . $currentVersion, array('app' => 'core'));
 
                $success = true;
@@ -617,12 +612,6 @@ class Updater extends BasicEmitter {
                });
 
        }
-       private function waitForCronToFinish() {
-               while ($this->jobList->isAnyJobRunning()) {
-                       $this->emit('\OC\Updater', 'waitForCronToFinish');
-                       sleep(5);
-               }
-       }
 
 }
 
index 02fea9c3c0ad9eec6278878706d3eb827b6f22c8..afc9f9b1f862ee1734733a5edb1e80d73d0b8e95 100644 (file)
@@ -24,7 +24,6 @@ namespace Test;
 
 use OC\Installer;
 use OC\Updater;
-use OCP\BackgroundJob\IJobList;
 use OCP\IConfig;
 use OCP\ILogger;
 use OC\IntegrityCheck\Checker;
@@ -40,8 +39,6 @@ class UpdaterTest extends TestCase {
        private $checker;
        /** @var Installer|\PHPUnit_Framework_MockObject_MockObject */
        private $installer;
-       /** @var IJobList|\PHPUnit_Framework_MockObject_MockObject */
-       private $jobList;
 
        public function setUp() {
                parent::setUp();
@@ -57,16 +54,12 @@ class UpdaterTest extends TestCase {
                $this->installer = $this->getMockBuilder(Installer::class)
                        ->disableOriginalConstructor()
                        ->getMock();
-               $this->jobList = $this->getMockBuilder(IJobList::class)
-                       ->disableOriginalConstructor()
-                       ->getMock();
 
                $this->updater = new Updater(
                        $this->config,
                        $this->checker,
                        $this->logger,
-                       $this->installer,
-                       $this->jobList
+                       $this->installer
                );
        }