Skip to content

Conversation

@amaliujia
Copy link
Contributor

What changes were proposed in this pull request?

Add namespace quota to documentation

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-4594

How was this patch tested?

UT

@amaliujia
Copy link
Contributor Author

R: @captainzmc


c. When bucket namespace quota is enabled, the totoal number of keys under the bucket, cannot exceed the bucket namespace quota.

d. Linked buckets do not consume namespace quota.
Copy link
Member

Choose a reason for hiding this comment

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

Whether 'e' in Storage Space applies equally to Namespace quota. If so, we need to list.

@captainzmc
Copy link
Member

@amaliujia Thanks for your patch. Could you also update the Client Usage about Namespace quota?

@amaliujia
Copy link
Contributor Author

@captainzmc

comments addressed.

Copy link
Contributor

@linyiqun linyiqun left a comment

Choose a reason for hiding this comment

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

When reading this quota documentation, I am thinking for the set quota cases: as we support both volume and bucket quota, will we check the below scenarios that only either one quota was set, then try to enable another quota:

  • Bucket quota already set, then user try to set volume quota, will we check volume quota must more than total bucket quota?
  • Volume quota already set, then user try to set bucket quota, will we check the total bucket quota, so that will not exceed volume quota? (I remember this check already contained)

@amaliujia
Copy link
Contributor Author

amaliujia commented Dec 27, 2020

@linyiqun

I think for namespace, it will be different than space. Namespace quota is independent on each level (volume, bucket, etc.)

Namespace quota on volume only controls number of bucket names, and namespace on bucket only controls keys in that bucket. This is different from space quota in which volume quota has a relationship with its buckets quotas.

What currently is missing is the ability to control all keys within a volume. If there is use case, we could add support for it.

@captainzmc
Copy link
Member

Thanks for @amaliujia's update. I think there are some case we need to confirm.
Whether 'f' in Storage Space applies equally to Namespace quota? If so, we need to list. This part of namespace has been modified in #1723 (reviewing).

bin/ozone sh volume clrquota --space-quota /volume1
bin/ozone sh bucket clrquota --space-quota /volume1/bucket1
bin/ozone sh volume clrquota --space-quota --namespace-quota /volume1
bin/ozone sh bucket clrquota --space-quota --namespace-quota /volume1/bucket1
Copy link
Member

Choose a reason for hiding this comment

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

There's something wrong with the directory structure. At present is:
#space quota
……
#namespace quota
……
## clear quota
## check quota and usage

We can change it to:

#space quota
……
## clear quota
## check quota and usage

#namespace quota
……
## clear quota
## check quota and usage

or

#space quota and namespace quota

……
## clear quota
## check quota and usage

@amaliujia
Copy link
Contributor Author

@captainzmc
f. does not apply to namespace in my opinion. Note that the difference so far for namespace quota is that it is set independently on volume and bucket (not like space quota).

@captainzmc
Copy link
Member

captainzmc commented Dec 28, 2020

@captainzmc
f. does not apply to namespace in my opinion. Note that the difference so far for namespace quota is that it is set independently on volume and bucket (not like space quota).

Agree, I'll remove the namespace restriction in #1723.

@amaliujia
Copy link
Contributor Author

@captainzmc

finally I was able to re-trigger the CI (seemed that CI didn't work for a while). Any other comments on this PR?

Copy link
Member

@captainzmc captainzmc left a comment

Choose a reason for hiding this comment

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

LGTM +1
hi @linyiqun, any other suggestions about this PR?

@linyiqun
Copy link
Contributor

Also +1 from me.

@captainzmc captainzmc merged commit 1e84d17 into apache:master Dec 29, 2020
@captainzmc
Copy link
Member

Merged this PR. Thanks for @amaliujia's patch and @linyiqun‘s review.

@amaliujia amaliujia deleted the HDDS-4594 branch December 29, 2020 19:40
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