Can we use something like the file plugin? thread pool of workers, each keeping open vddk handle, and serving requests in parallel from the same nbd socket?

This is kind of ugly but simple, and it works great for the file plugin - we get better performance than qemu-nbd. But since we get low throughput even when we have 10 concurrent handles for 10 different disks, I'm sure this will help, and the issue may be deeper in vmware. Maybe they intentionally throttle the clients?

On Wed, Aug 5, 2020 at 2:58 PM Richard W.M. Jones <rjones(a)redhat.com&gt; wrote: > > On Wed, Aug 05, 2020 at 02:39:44PM +0300, Nir Soffer wrote: > > Can we use something like the file plugin? thread pool of workers, > > each keeping open vddk handle, and serving requests in parallel from > > the same nbd socket? > > Yes, but this isn't implemented in the plugins, it's implemented in > the server. The server always uses a thread pool, but plugins can opt > for more or less concurrency by adjusting the thread model: > > http://libguestfs.org/nbdkit-plugin.3.html#Threads > > The file plugin uses PARALLEL: > > $ nbdkit file --dump-plugin | grep thread > max_thread_model=parallel > thread_model=parallel > > The VDDK plugin currently uses SERIALIZE_ALL_REQUESTS: > > $ nbdkit vddk --dump-plugin | grep thread > max_thread_model=serialize_all_requests > thread_model=serialize_all_requests > > The proposal is to use SERIALIZE_REQUESTS, with an extra mutex added > by the plugin around VixDiskLib_Open and _Close calls. I'm not sure what is the difference between SERIALIZE_REQUESTS and SERIALIZE_ALL_REQUESTS,

but it sounds to me like we need PARALLEL. With parallel we will have multiple threads using the same vddk_handle, which can be thread safe since we control this struct. The struct can be: struct vddk_item { VixDiskLibConnectParams *params; /* connection parameters */ VixDiskLibConnection connection; /* connection */ VixDiskLibHandle handle; /* disk handle */ struct vddk_item *next; /* next handle in the list */ } struct vddk_handle { struct vddm_items *pool; pthread_mutex_t *mutex; } open() will initialize the pool of vddk_item. pread() will: - lock the mutex - take an item from the pool - unlock the mutex - perform a single request - lock the mutex - return the item to the pool - unlock the mutex Since we don't need lot of connections, and most of the time is spent waiting on I/O, the time to lock/unlock the pool should not be significant. The server thread pool should probably use the same size of the pool, so there is always a free item in the pool for every thread. Or maybe something simpler, every thread will create a vddk_item and keep it in thread local storage, no locking required (maybe only for open and close). Do you see any reason why this will not work?

I see, can change the python plugin to support multiple connections to imageio using SERIALIZE_REQUESTS? The GiL should not limit us since the GIL is released when you write to imageio socket, and this is likely where the plugin spends most of the time.

I'm not sure what triggers using multiple connections in qemu-img and how it decide how many connections should be used, but we report the number of writers in OPTIONS: http://ovirt.github.io/ovirt-imageio/images.html#max_writers There is a hard limit in vdsm, because it runs qemu-nbd with --shared=8, so you should not use more than 8 connections, they will just block on qemu-nbd forever.

We use 4 connections by default, giving about 100% speed up compared with one connection. 2 connections give about 80% speed up. If the number of connections is related to the number of coroutines, you can use -m 4 to use 4 coroutines. Using -W will improve performance. In this mode every coroutine will do the I/O when it is ready, instead of waiting for other coroutines and submit the I/O in the right order.

On Wed, Aug 5, 2020 at 5:10 PM Richard W.M. Jones <rjones(a)redhat.com&gt; wrote: > > On Wed, Aug 05, 2020 at 04:49:04PM +0300, Nir Soffer wrote: > > I see, can change the python plugin to support multiple connections to imageio > > using SERIALIZE_REQUESTS? > > > > The GiL should not limit us since the GIL is released when you write to > > imageio socket, and this is likely where the plugin spends most of the time. > > It's an interesting question and one I'd not really considered at all. > Does the Python GIL actively mutex different threads if they call into > Python code at the same time? Yes, only one thread can run python code at the same time, there is no way around this. But before calling most syscalls, python releases the GIL. When the syscall returns, python acquires the GIL again. But thread can be switched at any point in time, so must use locking in the python if you modify or access shared state. > If it's truly a lock, then it should, > in which case it should be safe to change the Python plugin to > PARALLEL ... Using multiple threads on the same http socket will not work, you must have multiple sockets to imageio.

> I'll try it out and get back to you. > > > I'm not sure what triggers using multiple connections in qemu-img and > > how it decide how many connections should be used, but we report > > the number of writers in OPTIONS: > > http://ovirt.github.io/ovirt-imageio/images.html#max_writers > > > > There is a hard limit in vdsm, because it runs qemu-nbd with > > --shared=8, so you should > > not use more than 8 connections, they will just block on qemu-nbd forever. > > It's different for qemu NBD client and server. Eric told me on IRC a > few minutes ago that qemu NBD client does not "yet" support > multi-conn. However it is implemented by qemu-nbd (the server) for > readonly connections. > > Note that multi-conn and --shared are (a bit) different. Multi-conn > is a flag which the server can send to clients to indicate not only > that multiple connections are possible, but also that they are come > with certain guarantees: > > bit 8, NBD_FLAG_CAN_MULTI_CONN: Indicates that the server operates > entirely without cache, or that the cache it uses is shared among > all connections to the given device. In particular, if this flag is > present, then the effects of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA MUST > be visible across all connections when the server sends its reply to > that command to the client. In the absence of this flag, clients > SHOULD NOT multiplex their commands over more than one connection to > the export. > > “--shared” limits the number of connections that the server will > accept at the same time (like the nbdkit limit filter). But it would > still be possible for an NBD server to accept multiple connections > from a single client but not be multi-conn safe. > > Also NBD lets you multiplex commands on a single connection (which > does not require multi-conn or --shared). > > BTW I found that multi-conn is a big win with the Linux kernel NBD > client. > > > We use 4 connections by default, giving about 100% speed up compared > > with one connection. 2 connections give about 80% speed up. If the > > number of connections is related to the number of coroutines, you > > can use -m 4 to use 4 coroutines. > > > > Using -W will improve performance. In this mode every coroutine will > > do the I/O when it is ready, instead of waiting for other coroutines > > and submit the I/O in the right order. > > I think Eric might have a better idea about what -m and -W really do > for qemu NBD client. Maybe improve multiplexing? They don't enable > multi-conn :-( > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > virt-df lists disk usage of guests without needing to install any > software inside the virtual machine. Supports Linux and Windows. > http://people.redhat.com/~rjones/virt-df/ >

Correct.  Using -W doesn't make sense without -m (if you only have one worker, you might as well proceed linearly than trying to randomize access, but even when you have multiple threads, there are cases where linear operations are still useful, such as 'nbdkit streaming'.  But -m is definitely the knob that controls how many outstanding I/O requests qemu-img is willing to use; and once you are using -m, using -W makes life easier for those coroutines to stay active.  The default -m1 says that at most one request is outstanding, so parallelism in the server is not utilized.  With higher -m, qemu-img issues up to that many requests without waiting for server answers, but all on the same NBD connection. Ideally, you'll get the maximum behavior as 'qemu-img -m' and 'nbdkit --threads' choose the same values; if either side has fewer in-flight operations permitted than the other, then that side has the potential to become a bottleneck.  Right now, nbdkit defaults to 16 threads (that is, up to 16 in-flight operations) for any PARALLEL plugin.

On Wed, Aug 5, 2020 at 3:28 PM Richard W.M. Jones <rjones(a)redhat.com&gt; wrote: > > > Nir, BTW what are you using for performance testing? virt-v2v with local image, or imageio client with local image. > As far as I can tell it's not possible to make qemu-img convert use > multi-conn when connecting to the source (which is going to be a > problem if we want to use this stuff in virt-v2v). But do we need multiple connections? qemu can send multiple requests on one connection.

Did you try to copy an image from nbdkit file plugin to another nbdkit file plugin using qemu-img convert? nbdkit file pluing -> qemu-img convert -W nbd:///?socket=src.sock nbd:///?socket=dst.sock-> nbdkit file plugin I did not try it but I will be surprised if we don't get all 8 threads busy in both sides

The reason we use multiple connection in imageio is that we don't support async I/O in http client, http server, and nbd client, and it is much easier to open new connection with the entire stuck compared to rewriting the http server and nbd client. It also much hard to provide easy to use interface for users supporting async I/O.

Here are some results anyway. The command I'm using is: $ ./nbdkit -r -U - vddk \ libdir=/path/to/vmware-vix-disklib-distrib \ user=root password='***' \ server='***' thumbprint=aa:bb:cc:... \ vm=moref=3 \ file='[datastore1] Fedora 28/Fedora 28.vmdk' \ --run 'time /var/tmp/threaded-reads $unixsocket' Source for threaded-reads is attached.

Tests (1) and (2) are about the same within noise. Test (3) is making 8 times as many requests as test (1), so I think it's fair to compare the 8 x time taken by test (1) (ie. the time it would have taking to make 80,000 requests): Test (1) * 8 = 11m28 Test (3) = 7m11 So if we had a client which could actually use multi-conn then this would be a reasonable win. It seems like there's still a lot of locking going on somewhere, perhaps inside VDDK or in the server. It's certainly nowhere near a linear speedup.

The patch does at least seem stable. I'll post it in a minute.

On Wed, Aug 5, 2020 at 3:47 PM Richard W.M. Jones <rjones(a)redhat.com&gt; wrote: > > > Here are some results anyway. The command I'm using is: > > $ ./nbdkit -r -U - vddk \ > libdir=/path/to/vmware-vix-disklib-distrib \ > user=root password='***' \ > server='***' thumbprint=aa:bb:cc:... \ > vm=moref=3 \ > file='[datastore1] Fedora 28/Fedora 28.vmdk' \ > --run 'time /var/tmp/threaded-reads $unixsocket' > > Source for threaded-reads is attached. > > (1) Existing nbdkit VDDK plugin. > > NR_MULTI_CONN = 1 > NR_CYCLES = 10000 > > Note this is making 10,000 pread requests. > > real 1m26.103s > user 0m0.283s > sys 0m0.571s > > (2) VDDK plugin patched to support SERIALIZE_REQUESTS. > > NR_MULTI_CONN = 1 > NR_CYCLES = 10000 > > Note this is making 10,000 pread requests. > > real 1m26.755s > user 0m0.230s > sys 0m0.539s > > (3) VDDK plugin same as in (2). > > NR_MULTI_CONN = 8 > NR_CYCLES = 10000 > > Note this is making 80,000 pread requests in total. > > real 7m11.729s > user 0m2.891s > sys 0m6.037s > > My observations: > > Tests (1) and (2) are about the same within noise. > > Test (3) is making 8 times as many requests as test (1), so I think > it's fair to compare the 8 x time taken by test (1) (ie. the time it > would have taking to make 80,000 requests): > > Test (1) * 8 = 11m28 > Test (3) = 7m11 That's pretty good results, 62% faster. What is the request size used? I would test 1, 2, 4, 8 MiB reads.

> So if we had a client which could actually use multi-conn then this > would be a reasonable win. It seems like there's still a lot of > locking going on somewhere, perhaps inside VDDK or in the server. > It's certainly nowhere near a linear speedup. > > The patch does at least seem stable. I'll post it in a minute. > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > libguestfs lets you edit virtual machines. Supports shell scripting, > bindings from many languages. http://libguestfs.org