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

Feature request: limit repo size #65

Closed
dotlambda opened this issue Apr 10, 2018 · 8 comments
Closed

Feature request: limit repo size #65

dotlambda opened this issue Apr 10, 2018 · 8 comments

Comments

@dotlambda
Copy link

I'd like to host a REST server for multiple users and I don't want them to use up all of my server's disk space.
So it would be nice to be able limit the overall disk space the repos take, i.e. the recursive size of --path.
Furthermore, I'd optimally be able to set different repo sizes for different users.

@mholt
Copy link
Contributor

mholt commented Apr 13, 2018

This is similar to #50 I think.

But I'm also interested in this. I would even implement it. My main question is how to trade off efficiency and accuracy?

For example, limiting the size of a SaveBlob is easy enough: just wrap r.Body in a io.LimitReader where the size of the limit reader is the available space left in the repo. The question is how to get the space left efficiently, without scanning the repo at every request.

(Oh, and this would necessarily cause a backup to be "corrupt" in the sense that a SaveBlob will fail when the repo reaches the size limit. If we wanted to avoid that and sacrifice some accuracy and allow repos to go over the size limit, we could allow the write to finish, then forbid future writes while the repo is too big.)

Any thoughts on this, @fd0?

@fd0
Copy link
Member

fd0 commented Apr 15, 2018

I think it's a nice feature to have, although I'm not sure how many people will actually use it (at least you two) ;)

At the start, we need to scan the repo and sum all file sizes, e.g. by using filepath.Walk. Then after each SaveBlob, we need to add the size of the new blob to the global size, which needs to be guarded by a mutex. A recent version of restic will announce the blob size in the Content-Length HTTP header, so we can reject a new blob early if it goes over the limit.

These are my thoughts so far.

@mholt
Copy link
Contributor

mholt commented Apr 15, 2018

@fd0 @dotlambda See https://github.com/mholt/rest-server/pull/1 for my implementation. It requires #67 so I made the PR to my own fork so the diff was more focused.

Note that it will require to stop retrying if the server responds with a status code of 507 (Insufficient Storage). (Well, not strictly -- but it would be polite.)

@wojas
Copy link
Contributor

wojas commented Apr 17, 2018

I think that an error response like 413 Payload Too Large might be better here than 507 Insufficient Storage. 5xx errors indicate something is temporarily wrong with the server, while 4xx is for errors caused by the client.

According to RFC 4918:

11.5. 507 Insufficient Storage

The 507 (Insufficient Storage) status code means the method could not
be performed on the resource because the server is unable to store
the representation needed to successfully complete the request. This
condition is considered to be temporary.
If the request that
received this status code was the result of a user action, the
request MUST NOT be repeated until it is requested by a separate user
action.

Being out of quota is not temporary: the current backup cannot complete and resources need to be freed first.

RFC 7231:

6.5.11. 413 Payload Too Large

The 413 (Payload Too Large) status code indicates that the server is
refusing to process a request because the request payload is larger
than the server is willing or able to process. The server MAY close
the connection to prevent the client from continuing the request.

If the condition is temporary, the server SHOULD generate a
Retry-After header field to indicate that it is temporary and after
what time the client MAY try again.

This is assumed to be permanent by default.

I'm not sure if 413 would be the right one here, as the restic client might want to use that one in the future to detect request size limits. Alternatively, maybe 400 can be used with a special header.

@mholt
Copy link
Contributor

mholt commented Apr 17, 2018

@wojas I forgot about 413 -- thanks! I've pushed an update that uses 413 instead of 507.

@mholt
Copy link
Contributor

mholt commented Apr 17, 2018

@fd0 or other collaborator: Would you be able to give https://github.com/mholt/rest-server/pull/1 a once-over as well? Then I think it will be smoother to merge #67. (I know you're busy with the new archiver, I just didn't want this to slip though.)

@fd0
Copy link
Member

fd0 commented Jun 11, 2018

Sure, thanks for the reminder. I'm using https://octobox.io now to keep track of things I still need to do, works great :)

@mholt mholt closed this as completed in #72 Jun 14, 2018
@yvolchkov
Copy link

Hi. Does the current implementation of --max-size limits the size for all repositories combined, or one can limit available disk space on per-repo basis?

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

No branches or pull requests

6 participants
@wojas @fd0 @mholt @dotlambda @yvolchkov and others