Skip to content
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

hammer: osd: smaller object_info_t xattrs #6544

Merged
merged 4 commits into from Dec 17, 2015

Conversation

liewegas
Copy link
Member

http://tracker.ceph.com/issues/14803

Avoid later bits of object_info_t encoding ot reduce the xattr size, so that it can be inlined by xfs (< 255 bytes).

We lose:

  • local_mtime (degrades cache efficiency)
  • omap digest when there is no omap (it's useless in that case anyway, and always -1)

@liewegas liewegas changed the title asdf osd: smaller object_info_t xattrs (hammer) Nov 12, 2015
@liewegas liewegas added this to the hammer milestone Nov 12, 2015
@liewegas liewegas changed the title osd: smaller object_info_t xattrs (hammer) DNM: osd: smaller object_info_t xattrs (hammer) Nov 12, 2015
@liewegas liewegas force-pushed the wip-smaller-object-info branch 2 times, most recently from c2ced2e to 17a217c Compare November 12, 2015 02:43
@athanatos
Copy link
Contributor

This seems reasonable to me, but should go into master first?

@liewegas
Copy link
Member Author

liewegas commented Nov 16, 2015 via email

@ghost ghost assigned ghost and unassigned athanatos Nov 19, 2015
@ghost
Copy link

ghost commented Nov 19, 2015

@liewegas could you please rebase & repush ?

@ghost
Copy link

ghost commented Dec 14, 2015

@liewegas would you mind rebasing & repushing to get a new run of the bot ?

!has_flag(DATA_DIGEST) &&
watchers.empty()) { // no watchers
// note: we dropped local_mtime too, even though it may have been used.
ev = 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

assert omap_digest == -1 and data_digest == -1?

@liewegas liewegas force-pushed the wip-smaller-object-info branch 7 times, most recently from 7f2d967 to aff84b4 Compare December 14, 2015 18:17
We want to avoid encoding it if we can.  And if the FLAG_OMAP is not set
we don't need to *also* store an empty crc.

Signed-off-by: Sage Weil <sage@redhat.com>
If we don't have a local_mtime value, use mtime instead, for the purposes
of deciding if we should record a digest after scrub.

Signed-off-by: Sage Weil <sage@redhat.com>
If a pool isn't tiered, don't bother with setting local_mtime.  The only
users are the tiering agent (which isn't needed if there is not tiering)
and scrub for deciding if an object should get its digest recorded (we can
use mtime instead).

Signed-off-by: Sage Weil <sage@redhat.com>
…nused

This reduces the size of the encoded object_info_t in most cases,
enough to get us under the 255 byte limit for a single inline
xattr in XFS.

Signed-off-by: Sage Weil <sage@redhat.com>
@athanatos
Copy link
Contributor

I think this looks right

liewegas added a commit that referenced this pull request Dec 17, 2015
osd: make encoded object_info_t smaller to fit inside the XFS inode

Reviewed-by: Samuel Just <sjust@redhat.com>
@liewegas liewegas merged commit 9739d4d into ceph:hammer Dec 17, 2015
@liewegas
Copy link
Member Author

Still need to run upgrade/hammer-x test; do not release or take this downstream yet.

@liewegas liewegas deleted the wip-smaller-object-info branch December 17, 2015 15:17
@ktdreyer
Copy link
Member

Ok do you want to update this PR to communicate the status?

@liewegas
Copy link
Member Author

liewegas commented Dec 17, 2015 via email

@liewegas
Copy link
Member Author

@ktdreyer ktdreyer changed the title DNM: osd: smaller object_info_t xattrs (hammer) osd: smaller object_info_t xattrs (hammer) Dec 18, 2015
@ghost ghost changed the title osd: smaller object_info_t xattrs (hammer) hammer: osd: smaller object_info_t xattrs Feb 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants