-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix (slow) move on same object bucket.
This commit fixes the issue #47856. When you upload a file into a group folder and when you use a single S3 bucket as primary storage, the final move operation hangs for a long time. In the background, Nextcloud initiates a copy-delete sequence from the bucket into the bucket, with causes a lot unnecessary overhead. Nextcloud thinks that the file must be imported to another storage and does not recognize that everything is done on the same object bucket. In that case, the import step can be completely skipped, which saves time, network bandwidth and reduces the load on the object storage. The behavior improves a lot with #46013. However, there are still some put messages that are being sent to the object storage when you use an object storage as primary storage and upload files into a group folder. Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: cfiehe <cfiehe@users.noreply.github.com>
- Loading branch information
1 parent
6a6910f
commit a025436
Showing
4 changed files
with
186 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
39 changes: 39 additions & 0 deletions
39
tests/lib/Files/ObjectStore/ObjectStoreStoragesDifferentBucketTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
<?php | ||
/** | ||
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors | ||
* SPDX-FileCopyrightText: 2016 ownCloud, Inc. | ||
* SPDX-License-Identifier: AGPL-3.0-or-later | ||
*/ | ||
|
||
namespace Test\Files\ObjectStore; | ||
|
||
use Test\Files\Storage\StoragesTest; | ||
|
||
/** | ||
* @group DB | ||
*/ | ||
class ObjectStoreStoragesSameBucketTest extends StoragesTest { | ||
/** | ||
* @var IObjectStore | ||
*/ | ||
private $objectStorage1; | ||
|
||
/** | ||
* @var IObjectStore | ||
*/ | ||
private $objectStorage2; | ||
|
||
protected function setUp(): void { | ||
parent::setUp(); | ||
|
||
$baseStorage1 = new Temporary(); | ||
$this->objectStorage1 = new StorageObjectStore($baseStorage1); | ||
$config['objectstore'] = $this->objectStorage1; | ||
$this->storage1 = new ObjectStoreStorageOverwrite($config); | ||
|
||
$baseStorage2 = new Temporary(); | ||
$this->objectStorage2 = new StorageObjectStore($baseStorage2); | ||
$config['objectstore'] = $this->objectStorage2; | ||
$this->storage2 = new ObjectStoreStorageOverwrite($config); | ||
} | ||
} |
31 changes: 31 additions & 0 deletions
31
tests/lib/Files/ObjectStore/ObjectStoreStoragesSameBucketTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<?php | ||
/** | ||
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors | ||
* SPDX-FileCopyrightText: 2016 ownCloud, Inc. | ||
* SPDX-License-Identifier: AGPL-3.0-or-later | ||
*/ | ||
|
||
namespace Test\Files\ObjectStore; | ||
|
||
use Test\Files\Storage\StoragesTest; | ||
|
||
/** | ||
* @group DB | ||
*/ | ||
class ObjectStoreStoragesSameBucketTest extends StoragesTest { | ||
/** | ||
* @var \OCP\Files\ObjectStore\IObjectStore | ||
*/ | ||
private $objectStorage; | ||
|
||
protected function setUp(): void { | ||
parent::setUp(); | ||
|
||
$baseStorage = new Temporary(); | ||
$this->objectStorage = new StorageObjectStore($baseStorage); | ||
$config['objectstore'] = $this->objectStorage; | ||
// storage1 and storage2 share the same object store. | ||
$this->storage1 = new ObjectStoreStorageOverwrite($config); | ||
$this->storage2 = new ObjectStoreStorageOverwrite($config); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
<?php | ||
/** | ||
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors | ||
* SPDX-FileCopyrightText: 2016 ownCloud, Inc. | ||
* SPDX-License-Identifier: AGPL-3.0-or-later | ||
*/ | ||
|
||
namespace Test\Files\Storage; | ||
|
||
use Test\TestCase; | ||
|
||
abstract class StoragesTest extends TestCase { | ||
/** | ||
* @var \OC\Files\Storage\Storage | ||
*/ | ||
protected $storage1; | ||
|
||
/** | ||
* @var \OC\Files\Storage\Storage | ||
*/ | ||
protected $storage2; | ||
|
||
protected function tearDown(): void { | ||
if (is_null($this->storage1) && is_null($this->storage2)) { | ||
return; | ||
} | ||
$this->storage1->getCache()->clear(); | ||
$this->storage2->getCache()->clear(); | ||
|
||
parent::tearDown(); | ||
} | ||
|
||
public function testMoveFileFromStorage() { | ||
$source = 'source.txt'; | ||
$target = 'target.txt'; | ||
$storage2->file_put_contents($source, 'foo'); | ||
|
||
$storage1->moveFromStorage($storage2, $source, $target); | ||
|
||
$this->assertTrue($storage1->file_exists($target), $target.' was not created'); | ||
$this->assertFalse($storage2->file_exists($source), $source.' still exists'); | ||
$this->assertEquals('foo', $storage1->file_get_contents($target)); | ||
} | ||
|
||
public function testMoveDirectoryFromStorage() { | ||
$storage2->mkdir('source'); | ||
$storage2->file_put_contents('source/test1.txt', 'foo'); | ||
$storage2->file_put_contents('source/test2.txt', 'qwerty'); | ||
$storage2->mkdir('source/subfolder'); | ||
$storage2->file_put_contents('source/subfolder/test.txt', 'bar'); | ||
|
||
$storage1->moveFromStorage($storage2, 'source', 'target'); | ||
|
||
$this->assertTrue($storage1->file_exists('target')); | ||
$this->assertTrue($storage1->file_exists('target/test1.txt')); | ||
$this->assertTrue($storage1->file_exists('target/test2.txt')); | ||
$this->assertTrue($storage1->file_exists('target/subfolder')); | ||
$this->assertTrue($storage1->file_exists('target/subfolder/test.txt')); | ||
|
||
$this->assertFalse($storage2->file_exists('source')); | ||
$this->assertFalse($storage2->file_exists('source/test1.txt')); | ||
$this->assertFalse($storage2->file_exists('source/test2.txt')); | ||
$this->assertFalse($storage2->file_exists('source/subfolder')); | ||
$this->assertFalse($storage2->file_exists('source/subfolder/test.txt')); | ||
|
||
$this->assertEquals('foo', $storage1->file_get_contents('target/test1.txt')); | ||
$this->assertEquals('qwerty', $storage1->file_get_contents('target/test2.txt')); | ||
$this->assertEquals('bar', $storage1->file_get_contents('target/subfolder/test.txt')); | ||
} | ||
|
||
/** | ||
* @dataProvider copyAndMoveProvider | ||
*/ | ||
public function testCopyFileFromStorage($source, $target) { | ||
$source = 'source.txt'; | ||
$target = 'target.txt'; | ||
$storage2->file_put_contents($source, 'foo'); | ||
|
||
$storage1->copyFromStorage($storage2, $source, $target); | ||
|
||
$this->assertTrue($storage1->file_exists($target), $target.' was not created'); | ||
$this->assertTrue($storage2->file_exists($source), $source.' was deleted'); | ||
$this->assertEquals('foo', $storage1->file_get_contents($target)); | ||
} | ||
|
||
public function testCopyDirectoryFromStorage() { | ||
$storage2->mkdir('source'); | ||
$storage2->file_put_contents('source/test1.txt', 'foo'); | ||
$storage2->file_put_contents('source/test2.txt', 'qwerty'); | ||
$storage2->mkdir('source/subfolder'); | ||
$storage2->file_put_contents('source/subfolder/test.txt', 'bar'); | ||
|
||
$storage1->copyFromStorage($storage2, 'source', 'target'); | ||
|
||
$this->assertTrue($storage1->file_exists('target')); | ||
$this->assertTrue($storage1->file_exists('target/test1.txt')); | ||
$this->assertTrue($storage1->file_exists('target/test2.txt')); | ||
$this->assertTrue($storage1->file_exists('target/subfolder')); | ||
$this->assertTrue($storage1->file_exists('target/subfolder/test.txt')); | ||
|
||
$this->assertTrue($storage2->file_exists('source')); | ||
$this->assertTrue($storage2->file_exists('source/test1.txt')); | ||
$this->assertTrue($storage2->file_exists('source/test2.txt')); | ||
$this->assertTrue($storage2->file_exists('source/subfolder')); | ||
$this->assertTrue($storage2->file_exists('source/subfolder/test.txt')); | ||
|
||
$this->assertEquals('foo', $storage1->file_get_contents('target/test1.txt')); | ||
$this->assertEquals('qwerty', $storage1->file_get_contents('target/test2.txt')); | ||
$this->assertEquals('bar', $storage1->file_get_contents('target/subfolder/test.txt')); | ||
} | ||
} |