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

fix(db): pre- SELECT ids with a LIMIT to control the amount of rows deleted for MySQL #1309

Merged
merged 1 commit into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 3 additions & 1 deletion lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
use OCP\User\Events\UserDeletedEvent;
use OCP\Util;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;

class Application extends App implements IBootstrap {
public const APP_ID = 'activity';
Expand Down Expand Up @@ -110,7 +111,8 @@ public function register(IRegistrationContext $context): void {
$context->registerService(Data::class, function (ContainerInterface $c) {
return new Data(
$c->get(IManager::class),
$c->get('ActivityConnectionAdapter')
$c->get('ActivityConnectionAdapter'),
$c->get(LoggerInterface::class),
);
});

Expand Down
84 changes: 66 additions & 18 deletions lib/Data.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<?php

declare(strict_types=1);
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
Expand Down Expand Up @@ -32,6 +34,7 @@
use OCP\Activity\IManager;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
use Psr\Log\LoggerInterface;

/**
* @brief Class for managing the data in the activities
Expand All @@ -48,14 +51,16 @@ class Data {

/** @var ?IQueryBuilder */
protected $insertMail;
private LoggerInterface $logger;

/**
* @param IManager $activityManager
* @param IDBConnection $connection
*/
public function __construct(IManager $activityManager, IDBConnection $connection) {
public function __construct(IManager $activityManager, IDBConnection $connection, LoggerInterface $logger) {
$this->activityManager = $activityManager;
$this->connection = $connection;
$this->logger = $logger;
}

/**
Expand Down Expand Up @@ -368,9 +373,16 @@ public function expire($expireDays = 365) {
* 'field' => 'value' => `field` = 'value'
* 'field' => array('value', 'operator') => `field` operator 'value'
*/
public function deleteActivities($conditions) {
$delete = $this->connection->getQueryBuilder();
$delete->delete('activity');
public function deleteActivities($conditions): void {
$platform = $this->connection->getDatabasePlatform();
if($platform instanceof MySQLPlatform) {
$this->logger->debug('Choosing chunked activity delete for MySQL/MariaDB', ['app' => 'activity']);
$this->deleteActivitiesForMySQL($conditions);
return;
}
$this->logger->debug('Choosing regular activity delete', ['app' => 'activity']);
$deleteQuery = $this->connection->getQueryBuilder();
$deleteQuery->delete('activity');

foreach ($conditions as $column => $comparison) {
if (is_array($comparison)) {
Expand All @@ -381,22 +393,14 @@ public function deleteActivities($conditions) {
$value = $comparison;
}

$delete->andWhere($delete->expr()->comparison($column, $operation, $delete->createNamedParameter($value)));
$deleteQuery->andWhere($deleteQuery->expr()->comparison($column, $operation, $deleteQuery->createNamedParameter($value)));
}

// Add galera safe delete chunking if using mysql
// Stops us hitting wsrep_max_ws_rows when large row counts are deleted
if ($this->connection->getDatabasePlatform() instanceof MySQLPlatform) {
// Then use chunked delete
$max = 100000;
$delete->setMaxResults($max);
do {
$deleted = $delete->executeStatement();
} while ($deleted === $max);
} else {
// Dont use chunked delete - let the DB handle the large row count natively
$delete->executeStatement();
}



// Dont use chunked delete - let the DB handle the large row count natively
$deleteQuery->executeStatement();
}

public function getById(int $activityId): ?IEvent {
Expand Down Expand Up @@ -467,4 +471,48 @@ public function getActivitySince(string $user, int $since, bool $byOthers) {

return $query->execute()->fetch();
}

/**
* Add galera safe delete chunking if using mysql
* Stops us hitting wsrep_max_ws_rows when large row counts are deleted
*
* @param array $conditions
* @return void
*/
private function deleteActivitiesForMySQL(array $conditions): void {
$query = $this->connection->getQueryBuilder();
$query->select('activity_id')
->from('activity');

foreach ($conditions as $column => $comparison) {
if (is_array($comparison)) {
$operation = $comparison[1] ?? '=';
$value = $comparison[0];
} else {
$operation = '=';
$value = $comparison;
}
$query->where($query->expr()->comparison($column, $operation, $query->createNamedParameter($value)));
}

$query->setMaxResults(10000);
$result = $query->executeQuery();
$count = $result->rowCount();
if($count === 0) {
return;
}
$ids = array_map(static function (array $id) {
return (int)$id[0];
}, $result->fetchAll(\PDO::FETCH_NUM));
$result->closeCursor();

$deleteQuery = $this->connection->getQueryBuilder();
$deleteQuery->delete('activity');
$deleteQuery->where($deleteQuery->expr()->in('activity_id', $deleteQuery->createParameter('ids'), IQueryBuilder::PARAM_INT_ARRAY));
$deleteQuery->setParameter('ids', $ids, IQueryBuilder::PARAM_INT_ARRAY);
$queryResult = $deleteQuery->executeStatement();
if($queryResult === 10000) {
$this->deleteActivitiesForMySQL($conditions);
}
}
}
1 change: 1 addition & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<UndefinedClass>
<errorLevel type="suppress">
<referencedClass name="Doctrine\DBAL\Platforms\MySQLPlatform" />
<referencedClass name="Doctrine\DBAL\Platforms\AbstractMySQLPlatform" />
<referencedClass name="Doctrine\DBAL\Types\Types" />
<referencedClass name="OC" />
<referencedClass name="OC\Core\Command\Base" />
Expand Down
9 changes: 7 additions & 2 deletions tests/Controller/APIv1ControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
use OCP\IRequest;
use OCP\IUserSession;
use OCP\RichObjectStrings\IValidator;
use Psr\Log\LoggerInterface;

/**
* Class APIv1Test
Expand Down Expand Up @@ -106,7 +107,8 @@ protected function tearDown(): void {
protected function cleanUp(): void {
$data = new Data(
$this->createMock(IManager::class),
\OC::$server->getDatabaseConnection()
\OC::$server->getDatabaseConnection(),
$this->createMock(LoggerInterface::class)
);

$this->deleteUser($data, 'activity-api-user1');
Expand Down Expand Up @@ -220,7 +222,10 @@ public function testGet(string $user, int $start, int $count, array $expected):
->method('getUID')
->willReturn($user);

$data = new Data($activityManager, \OC::$server->getDatabaseConnection());
$data = new Data($activityManager,
\OC::$server->getDatabaseConnection(),
$this->createMock(LoggerInterface::class)
);

$controller = new APIv1Controller(
'activity',
Expand Down
4 changes: 2 additions & 2 deletions tests/DataDeleteActivitiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
use OCP\BackgroundJob\IJobList;
use OCP\DB\IPreparedStatement;
use OCP\IConfig;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;

/**
* Class DataDeleteActivitiesTest
Expand Down Expand Up @@ -74,7 +74,7 @@ protected function setUp(): void {
$this->data = new Data(
$this->createMock(IManager::class),
\OC::$server->getDatabaseConnection(),
$this->createMock(IUserSession::class)
$this->createMock(LoggerInterface::class)
);
}

Expand Down
8 changes: 7 additions & 1 deletion tests/DataTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use OCP\IUserSession;
use OCP\Util;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\NullLogger;

/**
* Class DataTest
Expand All @@ -54,17 +55,22 @@ class DataTest extends TestCase {
/** @var IManager */
protected $realActivityManager;

/** @var NullLogger */
protected $logger;

protected function setUp(): void {
parent::setUp();

$this->activityLanguage = Util::getL10N('activity', 'en');
$activityManager = $this->createMock(IManager::class);
$this->dbConnection = \OC::$server->get(IDBConnection::class);
$this->realActivityManager = \OC::$server->get(IManager::class);
$this->logger = \OC::$server->get(NullLogger::class);

$this->data = new Data(
$activityManager,
$this->dbConnection
$this->dbConnection,
$this->logger
);
}

Expand Down
Loading