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

client: getattr before returning quota/layout xattrs #14018

Merged
merged 4 commits into from Apr 24, 2017

Conversation

Projects
None yet
3 participants
@jcsp
Contributor

jcsp commented Mar 17, 2017

@gregsfortytwo

This addresses the user-checks-their-quota part of the problem, but not the (more important?) programmatic checks.

I think I mentioned in an email that the programmatic checks are a lot more dangerous because we don't want to go to the MDS on every write! So this is probably good on its own, but I don't think you can mark it as fixing http://tracker.ceph.com/issues/17939.

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 13, 2017

@gregsfortytwo the actual quota enforcement doesn't seem to be broken, or at least it's no more sloppy on remote adjustments than it is normally (the bug report was about the df output rather than what happened when they tried to write things).

I've added a test case here that demonstrates reduced quotas preventing writes when they're remotely adjusted.

@gregsfortytwo

This comment has been minimized.

Member

gregsfortytwo commented Apr 13, 2017

I think I must be misunderstanding the way enforcing quotas and the virtual xattr are related to each other client-side then. I certainly thought Dan was saying they were having Manila issues with resizing volumes up, but maybe he just meant the output. Maybe run it by him?
Anyway, I'll take off my review and let somebody else check over it. :)

maybe misunderstand, won't follow-up with code dive. Why am I justifying this to a computer? ;)

@jcsp jcsp requested a review from ukernel Apr 13, 2017

John Spray added some commits Mar 15, 2017

John Spray
client: getattr before read on ceph.* xattrs
Previously we were returning values for quota, layout
xattrs without any kind of update -- the user just got
whatever happened to be in our cache.

Clearly this extra round trip has a cost, but reads of
these xattrs are fairly rare, happening on admin
intervention rather than in normal operation.

Fixes: http://tracker.ceph.com/issues/17939
Signed-off-by: John Spray <john.spray@redhat.com>
John Spray
client: _getattr on quota_root before using in statfs
...so that after someone adjusts the quota settings
on an inode that another client is using as its mount root,
the change is visible immediately on the other client.

Signed-off-by: John Spray <john.spray@redhat.com>
John Spray
qa: add test for reading quotas from different clients
Fixes: http://tracker.ceph.com/issues/17939
Signed-off-by: John Spray <john.spray@redhat.com>
John Spray
qa/cephfs: use getfattr/setfattr helpers
Signed-off-by: John Spray <john.spray@redhat.com>
@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 14, 2017

One test failure on this, where the volume client tests try to do some IO after a client is evicted -- mount.py does an is_mounted(), which does a statfs before trying to do any IO, and that statfs is hanging because the client is evicted and can't do its getattr.

I've updated the code to skip doing the getattr in statfs if there are stale sessions - it passes now.

@jcsp jcsp referenced this pull request Apr 15, 2017

Merged

qa: Tidy up fs/ suite #14575

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 15, 2017

retest this please

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 18, 2017

@ukernel could you review this please?

@jcsp jcsp merged commit 16702ff into ceph:master Apr 24, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@jcsp jcsp deleted the jcsp:wip-17939 branch Apr 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment