[#418] Implement support for physical quotas (main)#457
[#418] Implement support for physical quotas (main)#457korydraughn wants to merge 4 commits intoirods:mainfrom
Conversation
53a6fd9 to
dd4fe0f
Compare
trel
left a comment
There was a problem hiding this comment.
noting here that there are more open quota issues in the server than i remembered...
|
Need to rebase and rerun tests. |
|
Just noticed failures with connection pooling disabled. Putting this back in draft to avoid accidental merging. |
| } | ||
| ``` | ||
|
|
||
| ## Quota Operations |
There was a problem hiding this comment.
Is this all under an "admin only" heading? if not, should we mention that this is all admin only?
There was a problem hiding this comment.
are all quota operations admin only? iquota?
There was a problem hiding this comment.
All of the operations in this PR except stat require admin privileges.
I haven't looked at what iquota does too closely. Will take a peek.
There was a problem hiding this comment.
Is this all under an "admin only" heading? if not, should we mention that this is all admin only?
Forgot about the admin-ness of these ops. Will add a note to each operation.
There was a problem hiding this comment.
also, not sure about groupadmins...
| input.arg2 = group_iter->second.c_str(); | ||
| input.arg4 = quota_iter->second.c_str(); |
There was a problem hiding this comment.
This may or may not be an issue, but is there any potential to provide a string that is too long to get (correctly) packed? Or is there some kind of safety mechanism in place to prevent that?
There was a problem hiding this comment.
I believe the only limit for rxGeneralAdmin is memory. We need to apply a reasonable limit to guard against malicious input like that.
I'm thinking the limit for each of these should be the max length of a group name and signed 64bit integer (i.e. try to convert it to int64_t).
Thoughts?
| # Recalculate the quotas. | ||
| r = requests.post(self.url_endpoint, headers=rodsadmin_headers, data={ | ||
| 'op': 'recalculate' | ||
| }) | ||
| self.logger.debug(r.content) | ||
|
|
||
| # Show the quotas. | ||
| r = requests.get(self.url_endpoint, headers=rodsadmin_headers, params={ | ||
| 'op': 'stat', | ||
| 'group': 'public' | ||
| }) | ||
| self.logger.debug(r.content) |
There was a problem hiding this comment.
Is this here for debugging, or is it required in order to properly clean up?
There was a problem hiding this comment.
It's for debugging purposes only.
There was a problem hiding this comment.
Would like to see some tests with incomplete / invalid inputs to make sure the things we care about in this client are being enforced.
This commit introduces a new endpoint, /quotas, which supports the
following operations:
- stat
- set_group_quota
- recalculate
|
This was put in draft due to there being a problem with recalculating totals for quotas. See irods/irods#8633 for details. |
|
I've confirmed that the PR at irods/irods#8657 resolves the inconsistency in quota calculations. This PR can now move forward. |
|
Just learned that irods/irods#8633 is still reproducible. All that's required is to run the test enough times. |
|
The problem might be due to the test. I don't have any evidence supporting that thought yet, but investigating. |
I'm not able to reproduce this anymore. I think my test zone may have been dirty. |
|
Okay, good - because I was very confused. |
This PR adds support for system/resource quotas (not logical quotas).
Through this work, I've noticed that the recalculation of quotas doesn't always produce the expected results. This might be caused by misuse of transactions and/or bad math in the iRODS quota code. I'll open an issue for that soon.
As of right now, support behaves as expected when the connection pool is disabled or carefully placed calls to
time.sleep()are inserted into the tests.Putting this in draft until we can decide on whether this will be part of the 0.6.0 release. The design of the operations have not been settled on yet either.
Feedback is welcome.