Skip to content

Commit

Permalink
repair -1 folder sizes for object store background scan
Browse files Browse the repository at this point in the history
Signed-off-by: Robin Appelman <robin@icewind.nl>
  • Loading branch information
icewind1991 committed May 10, 2023
1 parent f734a76 commit 2ea41da
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 112 deletions.
5 changes: 3 additions & 2 deletions apps/files_sharing/lib/Scanner.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

namespace OCA\Files_Sharing;

use OC\Files\ObjectStore\NoopScanner;
use OC\Files\ObjectStore\ObjectStoreScanner;

/**
* Scanner for SharedStorage
Expand Down Expand Up @@ -72,7 +72,8 @@ private function getSourceScanner() {

public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) {
$sourceScanner = $this->getSourceScanner();
if ($sourceScanner instanceof NoopScanner) {
if ($sourceScanner instanceof ObjectStoreScanner) {
// ObjectStoreScanner doesn't scan
return [];
} else {
return parent::scanFile($file, $reuseExisting, $parentId, $cacheData, $lock);
Expand Down
6 changes: 4 additions & 2 deletions apps/files_sharing/tests/External/ScannerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@
use OCA\Files_Sharing\External\Scanner;
use Test\TestCase;

/**
* @group DB
*/
class ScannerTest extends TestCase {
/** @var \OCA\Files_Sharing\External\Scanner */
protected $scanner;
protected Scanner $scanner;
/** @var \OCA\Files_Sharing\External\Storage|\PHPUnit\Framework\MockObject\MockObject */
protected $storage;
/** @var \OC\Files\Cache\Cache|\PHPUnit\Framework\MockObject\MockObject */
Expand Down
2 changes: 1 addition & 1 deletion lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@
'OC\\Files\\ObjectStore\\Azure' => $baseDir . '/lib/private/Files/ObjectStore/Azure.php',
'OC\\Files\\ObjectStore\\HomeObjectStoreStorage' => $baseDir . '/lib/private/Files/ObjectStore/HomeObjectStoreStorage.php',
'OC\\Files\\ObjectStore\\Mapper' => $baseDir . '/lib/private/Files/ObjectStore/Mapper.php',
'OC\\Files\\ObjectStore\\NoopScanner' => $baseDir . '/lib/private/Files/ObjectStore/NoopScanner.php',
'OC\\Files\\ObjectStore\\ObjectStoreScanner' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreScanner.php',
'OC\\Files\\ObjectStore\\ObjectStoreStorage' => $baseDir . '/lib/private/Files/ObjectStore/ObjectStoreStorage.php',
'OC\\Files\\ObjectStore\\S3' => $baseDir . '/lib/private/Files/ObjectStore/S3.php',
'OC\\Files\\ObjectStore\\S3ConnectionTrait' => $baseDir . '/lib/private/Files/ObjectStore/S3ConnectionTrait.php',
Expand Down
2 changes: 1 addition & 1 deletion lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1291,7 +1291,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Files\\ObjectStore\\Azure' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/Azure.php',
'OC\\Files\\ObjectStore\\HomeObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/HomeObjectStoreStorage.php',
'OC\\Files\\ObjectStore\\Mapper' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/Mapper.php',
'OC\\Files\\ObjectStore\\NoopScanner' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/NoopScanner.php',
'OC\\Files\\ObjectStore\\ObjectStoreScanner' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreScanner.php',
'OC\\Files\\ObjectStore\\ObjectStoreStorage' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/ObjectStoreStorage.php',
'OC\\Files\\ObjectStore\\S3' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3.php',
'OC\\Files\\ObjectStore\\S3ConnectionTrait' => __DIR__ . '/../../..' . '/lib/private/Files/ObjectStore/S3ConnectionTrait.php',
Expand Down
17 changes: 10 additions & 7 deletions lib/private/Files/Cache/Scanner.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@
use OCP\Files\ForbiddenException;
use OCP\Files\NotFoundException;
use OCP\Files\Storage\IReliableEtagStorage;
use OCP\IDBConnection;
use OCP\Lock\ILockingProvider;
use OC\Files\Storage\Wrapper\Encoding;
use OC\Files\Storage\Wrapper\Jail;
use OC\Hooks\BasicEmitter;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -89,12 +89,15 @@ class Scanner extends BasicEmitter implements IScanner {
*/
protected $lockingProvider;

protected IDBConnection $connection;

public function __construct(\OC\Files\Storage\Storage $storage) {
$this->storage = $storage;
$this->storageId = $this->storage->getId();
$this->cache = $storage->getCache();
$this->cacheActive = !\OC::$server->getConfig()->getSystemValueBool('filesystem_cache_readonly', false);
$this->lockingProvider = \OC::$server->getLockingProvider();
$this->connection = \OC::$server->get(IDBConnection::class);
}

/**
Expand Down Expand Up @@ -430,7 +433,7 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
}

if ($this->useTransactions) {
\OC::$server->getDatabaseConnection()->beginTransaction();
$this->connection->beginTransaction();
}

$exceptionOccurred = false;
Expand Down Expand Up @@ -473,8 +476,8 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
// process is running in parallel
// log and ignore
if ($this->useTransactions) {
\OC::$server->getDatabaseConnection()->rollback();
\OC::$server->getDatabaseConnection()->beginTransaction();
$this->connection->rollback();
$this->connection->beginTransaction();
}
\OC::$server->get(LoggerInterface::class)->debug('Exception while scanning file "' . $child . '"', [
'app' => 'core',
Expand All @@ -483,7 +486,7 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
$exceptionOccurred = true;
} catch (\OCP\Lock\LockedException $e) {
if ($this->useTransactions) {
\OC::$server->getDatabaseConnection()->rollback();
$this->connection->rollback();
}
throw $e;
}
Expand All @@ -494,7 +497,7 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
$this->removeFromCache($child);
}
if ($this->useTransactions) {
\OC::$server->getDatabaseConnection()->commit();
$this->connection->commit();
}
if ($exceptionOccurred) {
// It might happen that the parallel scan process has already
Expand Down Expand Up @@ -558,7 +561,7 @@ public function backgroundScan() {
}
}

private function runBackgroundScanJob(callable $callback, $path) {
protected function runBackgroundScanJob(callable $callback, $path) {
try {
$callback();
\OC_Hook::emit('Scanner', 'correctFolderSize', ['path' => $path]);
Expand Down
81 changes: 0 additions & 81 deletions lib/private/Files/ObjectStore/NoopScanner.php

This file was deleted.

98 changes: 98 additions & 0 deletions lib/private/Files/ObjectStore/ObjectStoreScanner.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<?php
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
* @author Joas Schilling <coding@schilljs.com>
* @author Jörn Friedrich Dreyer <jfd@butonic.de>
* @author Morris Jobke <hey@morrisjobke.de>
* @author Robin Appelman <robin@icewind.nl>
* @author Roeland Jago Douma <roeland@famdouma.nl>
*
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OC\Files\ObjectStore;

use OC\Files\Cache\Scanner;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Files\FileInfo;

class ObjectStoreScanner extends Scanner {
public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) {
return [];
}

public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) {
return [];
}

protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderId = null, $lock = true, array $data = []) {
return 0;
}

public function backgroundScan() {
$lastPath = null;
// find any path marked as unscanned and run the scanner until no more paths are unscanned (or we get stuck)
// we sort by path DESC to ensure that contents of a folder are handled before the parent folder
while (($path = $this->getIncomplete()) !== false && $path !== $lastPath) {
$this->runBackgroundScanJob(function () use ($path) {
$item = $this->cache->get($path);
if ($item && $item->getMimeType() !== FileInfo::MIMETYPE_FOLDER) {
$fh = $this->storage->fopen($path, 'r');
if ($fh) {
$stat = fstat($fh);
if ($stat['size']) {
$this->cache->update($item->getId(), ['size' => $stat['size']]);
}
}
}
}, $path);
// FIXME: this won't proceed with the next item, needs revamping of getIncomplete()
// to make this possible
$lastPath = $path;
}
}

/**
* Unlike the default Cache::getIncomplete this one sorts by path.
*
* This is needed since self::backgroundScan doesn't fix child entries when running on a parent folder.
* By sorting by path we ensure that we encounter the child entries first.
*
* @return false|string
* @throws \OCP\DB\Exception
*/
private function getIncomplete() {
$query = $this->connection->getQueryBuilder();
$query->select('path')
->from('filecache')
->where($query->expr()->eq('storage', $query->createNamedParameter($this->cache->getNumericStorageId(), IQueryBuilder::PARAM_INT)))
->andWhere($query->expr()->lt('size', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT)))
->orderBy('path', 'DESC')
->setMaxResults(1);

$result = $query->executeQuery();
$path = $result->fetchOne();
$result->closeCursor();

if ($path === false) {
return false;
}

// Make sure Oracle does not continue with null for empty strings
return (string)$path;
}
}
4 changes: 2 additions & 2 deletions lib/private/Files/ObjectStore/ObjectStoreStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,14 @@ private function normalizePath($path) {
*
* @param string $path
* @param \OC\Files\Storage\Storage (optional) the storage to pass to the scanner
* @return \OC\Files\ObjectStore\NoopScanner
* @return \OC\Files\ObjectStore\ObjectStoreScanner
*/
public function getScanner($path = '', $storage = null) {
if (!$storage) {
$storage = $this;
}
if (!isset($this->scanner)) {
$this->scanner = new NoopScanner($storage);
$this->scanner = new ObjectStoreScanner($storage);
}
return $this->scanner;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,29 @@

namespace Test\Files\ObjectStore;

class NoopScannerTest extends \Test\TestCase {
/** @var \OC\Files\Storage\Storage $storage */
private $storage;
use OC\Files\Cache\Scanner;
use OC\Files\ObjectStore\ObjectStoreScanner;
use OC\Files\Storage\Temporary;
use OCP\Files\Cache\ICache;
use OCP\Files\Storage\IStorage;
use Test\TestCase;

/** @var \OC\Files\ObjectStore\NoopScanner $scanner */
private $scanner;
/**
* @group DB
*/
class ObjectStoreScannerTest extends TestCase {
private IStorage $storage;
private ICache $cache;
private ObjectStoreScanner $scanner;
private Scanner $realScanner;

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

$this->storage = new \OC\Files\Storage\Temporary([]);
$this->scanner = new \OC\Files\ObjectStore\NoopScanner($this->storage);
$this->storage = new Temporary([]);
$this->cache = $this->storage->getCache();
$this->scanner = new ObjectStoreScanner($this->storage);
$this->realScanner = new Scanner($this->storage);
}

public function testFile() {
Expand Down Expand Up @@ -61,17 +72,16 @@ public function testBackgroundScan() {
$this->storage->mkdir('folder2');
$this->storage->file_put_contents('folder2/bar.txt', 'foobar');

$this->assertEquals(
[],
$this->scanner->scan('', \OC\Files\Cache\Scanner::SCAN_SHALLOW),
'Asserting that no error occurred while scan(SCAN_SHALLOW)'
);
$this->realScanner->scan('');

$this->assertEquals(6, $this->cache->get('folder2')->getSize());

$this->cache->put('folder2', ['size' => -1]);

$this->assertEquals(-1, $this->cache->get('folder2')->getSize());

$this->scanner->backgroundScan();

$this->assertTrue(
true,
'Asserting that no error occurred while backgroundScan()'
);
$this->assertEquals(6, $this->cache->get('folder2')->getSize());
}
}

0 comments on commit 2ea41da

Please sign in to comment.