diff options
author | Roeland Jago Douma <rullzer@users.noreply.github.com> | 2020-10-04 11:07:49 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-10-04 11:07:49 +0300 |
commit | f4707d178e4d0a85ca63b2e13bf8f897de9a941e (patch) | |
tree | d9e4756b16d5fef6460aab4815dc028fc6479c87 /lib | |
parent | eba83d22bbcf45caf400704b8794acce180c5ba9 (diff) | |
parent | 95a301ea570bfc724877530975297aad1e7536c8 (diff) |
Merge pull request #23047 from nextcloud/techdebt/noid/warn-on-database-abuse
Log the number of queries built and executed
Diffstat (limited to 'lib')
-rw-r--r-- | lib/private/AppFramework/DependencyInjection/DIContainer.php | 6 | ||||
-rw-r--r-- | lib/private/AppFramework/Http/Dispatcher.php | 53 | ||||
-rw-r--r-- | lib/private/DB/Connection.php | 18 |
3 files changed, 74 insertions, 3 deletions
diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 875597a1fba..3ef816f503e 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -61,6 +61,7 @@ use OCP\Files\Folder; use OCP\Files\IAppData; use OCP\Group\ISubAdmin; use OCP\IConfig; +use OCP\IDBConnection; use OCP\IInitialStateService; use OCP\IL10N; use OCP\ILogger; @@ -180,7 +181,10 @@ class DIContainer extends SimpleContainer implements IAppContainer { $c->get('Protocol'), $c->get(MiddlewareDispatcher::class), $c->get(IControllerMethodReflector::class), - $c->get(IRequest::class) + $c->get(IRequest::class), + $c->get(IConfig::class), + $c->get(IDBConnection::class), + $c->get(LoggerInterface::class) ); }); diff --git a/lib/private/AppFramework/Http/Dispatcher.php b/lib/private/AppFramework/Http/Dispatcher.php index 3892bb667f0..4a05ecaead0 100644 --- a/lib/private/AppFramework/Http/Dispatcher.php +++ b/lib/private/AppFramework/Http/Dispatcher.php @@ -35,11 +35,14 @@ namespace OC\AppFramework\Http; use OC\AppFramework\Http; use OC\AppFramework\Middleware\MiddlewareDispatcher; use OC\AppFramework\Utility\ControllerMethodReflector; - +use OC\DB\Connection; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\Response; +use OCP\IConfig; +use OCP\IDBConnection; use OCP\IRequest; +use Psr\Log\LoggerInterface; /** * Class to dispatch the request to the middleware dispatcher @@ -58,6 +61,15 @@ class Dispatcher { /** @var IRequest */ private $request; + /** @var IConfig */ + private $config; + + /** @var IDBConnection|Connection */ + private $connection; + + /** @var LoggerInterface */ + private $logger; + /** * @param Http $protocol the http protocol with contains all status headers * @param MiddlewareDispatcher $middlewareDispatcher the dispatcher which @@ -65,15 +77,24 @@ class Dispatcher { * @param ControllerMethodReflector $reflector the reflector that is used to inject * the arguments for the controller * @param IRequest $request the incoming request + * @param IConfig $config + * @param IDBConnection $connection + * @param LoggerInterface $logger */ public function __construct(Http $protocol, MiddlewareDispatcher $middlewareDispatcher, ControllerMethodReflector $reflector, - IRequest $request) { + IRequest $request, + IConfig $config, + IDBConnection $connection, + LoggerInterface $logger) { $this->protocol = $protocol; $this->middlewareDispatcher = $middlewareDispatcher; $this->reflector = $reflector; $this->request = $request; + $this->config = $config; + $this->connection = $connection; + $this->logger = $logger; } @@ -97,8 +118,36 @@ class Dispatcher { $this->middlewareDispatcher->beforeController($controller, $methodName); + + $databaseStatsBefore = []; + if ($this->config->getSystemValueBool('debug', false)) { + $databaseStatsBefore = $this->connection->getStats(); + } + $response = $this->executeController($controller, $methodName); + if (!empty($databaseStatsBefore)) { + $databaseStatsAfter = $this->connection->getStats(); + $numBuilt = $databaseStatsAfter['built'] - $databaseStatsBefore['built']; + $numExecuted = $databaseStatsAfter['executed'] - $databaseStatsBefore['executed']; + + if ($numBuilt > 50) { + $this->logger->debug('Controller {class}::{method} created {count} QueryBuilder objects, please check if they are created inside a loop by accident.' , [ + 'class' => (string) get_class($controller), + 'method' => $methodName, + 'count' => $numBuilt, + ]); + } + + if ($numExecuted > 100) { + $this->logger->warning('Controller {class}::{method} executed {count} queries.' , [ + 'class' => (string) get_class($controller), + 'method' => $methodName, + 'count' => $numExecuted, + ]); + } + } + // if an exception appears, the middleware checks if it can handle the // exception and creates a response. If no response is created, it is // assumed that theres no middleware who can handle it and the error is diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index c5766dcd7e0..ba3e6aae5bc 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -59,6 +59,12 @@ class Connection extends ReconnectWrapper implements IDBConnection { protected $lockedTable = null; + /** @var int */ + protected $queriesBuilt = 0; + + /** @var int */ + protected $queriesExecuted = 0; + public function connect() { try { return parent::connect(); @@ -68,12 +74,20 @@ class Connection extends ReconnectWrapper implements IDBConnection { } } + public function getStats(): array { + return [ + 'built' => $this->queriesBuilt, + 'executed' => $this->queriesExecuted, + ]; + } + /** * Returns a QueryBuilder for the connection. * * @return \OCP\DB\QueryBuilder\IQueryBuilder */ public function getQueryBuilder() { + $this->queriesBuilt++; return new QueryBuilder( $this, \OC::$server->getSystemConfig(), @@ -90,6 +104,7 @@ class Connection extends ReconnectWrapper implements IDBConnection { public function createQueryBuilder() { $backtrace = $this->getCallerBacktrace(); \OC::$server->getLogger()->debug('Doctrine QueryBuilder retrieved in {backtrace}', ['app' => 'core', 'backtrace' => $backtrace]); + $this->queriesBuilt++; return parent::createQueryBuilder(); } @@ -102,6 +117,7 @@ class Connection extends ReconnectWrapper implements IDBConnection { public function getExpressionBuilder() { $backtrace = $this->getCallerBacktrace(); \OC::$server->getLogger()->debug('Doctrine ExpressionBuilder retrieved in {backtrace}', ['app' => 'core', 'backtrace' => $backtrace]); + $this->queriesBuilt++; return parent::getExpressionBuilder(); } @@ -191,6 +207,7 @@ class Connection extends ReconnectWrapper implements IDBConnection { public function executeQuery($query, array $params = [], $types = [], QueryCacheProfile $qcp = null) { $query = $this->replaceTablePrefix($query); $query = $this->adapter->fixupStatement($query); + $this->queriesExecuted++; return parent::executeQuery($query, $params, $types, $qcp); } @@ -211,6 +228,7 @@ class Connection extends ReconnectWrapper implements IDBConnection { public function executeUpdate($query, array $params = [], array $types = []) { $query = $this->replaceTablePrefix($query); $query = $this->adapter->fixupStatement($query); + $this->queriesExecuted++; return parent::executeUpdate($query, $params, $types); } |