extstore: support direct I/O and async I/O #703

mita · 2020-07-31T15:01:15Z

This adds support for direct I/O and async I/O on extstore.

With -o ext_direct at startup, memcached opens the extstore files with
the O_DIRECT flag. When direct I/O is used, extstore reads and writes
bypass the operating system caches.

To build with async I/O support you will have to install io_uring library
(http://git.kernel.dk/cgit/liburing/) and ./configure --enable-liburing.
With -o ext_io_engine=io_uring and I/O depth > 1
(e.g. `-o ext_io_depth=64), the IO threads execute each IO object on their
queues in asynchronous manner.

This maximizes I/O throughput with smaller number of the IO threads when
direct I/O is used.

dormando · 2020-07-31T20:04:17Z

Thanks! This is fun, I've been wanting to do both of these things for a while but my priorities are elsewhere.

Might take a while to review due to time right now. Do you have any performance test results for the various modes, etc? Not a lot of comments in the patch and a lot of new things are going on in there.

With io_uring I was hoping to replace the entire event loop so extstore requests don't have to push to a separate thread anymore, but of course that's a lot more work.

mita · 2020-08-01T16:00:21Z

In order to make it easy to review, I plan to split this single change into at least three smaller changes.

Add extstore I/O engine infrastructure
Support direct I/O
Support async I/O

I usually do performance tests against extstore intensive read-only workloads.

# start memcached
memcached -t 16 -c 48000 -m 40000 -b 4096 \
  -o ext_wbuf_size=64,ext_item_size=240,ext_max_frag=0.80,hashpower=30,no_lru_crawler,ext_recache_rate=0,ext_item_age=5,ext_path=/mnt/ssd/extstore:200G
# mget benchmark (1000 mget, 8 io threads, 256 bytes value, 10000000 requests)
memstrike -d 127.0.0.1:11211 -m 1000 -t 8 -v 256 10000000

The ext_path is a file in ext4 file system on Intel SSD 900P.

With additional extended options -o ext_threads=4,ext_io_engine=io_uring,ext_direct,ext_io_depth=64
(direct I/O, async I/O, and four IO threads): 527712.951173 req/sec
This saturates the SSD bandwidth.

With additional extended options-o ext_threads=20,ext_io_engine=sync,ext_direct
(direct I/O, normal blocking I/O, and twenty IO threads): 483425.397672 req/sec

pkarumanchi9

thanks for doing this change-set. I've done one pass and left some comments for review.
The usage information is missing in memcached.c file. Can you add that as well?

pkarumanchi9 · 2020-08-01T23:18:20Z

extstore.c

+        if (e->ops->thread_init) {
+            *res = e->ops->thread_init(&e->io_threads[i]);
+            if (*res) {
+                free(e);


free e->pages and e->io_threads before returning NULL. Better yet introduce goto label and handle this there.

Fixed by commit 884e3b1
The change also releases e->wbuf_stack and e->io_stack.

pkarumanchi9 · 2020-08-01T23:18:26Z

extstore.c

+
+    if ((e->max_io_size % e->io_align) != 0 || !mc_powerof2(e->io_align)) {
+        *res = EXTSTORE_INIT_BAD_IO_SIZE;
+        free(e);


need to free e->pages before returning NULL

Fixed by commit 884e3b1

pkarumanchi9 · 2020-08-01T23:41:34Z

extstore.c

+
+    ret = io_uring_queue_init(e->io_depth, &priv->ring, 0);
+    if (ret < 0) {
+        E_DEBUG("io_uring_queue_init: %s\n", strerror(-ret));


free priv before returning

Fixed by commit 884e3b1

pkarumanchi9 · 2020-08-01T23:47:00Z

extstore.c

+        struct store_io_data *data = malloc(sizeof(*data));
+
+        if (e->direct) {
+            if (posix_memalign(&data->read_buf, e->io_align, e->max_io_size)) {


missing the following before returning EXTSTORE_INIT_OOM

io_uring_queue_exit(&priv->ring); // free data and data->read_buf that are added to priv->io_free list. free(priv);

Fixed by commit 884e3b1

pkarumanchi9 · 2020-08-02T00:04:02Z

extstore.c

+
+static void extstore_io_submit(store_io_thread *t, obj_io *io) {
+    if (t->e->ops->submit(t, io))
+        t->inflight++;


missing mutex lock and unlock around t->inflight++?

t->inflight is only updated by the thread `t', so it doesn't need to be protected by mutex.

pkarumanchi9 · 2020-08-02T00:05:50Z

extstore.c

+static void extstore_io_complete(store_io_thread *t) {
+    if (t->e->ops->complete) {
+        if (t->e->ops->complete(t))
+            t->inflight--;


missing mutex lock and unlock around t->inflight--?

Same as above.

pkarumanchi9 · 2020-08-02T00:26:47Z

extstore.c

+
+    extstore_io_done(t, io, ret);
+
+    return false;


this function returns false all the time and won't increment the inflight counter. Can you please add a comment to the struct store_io_thread that inflight is used for only async io.

Updated comment about inflight in struct store_io_thread:

unsigned int inflight; // number of inflight I/O requests, this is used only for async I/O

pkarumanchi9 · 2020-08-02T01:02:27Z

extstore.c

+    uint64_t iov_offset;
+    int fd;
+    void *read_buf;
+    struct iovec iov_oneshot;


can you clarify when do we use iov_oneshot vs array of iov

I've added comment to clarify:
/* this is used when io->iov is NULL or resubmitting after short read/write */

pkarumanchi9 · 2020-08-02T03:45:29Z

extstore.c

+    data = io_uring_cqe_get_data(cqe);
+
+    ret = cqe->res;
+    if (ret <= 0) {


shouldn't this be ret < 0?

Presumably 'ret == 0' can only happen when reading the end of file (e.g. the extstore is accidentally truncated).
So I would like to avoid infinite loop by this condition.

pkarumanchi9 · 2020-08-02T03:50:39Z

extstore.c

+    if (done) {
+        ret = data->io->len;
+    } else {
+        struct io_uring_sqe *sqe = io_uring_get_sqe(&priv->ring);


For -EAGAIN error on line 1166, can the request be tried again? - similar to lines 1185-1199, where we re-submit the partial request? Can you please clarify this part?

Fixed by commit f2328d4

tharanga · 2020-08-03T20:21:13Z

@mita we are excited about this change. What are the liburing and kernel versions you used?

mita · 2020-08-04T14:13:10Z

@mita we are excited about this change. What are the liburing and kernel versions you used?

I get the latest liburing (0.7+) and build from source on Ubuntu 18.04 LTS.
The kernel version is 5.4.0-42-generic.

tharanga · 2020-08-04T15:00:49Z

@mita we are excited about this change. What are the liburing and kernel versions you used?

I get the latest liburing (0.7+) and build from source on Ubuntu 18.04 LTS.
The kernel version is 5.4.0-42-generic.

Thanks! I saw the liburing is updated to match with the kernel 5.7 and wondering it's compatibility. Usually it's a good idea to stick with a version, so the behavior is deterministic. You may consider upgrading Travis builds to use focal/kernel 5.4 with liburing.

tharanga · 2020-08-04T15:10:01Z

configure.ac

@@ -228,6 +234,7 @@ AM_CONDITIONAL([ENABLE_SASL],[test "$enable_sasl" = "yes"])
 AM_CONDITIONAL([ENABLE_EXTSTORE],[test "$enable_extstore" != "no"])
 AM_CONDITIONAL([ENABLE_ARM_CRC32],[test "$enable_arm_crc32" = "yes"])
 AM_CONDITIONAL([ENABLE_TLS],[test "$enable_tls" = "yes"])
+AM_CONDITIONAL([ENABLE_LIBURING],[test "$enable_liburing" = "yes"])


If you haven't done already, it's a good idea to run extstore tests in the async mode. You may do something similar to TLS (https://github.com/memcached/memcached/blob/master/Makefile.am#L128), run all extstore tests with io_uring.

Sounds good.

Commit 5dee70a introduced a new environment variable which can be used to test with additional memcached server options.

dormando · 2020-08-07T07:30:40Z

What's memstrike from?

I'm still a few days behind being able to look closely. I like the idea of splitting up the patch; I'm very likely to take the direct io patch. I probably want to at least keep the io_uring one around for reference but I had different plans for it. A closer look will help me figure this out though. I feel like there are also too many tunables now.

For direct io at least, you'll probably see the biggest differences in performance under mixed read/write workloads. The performance could go either way.

mita · 2020-08-10T15:55:35Z

What's memstrike from?

The memstrike is a memcached benchmark tool which supports multiple server and mget, but does not support read/write mixed workloads.

https://github.com/frsyuki/memstrike

I have split up the direct IO and async IO patch into three patches.

  extstore: support direct I/O
  extstore: introduce extstore IO engine infrastructure
  extstore: support async I/O

dormando · 2020-10-27T21:21:58Z

Hey,

I fell off the earth for a while. This is a pretty involved change so I'm not going to get away with some quick testing :)

What would be nice if you or anyone else wants to, please do mixed read/write workload tests. Use mc-crusher(https://github.com/memcached/mc-crusher) and create some test configs to share here. You can use the test-suites code to design a quick suite of tests which will do ramp-to-failure and other interesting things. I've had to do all this (and more!) while working on larger changes like this.

The tunables I'd have to think about one by one; I'm trying pretty hard to not add more of those and keep sensible defaults. It's not quite obvious to me from a skim what those might be.

As far as plans go, I'm hoping to have an entire io_uring event loop. Then extstore requests can get executed internally to each worker and we avoid the sidethread push and multiple syscalls on pipes. This code still helps a lot as a point of reference at very least, and I hope I can take the direct io code still.

mita · 2020-11-24T15:07:25Z

I've created a test script based on test-suite-example.

https://github.com/mita/mc-crusher/blob/direct_io/test-suites/test-direct

In addition to the normal extstore test and RAM test without extstore,This script adds direct IO and direct IO + io_uring test configurations.

I agree that extstore requests with io_uring should not be pushed to a separate thread. I plan to work on this.

This adds support for direct I/O on extstore. With `-o ext_direct` at startup, memcached opens the extstore files with the O_DIRECT flag. When direct I/O is used, extstore reads and writes bypass the operating system caches.

This adds support for async I/O on extstore. To build with async I/O support you will have to install io_uring library (http://git.kernel.dk/cgit/liburing/) and ./configure --enable-liburing. With `-o ext_io_engine=io_uring` and I/O depth > 1 (e.g. `-o ext_io_depth=64), the worker threads directly read and write asynchronouly without the IO threads.

This adds a new MEMCACHED_EXTRA_ARGS environment variable for running tests with additional memcached server options. A test with direct I/O: MEMCACHED_EXTRA_ARGS="-o ext_direct" make test A test with direct I/O and async I/O: MEMCACHED_EXTRA_ARGS="-o ext_io_engine=io_uring,ext_direct,ext_io_depth=64" make test

mita · 2021-03-18T14:24:02Z

The latest version does not use the I/O threads when the io_uring engine is selected.
The worker threads directly submit I/O requests to extstore asynchronously.

mita · 2021-03-19T15:14:35Z

This table shows performance comparison between default sync engine and io_uring engine.
The io_uring engine gets better throughput (requets/sec) with smaller number of client threads.

client threads	2	4	8	12	16
sync engine	338768	712797	1145488	1351642	1323820
io_uring engine	455830	855805	1266225	1384047	1323683
sync engine, direct I/O	95181	177153	305078	419204	490403
io_uring engine, direct I/O	289145	481540	507430	511633	512510

Benchmark details:

Run memcached

./memcached/memcached -t 16 -m 8000 -o ext_item_age=0,ext_path=/mnt/ssd/extstore:200G $EXTRA_ARGS

The value of EXTRA_ARGS varies depending on the configuration.

sync engine: -o ext_io_engine=sync,ext_threads=16,ext_io_depth=64
io_uring engine: -o ext_io_engine=io_uring,ext_io_depth=64
sync engine, direct I/O: -o ext_io_engine=sync,ext_direct,ext_threads=16,ext_io_depth=64
io_uring engine, direct I/O: -o ext_io_engine=io_uring,ext_direct,ext_io_depth=64

The ext_path is a file in ext4 file system on Intel SSD 900P.

Load 10,000,000 records.

./memstrike/memstrike -d '127.0.0.1:11211' -t 16 -v 512 10000000 -s

Wait for extstore settle

./mc-crusher/bench-sample 127.0.0.1:11211 0 2 extstore_objects_written

Run read only workloads with different number of client threads

for i in 2 4 8 12 16; do
  ./memstrike/memstrike -d '127.0.0.1:11211' -m 10 -t $i -v 512 10000000 -g
done

dormando · 2021-04-17T19:32:06Z

Whoops I missed this update. Any idea why direct IO is so much slower?

mita · 2021-04-20T14:12:15Z

Whoops I missed this update. Any idea why direct IO is so much slower?

Thanks for looking at update.

The direct IO performance is limited by the random read IOPS.
These results are measured with a single Intel SSD 900P.
We'll get better numbers with using multiple devices for extstore.

dormando · 2022-08-29T02:15:22Z

@mita did you end up using this in a production context anywhere?

I've been both too busy and too-allocated to do the work necessary to validate this change, but that might be changing. I finally spent some time with io_uring and have more thoughts on it now as well.

lidaohang · 2023-02-23T03:57:39Z

mark

pkarumanchi9 suggested changes Aug 2, 2020

View reviewed changes

tharanga reviewed Aug 4, 2020

View reviewed changes

dormando force-pushed the next branch from 8c03512 to 31956fd Compare October 29, 2020 23:35

mita force-pushed the direct_io branch from 34b272d to c33dbde Compare November 5, 2020 14:42

mita added 4 commits March 18, 2021 22:14

extstore: introduce extstore IO engine infrastructure

f20c936

extstore: support direct I/O

0834a03

This adds support for direct I/O on extstore. With `-o ext_direct` at startup, memcached opens the extstore files with the O_DIRECT flag. When direct I/O is used, extstore reads and writes bypass the operating system caches.

mita force-pushed the direct_io branch from c33dbde to e36283f Compare March 18, 2021 13:39

dormando force-pushed the next branch from ad4b179 to 7d6907e Compare March 8, 2023 21:00

extstore: support direct I/O and async I/O #703

Are you sure you want to change the base?

extstore: support direct I/O and async I/O #703

Conversation

mita commented Jul 31, 2020

dormando commented Jul 31, 2020

mita commented Aug 1, 2020

pkarumanchi9 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tharanga commented Aug 3, 2020

mita commented Aug 4, 2020

tharanga commented Aug 4, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dormando commented Aug 7, 2020

mita commented Aug 10, 2020

dormando commented Oct 27, 2020

mita commented Nov 24, 2020

mita commented Mar 18, 2021

mita commented Mar 19, 2021

dormando commented Apr 17, 2021

mita commented Apr 20, 2021

dormando commented Aug 29, 2022

lidaohang commented Feb 23, 2023

tharanga commented Aug 4, 2020 •

edited