Added object cache for Swift ext storage #7897

PVince81 · 2014-03-26T16:19:58Z

This avoid calling the remote API for repeated calls to is_dir,
filetype, etc

I haven't fully tested this yet, but it does seem to be a bit faster.

I'm starting to think we might want to provide a generic cache for remote storages, because the Amazon implementation would also need it. The Dropbox one also has something similar.

@icewind1991

karlitschek · 2014-03-26T17:55:36Z

looks good but not tested 👍

icewind1991 · 2014-03-27T00:24:06Z

apps/files_external/lib/swift.php

 	 * @param string $path
+	 * @return \OpenCloud\OpenStack\ObjectStorage\Resource\DataObject


Does this return the fetched object or a bool?

If the object exists, it returns it.
If the object does not exist, it returns false, which is also cached. The point being that we don't want to check X times whether a file doesn't exist when we already knew it doesn't. (check is_dir/filetype, that one is nasty)

I'll add the bool in the doc

leonardofln · 2014-03-27T01:48:18Z

This improvement was excellent, greatly reduced the cURL requests and response time of the application. Good job!

PVince81 · 2014-03-27T10:27:41Z

I've updated it. For some parts, objects need to be removed from the cache to force refetch/repopulate.

Please review/test.

PVince81 · 2014-03-27T11:39:30Z

The branch should be ready now (rebased/squased). I've done some testing and fixed the remaining issue with caching.

PVince81 · 2014-03-28T10:15:53Z

Rebased.
I've tested most file operations including sync client, all should work correctly.

Please review @icewind1991 @schiesbn

icewind1991 · 2014-03-28T12:02:17Z

Changes look good 👍

DeepDiver1975 · 2014-03-28T12:17:15Z

testing ....

DeepDiver1975 · 2014-03-28T12:40:49Z

@PVince81 testStat failed:

PHPUnit_Framework_ExpectationFailedException : Failed asserting that 100 matches expected 1396009775.0.
Expected :1396009775
Actual   :100

DeepDiver1975 · 2014-03-28T12:43:29Z

debugging ....

DeepDiver1975 · 2014-03-28T13:15:38Z

the touch operation succeeds, but when retrieving the mtime the value is not as expected

PVince81 · 2014-03-28T14:17:23Z

Ok, I'll look into this.

PVince81 · 2014-03-28T14:17:56Z

I'll also check master, because the touch operation was fixed in a separate PR.

PVince81 · 2014-03-28T14:52:14Z

@DeepDiver1975 I ran the unit tests and I'm getting something different on this branch:

1) Test\Files\Storage\Swift::testStat
Failed asserting that false is true.

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

2) Test\Files\Storage\Swift::testUnlink
Failed asserting that true is false.

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

Not sure if it's due to connection speed being different from yours.
I'll try and find out what's going on.

PVince81 · 2014-03-28T15:39:18Z

@DeepDiver1975 I have the feeling that OpenStack doesn't automatically update the mtime of its root. Also even stranger it always returns the same timestamp, even when I delete the bucket and re-create it. It seems to be the bucket's creation time, seen this with a newly created bucket.

Do you get the same results on master ? If yes I suggest to merge this first and I'll have a look at that issue separately (with potential backport). Maybe we need to "touch" the root as well (this adds our custom mtime attribute) to manually propagate the change.

PVince81 · 2014-03-28T15:47:58Z

I also realized that mtime won't propagate to parent folders either since OpenStack is a flat object hierarchy. To fix this we'd need to touch (update the mtime) every parent folder up to the root...
I'll investigate this separately.

DeepDiver1975 · 2014-04-01T20:58:51Z

@PVince81 I'll test this and master tomorrow - I've been asking myself if we should put each external fs into it's own repo and prepare travis to execute the tests on - as far as possible - locally installer services (e.g. Ceph as S3 backend etc)

PVince81 · 2014-04-01T21:01:04Z

Thanks. I'd like to get this merged first, then possibly look into making the mtime propagation and update detection work.

What do you mean with own repo ? Having each external storage as separate GitHub repo app ?

DeepDiver1975 · 2014-04-01T21:10:34Z

Having each external storage as separate GitHub repo app ?

Yes - divide and conquer!

PVince81 · 2014-04-01T21:13:25Z

That would be quite a lot of backends. Might make sense for some of them, or at least the "exotic" ones like object store. The FTP, SFTP ones could stay here. Or group them somehow.

Changes will be needed in the app to make it possible for external apps to register their backends, which makes sense as this is currently not supported and makes it impossible for 3rd party to provide their own ext storages.

bantu · 2014-04-02T02:14:40Z

apps/files_external/lib/swift.php

@@ -53,6 +53,12 @@ class Swift extends \OC\Files\Storage\Common {
 	private static $tmpFiles = array();

 	/**
+	 * Object cache, map of path to object


This description does not correctly consider https://github.com/owncloud/core/pull/7897/files#diff-2ab592dfd95e06115a358d1b5b20cdc5R86

RobinMcCorkell · 2015-09-21T09:20:36Z

It seems that Flysystem performs no caching at all... but if we were to have a single storage API provided by Flysystem, implementing generic caching would be a possibility

DeepDiver1975 · 2015-09-21T09:23:27Z

It seems that Flysystem performs no caching at all...

aha .... http://flysystem.thephpleague.com/caching/

RobinMcCorkell · 2015-09-21T09:28:05Z

@DeepDiver1975 Or that 🙈

PVince81 · 2015-10-29T10:38:56Z

Rebased.

Has failing tests:

2) Test\Files\Storage\Swift::testRenameDirectory
Failed asserting that false is true.

/var/www/html/owncloud/tests/lib/files/storage/storage.php:468

3) Test\Files\Storage\Swift::testCopyDirectory
Failed asserting that false is true.

/var/www/html/owncloud/tests/lib/files/storage/storage.php:522

PVince81 · 2016-05-20T14:13:55Z

Rebased and fixed unit tests.
Also now using the CappedCache for the stat cache.

Please review @owncloud/filesystem
@owncloud/qa can you help regression test this ? All file operations should still work, this makes SWIFT external storage much faster.

PVince81 · 2016-05-23T12:40:42Z

also @tjdett @MorrisJobke in case you are interested

icewind1991 · 2016-05-24T10:53:41Z

👍

This avoid calling the remote API for repeated calls to is_dir, filetype, etc

PVince81 · 2016-05-24T11:30:57Z

Rebased due to the PSR-4 changes. (no conflicts, automatically resolved)

PVince81 · 2016-05-24T14:21:25Z

Ran the Swift tests locally and they passed, merging.

lock · 2019-08-05T18:02:46Z

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.

PVince81 added Performance labels Mar 26, 2014

This was referenced Mar 26, 2014

Openstack Swift Errors #7191

Closed

Slowness in owncloud (storage in swift) #7882

Closed

icewind1991 reviewed Mar 27, 2014
View reviewed changes

PVince81 mentioned this pull request Mar 27, 2014

Improve ext storage performance with stat cache [$5] #7910

Closed

8 tasks

PVince81 mentioned this pull request Mar 27, 2014

problem retrieving object with swift backend #7667

Closed

bantu reviewed Apr 2, 2014
View reviewed changes

PVince81 removed this from the 8.2-current milestone Sep 21, 2015

PVince81 force-pushed the ext-swiftcaching branch from 101312b to ecba66e Compare October 29, 2015 10:36

PVince81 mentioned this pull request Nov 16, 2015

External Storage: Google Drive: 403 User Rate Limit Exceeded #20481

Closed

PVince81 added 2 - Developing and removed 3 - To Review labels Nov 16, 2015

DanielTosello mentioned this pull request Jan 7, 2016

Streaming download from Swift in files_external #19002

Merged

MorrisJobke modified the milestones: 9.1-next, 9.0-current Mar 4, 2016

PVince81 self-assigned this Mar 31, 2016

PVince81 force-pushed the ext-swiftcaching branch from ecba66e to 39046a8 Compare May 20, 2016 14:12

PVince81 added 3 - To Review and removed 2 - Developing labels May 20, 2016

Added object cache for Swift ext storage

f77a11a

This avoid calling the remote API for repeated calls to is_dir, filetype, etc

PVince81 force-pushed the ext-swiftcaching branch from 39046a8 to f77a11a Compare May 24, 2016 11:30

PVince81 added 4 - To release and removed 3 - To Review labels May 24, 2016

PVince81 merged commit e8c3794 into master May 24, 2016

PVince81 deleted the ext-swiftcaching branch May 24, 2016 14:21

PVince81 mentioned this pull request Apr 13, 2017

Openstack SWIFT storage performance #25947

Closed

lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added object cache for Swift ext storage #7897

Added object cache for Swift ext storage #7897

PVince81 commented Mar 26, 2014

karlitschek commented Mar 26, 2014

icewind1991 Mar 27, 2014

PVince81 Mar 27, 2014

PVince81 Mar 27, 2014

leonardofln commented Mar 27, 2014

PVince81 commented Mar 27, 2014

PVince81 commented Mar 27, 2014

PVince81 commented Mar 28, 2014

icewind1991 commented Mar 28, 2014

DeepDiver1975 commented Mar 28, 2014

DeepDiver1975 commented Mar 28, 2014

DeepDiver1975 commented Mar 28, 2014

DeepDiver1975 commented Mar 28, 2014

PVince81 commented Mar 28, 2014

PVince81 commented Mar 28, 2014

PVince81 commented Mar 28, 2014

PVince81 commented Mar 28, 2014

PVince81 commented Mar 28, 2014

DeepDiver1975 commented Apr 1, 2014

PVince81 commented Apr 1, 2014

DeepDiver1975 commented Apr 1, 2014

PVince81 commented Apr 1, 2014

bantu Apr 2, 2014

RobinMcCorkell commented Sep 21, 2015

DeepDiver1975 commented Sep 21, 2015

RobinMcCorkell commented Sep 21, 2015

PVince81 commented Oct 29, 2015

PVince81 commented May 20, 2016

PVince81 commented May 23, 2016

icewind1991 commented May 24, 2016

PVince81 commented May 24, 2016

PVince81 commented May 24, 2016

lock bot commented Aug 5, 2019

		* @param string $path
		* @return \OpenCloud\OpenStack\ObjectStorage\Resource\DataObject

Added object cache for Swift ext storage #7897

Added object cache for Swift ext storage #7897

Conversation

PVince81 commented Mar 26, 2014

karlitschek commented Mar 26, 2014

icewind1991 Mar 27, 2014

Choose a reason for hiding this comment

PVince81 Mar 27, 2014

Choose a reason for hiding this comment

PVince81 Mar 27, 2014

Choose a reason for hiding this comment

leonardofln commented Mar 27, 2014

PVince81 commented Mar 27, 2014

PVince81 commented Mar 27, 2014

PVince81 commented Mar 28, 2014

icewind1991 commented Mar 28, 2014

DeepDiver1975 commented Mar 28, 2014

DeepDiver1975 commented Mar 28, 2014

DeepDiver1975 commented Mar 28, 2014

DeepDiver1975 commented Mar 28, 2014

PVince81 commented Mar 28, 2014

PVince81 commented Mar 28, 2014

PVince81 commented Mar 28, 2014

PVince81 commented Mar 28, 2014

PVince81 commented Mar 28, 2014

DeepDiver1975 commented Apr 1, 2014

PVince81 commented Apr 1, 2014

DeepDiver1975 commented Apr 1, 2014

PVince81 commented Apr 1, 2014

bantu Apr 2, 2014

Choose a reason for hiding this comment

RobinMcCorkell commented Sep 21, 2015

DeepDiver1975 commented Sep 21, 2015

RobinMcCorkell commented Sep 21, 2015

PVince81 commented Oct 29, 2015

PVince81 commented May 20, 2016

PVince81 commented May 23, 2016

icewind1991 commented May 24, 2016

PVince81 commented May 24, 2016

PVince81 commented May 24, 2016

lock bot commented Aug 5, 2019