Skip to content

HDDS-4672. Add warning log when old volume and bucket set quota#1781

Merged
ChenSammi merged 3 commits intoapache:masterfrom
captainzmc:add-warn-quota
Jan 18, 2021
Merged

HDDS-4672. Add warning log when old volume and bucket set quota#1781
ChenSammi merged 3 commits intoapache:masterfrom
captainzmc:add-warn-quota

Conversation

@captainzmc
Copy link
Member

@captainzmc captainzmc commented Jan 12, 2021

What changes were proposed in this pull request?

https://github.com/apache/ozone/blob/master/hadoop-hdds/docs/content/feature/Quota.md
image

Currently we only advise users in the doc document not to enable quota for old volumes and buckets. Sometimes the user might not notice. So we added a warning on the client side, when user set quota on old volume and bucket.
image

What is the link to the Apache JIRA

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

How was this patch tested?

Add log no need UT

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: better to undo this line of change.

Copy link
Contributor

@amaliujia amaliujia Jan 12, 2021

Choose a reason for hiding this comment

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

I feel like "old" does not provide enough context.

Will it better to say that " {} is a volume created before version 1.1.0"? Same for the following warning messages.

@captainzmc captainzmc requested a review from ChenSammi January 12, 2021 08:15
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we pass this warnning message to client side? Put it in log is not enough to raise the alert to user.

Copy link
Member Author

@captainzmc captainzmc Jan 12, 2021

Choose a reason for hiding this comment

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

Yes, the warning here is pass to the user client directly. Users can see this whether they're using shell or API.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

@captainzmc , we can see this output on console because LOG is enabled. When LOG is disabled on client side, we cannot see any message. My suggestion is we return this message as the command response, and then output to console.

@amaliujia
Copy link
Contributor

overall LGTM

@captainzmc
Copy link
Member Author

captainzmc commented Jan 14, 2021

Had added message as the command response, and output to console. CI runs normally on my personal branch.
cc @ChenSammi

@ChenSammi
Copy link
Contributor

+1

@ChenSammi ChenSammi merged commit a02ea47 into apache:master Jan 18, 2021
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

Comments