* RBD client cache
@ 2015-11-19 15:37 Jens Rosenboom
2015-11-19 20:31 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Jens Rosenboom @ 2015-11-19 15:37 UTC (permalink / raw)
To: fio
A couple of releases ago, ceph has changed the default value for
rbd cache writethrough until flush
from false to true. As the fio rbd backend currently does not emit the
necessary fsync to trigger writeback mode, this implies that tests
will run in writethrough, possibly giving a significantly reduced
performance when compared to other clients, e.g. when running fio
within a qemu-based instance accessing the same RBD volume.
Would it make sense for fio to simply issue a rbd_flush after each
rbd_open at https://github.com/axboe/fio/blob/master/engines/rbd.c#L138
? Or do you think that this would need a dedicated flag to be enabled?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RBD client cache
2015-11-19 15:37 RBD client cache Jens Rosenboom
@ 2015-11-19 20:31 ` Jens Axboe
2015-11-26 15:51 ` Jens Rosenboom
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2015-11-19 20:31 UTC (permalink / raw)
To: Jens Rosenboom, fio
On 11/19/2015 08:37 AM, Jens Rosenboom wrote:
> A couple of releases ago, ceph has changed the default value for
>
> rbd cache writethrough until flush
>
> from false to true. As the fio rbd backend currently does not emit the
> necessary fsync to trigger writeback mode, this implies that tests
> will run in writethrough, possibly giving a significantly reduced
> performance when compared to other clients, e.g. when running fio
> within a qemu-based instance accessing the same RBD volume.
>
> Would it make sense for fio to simply issue a rbd_flush after each
> rbd_open at https://github.com/axboe/fio/blob/master/engines/rbd.c#L138
> ? Or do you think that this would need a dedicated flag to be enabled?
I'd probably make that an rbd engine option, ala flush_on_open or
something like that.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RBD client cache
2015-11-19 20:31 ` Jens Axboe
@ 2015-11-26 15:51 ` Jens Rosenboom
2015-11-27 16:06 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Jens Rosenboom @ 2015-11-26 15:51 UTC (permalink / raw)
To: fio
From dbd950891a43a7104bce5ec935cd992c7e2b27ff Mon Sep 17 00:00:00 2001
From: Jens Rosenboom <j.rosenboom@x-ion.de>
Date: Thu, 26 Nov 2015 16:27:49 +0100
Subject: [PATCH] Add an option to flush RBD after opening
With recent Ceph releases the default value for
rbd cache writethrough until flush
has been changed to true, so the librbd client cache will stay in
writethrough mode for fio.
This patch adds an option to perform an rbd_flush() call after opening
the RBD, causing the client cache to activate writeback mode.
Signed-off-by: Jens Rosenboom <j.rosenboom@x-ion.de>
---
This works in my test enviroment and boosts 4k randwrites at iodepth=1 from 100 to 6000.
Still it would probably be good if other people could test this, too.
engines/rbd.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/engines/rbd.c b/engines/rbd.c
index 2be9b55..b3efedf 100644
--- a/engines/rbd.c
+++ b/engines/rbd.c
@@ -30,6 +30,7 @@ struct rbd_options {
char *pool_name;
char *client_name;
int busy_poll;
+ int flush_on_open;
};
static struct fio_option options[] = {
@@ -71,6 +72,16 @@ static struct fio_option options[] = {
.group = FIO_OPT_G_RBD,
},
{
+ .name = "flush_on_open",
+ .lname = "Flush on open",
+ .type = FIO_OPT_BOOL,
+ .help = "Flush on opening the RBD to activate client cache",
+ .off1 = offsetof(struct rbd_options, flush_on_open),
+ .def = "0",
+ .category = FIO_OPT_C_ENGINE,
+ .group = FIO_OPT_G_RBD,
+ },
+ {
.name = NULL,
},
};
@@ -140,6 +151,13 @@ static int _fio_rbd_connect(struct thread_data *td)
log_err("rbd_open failed.\n");
goto failed_open;
}
+ if (o->flush_on_open) {
+ r = rbd_flush(rbd->image);
+ if (r < 0) {
+ log_err("rbd_flush failed.\n");
+ goto failed_open;
+ }
+ }
return 0;
failed_open:
--
2.4.10
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: RBD client cache
2015-11-26 15:51 ` Jens Rosenboom
@ 2015-11-27 16:06 ` Jens Axboe
2015-12-08 13:41 ` Jens Rosenboom
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2015-11-27 16:06 UTC (permalink / raw)
To: Jens Rosenboom, fio
On 11/26/2015 08:51 AM, Jens Rosenboom wrote:
> From dbd950891a43a7104bce5ec935cd992c7e2b27ff Mon Sep 17 00:00:00 2001
> From: Jens Rosenboom <j.rosenboom@x-ion.de>
> Date: Thu, 26 Nov 2015 16:27:49 +0100
> Subject: [PATCH] Add an option to flush RBD after opening
>
> With recent Ceph releases the default value for
>
> rbd cache writethrough until flush
>
> has been changed to true, so the librbd client cache will stay in
> writethrough mode for fio.
>
> This patch adds an option to perform an rbd_flush() call after opening
> the RBD, causing the client cache to activate writeback mode.
>
> Signed-off-by: Jens Rosenboom <j.rosenboom@x-ion.de>
> ---
>
> This works in my test enviroment and boosts 4k randwrites at iodepth=1 from 100 to 6000.
> Still it would probably be good if other people could test this, too.
>
>
> engines/rbd.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/engines/rbd.c b/engines/rbd.c
> index 2be9b55..b3efedf 100644
> --- a/engines/rbd.c
> +++ b/engines/rbd.c
> @@ -30,6 +30,7 @@ struct rbd_options {
> char *pool_name;
> char *client_name;
> int busy_poll;
> + int flush_on_open;
> };
>
> static struct fio_option options[] = {
> @@ -71,6 +72,16 @@ static struct fio_option options[] = {
> .group = FIO_OPT_G_RBD,
> },
> {
> + .name = "flush_on_open",
> + .lname = "Flush on open",
> + .type = FIO_OPT_BOOL,
> + .help = "Flush on opening the RBD to activate client cache",
> + .off1 = offsetof(struct rbd_options, flush_on_open),
> + .def = "0",
> + .category = FIO_OPT_C_ENGINE,
> + .group = FIO_OPT_G_RBD,
> + },
> + {
> .name = NULL,
> },
> };
> @@ -140,6 +151,13 @@ static int _fio_rbd_connect(struct thread_data *td)
> log_err("rbd_open failed.\n");
> goto failed_open;
> }
> + if (o->flush_on_open) {
> + r = rbd_flush(rbd->image);
> + if (r < 0) {
> + log_err("rbd_flush failed.\n");
> + goto failed_open;
> + }
> + }
> return 0;
Looks good, but we should probably limit this to td_write() being true,
and not do this for read-only opens.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RBD client cache
2015-11-27 16:06 ` Jens Axboe
@ 2015-12-08 13:41 ` Jens Rosenboom
2015-12-08 15:06 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Jens Rosenboom @ 2015-12-08 13:41 UTC (permalink / raw)
To: Jens Axboe, Fio
2015-11-27 17:06 GMT+01:00 Jens Axboe <axboe@kernel.dk>:
> On 11/26/2015 08:51 AM, Jens Rosenboom wrote:
>>
>> From dbd950891a43a7104bce5ec935cd992c7e2b27ff Mon Sep 17 00:00:00 2001
>> From: Jens Rosenboom <j.rosenboom@x-ion.de>
>> Date: Thu, 26 Nov 2015 16:27:49 +0100
>> Subject: [PATCH] Add an option to flush RBD after opening
>>
>> With recent Ceph releases the default value for
>>
>> rbd cache writethrough until flush
>>
>> has been changed to true, so the librbd client cache will stay in
>> writethrough mode for fio.
>>
>> This patch adds an option to perform an rbd_flush() call after opening
>> the RBD, causing the client cache to activate writeback mode.
>>
>> Signed-off-by: Jens Rosenboom <j.rosenboom@x-ion.de>
>> ---
>>
>> This works in my test enviroment and boosts 4k randwrites at iodepth=1
>> from 100 to 6000.
>> Still it would probably be good if other people could test this, too.
>>
>>
>> engines/rbd.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/engines/rbd.c b/engines/rbd.c
>> index 2be9b55..b3efedf 100644
>> --- a/engines/rbd.c
>> +++ b/engines/rbd.c
>> @@ -30,6 +30,7 @@ struct rbd_options {
>> char *pool_name;
>> char *client_name;
>> int busy_poll;
>> + int flush_on_open;
>> };
>>
>> static struct fio_option options[] = {
>> @@ -71,6 +72,16 @@ static struct fio_option options[] = {
>> .group = FIO_OPT_G_RBD,
>> },
>> {
>> + .name = "flush_on_open",
>> + .lname = "Flush on open",
>> + .type = FIO_OPT_BOOL,
>> + .help = "Flush on opening the RBD to activate
>> client cache",
>> + .off1 = offsetof(struct rbd_options,
>> flush_on_open),
>> + .def = "0",
>> + .category = FIO_OPT_C_ENGINE,
>> + .group = FIO_OPT_G_RBD,
>> + },
>> + {
>> .name = NULL,
>> },
>> };
>> @@ -140,6 +151,13 @@ static int _fio_rbd_connect(struct thread_data *td)
>> log_err("rbd_open failed.\n");
>> goto failed_open;
>> }
>> + if (o->flush_on_open) {
>> + r = rbd_flush(rbd->image);
>> + if (r < 0) {
>> + log_err("rbd_flush failed.\n");
>> + goto failed_open;
>> + }
>> + }
>> return 0;
>
>
> Looks good, but we should probably limit this to td_write() being true, and
> not do this for read-only opens.
I'm not sure that it will be helpful to make this option more
complicated in behaviour and documentation. I agree that currently it
will only be useful for write jobs, but it should not do any harm for
others. And maybe in the future the behaviour of librbd will also
change for read operations, rather than having to add yet another flag
then, I'd vote for KISS.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RBD client cache
2015-12-08 13:41 ` Jens Rosenboom
@ 2015-12-08 15:06 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2015-12-08 15:06 UTC (permalink / raw)
To: Jens Rosenboom, Fio
On 12/08/2015 06:41 AM, Jens Rosenboom wrote:
> 2015-11-27 17:06 GMT+01:00 Jens Axboe <axboe@kernel.dk>:
>> On 11/26/2015 08:51 AM, Jens Rosenboom wrote:
>>>
>>> From dbd950891a43a7104bce5ec935cd992c7e2b27ff Mon Sep 17 00:00:00 2001
>>> From: Jens Rosenboom <j.rosenboom@x-ion.de>
>>> Date: Thu, 26 Nov 2015 16:27:49 +0100
>>> Subject: [PATCH] Add an option to flush RBD after opening
>>>
>>> With recent Ceph releases the default value for
>>>
>>> rbd cache writethrough until flush
>>>
>>> has been changed to true, so the librbd client cache will stay in
>>> writethrough mode for fio.
>>>
>>> This patch adds an option to perform an rbd_flush() call after opening
>>> the RBD, causing the client cache to activate writeback mode.
>>>
>>> Signed-off-by: Jens Rosenboom <j.rosenboom@x-ion.de>
>>> ---
>>>
>>> This works in my test enviroment and boosts 4k randwrites at iodepth=1
>>> from 100 to 6000.
>>> Still it would probably be good if other people could test this, too.
>>>
>>>
>>> engines/rbd.c | 18 ++++++++++++++++++
>>> 1 file changed, 18 insertions(+)
>>>
>>> diff --git a/engines/rbd.c b/engines/rbd.c
>>> index 2be9b55..b3efedf 100644
>>> --- a/engines/rbd.c
>>> +++ b/engines/rbd.c
>>> @@ -30,6 +30,7 @@ struct rbd_options {
>>> char *pool_name;
>>> char *client_name;
>>> int busy_poll;
>>> + int flush_on_open;
>>> };
>>>
>>> static struct fio_option options[] = {
>>> @@ -71,6 +72,16 @@ static struct fio_option options[] = {
>>> .group = FIO_OPT_G_RBD,
>>> },
>>> {
>>> + .name = "flush_on_open",
>>> + .lname = "Flush on open",
>>> + .type = FIO_OPT_BOOL,
>>> + .help = "Flush on opening the RBD to activate
>>> client cache",
>>> + .off1 = offsetof(struct rbd_options,
>>> flush_on_open),
>>> + .def = "0",
>>> + .category = FIO_OPT_C_ENGINE,
>>> + .group = FIO_OPT_G_RBD,
>>> + },
>>> + {
>>> .name = NULL,
>>> },
>>> };
>>> @@ -140,6 +151,13 @@ static int _fio_rbd_connect(struct thread_data *td)
>>> log_err("rbd_open failed.\n");
>>> goto failed_open;
>>> }
>>> + if (o->flush_on_open) {
>>> + r = rbd_flush(rbd->image);
>>> + if (r < 0) {
>>> + log_err("rbd_flush failed.\n");
>>> + goto failed_open;
>>> + }
>>> + }
>>> return 0;
>>
>>
>> Looks good, but we should probably limit this to td_write() being true, and
>> not do this for read-only opens.
>
> I'm not sure that it will be helpful to make this option more
> complicated in behaviour and documentation. I agree that currently it
> will only be useful for write jobs, but it should not do any harm for
> others. And maybe in the future the behaviour of librbd will also
> change for read operations, rather than having to add yet another flag
> then, I'd vote for KISS.
I'm all for KISS, but issuing a flush on a read-only opened device is
not a great idea, period. And since the flush-to-enable-write-cache only
has an effect on writes, we should only do it for writes.
We should also rename the option to "write_cache_enable" or something
like that, flush_on_open tells us what a horrible interface rbd is, not
what the option actually does. The fact that this is the control
interface for write caching should not be imposed on the user, since
it's crazy. When you do that rename, the fact that it should only have
an effect on a writeable workload becomes more apparent.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-08 15:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-19 15:37 RBD client cache Jens Rosenboom
2015-11-19 20:31 ` Jens Axboe
2015-11-26 15:51 ` Jens Rosenboom
2015-11-27 16:06 ` Jens Axboe
2015-12-08 13:41 ` Jens Rosenboom
2015-12-08 15:06 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox