Implement storage quotas - #2565
Conversation
| re-acquiring quota data in the process, or | ||
| - edit the msgpacked ``hints.N`` file (not recommended and thus not | ||
| documented further). | ||
|
|
There was a problem hiding this comment.
The trade-offs documented here drastically simplify the implementation. (More than half of this PR is documentation!)
| a transaction modify the currently used quota: | ||
|
|
||
| - A PUT adds the size of the *log entry* to the quota, | ||
| i.e. the length of the data plus the 41 byte header. |
There was a problem hiding this comment.
maybe do not mention the specific size, would get out of sync if we ever change that.
so just "length of the put command header"?
|
|
||
| - A PUT adds the size of the *log entry* to the quota, | ||
| i.e. the length of the data plus the 41 byte header. | ||
| - A DELETE substracts the size of the deleted log entry to the quota, |
There was a problem hiding this comment.
subtracts ... from the quota.
| - A DELETE substracts the size of the deleted log entry to the quota, | ||
| which includes the header. | ||
|
|
||
| Thus, PUT and DELETE are symmetric and cancel each other out precisely. |
There was a problem hiding this comment.
not sure: "cancel out each other"?
| The quota is enforcible only if *all* :ref:`borg_serve` versions | ||
| accessible to clients support quotas (see next section). Further, quota is | ||
| per repository. Therefore, ensure clients can only access a defined set of repositories | ||
| with their quotas set, using ``--restrict-to-path``. |
There was a problem hiding this comment.
hmm, iirc --restrict-to-path was only meant to restrict the path prefix ("base dir"), but not meaning that there is only 1 repo there.
so, would a provider pre-create some repos below the base dir and revoke write permissions to the base dir afterwards for the client so the client can not create any further repos (directories)?
There was a problem hiding this comment.
See PR comment
More difficult to deploy compared to FS quotas
Perhaps a complementary option to --restrict-to-path that matches the path exactly would simplify handling of repo hosting in general?
| def parse_storage_quota(storage_quota): | ||
| parsed = parse_file_size(storage_quota) | ||
| if parsed < parse_file_size('10M'): | ||
| raise argparse.ArgumentTypeError('quota is too small (%s), must exceed 10M' % storage_quota) |
There was a problem hiding this comment.
"exceed" (from: excessive) sounds strange here. rather "must be at least 10M"?
also: refactor to not repeat '10M'?
|
|
||
| def __init__(self, path, create=False, exclusive=False, lock_wait=None, lock=True, append_only=False): | ||
| class StorageQuotaExceeded(Error): | ||
| """The storage quota ({}) has been exceeded ({}). Try deleting some archives.""" |
There was a problem hiding this comment.
add this to handle_error in remote.py?
There was a problem hiding this comment.
The generic error handling in protocol v2 means that we don't have to do that unless the type of the exception is relevant, i.e. some code expects that exception. For user-facing error messages there is no need.
There was a problem hiding this comment.
Actually that wasn't quite right, since the .traceback attributes of Error was not propagated this did not quite work as intended. Fixed below.
Codecov Report
@@ Coverage Diff @@
## master #2565 +/- ##
==========================================
+ Coverage 83.58% 83.62% +0.04%
==========================================
Files 22 22
Lines 8052 8166 +114
Branches 1375 1390 +15
==========================================
+ Hits 6730 6829 +99
- Misses 941 956 +15
Partials 381 381
Continue to review full report at Codecov.
|
| if self.storage_quota: | ||
| config.set('repository', 'storage_quota', str(self.storage_quota)) | ||
| else: | ||
| config.set('repository', 'storage_quota', '0') |
There was a problem hiding this comment.
set(..., str(self.storage_quota if self.storage_quota else 0))
| b'segments': self.segments, | ||
| b'compact': self.compact} | ||
| b'compact': self.compact, | ||
| b'storage_quota_use': self.storage_quota_use, } |
There was a problem hiding this comment.
nitpick: that way of using a trailing comma does not minimize the diff if a new element is added afterwards.
should be rather:
{a: 1,
b: 2,
}
|
|
||
| If no quota data is stored in the hints file, Borg assumes zero quota is used. | ||
| Thus, if a repository with an enabled quota is written to with an older version | ||
| that does not understand quotas, then the quota usage will be erased. |
There was a problem hiding this comment.
Does not this allow users to circumvent this feature? Say a server admins wants to enforce a 50 GB quota and puts this limit there. Then the user can just access the repo with an old borg client and voila… the quota is gone.
That's how I understand that sentence.
There was a problem hiding this comment.
see section headline - it is referring to servers here, not clients.
but maybe it makes sense to actually say that in that sentence, just to avoid misunderstandings.
4 participants
Fixes #2517
I gave #2517 some thought; implementing it in Borg has some advantages:
Disadvantages:
More difficult to deploy compared to FS quotas
Perhaps a complementary option to --restrict-to-path that matches the path exactly would simplify handling of repo hosting in general?
The storage quota and the actual bound on disk space use have a somewhat loose relationship.
(In case anyone feels reminded of file systems and filling them up completely... it's the same structures, the same math — an exercise for the reader: design a transactional data store that has a fixed upper bound X, independent of contents, such that using SPACE(X) the store's contents can still be cleaned up :)
TODO: Add tests for quota tracking.