Revert "Wait for cron to finish before running upgrade command"
This reverts commit 18e9631810
.
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
This commit is contained in:
parent
5bb7378bf3
commit
b292f919c6
|
@ -99,8 +99,7 @@ class Upgrade extends Command {
|
||||||
$this->config,
|
$this->config,
|
||||||
\OC::$server->getIntegrityCodeChecker(),
|
\OC::$server->getIntegrityCodeChecker(),
|
||||||
$this->logger,
|
$this->logger,
|
||||||
$this->installer,
|
$this->installer
|
||||||
\OC::$server->getJobList()
|
|
||||||
);
|
);
|
||||||
|
|
||||||
$dispatcher = \OC::$server->getEventDispatcher();
|
$dispatcher = \OC::$server->getEventDispatcher();
|
||||||
|
@ -192,9 +191,6 @@ class Upgrade extends Command {
|
||||||
$updater->listen('\OC\Updater', 'maintenanceActive', function () use($output) {
|
$updater->listen('\OC\Updater', 'maintenanceActive', function () use($output) {
|
||||||
$output->writeln('<info>Maintenance mode is kept active</info>');
|
$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',
|
$updater->listen('\OC\Updater', 'updateEnd',
|
||||||
function ($success) use($output, $self) {
|
function ($success) use($output, $self) {
|
||||||
if ($success) {
|
if ($success) {
|
||||||
|
|
|
@ -119,8 +119,7 @@ if (\OCP\Util::needUpgrade()) {
|
||||||
$config,
|
$config,
|
||||||
\OC::$server->getIntegrityCodeChecker(),
|
\OC::$server->getIntegrityCodeChecker(),
|
||||||
$logger,
|
$logger,
|
||||||
\OC::$server->query(\OC\Installer::class),
|
\OC::$server->query(\OC\Installer::class)
|
||||||
\OC::$server->getJobList()
|
|
||||||
);
|
);
|
||||||
$incompatibleApps = [];
|
$incompatibleApps = [];
|
||||||
|
|
||||||
|
@ -153,9 +152,6 @@ if (\OCP\Util::needUpgrade()) {
|
||||||
$updater->listen('\OC\Updater', 'maintenanceActive', function () use ($eventSource, $l) {
|
$updater->listen('\OC\Updater', 'maintenanceActive', function () use ($eventSource, $l) {
|
||||||
$eventSource->send('success', (string)$l->t('Maintenance mode is kept active'));
|
$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) {
|
$updater->listen('\OC\Updater', 'dbUpgradeBefore', function () use($eventSource, $l) {
|
||||||
$eventSource->send('success', (string)$l->t('Updating database schema'));
|
$eventSource->send('success', (string)$l->t('Updating database schema'));
|
||||||
});
|
});
|
||||||
|
|
|
@ -48,9 +48,6 @@ class JobList implements IJobList {
|
||||||
/**@var ITimeFactory */
|
/**@var ITimeFactory */
|
||||||
protected $timeFactory;
|
protected $timeFactory;
|
||||||
|
|
||||||
/** @var int - 12 hours * 3600 seconds*/
|
|
||||||
private $jobTimeOut = 43200;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param IDBConnection $connection
|
* @param IDBConnection $connection
|
||||||
* @param IConfig $config
|
* @param IConfig $config
|
||||||
|
@ -186,7 +183,7 @@ class JobList implements IJobList {
|
||||||
$query = $this->connection->getQueryBuilder();
|
$query = $this->connection->getQueryBuilder();
|
||||||
$query->select('*')
|
$query->select('*')
|
||||||
->from('jobs')
|
->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)))
|
->andWhere($query->expr()->lte('last_checked', $query->createNamedParameter($this->timeFactory->getTime(), IQueryBuilder::PARAM_INT)))
|
||||||
->orderBy('last_checked', 'ASC')
|
->orderBy('last_checked', 'ASC')
|
||||||
->setMaxResults(1);
|
->setMaxResults(1);
|
||||||
|
@ -346,39 +343,4 @@ class JobList implements IJobList {
|
||||||
->where($query->expr()->eq('id', $query->createNamedParameter($job->getId(), IQueryBuilder::PARAM_INT)));
|
->where($query->expr()->eq('id', $query->createNamedParameter($job->getId(), IQueryBuilder::PARAM_INT)));
|
||||||
$query->execute();
|
$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;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -38,7 +38,6 @@ use OC\DB\MigrationService;
|
||||||
use OC\Hooks\BasicEmitter;
|
use OC\Hooks\BasicEmitter;
|
||||||
use OC\IntegrityCheck\Checker;
|
use OC\IntegrityCheck\Checker;
|
||||||
use OC_App;
|
use OC_App;
|
||||||
use OCP\BackgroundJob\IJobList;
|
|
||||||
use OCP\IConfig;
|
use OCP\IConfig;
|
||||||
use OCP\ILogger;
|
use OCP\ILogger;
|
||||||
use OCP\Util;
|
use OCP\Util;
|
||||||
|
@ -67,9 +66,6 @@ class Updater extends BasicEmitter {
|
||||||
/** @var Installer */
|
/** @var Installer */
|
||||||
private $installer;
|
private $installer;
|
||||||
|
|
||||||
/** @var IJobList */
|
|
||||||
private $jobList;
|
|
||||||
|
|
||||||
private $logLevelNames = [
|
private $logLevelNames = [
|
||||||
0 => 'Debug',
|
0 => 'Debug',
|
||||||
1 => 'Info',
|
1 => 'Info',
|
||||||
|
@ -78,16 +74,20 @@ class Updater extends BasicEmitter {
|
||||||
4 => 'Fatal',
|
4 => 'Fatal',
|
||||||
];
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @param IConfig $config
|
||||||
|
* @param Checker $checker
|
||||||
|
* @param ILogger $log
|
||||||
|
* @param Installer $installer
|
||||||
|
*/
|
||||||
public function __construct(IConfig $config,
|
public function __construct(IConfig $config,
|
||||||
Checker $checker,
|
Checker $checker,
|
||||||
ILogger $log,
|
ILogger $log = null,
|
||||||
Installer $installer,
|
Installer $installer) {
|
||||||
IJobList $jobList) {
|
|
||||||
$this->log = $log;
|
$this->log = $log;
|
||||||
$this->config = $config;
|
$this->config = $config;
|
||||||
$this->checker = $checker;
|
$this->checker = $checker;
|
||||||
$this->installer = $installer;
|
$this->installer = $installer;
|
||||||
$this->jobList = $jobList;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -114,11 +114,6 @@ class Updater extends BasicEmitter {
|
||||||
$installedVersion = $this->config->getSystemValue('version', '0.0.0');
|
$installedVersion = $this->config->getSystemValue('version', '0.0.0');
|
||||||
$currentVersion = implode('.', \OCP\Util::getVersion());
|
$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'));
|
$this->log->debug('starting upgrade from ' . $installedVersion . ' to ' . $currentVersion, array('app' => 'core'));
|
||||||
|
|
||||||
$success = true;
|
$success = true;
|
||||||
|
@ -617,12 +612,6 @@ class Updater extends BasicEmitter {
|
||||||
});
|
});
|
||||||
|
|
||||||
}
|
}
|
||||||
private function waitForCronToFinish() {
|
|
||||||
while ($this->jobList->isAnyJobRunning()) {
|
|
||||||
$this->emit('\OC\Updater', 'waitForCronToFinish');
|
|
||||||
sleep(5);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -24,7 +24,6 @@ namespace Test;
|
||||||
|
|
||||||
use OC\Installer;
|
use OC\Installer;
|
||||||
use OC\Updater;
|
use OC\Updater;
|
||||||
use OCP\BackgroundJob\IJobList;
|
|
||||||
use OCP\IConfig;
|
use OCP\IConfig;
|
||||||
use OCP\ILogger;
|
use OCP\ILogger;
|
||||||
use OC\IntegrityCheck\Checker;
|
use OC\IntegrityCheck\Checker;
|
||||||
|
@ -40,8 +39,6 @@ class UpdaterTest extends TestCase {
|
||||||
private $checker;
|
private $checker;
|
||||||
/** @var Installer|\PHPUnit_Framework_MockObject_MockObject */
|
/** @var Installer|\PHPUnit_Framework_MockObject_MockObject */
|
||||||
private $installer;
|
private $installer;
|
||||||
/** @var IJobList|\PHPUnit_Framework_MockObject_MockObject */
|
|
||||||
private $jobList;
|
|
||||||
|
|
||||||
public function setUp() {
|
public function setUp() {
|
||||||
parent::setUp();
|
parent::setUp();
|
||||||
|
@ -57,16 +54,12 @@ class UpdaterTest extends TestCase {
|
||||||
$this->installer = $this->getMockBuilder(Installer::class)
|
$this->installer = $this->getMockBuilder(Installer::class)
|
||||||
->disableOriginalConstructor()
|
->disableOriginalConstructor()
|
||||||
->getMock();
|
->getMock();
|
||||||
$this->jobList = $this->getMockBuilder(IJobList::class)
|
|
||||||
->disableOriginalConstructor()
|
|
||||||
->getMock();
|
|
||||||
|
|
||||||
$this->updater = new Updater(
|
$this->updater = new Updater(
|
||||||
$this->config,
|
$this->config,
|
||||||
$this->checker,
|
$this->checker,
|
||||||
$this->logger,
|
$this->logger,
|
||||||
$this->installer,
|
$this->installer
|
||||||
$this->jobList
|
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue