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

quota querying and tree accounting #1405

Merged
merged 9 commits into from
Feb 18, 2021
Merged

quota querying and tree accounting #1405

merged 9 commits into from
Feb 18, 2021

Conversation

butonic
Copy link
Contributor

@butonic butonic commented Jan 18, 2021

The ocs api now returns the user quota for the users home storage. Furthermore, the ocis storage driver now reads the quota from the extended attributes of the user home or root node and implements tree size accounting. Finally, ocdav PROPFINDS now handle the DAV:quota-used-bytes and DAV:quote-available-bytes properties.

relvated: owncloud/ocis#1313

@butonic butonic requested a review from labkode as a code owner January 18, 2021 21:12
@update-docs
Copy link

update-docs bot commented Jan 18, 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.

@butonic butonic marked this pull request as draft January 18, 2021 21:17
@lgtm-com
Copy link

lgtm-com bot commented Feb 1, 2021

This pull request fixes 4 alerts when merging 99864e4 into d919d59 - view on LGTM.com

fixed alerts:

  • 4 for Incorrect conversion between integer types

@butonic
Copy link
Contributor Author

butonic commented Feb 2, 2021

the only api that end users can use to access the free and used space of a storage is through webdav rfc 4331

@lgtm-com
Copy link

lgtm-com bot commented Feb 3, 2021

This pull request fixes 4 alerts when merging 60e4c47 into ab3be6b - view on LGTM.com

fixed alerts:

  • 4 for Incorrect conversion between integer types

@lgtm-com
Copy link

lgtm-com bot commented Feb 3, 2021

This pull request fixes 4 alerts when merging 790a8c7 into ffb0fb4 - view on LGTM.com

fixed alerts:

  • 4 for Incorrect conversion between integer types

@lgtm-com
Copy link

lgtm-com bot commented Feb 3, 2021

This pull request fixes 4 alerts when merging bb1c3f9 into 82be569 - view on LGTM.com

fixed alerts:

  • 4 for Incorrect conversion between integer types

@lgtm-com
Copy link

lgtm-com bot commented Feb 3, 2021

This pull request fixes 4 alerts when merging 6f484a8 into 6f07395 - view on LGTM.com

fixed alerts:

  • 4 for Incorrect conversion between integer types

@lgtm-com
Copy link

lgtm-com bot commented Feb 3, 2021

This pull request fixes 4 alerts when merging 0336464 into 6f07395 - view on LGTM.com

fixed alerts:

  • 4 for Incorrect conversion between integer types

@lgtm-com
Copy link

lgtm-com bot commented Feb 3, 2021

This pull request fixes 4 alerts when merging 9326e3b into 6f07395 - view on LGTM.com

fixed alerts:

  • 4 for Incorrect conversion between integer types

@butonic butonic marked this pull request as ready for review February 3, 2021 20:34
@butonic butonic changed the title initial quota support quota querying and tree accounting Feb 3, 2021
@lgtm-com
Copy link

lgtm-com bot commented Feb 3, 2021

This pull request fixes 4 alerts when merging 0c6c3d4 into 6f07395 - view on LGTM.com

fixed alerts:

  • 4 for Incorrect conversion between integer types

@butonic butonic requested a review from ishank011 February 3, 2021 20:53
@butonic
Copy link
Contributor Author

butonic commented Feb 9, 2021

To test this using ocis

  1. replace reva in ocis/ocis/go.mod and
  2. make the proxy forward all ocs requests to reva:
diff --git a/proxy/pkg/proxy/proxy.go b/proxy/pkg/proxy/proxy.go
index a6c06bc0..8e2b0c76 100644
--- a/proxy/pkg/proxy/proxy.go
+++ b/proxy/pkg/proxy/proxy.go
@@ -284,11 +284,13 @@ func defaultPolicies() []config.Policy {
                                        Endpoint: "/signin/",
                                        Backend:  "http://localhost:9130",
                                },
-                               {
-                                       Type:     config.RegexRoute,
-                                       Endpoint: "/ocs/v[12].php/cloud/(users?|groups)", // we have `user`, `users` and `groups` in ocis-ocs
-                                       Backend:  "http://localhost:9110",
-                               },
+                               /*
+                                       {
+                                               Type:     config.RegexRoute,
+                                               Endpoint: "/ocs/v[12].php/cloud/(users?|groups)", // we have `user`, `users` and `groups` in ocis-ocs
+                                               Backend:  "http://localhost:9110",
+                                       },
+                               */
                                {
                                        Endpoint: "/ocs/",
                                        Backend:  "http://localhost:9140",
  1. set a quota on a users home by using this one weird extended attribute trick:
    setfattr -n user.ocis.quota -v 2024000 /var/tmp/ocis/storage/users/nodes/root/f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/ which should set the attribute on the linked directory node, not the symlink.

Now test fetching the quota using ocs:

curl --location --request GET 'https://demo.owncloud.com/ocs/v1.php/cloud/users/demo' \
--header 'ocs-apirequest: true' \
--header 'Authorization: Basic ZGVtbzpkZW1v'
<?xml version="1.0"?>
<ocs>
    <meta>
        <status>ok</status>
        <statuscode>100</statuscode>
        <message/>
    </meta>
    <data>
        <quota>
            <free>25953636352</free>
            <used>7831162</used>
            <total>25961467514</total>
            <relative>0.03</relative>
            <definition>default</definition>
        </quota>
        <email/>
        <displayname>demo</displayname>
        <home>/var/lib/owncloud/files/demo</home>
        <two_factor_auth_enabled>false</two_factor_auth_enabled>
    </data>
</ocs>

or using propfinds:

curl --location --request PROPFIND 'https://demo.owncloud.com/remote.php/dav/files/demo' \
--header 'Authorization: Basic ZGVtbzpkZW1v' \
--header 'Content-Type: text/plain' \
--header 'Cookie: cookie_test=test; oc_sessionPassphrase=9u%2FvsPAzRkzHqPl6tDokV6uFQiSn9MsSOonY1r0IsJhlym1d%2FOYxK1NOIHQ%2F%2FLtM%2BYV6N5fZuJg%2FyK40W4QEqrm%2BFLWeDA6ZZmbgvLuWKseRzZZ%2FT0VP6PXAa1kjV1hT; ocxahbm4e6a4=mmdhqh82puj3bcm38pus26ac3j' \
--data-raw '<?xml version="1.0"?>
<d:propfind  xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
<d:prop>
<oc:permissions />
<oc:favorite />
<oc:fileid />
<oc:owner-id />
<oc:owner-display-name />
<oc:share-types />
<oc:privatelink />
<d:getcontentlength />
<oc:size />
<d:getlastmodified />
<d:getetag />
<d:resourcetype />
<oc:public-link-item-type />
<oc:public-link-permission />
<oc:public-link-expiration />
<oc:public-link-share-datetime />
<oc:public-link-share-owner />
<oc:downloadURL />
<d:quota-used-bytes />
<d:quota-available-bytes />
  </d:prop>
</d:propfind>'

@kulmann
Copy link
Member

kulmann commented Feb 10, 2021

For the sake of completeness, the command for setting the quota on a mac is:
xattr -w user.ocis.quota 2024000 /var/tmp/ocis/storage/users/nodes/root/f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/
(that should write to the node, not the symlink. And additional -s would be required for writing to the symlink instead)

@kulmann
Copy link
Member

kulmann commented Feb 10, 2021

I can confirm the results with the instructions from above.

<?xml version="1.0" encoding="utf-8"?>
<ocs>
  <meta>
    <status>ok</status>
    <statuscode>100</statuscode>
    <message>OK</message>
  </meta>
  <data>
    <quota>
      <free>1799840</free>
      <used>224160</used>
      <total>2024000</total>
      <relative>0.11075099</relative>
      <definition>default</definition>
    </quota>
    <email>[email protected]</email>
    <displayname>Albert Einstein</displayname>
    <two_factor_auth_enabled>false</two_factor_auth_enabled>
  </data>
</ocs>

return 0, 0, err
}

total := stat.Blocks * uint64(stat.Bsize) // Total data blocks in filesystem
Copy link
Member

@kulmann kulmann Feb 10, 2021

Choose a reason for hiding this comment

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

Shouldn't total be the available disk space? 🤔 at least from a user perspective it's misleading that with unlimited quota the response will tell that (on my machine) 495GB are free, while my actual free disk space is at 195GB.

Copy link
Contributor Author

@butonic butonic Feb 10, 2021

Choose a reason for hiding this comment

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

uh ... that is weird. maybe the total is calculated differently on mac? what does this look like on mac:

> stat .
  File: .
  Size: 4096            Blocks: 8          IO Block: 4096   directory
Device: 820h/2080d      Inode: 420580      Links: 29
Access: (0755/drwxr-xr-x)  Uid: ( 1000/  vscode)   Gid: ( 1000/  vscode)
Access: 2021-02-09 20:43:26.250000000 +0000
Modify: 2021-02-09 20:43:26.100000000 +0000
Change: 2021-02-09 20:43:26.100000000 +0000
 Birth: -

Copy link
Member

Choose a reason for hiding this comment

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

macOS is weird for stat... being in the ocis user home dir of einstein:

> stat -x .
  File: "."
  Size: 128          FileType: Directory
  Mode: (0700/drwx------)         Uid: (  501/    bene)  Gid: (    0/   wheel)
Device: 1,5   Inode: 62850061    Links: 4
Access: Wed Feb 10 07:33:06 2021
Modify: Wed Feb 10 07:33:06 2021
Change: Wed Feb 10 09:56:26 2021
> stat .
16777221 62850061 drwx------ 4 bene wheel 0 128 "Feb 10 07:33:06 2021" "Feb 10 07:33:06 2021" "Feb 10 09:56:26 2021" "Feb 10 07:23:36 2021" 4096 0 0 .

Copy link
Contributor Author

@butonic butonic Feb 10, 2021

Choose a reason for hiding this comment

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

ok, what about filesystem stat and df:

> stat -f .
  File: "."
    ID: fedc9aa3bd65bc57 Namelen: 255     Type: ext2/ext3
Block size: 4096       Fundamental block size: 4096
Blocks: Total: 65793553   Free: 58593039   Available: 55233500
Inodes: Total: 16777216   Free: 16011441
> df -B4K
Filesystem     4K-blocks     Used Available Use% Mounted on
overlay         65793553  7200514  55233500  12% /
tmpfs              16384        0     16384   0% /dev
tmpfs            2046465        0   2046465   0% /sys/fs/cgroup
shm                16384        0     16384   0% /dev/shm
/dev/sdc        65793553  7200514  55233500  12% /etc/hosts
drvfs           31096934 19848635  11248299  64% /home/vscode/.ssh

Copy link
Member

Choose a reason for hiding this comment

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

macOS doesn't have stat for the entire filesystem AFAICT.

> BLOCKSIZE=4K df
Filesystem     4K-blocks     Used Available Capacity iused      ifree %iused  Mounted on
/dev/disk1s5s1 122061322  4756743  34314966    13%  568975 4881883905    0%   /
devfs                 47       47         0   100%     658          0  100%   /dev
/dev/disk1s4   122061322   524298  34314966     2%       2 4882452878    0%   /System/Volumes/VM
/dev/disk1s2   122061322    80331  34314966     1%     792 4882452088    0%   /System/Volumes/Preboot
/dev/disk1s6   122061322    27269  34314966     1%     396 4882452484    0%   /System/Volumes/Update
/dev/disk1s1   122061322 82168442  34314966    71% 4246575 4878206305    0%   /System/Volumes/Data
map auto_home          0        0         0   100%       0          0  100%   /System/Volumes/Data/home
/dev/disk1s5   122061322  4756743  34314966    13%  568977 4881883903    0%   /System/Volumes/Update/mnt1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah now I get it ... the switch case in https://github.com/cs3org/reva/pull/1405/files#diff-62fe53c34a46cf0bf6e8f27409f0ba1ba6b0687387b8c9cde985299ba10b839fR229-R240 should limit the total to the quota if the latter is set. the free space is then calculated in the ocs / ocdav handler by calculating free = total-used. The problem is that we currently cannot return 'unlimited' quota. It would be a dedicated flag because currently quota is an unsigned int...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, if no quote is set we should return free+used as the total, because other users files also count against the total.

@lgtm-com
Copy link

lgtm-com bot commented Feb 10, 2021

This pull request fixes 4 alerts when merging d080ab3 into 7a9a5ef - view on LGTM.com

fixed alerts:

  • 4 for Incorrect conversion between integer types

kulmann
kulmann previously approved these changes Feb 10, 2021
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

👍

@butonic
Copy link
Contributor Author

butonic commented Feb 18, 2021

@ishank011 can you test on eos?

Copy link
Contributor

@ishank011 ishank011 left a comment

Choose a reason for hiding this comment

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

@butonic Looks good, a few minor nits. And works for EOS, nice!

internal/grpc/services/gateway/storageprovider.go Outdated Show resolved Hide resolved
pkg/storage/utils/localfs/localfs.go Outdated Show resolved Hide resolved
pkg/storage/fs/ocis/ocis.go Outdated Show resolved Hide resolved
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@lgtm-com
Copy link

lgtm-com bot commented Feb 18, 2021

This pull request fixes 4 alerts when merging 3f69dc2 into 8a29177 - view on LGTM.com

fixed alerts:

  • 4 for Incorrect conversion between integer types

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.

3 participants