-
Notifications
You must be signed in to change notification settings - Fork 244
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
Bugfix/cldsrv 533 #5570
Bugfix/cldsrv 533 #5570
Conversation
Hello williamlardier,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
591a9f6
to
5fcaad8
Compare
A corner case was found, where any PUT from the cold backend would fail if the quota is already exceeded, as the storage was reserved for the restore, but the restore itself requires some more bytes as inflights when evaluating quotas. By passing a flag in the quota evaluation function, we ensure that we can, in these cases, evaluate the quotas with 0 inflight.
5fcaad8
to
918ac53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
lib/metadata/metadataUtils.js
Outdated
// withVersionId cover cases when an object is being restored with a specific version ID. | ||
// In this case, the storage space was already accounted for when the RestoreObject API call | ||
// was made, so we don't need to add any inflight, but quota must be evaluated. | ||
if (!checkQuota && !withVersionId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this really needed?
// withVersionId cover cases when an object is being restored with a specific version ID. | |
// In this case, the storage space was already accounted for when the RestoreObject API call | |
// was made, so we don't need to add any inflight, but quota must be evaluated. | |
if (!checkQuota && !withVersionId) { | |
if (!checkQuota) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, it had been removed (and squashed)
When the API is being called by a cold backend, the x-scal-s3-version-id header is set. In this case, the quotas must be evaluated with a 0 inflight.
918ac53
to
926242b
Compare
/approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue CLDSRV-533. Goodbye williamlardier. The following options are set: approve |
Fixes a limitation during a corner case, when restoring an object within account/bucket with enabled exceeded quota:
RestoreObject
x-scal-s3-version-id
). But then tries to increase the inflights: in this case, if the quota is exceeded already, or the bytes are near to the limit, we fail, even if the space was reserved.This PR introduces a way to detect DMF calls (the specific header) and, in this case, only check the quotas, without increasing the in-flights.