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

Handling files which require Swift large object storage #24606

Closed
wants to merge 2 commits into from

Conversation

PVince81
Copy link
Contributor

Thanks @tjdett for this commit, from #19004

At a particular threshold, php-opencloud DLO support is used to upload
the object in segments.

Large object files are detected when copying, renaming and deleting.
Deletions remove any segments that follow the naming convention used by
php-opencloud.

For now, copy/rename simply fails for large files. The best solution for
implementing it involves upgrading php-opencloud.

@PVince81 PVince81 added this to the 9.1-current milestone May 12, 2016
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @icewind1991, @butonic, @berendt and @Xenopathic to be potential reviewers

@PVince81
Copy link
Contributor Author

CLA here #19004 (comment)

@PVince81
Copy link
Contributor Author

@rullzer @butonic can we get this in ?

@rullzer
Copy link
Contributor

rullzer commented May 24, 2016

It works with my local setup. But I'm not that familiar with SWIFT....

@PVince81
Copy link
Contributor Author

I don't know enough about Swift to be able to make a decision here.
@butonic can you help ?

@butonic
Copy link
Member

butonic commented May 31, 2016

👍 and should be added to https://github.com/owncloud/core/blob/master/lib/private/Files/ObjectStore/Swift.php#L111 as well. in a separate PR

@PVince81
Copy link
Contributor Author

Hmmmshit

S...................FFFFFFFF........FFFFFFFF..F..F.F.....FFFF.F.. 65 / 80 ( 81%)
...............                                                   80 / 80 (100%)

Time: 39.83 seconds, Memory: 38.50Mb

There were 24 failures:

1) OCA\Files_External\Tests\Storage\SwiftTest::testMove with data set #0 ('/source.txt', '/target.txt')
/source.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:222

2) OCA\Files_External\Tests\Storage\SwiftTest::testMove with data set #1 ('/source.txt', '/target with space.txt')
/source.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:222

3) OCA\Files_External\Tests\Storage\SwiftTest::testMove with data set #2 ('/source with space.txt', '/target.txt')
/source with space.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:222

4) OCA\Files_External\Tests\Storage\SwiftTest::testMove with data set #3 ('/source with space.txt', '/target with space.txt')
/source with space.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:222

5) OCA\Files_External\Tests\Storage\SwiftTest::testMove with data set #4 ('/source.txt', '/tärgét.txt')
/source.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:222

6) OCA\Files_External\Tests\Storage\SwiftTest::testMove with data set #5 ('/sòurcē.txt', '/target.txt')
/sòurcē.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:222

7) OCA\Files_External\Tests\Storage\SwiftTest::testMove with data set #6 ('/sòurcē.txt', '/tärgét.txt')
/sòurcē.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:222

8) OCA\Files_External\Tests\Storage\SwiftTest::testMove with data set #7 ('/single ' quote.txt', '/tar'get.txt')
/single ' quote.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:222

9) OCA\Files_External\Tests\Storage\SwiftTest::testMoveOverwrite with data set #0 ('/source.txt', '/target.txt')
/source.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:249

10) OCA\Files_External\Tests\Storage\SwiftTest::testMoveOverwrite with data set #1 ('/source.txt', '/target with space.txt')
/source.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:249

11) OCA\Files_External\Tests\Storage\SwiftTest::testMoveOverwrite with data set #2 ('/source with space.txt', '/target.txt')
/source with space.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:249

12) OCA\Files_External\Tests\Storage\SwiftTest::testMoveOverwrite with data set #3 ('/source with space.txt', '/target with space.txt')
/source with space.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:249

13) OCA\Files_External\Tests\Storage\SwiftTest::testMoveOverwrite with data set #4 ('/source.txt', '/tärgét.txt')
/source.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:249

14) OCA\Files_External\Tests\Storage\SwiftTest::testMoveOverwrite with data set #5 ('/sòurcē.txt', '/target.txt')
/sòurcē.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:249

15) OCA\Files_External\Tests\Storage\SwiftTest::testMoveOverwrite with data set #6 ('/sòurcē.txt', '/tärgét.txt')
/sòurcē.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:249

16) OCA\Files_External\Tests\Storage\SwiftTest::testMoveOverwrite with data set #7 ('/single ' quote.txt', '/tar'get.txt')
/single ' quote.txt still exists
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:249

17) OCA\Files_External\Tests\Storage\SwiftTest::testUnlink
Failed asserting that false is true.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:333

18) OCA\Files_External\Tests\Storage\SwiftTest::testRecursiveRmdir
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:374

19) OCA\Files_External\Tests\Storage\SwiftTest::testRecursiveUnlink
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:394

20) OCA\Files_External\Tests\Storage\SwiftTest::testRenameOverWriteFile
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:448

21) OCA\Files_External\Tests\Storage\SwiftTest::testRenameDirectory
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:460

22) OCA\Files_External\Tests\Storage\SwiftTest::testRenameOverWriteDirectory
source/test1.txt has not been removed
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:487

23) OCA\Files_External\Tests\Storage\SwiftTest::testRenameOverWriteDirectoryOverFile
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:501

24) OCA\Files_External\Tests\Storage\SwiftTest::testCopyOverWriteDirectory
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:540

@PVince81 PVince81 self-assigned this May 31, 2016
@PVince81
Copy link
Contributor Author

Rebased.

Had to fix the unlink method to use the "old" method when there's 0 file segments and only use bulkDelete when there are more, as inspired from the other ones.

This fixes the SWIFT tests:

$objects = $this->getContainer()->objectList(array(
'path' => $defaultSegmentPath
));
$objectPaths = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return array_map(function($object){
    return $object->getName();
}, $objects);

😄

@icewind1991
Copy link
Contributor

Needs unit tests (is there a way to force it to use chunking for smaller files?)

Also I would expect custom logic for reading files.

$this->objectCache->remove($path2 . '/');
} else {
// Large object file
// TODO: Implement after PHP OpenCloud is upgraded
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo it would make more sense to update opencloud before we merge this

merging a half working solution can only lead to unexpected behaviour

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, indeed I also have some concerns about this.

The problem is that he opencloud lib we need for large object storage only supports PHP > 5.4 so we can't update. (from what I heard)

@PVince81
Copy link
Contributor Author

Needs unit tests (is there a way to force it to use chunking for smaller files?)

Thought about this a bit. It would mean we'd need a test that uploads a virtual 4-5 Gb file, maybe generated from random data as we don't want to check in such file in git.

Due to the many concerns voiced here I'll move this to 9.2 as it seems this isn't ready.

@PVince81 PVince81 modified the milestones: 9.2-next, 9.1-current May 31, 2016
@PVince81
Copy link
Contributor Author

(and with a bit of luck maybe we can drop support of PHP 5.4 in 9.2 😉)

@PVince81
Copy link
Contributor Author

9.2 will only support PHP >= 5.6, so there's a chance to update the lib maybe ?

@PVince81
Copy link
Contributor Author

update php-opencloud #26111

tjdett and others added 2 commits September 29, 2016 16:14
At a particular threshold, php-opencloud DLO support is used to upload
the object in segments.

Large object files are detected when copying, renaming and deleting.
Deletions remove any segments that follow the naming convention used by
php-opencloud.

For now, copy/rename simply fails for large files. The best solution for
implementing it involves upgrading php-opencloud.
@PVince81 PVince81 modified the milestones: backlog, 10.0 Apr 6, 2017
@PVince81
Copy link
Contributor Author

PVince81 commented Jul 4, 2017

to be reconsidered in separate SWIFT app

cc @DeepDiver1975

@PVince81 PVince81 closed this Jul 4, 2017
@PVince81 PVince81 deleted the swift-largeobjectsupport branch July 4, 2017 14:37
@lock
Copy link

lock bot commented Aug 3, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants