mds: align quota.max_bytes to 4MB or 4KB by lxbsz · Pull Request #46905 · ceph/ceph

lxbsz · 2022-06-30T07:21:09Z

The quota.max_bytes must align to 4MB if it's larger than or equals
to 4MB, or must align to 4KB.

This will make the statfs report correct block size and total blocks,
then the df could show expected capacity.

Signed-off-by: Xiubo Li xiubli@redhat.com

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows

src/mds/Server.cc

kotreshhr

Looks good to me otherwise.

doc/cephfs/quota.rst

gregsfortytwo · 2022-07-08T02:48:28Z

@Madhu-1 would this be okay with CSI? It really simplifies our life but I don't know if it'll be okay for users on top of us.

vshankar

LGTM

kotreshhr · 2022-07-12T05:27:13Z

@lxbsz What happens to the quota which is already set ?

lxbsz · 2022-07-12T07:01:11Z

@lxbsz What happens to the quota which is already set ?

The size from df output will always be rounded down to 4MB, for example, if you set quota size to any of (4~8)MB, it always show 4MB capacity.

mchangir · 2022-07-12T08:51:51Z

@lxbsz What happens to the quota which is already set ?

The size from df output will always be rounded down to 4MB, for example, if you set quota size to any of (4~8)MB, it always show 4MB capacity.

If quota is enforced to the exact size that is used when setting the quota, then there should not be any rounding up or down done specifically for df output.
If quota is rounded up to a multiple of 4KB or 4MB at setting time, then there's no special care required for df output.

Proposal:
I think, quota rounding up to a multiple of 4KB at setting time is an optimal solution. We needn't take any special care of the 4MB case. And we should enforce the quota to the exact number of rounded bytes, i.e. the size to the latest 4KB number of blocks.

Due to the bug in the df stat we need to roundoff the subvolume size to allign with 4Mib. Note:- Minimum supported size in cephcsi is 1Mib, we dont need to take care of Kib. fixes ceph#3240 More details at ceph/ceph#46905 Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

Due to the bug in the df stat we need to round off the subvolume size to align with 4Mib. Note:- Minimum supported size in cephcsi is 1Mib, we dont need to take care of Kib. fixes ceph#3240 More details at ceph/ceph#46905 Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

Madhu-1 · 2022-07-12T08:56:11Z

@Madhu-1 would this be okay with CSI? It really simplifies our life but I don't know if it'll be okay for users on top of us.

@gregsfortytwo it should be fine ceph/ceph-csi#3241 should address it.

Due to the bug in the df stat we need to round off the subvolume size to align with 4Mib. Note:- Minimum supported size in cephcsi is 1Mib, we dont need to take care of Kib. fixes ceph#3240 More details at ceph/ceph#46905 Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

Due to the bug in the df stat we need to round off the subvolume size to align with 4Mib. Note:- Minimum supported size in cephcsi is 1Mib, we dont need to take care of Kib. fixes #3240 More details at ceph/ceph#46905 Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

lxbsz · 2023-02-23T12:16:41Z

Rebased it to resolve the conflicts in doc/cephfs/quota.rst.

github-actions · 2023-03-08T08:46:28Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

The quota.max_bytes must be aligned to 4MB if greater than or equal to 4MB, otherwise must align to 4KB. This will make the statfs report correct block size and total blocks, then the `df` could show expected capacity. Fixes: https://tracker.ceph.com/issues/56397 Signed-off-by: Xiubo Li <xiubli@redhat.com>

Fixes: https://tracker.ceph.com/issues/56397 Signed-off-by: Xiubo Li <xiubli@redhat.com>

For quota size less than 4KB, report the total=used=4KB,free=0 when quota is full and total=free=4KB, used=0 otherwise. This is from Kotresh's kceph's fix. Fixes: https://tracker.ceph.com/issues/56397 Signed-off-by: Xiubo Li <xiubli@redhat.com>

Fixes: https://tracker.ceph.com/issues/56397 Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz · 2023-03-08T10:58:17Z

Rebased it to resolve the conflicts in Server.cc

rishabh-d-dave · 2023-03-31T18:41:30Z

https://tracker.ceph.com/projects/cephfs/wiki/Main#31-Mar-2023

rishabh-d-dave · 2023-03-31T18:45:32Z

@lxbsz https://tracker.ceph.com/issues/56397#note-3

dparmar18 · 2023-04-06T12:28:38Z

src/include/cephfs/types.h

+#define CEPH_4M_BLOCK_SHIFT 22
+#define CEPH_4M_BLOCK_SIZE (1 << CEPH_4M_BLOCK_SHIFT) // 4MB
+#define CEPH_4K_BLOCK_SHIFT 12
+#define CEPH_4K_BLOCK_SIZE (1 << CEPH_4K_BLOCK_SHIFT) // 4KB


this needs a bit of correction. CEPH_4M_BLOCK_SIZE and CEPH_4K_BLOCK_SIZE are both power of 2. The comment here as well in other places say (M/K)B ( i.e power of 10) but it should be (M/K)iB. Will add this correction in the PR i raise for the affected test cases due to this patch.

* use a fixture for setup_test() * invoke test_cephfs.py with pytest Signed-off-by: Casey Bodley <cbodley@redhat.com> (cherry picked from commit 0bd813f) Conflicts: src/test/pybind/test_cephfs.py missing ceph#43546 missing ceph#46905

vshankar · 2023-08-25T10:54:32Z

@rishabh-d-dave @lxbsz @dparmar18 Is this PR causing issues which is being fixed by #50910? If that is true, then can be revert this change since #50910 is taking time to get reviewed and tested?

dparmar18 · 2023-08-25T12:43:41Z

@rishabh-d-dave @lxbsz @dparmar18 Is this PR causing issues which is being fixed by #50910? If that is true, then can be revert this change since #50910 is taking time to get reviewed and tested?

@vshankar I'm unaware of the issues caused by this PR but #50910 does solve the issues. One thing i'm concerned about reverting this PR is it might break #50910 since xiubo might've edited the code there that was added by this PR and it might be cumbersome to compare and fix the PR. I think we can prioritise that PR to get it merged ASAP. I can go through that PR again and approve it.

lxbsz · 2023-08-26T01:40:12Z

@rishabh-d-dave @lxbsz @dparmar18 Is this PR causing issues which is being fixed by #50910? If that is true, then can be revert this change since #50910 is taking time to get reviewed and tested?

@vshankar @rishabh-d-dave Sorry I missed Rishabh's last comment. I checked those 3 failure, they are not introduce by the pr #50910. They are just other failures that haven't fixed totally.

If except those 3 failures there is no any other issue let's merge #50910 first ? I'd prefer to create a new PR to fix them instead of reverting this PR, I am afraid reverting it will break the third part APP or the upper layer ODF.

vshankar · 2023-08-26T02:49:48Z

@rishabh-d-dave @lxbsz @dparmar18 Is this PR causing issues which is being fixed by #50910? If that is true, then can be revert this change since #50910 is taking time to get reviewed and tested?

@vshankar @rishabh-d-dave Sorry I missed Rishabh's last comment. I checked those 3 failure, they are not introduce by the pr #50910. They are just other failures that haven't fixed totally.

If except those 3 failures there is no any other issue let's merge #50910 first ? I'd prefer to create a new PR to fix them instead of reverting this PR, I am afraid reverting it will break the third part APP or the upper layer ODF.

ACK. My only concern was that quota related failures are looming around from ~4 months and they seemed to have cropped up around the time this changes got merged. If we can prioritise and get the fixes merged for quota failures, it would be good.

lxbsz · 2023-08-26T02:55:07Z

@rishabh-d-dave @lxbsz @dparmar18 Is this PR causing issues which is being fixed by #50910? If that is true, then can be revert this change since #50910 is taking time to get reviewed and tested?

@vshankar @rishabh-d-dave Sorry I missed Rishabh's last comment. I checked those 3 failure, they are not introduce by the pr #50910. They are just other failures that haven't fixed totally.
If except those 3 failures there is no any other issue let's merge #50910 first ? I'd prefer to create a new PR to fix them instead of reverting this PR, I am afraid reverting it will break the third part APP or the upper layer ODF.

ACK. My only concern was that quota related failures are looming around from ~4 months and they seemed to have cropped up around the time this changes got merged. If we can prioritise and get the fixes merged for quota failures, it would be good.

Okay, sorry I didn't notice that and before I didn't see any report about the test_volume failures. Just raised a new PR to fix the test_volume issue in #53160.

Thanks

vshankar · 2023-08-31T05:15:48Z

#53153 (comment) @lxbsz @dparmar18 @rishabh-d-dave

* use a fixture for setup_test() * invoke test_cephfs.py with pytest Signed-off-by: Casey Bodley <cbodley@redhat.com> (cherry picked from commit 0bd813f) Conflicts: src/test/pybind/test_cephfs.py missing ceph#43546 missing ceph#46905

lxbsz requested a review from a team June 30, 2022 07:21

github-actions bot added the cephfs Ceph File System label Jun 30, 2022

lxbsz mentioned this pull request Jun 30, 2022

client: switch to 4KB block size if quota size is not aligned to 4MB #46867

Closed

14 tasks

lxbsz force-pushed the wip-quota branch 2 times, most recently from dcdc82b to 5b4a51d Compare June 30, 2022 08:24

lxbsz requested a review from a team as a code owner June 30, 2022 08:24

github-actions bot added the documentation label Jun 30, 2022

mchangir reviewed Jul 1, 2022

View reviewed changes

src/mds/Server.cc Outdated Show resolved Hide resolved

lxbsz force-pushed the wip-quota branch from 5b4a51d to 4adee4f Compare July 1, 2022 08:45

mchangir approved these changes Jul 1, 2022

View reviewed changes

kotreshhr approved these changes Jul 1, 2022

View reviewed changes

doc/cephfs/quota.rst Outdated Show resolved Hide resolved

lxbsz force-pushed the wip-quota branch from 4adee4f to d80098f Compare July 4, 2022 01:58

anthonyeleven requested changes Jul 4, 2022

View reviewed changes

doc/cephfs/quota.rst Outdated Show resolved Hide resolved

doc/cephfs/quota.rst Outdated Show resolved Hide resolved

lxbsz force-pushed the wip-quota branch 2 times, most recently from 1009aa7 to 9300826 Compare July 4, 2022 06:46

anthonyeleven approved these changes Jul 4, 2022

View reviewed changes

vshankar approved these changes Jul 8, 2022

View reviewed changes

mchangir approved these changes Jul 8, 2022

View reviewed changes

Madhu-1 mentioned this pull request Jul 12, 2022

cephfs: round to cephfs size to multiple of 4Mib ceph/ceph-csi#3241

Merged

vshankar added the wip-vshankar-testing1 label Jul 14, 2022

github-actions bot added the needs-rebase label Mar 8, 2023

lxbsz added 5 commits March 8, 2023 18:56

client: switch to use CEPH_4K_BLOCK_SHIFT macro for general case

001aa2c

Fixes: https://tracker.ceph.com/issues/56397 Signed-off-by: Xiubo Li <xiubli@redhat.com>

qa: use 4KB aligned quota bytes

fdf0e24

Fixes: https://tracker.ceph.com/issues/56397 Signed-off-by: Xiubo Li <xiubli@redhat.com>

pybind/cephfs: use 4KB aligned quota bytes

0d2d61f

Fixes: https://tracker.ceph.com/issues/56397 Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz force-pushed the wip-quota branch from a7e56a9 to 0d2d61f Compare March 8, 2023 10:57

github-actions bot removed the needs-rebase label Mar 8, 2023

rishabh-d-dave removed the needs-qa label Mar 31, 2023

rishabh-d-dave merged commit 989f7ec into ceph:main Mar 31, 2023

dparmar18 reviewed Apr 6, 2023

View reviewed changes

rptaylor mentioned this pull request Apr 14, 2023

mds,client,qa: misc fixes for quota and quota.max_bytes #50910

Closed

14 tasks

cbodley mentioned this pull request Jul 20, 2023

reef: test/pybind: replace nose with pytest #52566

Merged

vshankar mentioned this pull request Aug 25, 2023

Revert "mds: align quota.max_bytes to 4MB or 4KB" #53153

Merged

Conversation

lxbsz commented Jun 30, 2022

Contribution Guidelines

Checklist

Uh oh!

Uh oh!

kotreshhr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gregsfortytwo commented Jul 8, 2022

Uh oh!

vshankar left a comment

Choose a reason for hiding this comment

Uh oh!

kotreshhr commented Jul 12, 2022

Uh oh!

lxbsz commented Jul 12, 2022

Uh oh!

mchangir commented Jul 12, 2022

Uh oh!

Madhu-1 commented Jul 12, 2022

Uh oh!

lxbsz commented Feb 23, 2023

Uh oh!

github-actions bot commented Mar 8, 2023

Uh oh!

lxbsz commented Mar 8, 2023

Uh oh!

rishabh-d-dave commented Mar 31, 2023

Uh oh!

rishabh-d-dave commented Mar 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dparmar18 Apr 6, 2023

Choose a reason for hiding this comment

Uh oh!

vshankar commented Aug 25, 2023

Uh oh!

dparmar18 commented Aug 25, 2023

Uh oh!

lxbsz commented Aug 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vshankar commented Aug 26, 2023

Uh oh!

lxbsz commented Aug 26, 2023

Uh oh!

vshankar commented Aug 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

rishabh-d-dave commented Mar 31, 2023 •

edited

Loading

lxbsz commented Aug 26, 2023 •

edited

Loading