Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a setup and cli check for the recommended memory limit #10539

Merged
merged 7 commits into from
Aug 21, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion console.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,13 @@ function exceptionHandler($exception) {
echo "The process control (PCNTL) extensions are required in case you want to interrupt long running commands - see http://php.net/manual/en/book.pcntl.php" . PHP_EOL;
}

$application = new Application(\OC::$server->getConfig(), \OC::$server->getEventDispatcher(), \OC::$server->getRequest(), \OC::$server->getLogger());
$application = new Application(
\OC::$server->getConfig(),
\OC::$server->getEventDispatcher(),
\OC::$server->getRequest(),
\OC::$server->getLogger(),
\OC::$server->query(\OC\MemoryInfo::class)
);
$application->loadCommands(new ArgvInput(), new ConsoleOutput());
$application->run();
} catch (Exception $ex) {
Expand Down
9 changes: 9 additions & 0 deletions core/js/setupchecks.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,15 @@
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
});
}
if (!data.isMemoryLimitSufficient) {
messages.push({
msg: t(
'core',
'The PHP memory limit is below the recommended value of 512MB.'
),
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
})
}
} else {
messages.push({
msg: t('core', 'Error occurred while checking server setup'),
Expand Down
78 changes: 67 additions & 11 deletions core/js/tests/specs/setupchecksSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand Down Expand Up @@ -217,7 +218,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand Down Expand Up @@ -265,7 +267,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand Down Expand Up @@ -311,7 +314,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand Down Expand Up @@ -355,7 +359,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand Down Expand Up @@ -399,7 +404,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand Down Expand Up @@ -443,7 +449,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand All @@ -456,6 +463,51 @@ describe('OC.SetupChecks tests', function() {
});
});

it('should return a warning if the memory limit is below the recommended value', function(done) {
var async = OC.SetupChecks.checkSetup();

suite.server.requests[0].respond(
200,
{
'Content-Type': 'application/json',
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
hasWorkingFileLocking: true,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isUrandomAvailable: true,
serverHasInternetConnection: true,
isMemcacheConfigured: true,
forwardedForHeadersWorking: true,
reverseProxyDocs: 'https://docs.owncloud.org/foo/bar.html',
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isOpcacheProperlySetup: true,
hasOpcacheLoaded: true,
isSettimelimitAvailable: true,
hasFreeTypeSupport: true,
missingIndexes: [],
outdatedCaches: [],
cronErrors: [],
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: false
})
);

async.done(function( data, s, x ){
expect(data).toEqual([{
msg: 'The PHP memory limit is below the recommended value of 512MB.',
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
}]);
done();
});
});

it('should return an error if the response has no statuscode 200', function(done) {
var async = OC.SetupChecks.checkSetup();

Expand Down Expand Up @@ -508,7 +560,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand Down Expand Up @@ -553,7 +606,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand Down Expand Up @@ -598,7 +652,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand Down Expand Up @@ -643,7 +698,8 @@ describe('OC.SetupChecks tests', function() {
cronErrors: [],
cronInfo: {
diffInSeconds: 0
}
},
isMemoryLimitSufficient: true
})
);

Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,7 @@
'OC\\Memcache\\Memcached' => $baseDir . '/lib/private/Memcache/Memcached.php',
'OC\\Memcache\\NullCache' => $baseDir . '/lib/private/Memcache/NullCache.php',
'OC\\Memcache\\Redis' => $baseDir . '/lib/private/Memcache/Redis.php',
'OC\\MemoryInfo' => $baseDir . '/lib/private/MemoryInfo.php',
'OC\\Migration\\BackgroundRepair' => $baseDir . '/lib/private/Migration/BackgroundRepair.php',
'OC\\Migration\\ConsoleOutput' => $baseDir . '/lib/private/Migration/ConsoleOutput.php',
'OC\\Migration\\SimpleOutput' => $baseDir . '/lib/private/Migration/SimpleOutput.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\Memcache\\Memcached' => __DIR__ . '/../../..' . '/lib/private/Memcache/Memcached.php',
'OC\\Memcache\\NullCache' => __DIR__ . '/../../..' . '/lib/private/Memcache/NullCache.php',
'OC\\Memcache\\Redis' => __DIR__ . '/../../..' . '/lib/private/Memcache/Redis.php',
'OC\\MemoryInfo' => __DIR__ . '/../../..' . '/lib/private/MemoryInfo.php',
'OC\\Migration\\BackgroundRepair' => __DIR__ . '/../../..' . '/lib/private/Migration/BackgroundRepair.php',
'OC\\Migration\\ConsoleOutput' => __DIR__ . '/../../..' . '/lib/private/Migration/ConsoleOutput.php',
'OC\\Migration\\SimpleOutput' => __DIR__ . '/../../..' . '/lib/private/Migration/SimpleOutput.php',
Expand Down
30 changes: 29 additions & 1 deletion lib/private/Console/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/
namespace OC\Console;

use OC\MemoryInfo;
use OC\NeedsUpdateException;
use OC_App;
use OCP\AppFramework\QueryException;
Expand All @@ -52,20 +53,28 @@ class Application {
private $request;
/** @var ILogger */
private $logger;
/** @var MemoryInfo */
private $memoryInfo;

/**
* @param IConfig $config
* @param EventDispatcherInterface $dispatcher
* @param IRequest $request
* @param ILogger $logger
* @param MemoryInfo $memoryInfo
*/
public function __construct(IConfig $config, EventDispatcherInterface $dispatcher, IRequest $request, ILogger $logger) {
public function __construct(IConfig $config,
EventDispatcherInterface $dispatcher,
IRequest $request,
ILogger $logger,
MemoryInfo $memoryInfo) {
$defaults = \OC::$server->getThemingDefaults();
$this->config = $config;
$this->application = new SymfonyApplication($defaults->getName(), \OC_Util::getVersionString());
$this->dispatcher = $dispatcher;
$this->request = $request;
$this->logger = $logger;
$this->memoryInfo = $memoryInfo;
}

/**
Expand Down Expand Up @@ -97,6 +106,11 @@ public function loadCommands(
if ($input->getOption('no-warnings')) {
$output->setVerbosity(OutputInterface::VERBOSITY_QUIET);
}

if ($this->memoryInfo->isMemoryLimitSufficient() === false) {
$this->writeMemoryLimitInfo($output);
}

try {
require_once __DIR__ . '/../../../core/register_command.php';
if ($this->config->getSystemValue('installed', false)) {
Expand Down Expand Up @@ -173,6 +187,20 @@ private function writeMaintenanceModeInfo(
}
}

/**
* Write a memory info output if the limit is below the recommended value.
*
* @param ConsoleOutputInterface $output
* @return void
*/
private function writeMemoryLimitInfo(ConsoleOutputInterface $output) {
$errOutput = $output->getErrorOutput();
$errOutput->writeln(
'<comment>The current PHP memory limit ' .
'is below the recommended value of 512MB.</comment>'
);
}

/**
* Sets whether to automatically exit after a command execution or not.
*
Expand Down
60 changes: 60 additions & 0 deletions lib/private/MemoryInfo.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declare strict types, please, and also the license header as found on other PHP files

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. I added the license texts and PHP strict mode.


namespace OC;

/**
* Helper class that covers memory info.
*/
class MemoryInfo {

const RECOMMENDED_MEMORY_LIMIT = 512 * 1024 * 1024;

/**
* Tests if the memory limit is greater or equal the recommended value.
*
* @return bool
*/
public function isMemoryLimitSufficient(): bool {
$memoryLimit = $this->getMemoryLimit();
return $memoryLimit === -1 || $memoryLimit >= self::RECOMMENDED_MEMORY_LIMIT;
}

/**
* Returns the php memory limit.
*
* @return int The memory limit in bytes.
*/
public function getMemoryLimit(): int {
$iniValue = trim(ini_get('memory_limit'));
if ($iniValue === '-1') {
return -1;
} else if (is_numeric($iniValue) === true) {
return (int)$iniValue;
} else {
return $this->memoryLimitToBytes($iniValue);
}
}

/**
* Converts the ini memory limit to bytes.
*
* @param string $memoryLimit The "memory_limit" ini value
* @return int
*/
private function memoryLimitToBytes(string $memoryLimit): int {
$last = strtolower(substr($memoryLimit, -1));
$memoryLimit = (int)substr($memoryLimit, 0, -1);

// intended fall trough
switch($last) {
case 'g':
$memoryLimit *= 1024;
case 'm':
$memoryLimit *= 1024;
case 'k':
$memoryLimit *= 1024;
}

return $memoryLimit;
}
}
10 changes: 8 additions & 2 deletions settings/Controller/CheckSetupController.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
use OC\DB\MissingIndexInformation;
use OC\IntegrityCheck\Checker;
use OC\Lock\NoopLockingProvider;
use OC\MemoryInfo;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\DataDisplayResponse;
use OCP\AppFramework\Http\DataResponse;
Expand Down Expand Up @@ -81,6 +82,8 @@ class CheckSetupController extends Controller {
private $lockingProvider;
/** @var IDateTimeFormatter */
private $dateTimeFormatter;
/** @var MemoryInfo */
private $memoryInfo;

public function __construct($AppName,
IRequest $request,
Expand All @@ -94,7 +97,8 @@ public function __construct($AppName,
EventDispatcherInterface $dispatcher,
IDBConnection $db,
ILockingProvider $lockingProvider,
IDateTimeFormatter $dateTimeFormatter) {
IDateTimeFormatter $dateTimeFormatter,
MemoryInfo $memoryInfo) {
parent::__construct($AppName, $request);
$this->config = $config;
$this->clientService = $clientService;
Expand All @@ -107,6 +111,7 @@ public function __construct($AppName,
$this->db = $db;
$this->lockingProvider = $lockingProvider;
$this->dateTimeFormatter = $dateTimeFormatter;
$this->memoryInfo = $memoryInfo;
}

/**
Expand Down Expand Up @@ -565,7 +570,8 @@ public function check() {
'isSqliteUsed' => $this->isSqliteUsed(),
'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'),
'isPhpMailerUsed' => $this->isPhpMailerUsed(),
'mailSettingsDocumentation' => $this->urlGenerator->getAbsoluteURL('index.php/settings/admin')
'mailSettingsDocumentation' => $this->urlGenerator->getAbsoluteURL('index.php/settings/admin'),
'isMemoryLimitSufficient' => $this->memoryInfo->isMemoryLimitSufficient(),
]
);
}
Expand Down
Loading