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

wip-hint #1284

Closed
wants to merge 5 commits into from
Closed

wip-hint #1284

wants to merge 5 commits into from

Conversation

idryomov
Copy link
Contributor

Sam has reviewed the ECTransaction* portion, Josh has reviewed the librbd portion and took a quick look at the bindings.

@@ -168,5 +170,5 @@ These are the rpm packages needed to install in an rpm-based OS:

For example:

$ yum install autoconf automake gcc gcc-c++ make libtool python-argparse python-flask libuuid-devel libblkid-devel keyutils-libs-devel cryptopp-devel nss-devel fcgi-devel expat-devel libcurl-devel fuse-devel gperftools-devel libedit-devel libatomic_ops-devel snappy-devel leveldb-devel libaio-devel boost-devel
$ yum install autoconf automake gcc gcc-c++ make libtool python-argparse python-flask libuuid-devel libblkid-devel keyutils-libs-devel cryptopp-devel nss-devel fcgi-devel expat-devel libcurl-devel xfsprogs-devel fuse-devel gperftools-devel libedit-devel libatomic_ops-devel snappy-devel leveldb-devel libaio-devel boost-devel
Copy link
Member

Choose a reason for hiding this comment

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

xfs{libs,progs}-dev should be added to debian/control and ceph.spec.in as build dependencies for the ceph package too

@jdurgin
Copy link
Member

jdurgin commented Feb 21, 2014

This looks good to me other than the packaging update. Nice work! Reviewed-by: Josh Durgin josh.durgin@inktank.com with that fixed.

@jdurgin
Copy link
Member

jdurgin commented Feb 21, 2014

@athanatos should probably check out the osd-side stuff too

@idryomov
Copy link
Contributor Author

I held up squashing the packaging commit, because @liewegas commented on the specific commit instead of the diff. I'll squash it and hit the merge button once everybody saw his comment and/if we are ready to merge.

__le64 expected_size;
__le64 expected_write_size;
__u8 expected_size_probability;
} __attribute__ ((packed)) hint;
Copy link
Member

Choose a reason for hiding this comment

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

Missed this before: we should s/hint/alloc_hint/ or similar so that it matches the op name and (in this case) isn't confused with other future hint types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Sun, Feb 23, 2014 at 5:59 PM, Sage Weil notifications@github.com wrote:

In src/include/rados.h:

@@ -463,6 +466,11 @@ struct ceph_osd_op {
struct {
u8 flags;
} __attribute
((packed)) tmap2omap;

  • struct {
  • __le64 expected_size;
  • __le64 expected_write_size;
  • __u8 expected_size_probability;
  • } attribute ((packed)) hint;

Missed this before: we should s/hint/alloc_hint/ or similar so that it
matches the op name and (in this case) isn't confused with other future hint
types.

Heh, it was spelled alloc_hint in the previous version.. Will change it back.

This is primarily for librbd/krbd's benefit and is supposed to combat
fragmentation:

"... knowing that rbd images have a 4m size, librbd can pass a hint
that will let the osd do the xfs allocation size ioctl on new files so
that they are allocated in 1m or 4m chunks.  We've seen cases where
users with rbd workloads have very high levels of fragmentation in xfs
and this would mitigate that and probably have a pretty nice
performance benefit."

SETALLOCHINT is considered advisory, so our backwards compatibility
mechanism here is to set FAILOK flag for all SETALLOCHINT ops.

xfs is hooked up in the subsequent commits.

Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
Refactor FS detection checks in FileStore::_detect_fs() so that they
look the same as the ones in FileStore::mkfs().  This is in preparation
for adding XfsFileStoreBackend class.

Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
Introduce XfsFileStoreBackend class, currently the only filestore
backend implementing SETALLOCHINT op.  This commit adds a build-time
dependency on libxfs as xfs-specific ioctl (XFS_IOC_FSSETXATTR /
XFS_XFLAG_EXTSIZE) is used to implement the new set_alloc_hint()
method.

Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
Add a new config option, filestore_max_alloc_hint_size, to cap
SETALLOCHINT hint size.  The unit is a byte, the default value is
1 megabyte.

Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
In an effort to reduce fragmentation, prefix every rbd write with
a CEPH_OSD_OP_SETALLOCHINT osd op with an expected_write_size value set
to the object size (1 << order).  Backwards compatibility is taken care
of on the osd side.

"The CEPH_OSD_OP_SETALLOCHINT hint is durable, in that it's enough to
do it once.  The reason every rbd write is prefixed is that rbd doesn't
explicitly create objects and relies on writes creating them
implicitly, so there is no place to stick a single hint op into.  To
get around that we decided to prefix every rbd write with a hint (just
like write and setattr ops, hint op will create an object implicitly if
it doesn't exist)."

Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
@idryomov
Copy link
Contributor Author

idryomov commented Mar 3, 2014

Merged into firefly branch, closing.

@idryomov idryomov closed this Mar 3, 2014
@idryomov idryomov deleted the wip-hint branch March 4, 2014 12:12
liewegas added a commit to liewegas/ceph that referenced this pull request Dec 7, 2016
drop broken name length config args
smithfarm added a commit to SUSE/ceph that referenced this pull request Aug 21, 2018
After running Stages 0-4 to deploy RGW, "zypper ps -s" shows ceph-radosgw needs restart
SUSE/DeepSea#1284

To work around this, restart the RGW service before running the functional tests.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit to SUSE/ceph that referenced this pull request Aug 21, 2018
After running Stages 0-4 to deploy RGW, "zypper ps -s" shows ceph-radosgw needs restart
SUSE/DeepSea#1284

To work around this, restart the RGW service before running the functional tests.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
(cherry picked from commit 644ba74)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants