Skip to content

Commit fbda405

Browse files
solracsfbackportbot[bot]
solracsf
authored and
backportbot[bot]
committed
fix(files): clean up filecache companion rows of removed storages
cleanByMountId() deleted the filecache rows of an unmounted storage but left their filecache_extended and file metadata rows behind. It now goes through Storage::removeFileCacheEntries(), the same path clear() uses. files:cleanup also removes filecache_extended, files_metadata and files_metadata_index rows whose file is no longer in the filecache, so rows orphaned before this fix are cleaned up too. Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
1 parent c178ac8 commit fbda405

5 files changed

Lines changed: 235 additions & 94 deletions

File tree

‎apps/files/lib/Command/DeleteOrphanedFiles.php‎

Lines changed: 39 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@
1818
use OCP\IDBConnection;
1919

2020
/**
21-
* Delete all file entries that have no matching entries in the storage table.
21+
* Delete all file entries that have no matching entries in the storage table,
22+
* and the rows keyed by file id that have no matching file entry.
2223
*/
2324
#[AsCommand(
2425
name: 'files:cleanup',
2526
description: 'Clean up orphaned filecache and mount entries',
26-
help: 'Deletes orphaned filecache and mount entries (those without an existing storage).',
27+
help: 'Deletes orphaned filecache and mount entries (those without an existing storage), and filecache_extended and file metadata entries without a filecache entry.',
2728
)]
2829
class DeleteOrphanedFiles {
2930
public const int CHUNK_SIZE = 200;
@@ -38,23 +39,22 @@ public function __invoke(
3839
#[Option(name: 'skip-filecache-extended', description: 'don\'t remove orphaned entries from filecache_extended')]
3940
bool $skipFilecacheExtended = false,
4041
): ExitCode {
41-
$fileIdsByStorage = [];
42-
4342
$deletedStorages = array_diff($this->getReferencedStorages(), $this->getExistingStorages());
4443

45-
$deleteExtended = !$skipFilecacheExtended;
46-
if ($deleteExtended) {
47-
$fileIdsByStorage = $this->getFileIdsForStorages($deletedStorages);
48-
}
49-
5044
$deletedEntries = $this->cleanupOrphanedFileCache($deletedStorages);
5145
$output->writeln("$deletedEntries orphaned file cache entries deleted");
5246

53-
if ($deleteExtended) {
54-
$deletedFileCacheExtended = $this->cleanupOrphanedFileCacheExtended($fileIdsByStorage);
47+
if (!$skipFilecacheExtended) {
48+
$deletedFileCacheExtended = $this->cleanupEntriesWithoutFileCache('filecache_extended', 'fileid');
5549
$output->writeln("$deletedFileCacheExtended orphaned file cache extended entries deleted");
5650
}
5751

52+
$deletedMetadata = $this->cleanupEntriesWithoutFileCache('files_metadata', 'file_id');
53+
$output->writeln("$deletedMetadata orphaned file metadata entries deleted");
54+
55+
$deletedMetadataIndex = $this->cleanupEntriesWithoutFileCache('files_metadata_index', 'file_id');
56+
$output->writeln("$deletedMetadataIndex orphaned file metadata index entries deleted");
57+
5858
$deletedMounts = $this->cleanupOrphanedMounts();
5959
$output->writeln("$deletedMounts orphaned mount entries deleted");
6060

@@ -78,28 +78,6 @@ private function getExistingStorages(): array {
7878
return $query->executeQuery()->fetchFirstColumn();
7979
}
8080

81-
/**
82-
* @param int[] $storageIds
83-
* @return array<int, int[]>
84-
*/
85-
private function getFileIdsForStorages(array $storageIds): array {
86-
$query = $this->connection->getQueryBuilder();
87-
$query->select('storage', 'fileid')
88-
->from('filecache')
89-
->where($query->expr()->in('storage', $query->createParameter('storage_ids')));
90-
91-
$result = [];
92-
$storageIdChunks = array_chunk($storageIds, self::CHUNK_SIZE);
93-
foreach ($storageIdChunks as $storageIdChunk) {
94-
$query->setParameter('storage_ids', $storageIdChunk, IQueryBuilder::PARAM_INT_ARRAY);
95-
$chunk = $query->executeQuery()->fetchAllAssociative();
96-
foreach ($chunk as $row) {
97-
$result[$row['storage']][] = $row['fileid'];
98-
}
99-
}
100-
return $result;
101-
}
102-
10381
private function cleanupOrphanedFileCache(array $deletedStorages): int {
10482
$deletedEntries = 0;
10583

@@ -116,27 +94,38 @@ private function cleanupOrphanedFileCache(array $deletedStorages): int {
11694
return $deletedEntries;
11795
}
11896

119-
/**
120-
* @param array<int, int[]> $fileIdsByStorage
121-
* @return int
122-
*/
123-
private function cleanupOrphanedFileCacheExtended(array $fileIdsByStorage): int {
97+
private function cleanupEntriesWithoutFileCache(string $table, string $fileIdColumn): int {
12498
$deletedEntries = 0;
99+
$lastFileId = 0;
100+
101+
while (true) {
102+
$query = $this->connection->getQueryBuilder();
103+
$query->select($fileIdColumn)
104+
->from($table)
105+
->where($query->expr()->gt($fileIdColumn, $query->createNamedParameter($lastFileId, IQueryBuilder::PARAM_INT)))
106+
->orderBy($fileIdColumn)
107+
->setMaxResults(IQueryBuilder::MAX_IN_PARAMETERS)
108+
->runAcrossAllShards();
109+
$fileIds = array_unique(array_map(intval(...), $query->executeQuery()->fetchFirstColumn()));
110+
if ($fileIds === []) {
111+
return $deletedEntries;
112+
}
125113

126-
$deleteQuery = $this->connection->getQueryBuilder();
127-
$deleteQuery->delete('filecache_extended')
128-
->where($deleteQuery->expr()->in('fileid', $deleteQuery->createParameter('file_ids')));
129-
130-
foreach ($fileIdsByStorage as $storageId => $fileIds) {
131-
$deleteQuery->hintShardKey('storage', $storageId, true);
132-
$fileChunks = array_chunk($fileIds, self::CHUNK_SIZE);
133-
foreach ($fileChunks as $fileChunk) {
134-
$deleteQuery->setParameter('file_ids', $fileChunk, IQueryBuilder::PARAM_INT_ARRAY);
135-
$deletedEntries += $deleteQuery->executeStatement();
114+
$query = $this->connection->getQueryBuilder();
115+
$query->select('fileid')
116+
->from('filecache')
117+
->where($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)));
118+
$missingFileIds = array_diff($fileIds, $query->executeQuery()->fetchFirstColumn());
119+
120+
if ($missingFileIds !== []) {
121+
$query = $this->connection->getQueryBuilder();
122+
$query->delete($table)
123+
->where($query->expr()->in($fileIdColumn, $query->createNamedParameter($missingFileIds, IQueryBuilder::PARAM_INT_ARRAY)));
124+
$deletedEntries += $query->executeStatement();
136125
}
137-
}
138126

139-
return $deletedEntries;
127+
$lastFileId = max($fileIds);
128+
}
140129
}
141130

142131
private function cleanupOrphanedMounts(): int {

‎apps/files/tests/Command/DeleteOrphanedFilesTest.php‎

Lines changed: 89 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,15 @@
99

1010
namespace OCA\Files\Tests\Command;
1111

12+
use OC\Files\Storage\Temporary;
1213
use OC\Files\View;
1314
use OCA\Files\Command\DeleteOrphanedFiles;
1415
use OCP\Console\IOutput;
16+
use OCP\DB\QueryBuilder\IQueryBuilder;
17+
use OCP\Files\Cache\ICacheEntry;
1518
use OCP\Files\IRootFolder;
1619
use OCP\Files\StorageNotAvailableException;
20+
use OCP\FilesMetadata\IFilesMetadataManager;
1721
use OCP\IDBConnection;
1822
use OCP\IUserManager;
1923
use OCP\Server;
@@ -73,6 +77,31 @@ protected function getMountsCount(int $storageId): int {
7377
return (int)$query->executeQuery()->fetchOne();
7478
}
7579

80+
/**
81+
* @param list<int> $fileIds
82+
*/
83+
protected function countRows(string $table, string $column, array $fileIds): int {
84+
// selecting rows instead of COUNT(*), which a sharded query answers once per shard
85+
$query = $this->connection->getQueryBuilder();
86+
$query->select($column)
87+
->from($table)
88+
->where($query->expr()->in($column, $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)));
89+
return count($query->executeQuery()->fetchFirstColumn());
90+
}
91+
92+
/**
93+
* @param list<string> $calls
94+
*/
95+
protected function expectOutput(IOutput&\PHPUnit\Framework\MockObject\MockObject $output, array $calls): void {
96+
$output
97+
->expects($this->exactly(count($calls)))
98+
->method('writeln')
99+
->willReturnCallback(function (string $message) use (&$calls): void {
100+
$expected = array_shift($calls);
101+
$this->assertSame($expected, $message);
102+
});
103+
}
104+
76105
/**
77106
* Test clearing orphaned files
78107
*/
@@ -102,6 +131,16 @@ public function testClearFiles(): void {
102131
$this->assertCount(1, $this->getFile($fileInfo->getId()), 'Asserts that file is still available');
103132
$this->assertEquals(1, $this->getMountsCount($numericStorageId), 'Asserts that mount is still available');
104133

134+
$qb = $this->connection->getQueryBuilder();
135+
$storageFileIds = array_map('intval', $qb->select('fileid')
136+
->from('filecache')
137+
->where($qb->expr()->eq('storage', $qb->createNamedParameter($numericStorageId, IQueryBuilder::PARAM_INT)))
138+
->executeQuery()
139+
->fetchFirstColumn());
140+
$extendedEntries = $this->countRows('filecache_extended', 'fileid', $storageFileIds);
141+
$metadataEntries = $this->countRows('files_metadata', 'file_id', $storageFileIds);
142+
$metadataIndexEntries = $this->countRows('files_metadata_index', 'file_id', $storageFileIds);
143+
105144
$qb = $this->connection->getQueryBuilder();
106145
$deletedRows = $qb->delete('storages')
107146
->where($qb->expr()->eq('id', $qb->createNamedParameter($storageId)))
@@ -110,18 +149,13 @@ public function testClearFiles(): void {
110149
$this->assertSame(1, $deletedRows, 'Asserts that storage got deleted');
111150

112151
// parent folder, `files`, ´test` and `welcome.txt` => 4 elements
113-
$calls = [
152+
$this->expectOutput($output, [
114153
'3 orphaned file cache entries deleted',
115-
'0 orphaned file cache extended entries deleted',
154+
"$extendedEntries orphaned file cache extended entries deleted",
155+
"$metadataEntries orphaned file metadata entries deleted",
156+
"$metadataIndexEntries orphaned file metadata index entries deleted",
116157
'1 orphaned mount entries deleted',
117-
];
118-
$output
119-
->expects($this->exactly(3))
120-
->method('writeln')
121-
->willReturnCallback(function (string $message) use (&$calls): void {
122-
$expected = array_shift($calls);
123-
$this->assertSame($expected, $message);
124-
});
158+
]);
125159

126160
($this->command)($output);
127161

@@ -136,4 +170,49 @@ public function testClearFiles(): void {
136170
} catch (StorageNotAvailableException $e) {
137171
}
138172
}
173+
174+
public function testClearEntriesWithoutFileCacheEntry(): void {
175+
// remove orphans left behind by other tests so that the counts below only cover this test
176+
($this->command)($this->createMock(IOutput::class));
177+
178+
$storage = new Temporary([]);
179+
$cache = $storage->getCache();
180+
$cache->put('', ['size' => 0, 'mtime' => 0, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]);
181+
$data = ['size' => 1, 'mtime' => 1, 'mimetype' => 'text/plain', 'upload_time' => 25];
182+
$orphanId = $cache->put('orphan.txt', $data);
183+
$keptId = $cache->put('kept.txt', $data);
184+
185+
$metadataManager = Server::get(IFilesMetadataManager::class);
186+
foreach ([$orphanId, $keptId] as $fileId) {
187+
$metadata = $metadataManager->getMetadata($fileId, true);
188+
$metadata->setString('test-key', 'value', true);
189+
$metadataManager->saveMetadata($metadata);
190+
}
191+
192+
$qb = $this->connection->getQueryBuilder();
193+
$qb->delete('filecache')
194+
->where($qb->expr()->eq('fileid', $qb->createNamedParameter($orphanId, IQueryBuilder::PARAM_INT)))
195+
->executeStatement();
196+
197+
$output = $this->createMock(IOutput::class);
198+
$this->expectOutput($output, [
199+
'0 orphaned file cache entries deleted',
200+
'1 orphaned file cache extended entries deleted',
201+
'1 orphaned file metadata entries deleted',
202+
'1 orphaned file metadata index entries deleted',
203+
'0 orphaned mount entries deleted',
204+
]);
205+
206+
($this->command)($output);
207+
208+
$this->assertSame(0, $this->countRows('filecache_extended', 'fileid', [$orphanId]));
209+
$this->assertSame(0, $this->countRows('files_metadata', 'file_id', [$orphanId]));
210+
$this->assertSame(0, $this->countRows('files_metadata_index', 'file_id', [$orphanId]));
211+
212+
$this->assertSame(1, $this->countRows('filecache_extended', 'fileid', [$keptId]));
213+
$this->assertSame(1, $this->countRows('files_metadata', 'file_id', [$keptId]));
214+
$this->assertSame(1, $this->countRows('files_metadata_index', 'file_id', [$keptId]));
215+
216+
$cache->clear();
217+
}
139218
}

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

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -902,33 +902,7 @@ private function getChildIds(int $storageId, string $path): array {
902902
* remove all entries for files that are stored on the storage from the cache
903903
*/
904904
public function clear() {
905-
$storageId = $this->getNumericStorageId();
906-
907-
while (true) {
908-
$query = $this->getQueryBuilder();
909-
$query->select('fileid')
910-
->from('filecache')
911-
->whereStorageId($storageId)
912-
->setMaxResults(IQueryBuilder::MAX_IN_PARAMETERS);
913-
$fileIds = array_map(intval(...), $query->executeQuery()->fetchFirstColumn());
914-
if ($fileIds === []) {
915-
break;
916-
}
917-
918-
$query = $this->getQueryBuilder();
919-
$query->delete('filecache_extended')
920-
->where($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)))
921-
->hintShardKey('storage', $storageId);
922-
$query->executeStatement();
923-
924-
$this->metadataManager->deleteMetadataForFiles($storageId, $fileIds);
925-
926-
$query = $this->getQueryBuilder();
927-
$query->delete('filecache')
928-
->whereStorageId($storageId)
929-
->andWhere($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)));
930-
$query->executeStatement();
931-
}
905+
Storage::removeFileCacheEntries($this->getNumericStorageId());
932906

933907
$query = $this->connection->getQueryBuilder();
934908
$query->delete('storages')

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

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use OC\DB\Exceptions\DbalException;
1414
use OCP\DB\QueryBuilder\IQueryBuilder;
1515
use OCP\Files\Storage\IStorage;
16+
use OCP\FilesMetadata\IFilesMetadataManager;
1617
use OCP\IDBConnection;
1718
use OCP\Server;
1819
use Psr\Log\LoggerInterface;
@@ -170,14 +171,11 @@ public static function cleanByMountId(int $mountId): void {
170171
$query->select('storage_id')
171172
->from('mounts')
172173
->where($query->expr()->eq('mount_id', $query->createNamedParameter($mountId, IQueryBuilder::PARAM_INT)));
173-
$storageIds = $query->executeQuery()->fetchFirstColumn();
174-
$storageIds = array_unique($storageIds);
174+
$storageIds = array_unique(array_map(intval(...), $query->executeQuery()->fetchFirstColumn()));
175175

176-
$query = $db->getQueryBuilder();
177-
$query->delete('filecache')
178-
->where($query->expr()->in('storage', $query->createNamedParameter($storageIds, IQueryBuilder::PARAM_INT_ARRAY)))
179-
->runAcrossAllShards()
180-
->executeStatement();
176+
foreach ($storageIds as $storageId) {
177+
self::removeFileCacheEntries($storageId);
178+
}
181179

182180
$query = $db->getQueryBuilder();
183181
$query->delete('storages')
@@ -195,4 +193,38 @@ public static function cleanByMountId(int $mountId): void {
195193
throw $exception;
196194
}
197195
}
196+
197+
/**
198+
* Remove the filecache entries of a storage together with their filecache_extended and metadata rows
199+
*/
200+
public static function removeFileCacheEntries(int $numericStorageId): void {
201+
$db = Server::get(IDBConnection::class);
202+
$metadataManager = Server::get(IFilesMetadataManager::class);
203+
204+
while (true) {
205+
$query = $db->getQueryBuilder();
206+
$query->select('fileid')
207+
->from('filecache')
208+
->where($query->expr()->eq('storage', $query->createNamedParameter($numericStorageId, IQueryBuilder::PARAM_INT)))
209+
->setMaxResults(IQueryBuilder::MAX_IN_PARAMETERS);
210+
$fileIds = array_map(intval(...), $query->executeQuery()->fetchFirstColumn());
211+
if ($fileIds === []) {
212+
return;
213+
}
214+
215+
$query = $db->getQueryBuilder();
216+
$query->delete('filecache_extended')
217+
->where($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)))
218+
->hintShardKey('storage', $numericStorageId)
219+
->executeStatement();
220+
221+
$metadataManager->deleteMetadataForFiles($numericStorageId, $fileIds);
222+
223+
$query = $db->getQueryBuilder();
224+
$query->delete('filecache')
225+
->where($query->expr()->eq('storage', $query->createNamedParameter($numericStorageId, IQueryBuilder::PARAM_INT)))
226+
->andWhere($query->expr()->in('fileid', $query->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)))
227+
->executeStatement();
228+
}
229+
}
198230
}

0 commit comments

Comments
 (0)