Skip to content

Commit b10e23e

Browse files
author
Carl Schwan
committed
feat(filecache): Scale DB query created when deleting file from filecache
Instead of creating a CacheEntryRemovedEvent for each deleted files, create a single CacheEntriesRemovedEvent which wrap multiple CacheEntryRemovedEvent. This allow listener to optimize the query they do when multiple files are deleted at the same time (e.g. when deleting a folder). Signed-off-by: Carl Schwan <carl.schwan@nextclound.com>
1 parent b514d75 commit b10e23e

File tree

13 files changed

+170
-16
lines changed

13 files changed

+170
-16
lines changed

‎apps/files/lib/AppInfo/Application.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
use OCP\AppFramework\Bootstrap\IRegistrationContext;
3838
use OCP\Collaboration\Reference\RenderReferenceEvent;
3939
use OCP\Collaboration\Resources\IProviderManager;
40+
use OCP\Files\Cache\CacheEntriesRemovedEvent;
4041
use OCP\Files\Cache\CacheEntryRemovedEvent;
4142
use OCP\Files\Events\Node\BeforeNodeCopiedEvent;
4243
use OCP\Files\Events\Node\BeforeNodeDeletedEvent;
@@ -117,7 +118,7 @@ public function register(IRegistrationContext $context): void {
117118
$context->registerEventListener(RenderReferenceEvent::class, RenderReferenceEventListener::class);
118119
$context->registerEventListener(BeforeNodeRenamedEvent::class, SyncLivePhotosListener::class);
119120
$context->registerEventListener(BeforeNodeDeletedEvent::class, SyncLivePhotosListener::class);
120-
$context->registerEventListener(CacheEntryRemovedEvent::class, SyncLivePhotosListener::class);
121+
$context->registerEventListener(CacheEntriesRemovedEvent::class, SyncLivePhotosListener::class, 1);
121122
$context->registerEventListener(BeforeNodeCopiedEvent::class, SyncLivePhotosListener::class);
122123
$context->registerEventListener(NodeCopiedEvent::class, SyncLivePhotosListener::class);
123124
$context->registerEventListener(LoadSearchPlugins::class, LoadSearchPluginsListener::class);

‎apps/files/lib/Listener/SyncLivePhotosListener.php‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
use OCP\EventDispatcher\Event;
1818
use OCP\EventDispatcher\IEventListener;
1919
use OCP\Exceptions\AbortedEventException;
20-
use OCP\Files\Cache\CacheEntryRemovedEvent;
20+
use OCP\Files\Cache\CacheEntriesRemovedEvent;
2121
use OCP\Files\Events\Node\BeforeNodeCopiedEvent;
2222
use OCP\Files\Events\Node\BeforeNodeDeletedEvent;
2323
use OCP\Files\Events\Node\BeforeNodeRenamedEvent;
@@ -63,8 +63,8 @@ public function handle(Event $event): void {
6363
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getSource()->getId());
6464
} elseif ($event instanceof BeforeNodeDeletedEvent) {
6565
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getNode()->getId());
66-
} elseif ($event instanceof CacheEntryRemovedEvent) {
67-
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getFileId());
66+
} elseif ($event instanceof CacheEntriesRemovedEvent) {
67+
$this->handleCacheEntriesRemovedEvent($event);
6868
}
6969

7070
if ($peerFileId === null) {
@@ -83,12 +83,30 @@ public function handle(Event $event): void {
8383
$this->handleMove($event->getSource(), $event->getTarget(), $peerFile);
8484
} elseif ($event instanceof BeforeNodeDeletedEvent) {
8585
$this->handleDeletion($event, $peerFile);
86-
} elseif ($event instanceof CacheEntryRemovedEvent) {
87-
$peerFile->delete();
8886
}
8987
}
9088
}
9189

90+
public function handleCacheEntriesRemovedEvent(CacheEntriesRemovedEvent $cacheEntriesRemovedEvent): void {
91+
$entries = $cacheEntriesRemovedEvent->getCacheEntryRemovedEvents();
92+
$fileIds = [];
93+
foreach ($entries as $entry) {
94+
$fileIds[] = $entry->getFileId();
95+
}
96+
97+
$peerFileIds = $this->livePhotosService->getLivePhotoPeerIds($fileIds);
98+
99+
foreach ($peerFileIds as $peerFileId) {
100+
// Check the user's folder.
101+
$peerFile = $this->userFolder->getFirstNodeById($peerFileId);
102+
103+
if ($peerFile === null) {
104+
return; // Peer file not found.
105+
}
106+
$peerFile->delete();
107+
}
108+
}
109+
92110
private function runMoveOrCopyChecks(Node $sourceFile, Node $targetFile, Node $peerFile): void {
93111
$targetParent = $targetFile->getParent();
94112
$sourceExtension = $sourceFile->getExtension();

‎apps/files/lib/Service/LivePhotosService.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use OCP\FilesMetadata\Exceptions\FilesMetadataNotFoundException;
1212
use OCP\FilesMetadata\IFilesMetadataManager;
13+
use function PHPUnit\Framework\containsEqual;
1314

1415
class LivePhotosService {
1516
public function __construct(
@@ -33,4 +34,22 @@ public function getLivePhotoPeerId(int $fileId): ?int {
3334

3435
return (int)$metadata->getString('files-live-photo');
3536
}
37+
38+
/**
39+
* Get the associated live photo for multiple file ids
40+
* @param int[] $fileIds
41+
* @return int[]
42+
*/
43+
public function getLivePhotoPeerIds(array $fileIds): array {
44+
$metadata = $this->filesMetadataManager->getMetadataForFiles($fileIds);
45+
$peersIds = [];
46+
foreach ($metadata as $item) {
47+
if (!$item->hasKey('files-live-photo')) {
48+
continue;
49+
}
50+
51+
$peersIds[] = (int)$item->getString('files-live-photo');
52+
}
53+
return $peersIds;
54+
}
3655
}

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@
393393
'OCP\\Files\\AlreadyExistsException' => $baseDir . '/lib/public/Files/AlreadyExistsException.php',
394394
'OCP\\Files\\AppData\\IAppDataFactory' => $baseDir . '/lib/public/Files/AppData/IAppDataFactory.php',
395395
'OCP\\Files\\Cache\\AbstractCacheEvent' => $baseDir . '/lib/public/Files/Cache/AbstractCacheEvent.php',
396+
'OCP\\Files\\Cache\\CacheEntriesRemovedEvent' => $baseDir . '/lib/public/Files/Cache/CacheEntriesRemovedEvent.php',
396397
'OCP\\Files\\Cache\\CacheEntryInsertedEvent' => $baseDir . '/lib/public/Files/Cache/CacheEntryInsertedEvent.php',
397398
'OCP\\Files\\Cache\\CacheEntryRemovedEvent' => $baseDir . '/lib/public/Files/Cache/CacheEntryRemovedEvent.php',
398399
'OCP\\Files\\Cache\\CacheEntryUpdatedEvent' => $baseDir . '/lib/public/Files/Cache/CacheEntryUpdatedEvent.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
434434
'OCP\\Files\\AlreadyExistsException' => __DIR__ . '/../../..' . '/lib/public/Files/AlreadyExistsException.php',
435435
'OCP\\Files\\AppData\\IAppDataFactory' => __DIR__ . '/../../..' . '/lib/public/Files/AppData/IAppDataFactory.php',
436436
'OCP\\Files\\Cache\\AbstractCacheEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Cache/AbstractCacheEvent.php',
437+
'OCP\\Files\\Cache\\CacheEntriesRemovedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Cache/CacheEntriesRemovedEvent.php',
437438
'OCP\\Files\\Cache\\CacheEntryInsertedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Cache/CacheEntryInsertedEvent.php',
438439
'OCP\\Files\\Cache\\CacheEntryRemovedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Cache/CacheEntryRemovedEvent.php',
439440
'OCP\\Files\\Cache\\CacheEntryUpdatedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Cache/CacheEntryUpdatedEvent.php',

‎lib/private/Files/Cache/Cache.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OC\SystemConfig;
1717
use OCP\DB\QueryBuilder\IQueryBuilder;
1818
use OCP\EventDispatcher\IEventDispatcher;
19+
use OCP\Files\Cache\CacheEntriesRemovedEvent;
1920
use OCP\Files\Cache\CacheEntryInsertedEvent;
2021
use OCP\Files\Cache\CacheEntryRemovedEvent;
2122
use OCP\Files\Cache\CacheEntryUpdatedEvent;
@@ -611,15 +612,19 @@ private function removeChildren(ICacheEntry $entry) {
611612
$query->executeStatement();
612613
}
613614

615+
$cacheEntryRemovedEvents = [];
614616
foreach (array_combine($deletedIds, $deletedPaths) as $fileId => $filePath) {
615617
$cacheEntryRemovedEvent = new CacheEntryRemovedEvent(
616618
$this->storage,
617619
$filePath,
618620
$fileId,
619621
$this->getNumericStorageId()
620622
);
623+
$cacheEntryRemovedEvents[] = $cacheEntryRemovedEvent;
621624
$this->eventDispatcher->dispatchTyped($cacheEntryRemovedEvent);
622625
}
626+
$this->eventDispatcher->dispatchTyped(new CacheEntriesRemovedEvent($cacheEntryRemovedEvents));
627+
623628
}
624629

625630
/**

‎lib/private/FilesMetadata/FilesMetadataManager.php‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
use OCP\DB\Exception as DBException;
2121
use OCP\DB\QueryBuilder\IQueryBuilder;
2222
use OCP\EventDispatcher\IEventDispatcher;
23-
use OCP\Files\Cache\CacheEntryRemovedEvent;
23+
use OCP\Files\Cache\CacheEntriesRemovedEvent;
2424
use OCP\Files\Events\Node\NodeWrittenEvent;
2525
use OCP\Files\InvalidPathException;
2626
use OCP\Files\Node;
@@ -214,6 +214,20 @@ public function deleteMetadata(int $fileId): void {
214214
}
215215
}
216216

217+
public function deleteMetadataForFiles(array $fileIds): void {
218+
try {
219+
$this->metadataRequestService->dropMetadataForFiles($fileIds);
220+
} catch (Exception $e) {
221+
$this->logger->warning('issue while deleteMetadata', ['exception' => $e, 'fileIds' => $fileIds]);
222+
}
223+
224+
try {
225+
$this->indexRequestService->dropIndexForFiles($fileIds);
226+
} catch (Exception $e) {
227+
$this->logger->warning('issue while deleteMetadata', ['exception' => $e, 'fileIds' => $fileIds]);
228+
}
229+
}
230+
217231
/**
218232
* @param IQueryBuilder $qb
219233
* @param string $fileTableAlias alias of the table that contains data about files
@@ -301,6 +315,6 @@ public function initMetadata(
301315
*/
302316
public static function loadListeners(IEventDispatcher $eventDispatcher): void {
303317
$eventDispatcher->addServiceListener(NodeWrittenEvent::class, MetadataUpdate::class);
304-
$eventDispatcher->addServiceListener(CacheEntryRemovedEvent::class, MetadataDelete::class);
318+
$eventDispatcher->addServiceListener(CacheEntriesRemovedEvent::class, MetadataDelete::class);
305319
}
306320
}

‎lib/private/FilesMetadata/Listener/MetadataDelete.php‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,18 @@
99
namespace OC\FilesMetadata\Listener;
1010

1111
use Exception;
12+
use OC\Files\Cache\CacheEntry;
1213
use OCP\EventDispatcher\Event;
1314
use OCP\EventDispatcher\IEventListener;
15+
use OCP\Files\Cache\CacheEntriesRemovedEvent;
1416
use OCP\Files\Cache\CacheEntryRemovedEvent;
1517
use OCP\FilesMetadata\IFilesMetadataManager;
1618
use Psr\Log\LoggerInterface;
1719

1820
/**
1921
* Handle file deletion event and remove stored metadata related to the deleted file
2022
*
21-
* @template-implements IEventListener<CacheEntryRemovedEvent>
23+
* @template-implements IEventListener<CacheEntriesRemovedEvent>
2224
*/
2325
class MetadataDelete implements IEventListener {
2426
public function __construct(
@@ -28,17 +30,24 @@ public function __construct(
2830
}
2931

3032
public function handle(Event $event): void {
31-
if (!($event instanceof CacheEntryRemovedEvent)) {
33+
if (!($event instanceof CacheEntriesRemovedEvent)) {
3234
return;
3335
}
3436

35-
try {
36-
$nodeId = $event->getFileId();
37-
if ($nodeId > 0) {
38-
$this->filesMetadataManager->deleteMetadata($nodeId);
37+
$entries = $event->getCacheEntryRemovedEvents();
38+
$fileIds = [];
39+
40+
foreach ($entries as $entry) {
41+
try {
42+
$nodeId = $entry->getFileId();
43+
if ($nodeId > 0) {
44+
$fileIds[] = $nodeId;
45+
}
46+
} catch (Exception $e) {
47+
$this->logger->warning('issue while running MetadataDelete', ['exception' => $e]);
3948
}
40-
} catch (Exception $e) {
41-
$this->logger->warning('issue while running MetadataDelete', ['exception' => $e]);
4249
}
50+
51+
$this->filesMetadataManager->deleteMetadataForFiles($fileIds);
4352
}
4453
}

‎lib/private/FilesMetadata/Service/IndexRequestService.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,4 +175,30 @@ public function dropIndex(int $fileId, string $key = ''): void {
175175

176176
$qb->executeStatement();
177177
}
178+
179+
/**
180+
* Drop indexes related to multiple file ids
181+
* if a key is specified, only drop entries related to it
182+
*
183+
* @param int[] $fileIds file ids
184+
* @param string $key metadata key
185+
*
186+
* @throws DbException
187+
*/
188+
public function dropIndexForFiles(array $fileIds, string $key = ''): void {
189+
$chunks = array_chunk($fileIds, 1000);
190+
191+
foreach ($chunks as $chunk) {
192+
$qb = $this->dbConnection->getQueryBuilder();
193+
$expr = $qb->expr();
194+
$qb->delete(self::TABLE_METADATA_INDEX)
195+
->where($expr->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)));
196+
197+
if ($key !== '') {
198+
$qb->andWhere($expr->eq('meta_key', $qb->createNamedParameter($key)));
199+
}
200+
201+
$qb->executeStatement();
202+
}
203+
}
178204
}

‎lib/private/FilesMetadata/Service/MetadataRequestService.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,22 @@ public function dropMetadata(int $fileId): void {
143143
$qb->executeStatement();
144144
}
145145

146+
/**
147+
* @param int[] $fileIds
148+
* @return void
149+
* @throws Exception
150+
*/
151+
public function dropMetadataForFiles(array $fileIds): void {
152+
$chunks = array_chunk($fileIds, 1000);
153+
154+
foreach ($chunks as $chunk) {
155+
$qb = $this->dbConnection->getQueryBuilder();
156+
$qb->delete(self::TABLE_METADATA)
157+
->where($qb->expr()->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)));
158+
$qb->executeStatement();
159+
}
160+
}
161+
146162
/**
147163
* update metadata in the database
148164
*

0 commit comments

Comments
 (0)