HDDS-4588. If volume's quota is enabled then bucket's quota cannot be cleared

[captainzmc]

What changes were proposed in this pull request?

If volume's quota is enabled then bucket's quota cannot be cleared. We need to prompt the user to clear volume quota first.
For more details.

To prevent conflicts, I have updated the usage and behavior of clear space quota in #1677.

What is the link to the Apache JIRA

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

How was this patch tested?

ut added

[amaliujia]

Nit: will it better to use more descriptive names than variable names. For example, replace quotaInBytes by space quota?

[amaliujia]

Nit: same, maybe replace quotaInNamespace by namespace quota?

[linyiqun]

Agreed with @amaliujia 's suggestion. I also prefer to do below rename changing for more readable:

quotaInNamespace -> namespaceQuota
quotaInNamespace -> spaceQuota

[linyiqun]

I am thinking for this case, it's okay to just clear bucket quota no matter if the volume quota was set? Since the volume quota check will still make the sense, when we clear one bucket quota.

[ayushtkn]

Thanx @captainzmc for the work here.
dropped some minor comments,

[ayushtkn]

Can try something like this -

  LambdaTestUtils.intercept(IOException.class, "Before clear bucket "
            + "quotaInBytes, volume quotaInBytes need to be clear first.",
        () -> ozoneBucket.clearSpaceQuota());

Shall save lines and looks better

[ayushtkn]

Similarly here -

    LambdaTestUtils.intercept(IOException.class, "Before clear bucket "
            + "quotaInNamespace, volume quotaInNamespace need to be clear first.",
        () -> ozoneBucket.clearCountQuota());

[ayushtkn]

Seems some grammatical error in the message, Can you check and reframe if possible,
May be something like a warning, Can not clear bucket namespace quota because volume namespace quota is not unset. Or may be something better....

[captainzmc]

I am thinking for this case, it's okay to just clear bucket quota no matter if the volume quota was set? Since the volume quota check will still make the sense, when we clear one bucket quota.

There is a very detailed discussion of this.

[captainzmc]

Fixed review issues.

[linyiqun]

I am thinking for this case, it's okay to just clear bucket quota no matter if the volume quota was set? Since the volume quota check will still make the sense, when we clear one bucket quota.

There is a very detailed discussion of this.

@captainzmc , although we removed the volume usedBytes because of the quota update issue. Can we also try to sum the buckets used bytes when bucket is created that will be used for the volume quota check? As I see we already do the similar check for the quota valid check, OMBucketCreateRequest.java#L331

[captainzmc]

@captainzmc , although we removed the volume usedBytes because of the quota update issue. Can we also try to sum the buckets used bytes when bucket is created that will be used for the volume quota check? As I see we already do the similar check for the quota valid check, OMBucketCreateRequest.java#L331

Thanks for @linyiqun‘ advice.
We've considered this option before, but it can cause performance problems.
Let's assume that we have 1000 buckets under volume.
First set quota, which is not a very frequent action in a cluster, and unlike the write operation, has a low latency requirement. So we calculated that all bucket quota is acceptable.
However, each write operation calculating the usedBytes of all buckets for volume quota checker may affect write performance. The more buckets there are, the more performance is affected per write. This maybe unacceptable to the users.
Also if we do this, we need to lock the volume when we count all the usedBytes of our buckets, because we don't want the usedBytes of our buckets to change back and forth when we calculate the total usedBytes, which will also affect the performance.

[linyiqun]

However, each write operation calculating the usedBytes of all buckets for volume quota checker may affect write performance. The more buckets there are, the more performance is affected per write. This maybe unacceptable to the users.
Also if we do this, we need to lock the volume when we count all the usedBytes of our buckets, because we don't want the usedBytes of our buckets to change back and forth when we calculate the total usedBytes, which will also affect the performance.

If there is the performance consideration for this, I am okay for this.

[linyiqun]

+1 for this PR change.

[ayushtkn]

LGTM

[captainzmc]

Removed the namespace restriction, as discussed in #1736. cc @amaliujia

[amaliujia]

LGTM

[captainzmc]

Merged this patch, Thanks for @linyiqun @amaliujia @ayushtkn for the review.