ocis storage driver #559

butonic · 2020-03-13T20:55:35Z

A traditional filesystem uses paths to organize files in a tree. But path based references are not persistent because they change when a file is renamed or moved.
OCIS needs a persistent fileid to let other services reference files. Shares and favorites for example should not break when a file is moved or renamed.

A naive approach would use the fileid as the key and the path as the value for an efficient lookup. But this leads to n updates in the lookup table when a folder is renamed, because the path of all children changes.

Depending on the underlying storage a fast lookup by inode (btrfs, gpfs) or a truly unique fileid (eos) might be possible. For these a dedicated storage driver should be used. But keep in mind that inodes are reused, which might cause a purely inode based reference to point to a different file. Gpfs has an option to disable reusing inodes.

Ocisfs deconstructs a filesystem to provide a persistent fileid, an efficient file by ID lookup at the cost of more inodes.

/var/tmp/ocis/root/
├── nodes
│   ├── 1dd84abf-9466-4e14-bb86-02fc4ea3abcf
│   ├── 5425aaf2-8fa4-497e-b0e6-ed3f0809e350
│   │   └── bif -> ../d1bba60c-87a9-41a3-88a2-abd221abc100
│   ├── 8f16ec6c-6314-4f44-b553-d97abd464d06
│   │   ├── [MS-SFU].pdf -> ../b3951b72-cce9-4ab0-9992-f13b164ebb5b
│   │   └── foo -> ../9822b551-3c9a-4291-8a3a-4fccdbe8bcaa
│   ├── 9822b551-3c9a-4291-8a3a-4fccdbe8bcaa
│   │   └── bar -> ../5425aaf2-8fa4-497e-b0e6-ed3f0809e350
│   ├── b3951b72-cce9-4ab0-9992-f13b164ebb5b
│   └── d1bba60c-87a9-41a3-88a2-abd221abc100
├── trash
│   └── 1dd84abf-9466-4e14-bb86-02fc4ea3abcf -> ../nodes/1dd84abf-9466-4e14-bb86-02fc4ea3abcf
├── uploads
│   ├── 162c3dc6-b117-434d-b6fb-f1a350017d68.info
│   └── 322fa701-12ad-4a77-a165-28199a96aecc.info
├── users
│   └── 4c510ada-c86b-4815-8820-42cdf82c3d51 -> ../nodes/8f16ec6c-6314-4f44-b553-d97abd464d06
└── versions

Every file and folder is represented by an entry in the nodes folder. Directories have a children subfolder containing symlinks to child nodes. Files have a content file. A versions subfolder contains timestamp content entries.

While this node based layout may look confusing at first sight not everything is lost. The symlink pattern used in the children folder allow navigating the filesystem using the traditional tree hierarchy. In fact it is used to implement the path based lookup.

Why the content and children nodes? Why not use a uuid file or folder? Because we need a parent link? We can store that as an extended attribute instead of the parentname symlink.
We need the filename (changes on rename) and parentid (changes on move). This duplication might cause inconsistencies. Which could be mitigated using a journal with file operations.

The etag is an extended attribute anyway. Additional properties could be stored either in the fs as extended attributes or as an additional file in a root/properties/uuid Json or ini file. Might be an option

Versions could be stored in additional root/versions/uuid/timestamp files.

Basic file management is implemented, but this is missing:

Feedback welcome.

butonic · 2020-09-01T14:19:34Z

Move fails with

2020-09-01T16:17:44+02:00 INF unary code=OK end="01/Sep/2020:16:17:44 +0200" from=tcp://[::1]:37302 pkg=rgrpc service=reva start="01/Sep/2020:16:17:44 +0200" time_ns=4167182 traceid=eeac0c5e1646257d1cf24f3d4aaeab55 uri=/cs3.gateway.v1beta1.GatewayAPI/Stat user-agent=grpc-go/1.26.0
2020-09-01T16:17:44+02:00 DBG skipping auth method=/cs3.gateway.v1beta1.GatewayAPI/Move pkg=rgrpc service=reva traceid=eeac0c5e1646257d1cf24f3d4aaeab55
2020-09-01T16:17:44+02:00 INF unary code=OK end="01/Sep/2020:16:17:44 +0200" from=tcp://[::1]:37260 pkg=rgrpc service=reva start="01/Sep/2020:16:17:44 +0200" time_ns=362160 traceid=eeac0c5e1646257d1cf24f3d4aaeab55 uri=/cs3.storage.registry.v1beta1.RegistryAPI/GetStorageProvider user-agent=grpc-go/1.26.0
2020-09-01T16:17:44+02:00 INF unary code=OK end="01/Sep/2020:16:17:44 +0200" from=tcp://[::1]:37260 pkg=rgrpc service=reva start="01/Sep/2020:16:17:44 +0200" time_ns=285833 traceid=eeac0c5e1646257d1cf24f3d4aaeab55 uri=/cs3.storage.registry.v1beta1.RegistryAPI/GetStorageProvider user-agent=grpc-go/1.26.0
2020-09-01T16:17:44+02:00 ERR home/jfd/Repositories/reva/internal/grpc/services/storageprovider/storageprovider.go:439 > error moving file error="ocisfs: Move: could not propagate old node: ocisfs: Propagate: readlink error: ocisfs: invalid node info '&{ParentID:8ce51b56-4382-4838-8c36-0e340045a7a7 ID: Name:bam Exists:false}'" pkg=rgrpc service=reva traceid=eeac0c5e1646257d1cf24f3d4aaeab55
2020-09-01T16:17:44+02:00 INF unary code=OK end="01/Sep/2020:16:17:44 +0200" from=tcp://[::1]:33198 pkg=rgrpc service=reva start="01/Sep/2020:16:17:44 +0200" time_ns=1361495 traceid=eeac0c5e1646257d1cf24f3d4aaeab55 uri=/cs3.storage.provider.v1beta1.ProviderAPI/Move user-agent=grpc-go/1.26.0
2020-09-01T16:17:44+02:00 INF unary code=OK end="01/Sep/2020:16:17:44 +0200" from=tcp://[::1]:37302 pkg=rgrpc service=reva start="01/Sep/2020:16:17:44 +0200" time_ns=5021947 traceid=eeac0c5e1646257d1cf24f3d4aaeab55 uri=/cs3.gateway.v1beta1.GatewayAPI/Move user-agent=grpc-go/1.26.0
2020-09-01T16:17:44+02:00 ERR error handling move request move_request=CODE_INTERNAL pkg=rhttp service=reva traceid=eeac0c5e1646257d1cf24f3d4aaeab55
2020-09-01T16:17:44+02:00 ERR http end="01/Sep/2020:16:17:44 +0200" host=::1 method=MOVE pkg=rhttp proto=HTTP/1.1 service=reva size=0 start="01/Sep/2020:16:17:44 +0200" status=500 time_ns=23075986 traceid=eeac0c5e1646257d1cf24f3d4aaeab55 uri=/remote.php/webdav/bam url=/remote.php/webdav/bam

PVince81

See comments, many already discussed partly

pkg/storage/fs/ocis/node.go

pkg/storage/fs/ocis/ocis.go

pkg/storage/fs/ocis/tree.go

PVince81 · 2020-09-01T15:21:39Z

pkg/storage/fs/ocis/tree.go

+func (fs *Tree) Propagate(ctx context.Context, node *NodeInfo) (err error) {
+	// generate an etag
+	bytes := make([]byte, 16)
+	if _, err := rand.Read(bytes); err != nil {


I seem to remember you saying that we generate the etag based on mtime ? or was it just an idea ?

an idea ... for migrations / backup we need to be able to store an etag. the owncloud driver already has some logic for that.

PVince81 · 2020-09-01T15:21:52Z

pkg/storage/fs/ocis/tree.go

+
+		node.BecomeParent()
+	}
+	return


seems we don't have mtime propagation here

PVince81 · 2020-09-01T15:25:39Z

pkg/storage/fs/ocis/upload.go

+
+var defaultFilePerm = os.FileMode(0664)
+
+// TODO deprecated ... use tus


separate ticket or checkbox ?

pkg/storage/fs/ocis/upload.go

PVince81

See comments for latest additions

pkg/storage/fs/ocis/grants.go

pkg/storage/fs/ocis/tree.go

PVince81 · 2020-09-07T10:21:54Z

pkg/storage/fs/ocis/upload.go

+
+	_, err = io.Copy(tmp, r)
+	if err != nil {
+		return errors.Wrap(err, "ocisfs: error writing to tmp file "+tmp.Name())


delete temp file on error or does this happen automatically ? how often ?

hm ... the put is deprecated ... but yeah we should delete the file in this case

PVince81 · 2020-09-07T10:24:28Z

pkg/storage/fs/ocis/upload.go

+		// versions are stored alongside the actual file, so a rename can be efficient and does not cross storage / partition boundaries
+		versionsPath := filepath.Join(upload.fs.conf.Root, "nodes", n.ID+".REV."+fi.ModTime().UTC().Format(time.RFC3339Nano))
+
+		if err := os.Rename(targetPath, versionsPath); err != nil {


a copy might be better in case the below rename fails, in which case the file would disappear completely from view

PVince81 · 2020-09-07T10:25:40Z

pkg/storage/fs/ocis/upload.go

+		return err
+	}
+
+	if err := xattr.Set(targetPath, "user.ocis.parentid", []byte(n.ParentID)); err != nil {


wondering if we're going too low-level here and whether the upload code should talk to a higher level API, one that simply configures the node correctly, with xattrs and children.

PVince81 · 2020-09-08T13:24:11Z

let's get this merged, since the main file operations work

follow up for the remaining tasks:

sharing: Store share information in oCIS storage owncloud/product#145
trashbin implementation: Implement Trashbin Feature for ocis storage owncloud/product#209
versions implementation: Implement Versions Feature for ocis storage owncloud/product#210
extended attributes / arbitrary properties: Ability to store arbitrary properties in ocis storage owncloud/product#211
bring ocis storage to CI: CI for ocis storage owncloud/product#212
documentation: Document ocis storage owncloud/product#213

PVince81

Looks good, as discussed 👍

PVince81 · 2020-09-08T14:29:31Z

weird results:

runsh: Total unexpected failed scenarios throughout the test run:
apiTrashbin/trashbinFilesFolders.feature:185
apiTrashbin/trashbinFilesFolders.feature:186
apiTrashbin/trashbinFilesFolders.feature:203
apiTrashbin/trashbinFilesFolders.feature:204
apiTrashbin/trashbinFilesFolders.feature:227
apiTrashbin/trashbinFilesFolders.feature:228
apiTrashbin/trashbinFilesFolders.feature:241
apiTrashbin/trashbinFilesFolders.feature:242
apiTrashbin/trashbinFilesFolders.feature:255
apiTrashbin/trashbinFilesFolders.feature:256
runsh: Total unexpected passed scenarios throughout the test run:
apiTrashbin/trashbinFilesFolders.feature:217
apiTrashbin/trashbinFilesFolders.feature:218
apiTrashbin/trashbinFilesFolders.feature:235
apiTrashbin/trashbinFilesFolders.feature:236
apiTrashbin/trashbinFilesFolders.feature:259
apiTrashbin/trashbinFilesFolders.feature:260
apiTrashbin/trashbinFilesFolders.feature:273
apiTrashbin/trashbinFilesFolders.feature:274
apiTrashbin/trashbinFilesFolders.feature:287
apiTrashbin/trashbinFilesFolders.feature:288

so this broke something related to trashbin on OC storage but also fixed other things ?!

PVince81 · 2020-09-08T14:37:42Z

@butonic can you add instructions in the top POST about how to test it ? it is not clear to me what env variables to switch to get the proper working setup

PVince81 · 2020-09-08T14:42:09Z

we might want for #1148 to be merged first

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

phil-davis · 2020-09-08T14:44:40Z

so this broke something related to trashbin on OC storage but also fixed other things ?!

I guess that it needs a rebase. The core commit id to use for tests is embedded in .drone.star - so it will be using whatever is the core commit id in .drone.star in this branch, but the expected-failures list will be as it is in master.

PVince81 · 2020-09-08T14:51:02Z

the failing tests are related to accessing another user's trashbin.
instead of getting a 401 we receive an empty XML response.
this is reproducible locally

  Scenario: Listing other user's trashbin is prohibited                                    # /srv/www/htdocs/owncloud/tests/acceptance/features/apiTrashbin/trashbinFilesFolders.feature:173
    Given using old DAV path                                                               # FeatureContext::usingOldOrNewDavPath()
    And user "testtrashbin100" has been created with default attributes and skeleton files # FeatureContext::userHasBeenCreatedWithDefaultAttributes()
    And user "Brian" has been created with default attributes and without skeleton files   # FeatureContext::userHasBeenCreatedWithDefaultAttributesAndWithoutSkeletonFiles()
    And user "testtrashbin100" has deleted file "/textfile1.txt"                           # FeatureContext::userHasDeletedFile()
    When user "Brian" tries to list the trashbin content for user "testtrashbin100"        # TrashbinContext::userTriesToListTheTrashbinContentForUser()
      Received empty response where XML was expected (Exception)
    Then the HTTP status code should be "401"                                              # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And the last webdav response should not contain the following elements                 # TrashbinContext::theLastWebdavResponseShouldNotContainFollowingElements()
      | path          | user            |
      | textfile1.txt | testtrashbin100 |

there is a slight chance that the permission refactoring that happened affects this

PVince81 · 2020-09-08T15:01:26Z

more bad luck striking:

0. init.................. FAIL (connection refused by `revad-services' port 20080: Connection refused)

PVince81 · 2020-09-08T15:11:18Z

okay, so now all tests passed (except litmus hiccup)... so the rebase worked...

phil-davis · 2020-09-08T15:15:36Z

It would be nice to have #1148 merged - that will get the core API test suite having some more accept share scenarios, which are running in the ocis and ocis-reva repos today. I am not sure who is able to review-approve-merge that. Then a rebase here and it should be good.

butonic · 2020-09-11T08:54:46Z

we should close this in favor of #1155

exalate-issue-sync bot mentioned this pull request Sep 1, 2020

Test Etag propagation for oCIS storage owncloud/product#144

Closed

17 tasks

butonic force-pushed the ocis-driver branch 2 times, most recently from 8f73e01 to 817e639 Compare September 1, 2020 12:56

PVince81 reviewed Sep 1, 2020

View reviewed changes

PVince81 mentioned this pull request Sep 2, 2020

Prototyping for oCIS Storage Driver owncloud/product#119

Closed

2 tasks

butonic changed the title ~~[WIP] local ocis driver~~ [WIP] ocis storage driver Sep 7, 2020

PVince81 reviewed Sep 7, 2020

View reviewed changes

butonic force-pushed the ocis-driver branch from e420003 to da574ca Compare September 8, 2020 12:41

exalate-issue-sync bot mentioned this pull request Sep 8, 2020

Document ocis storage owncloud/product#213

Open

butonic force-pushed the ocis-driver branch from c86f9b4 to a2b8465 Compare September 8, 2020 13:28

butonic marked this pull request as ready for review September 8, 2020 13:28

butonic requested a review from labkode as a code owner September 8, 2020 13:28

butonic requested a review from PVince81 September 8, 2020 13:28

PVince81 previously approved these changes Sep 8, 2020

View reviewed changes

butonic changed the title ~~[WIP] ocis storage driver~~ ocis storage driver Sep 8, 2020

butonic added 10 commits September 8, 2020 16:44

initial version

c822662

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

introduce NodeInfo

4ebe568

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

fix metadata normalization

60335b0

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

stick to github.com/google/uuid

4942d3a

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

simpler layout, parentid and name as ext attr

4ff3ecf

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

manually adding mimetypes is no longer needed

ef23de5

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

rename Wrap to NodeFrom*

c188f79

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

refactor node implementation

c395440

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

fix delete and overwrite

b5e9dcf

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

initial sharing, store owner as ext attr, fix NewNode

745d4bc

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

butonic added 8 commits September 8, 2020 16:44

separate files for metadada, revisions and recycle

5710738

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

list revisions

4923121

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

refactor and use ACE package

aff7ef5

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

refactor node api

705e7fd

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

add license headers

0412aa7

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

add changelog

50b66e1

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

remove duplicate docs (moved to ace.go)

e0587cc

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

make linter happy

c3dd7a3

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

butonic dismissed PVince81’s stale review via c3dd7a3 September 8, 2020 14:44

butonic force-pushed the ocis-driver branch from a2b8465 to c3dd7a3 Compare September 8, 2020 14:44

phil-davis mentioned this pull request Sep 8, 2020

[Tests-Only] Bump core commit #1148

Merged

butonic mentioned this pull request Sep 9, 2020

ocis driver: access shares, trash, revisions, metadata #1155

Merged

PVince81 mentioned this pull request Sep 10, 2020

Reva ocis storage owncloud/ocis-reva#466

Merged

3 tasks

butonic closed this Sep 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ocis storage driver #559

ocis storage driver #559

butonic commented Mar 13, 2020 •

edited

Loading

butonic commented Sep 1, 2020

PVince81 left a comment

PVince81 Sep 1, 2020

butonic Sep 7, 2020

PVince81 Sep 1, 2020

PVince81 Sep 1, 2020

PVince81 left a comment

PVince81 Sep 7, 2020

butonic Sep 7, 2020

PVince81 Sep 7, 2020

PVince81 Sep 7, 2020

PVince81 commented Sep 8, 2020 •

edited

Loading

PVince81 left a comment

PVince81 commented Sep 8, 2020

PVince81 commented Sep 8, 2020

PVince81 commented Sep 8, 2020

phil-davis commented Sep 8, 2020

PVince81 commented Sep 8, 2020

PVince81 commented Sep 8, 2020

PVince81 commented Sep 8, 2020 •

edited

Loading

phil-davis commented Sep 8, 2020

butonic commented Sep 11, 2020 •

edited

Loading


		var defaultFilePerm = os.FileMode(0664)

		// TODO deprecated ... use tus

ocis storage driver #559

ocis storage driver #559

Conversation

butonic commented Mar 13, 2020 • edited Loading

butonic commented Sep 1, 2020

PVince81 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PVince81 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PVince81 commented Sep 8, 2020 • edited Loading

PVince81 left a comment

Choose a reason for hiding this comment

PVince81 commented Sep 8, 2020

PVince81 commented Sep 8, 2020

PVince81 commented Sep 8, 2020

phil-davis commented Sep 8, 2020

PVince81 commented Sep 8, 2020

PVince81 commented Sep 8, 2020

PVince81 commented Sep 8, 2020 • edited Loading

phil-davis commented Sep 8, 2020

butonic commented Sep 11, 2020 • edited Loading

butonic commented Mar 13, 2020 •

edited

Loading

PVince81 commented Sep 8, 2020 •

edited

Loading

PVince81 commented Sep 8, 2020 •

edited

Loading

butonic commented Sep 11, 2020 •

edited

Loading