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

os/filestore: queue ondisk completion before apply work #13918

Merged
merged 1 commit into from Jul 5, 2017

Conversation

Projects
None yet
7 participants
@liupan1111
Contributor

liupan1111 commented Mar 10, 2017

…and improve write op performance.

Signed-off-by: Pan Liu liupan1111@gmail.com

os/filestore: move ondisk in front, so that return oncommit earlier, …
…and improve write op performance.

Signed-off-by: Pan Liu <liupan1111@gmail.com>

@liupan1111 liupan1111 requested a review from liewegas Mar 10, 2017

@liupan1111

This comment has been minimized.

Contributor

liupan1111 commented Mar 11, 2017

@ceph-jenkins retest it please.

@shinobu-x

This comment has been minimized.

Contributor

shinobu-x commented Mar 11, 2017

retest this please

@liupan1111

This comment has been minimized.

Contributor

liupan1111 commented Mar 11, 2017

@liewegas , please help take a look, thanks!

@liupan1111

This comment has been minimized.

Contributor

liupan1111 commented Mar 14, 2017

@liewegas or @tchaikov , ping.

@liupan1111

This comment has been minimized.

Contributor

liupan1111 commented Mar 16, 2017

@dzafman , could help take a look? Thanks.

@xinxinsh

This comment has been minimized.

Member

xinxinsh commented Mar 17, 2017

how much performance improvement can be achieved

@liupan1111

This comment has been minimized.

Contributor

liupan1111 commented Mar 17, 2017

@xinxinsh not big, only few percent, but this change seems reasonable to me.

@dzafman

This comment has been minimized.

Member

dzafman commented Apr 11, 2017

@liupan1111 I have no idea if the reordering would break something or not.

@markhpc

This comment has been minimized.

Member

markhpc commented Apr 12, 2017

Ordering changes in filestore make me nervous. Channeling my inner-Sam, I think we need a really good reason to muck around with it. A few percent probably isn't enough to take on the risk, at least until bluestore is in production.

@liupan1111

This comment has been minimized.

Contributor

liupan1111 commented Apr 13, 2017

@markhpc, yeah... I agree, I will close it.

@liupan1111 liupan1111 closed this Apr 13, 2017

@liupan1111 liupan1111 reopened this Jun 15, 2017

@liupan1111 liupan1111 closed this Jun 15, 2017

@liupan1111 liupan1111 reopened this Jun 15, 2017

@liupan1111

This comment has been minimized.

Contributor

liupan1111 commented Jun 15, 2017

@markhpc
I made a test today: three hosts with each one has four NVME SSD, pool size = 3.
Fio configuration:
fio --ioengine=rbd --pool=test_pool --clientname=admin --rbdname=image --direct=1 --thread --rw=randwrite -numjobs=1 --iodepth=256 --bs=16k --group_reporting --name=test --time_based --runtime=600
Before change:
image
After change:
image
Seems there is an improvement of nearly 20%. Please correct me if you have any opinion about this.

@liupan1111

This comment has been minimized.

Contributor

liupan1111 commented Jun 18, 2017

@liewegas What do you think about this change?

@liupan1111

This comment has been minimized.

Contributor

liupan1111 commented Jun 27, 2017

retest this please

@liewegas liewegas added the needs-qa label Jun 27, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 27, 2017

This is so simple and should be safe. Let's test!

@liewegas liewegas changed the title from os/filestore: move ondisk in front, so that return oncommit earlier, … to os/filestore: queue ondisk completion before apply work Jun 27, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 1, 2017

@tchaikov tchaikov merged commit ab06dd4 into ceph:master Jul 5, 2017

5 of 6 checks passed

arm64 make check arm64 make check failed
Details
Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
Unmodified Submodules submodules for project are unmodified
Details
default Build finished.
Details
make check make check succeeded
Details
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 6, 2017

could be the cause of http://tracker.ceph.com/issues/20524, i am looking at it.

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