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

Separate blobs from metadata in the ocis driver #1452

Merged
merged 28 commits into from
Mar 10, 2021

Conversation

aduffeck
Copy link
Contributor

@aduffeck aduffeck commented Feb 4, 2021

This pr makes the ocis driver store the actual file content separately from the metadata, allowing for using a different (potentially faster) storage for the metadata.

Common code between the ocis and s3ng drivers is extracted into a decomposedfs package which leaves them with just a thin layer for the different blobstore implementations.

@update-docs
Copy link

update-docs bot commented Feb 4, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@aduffeck aduffeck force-pushed the streamline-s3ng-and-ocis branch 7 times, most recently from 4c17963 to 9f9b1b1 Compare February 10, 2021 13:18
Copy link
Member

@refs refs left a comment

Choose a reason for hiding this comment

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

some minimal preliminary comments. It makes sense to add another abstraction between the filesystem and a blob storage. I will try it locally later today and continue reviewing behavior on the meantime. 🚀 awesome awesome work

pkg/storage/fs/ocis/blobstore/blobstore.go Outdated Show resolved Hide resolved
pkg/storage/utils/decomposedfs/decomposed.go Outdated Show resolved Hide resolved
pkg/storage/utils/decomposedfs/decomposed.go Outdated Show resolved Hide resolved
}

// NewDefault returns an instance with default components
func NewDefault(m map[string]interface{}, bs tree.Blobstore) (storage.FS, error) {

This comment was marked as resolved.

pkg/storage/utils/decomposedfs/decomposed.go Outdated Show resolved Hide resolved
@aduffeck aduffeck force-pushed the streamline-s3ng-and-ocis branch 2 times, most recently from 10d0512 to 0815066 Compare March 1, 2021 08:30
@aduffeck aduffeck force-pushed the streamline-s3ng-and-ocis branch 4 times, most recently from c25b29e to ecc2efc Compare March 2, 2021 13:33
@aduffeck
Copy link
Contributor Author

aduffeck commented Mar 2, 2021

@butonic @phil-davis the ldap problem seems to be back unfortunately... (e.g. in https://drone.cernbox.cern.ch/cs3org/reva/710)

butonic
butonic previously approved these changes Mar 2, 2021
Copy link
Contributor

@butonic butonic left a comment

Choose a reason for hiding this comment

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

nice!

@aduffeck aduffeck marked this pull request as ready for review March 2, 2021 18:12
@aduffeck aduffeck requested a review from labkode as a code owner March 2, 2021 18:12
refs
refs previously approved these changes Mar 3, 2021
pkg/storage/fs/ocis/blobstore/blobstore.go Outdated Show resolved Hide resolved
pkg/storage/fs/ocis/blobstore/blobstore.go Outdated Show resolved Hide resolved
pkg/storage/utils/decomposedfs/decomposed.go Outdated Show resolved Hide resolved
@aduffeck aduffeck dismissed stale reviews from refs and butonic via 7ba87a6 March 3, 2021 13:54
C0rby
C0rby previously approved these changes Mar 3, 2021
Copy link
Contributor

@C0rby C0rby left a comment

Choose a reason for hiding this comment

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

Awesome! 👍

Copy link
Member

@labkode labkode left a comment

Choose a reason for hiding this comment

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

LGTM, just one minor nit, probably you forgot to rename the file:

pkg/storage/utils/decomposedfs/decomposed.go => pkg/storage/utils/decomposedfs/decomposedfs.go

Same for the test files.

@aduffeck
Copy link
Contributor Author

aduffeck commented Mar 8, 2021

@labkode Thanks for your comment. I renamed the file accordingly.

@ishank011 ishank011 merged commit a23b750 into cs3org:master Mar 10, 2021
@aduffeck aduffeck deleted the streamline-s3ng-and-ocis branch March 10, 2021 11:08
ffurano pushed a commit to ffurano/reva that referenced this pull request Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants