search for: serialize_requests

...This is a huge pain. +/* The rules on threads and VDDK are here: + * https://code.vmware.com/docs/11750/virtual-disk-development-kit-programming-guide/GUID-6BE903E8-DC70-46D9-98E4-E34A2002C2AD.html + * + * Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS. Since nbdkit + * 1.22 we changed this to SERIALIZE_REQUESTS and added a mutex around + * calls to VixDiskLib_Open and VixDiskLib_Close. This is not quite + * within the letter of the rules, but is within the spirit. */ -#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS +#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS + +/* Lock...

...The rules on threads and VDDK are here: > + * > https://code.vmware.com/docs/11750/virtual-disk-development-kit-programming-guide/GUID-6BE903E8-DC70-46D9-98E4-E34A2002C2AD.html > + * > + * Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS. Since nbdkit > + * 1.22 we changed this to SERIALIZE_REQUESTS and added a mutex around > + * calls to VixDiskLib_Open and VixDiskLib_Close. This is not quite > + * within the letter of the rules, but is within the spirit. > The document is very clear about using the same thread for open an close. Using a lock is not the same. I think Eric already...

...ad_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, SERIALIZE_ALL_REQUESTS serializes requests across all handles. SERIALIZE_REQUESTS serializes re...

...$ 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. PARALLEL is not possible. > 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...

...t; > 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, > > SERIALIZE_ALL_REQUESTS serializes requests across all handles. >...

...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,...

Nir, BTW what are you using for performance testing? 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). Instead I've hacked up a copy of this program from libnbd: https://github.com/libguestfs/libnbd/blob/master/examples/threaded-reads-and-writes.c so

...and it's possible to do better than what we are doing at the moment. Not sure at present if this will be easy or will add a lot of complexity. Read on ... I found through experimentation that it is possible to open multiple VDDK handles pointing to the same disk. This would allow us to use SERIALIZE_REQUESTS (instead of SERIALIZE_ALL_REQUESTS) and have overlapping calls through different handles all pointing back to the same server/disk. We should have to change all open/close calls to make the request through a single background thread - see document above for why. Adding a background thread and all...

...rce 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...

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...

I believe this roughly implements Nir's proposal here: https://www.redhat.com/archives/libguestfs/2020-August/msg00028.html Unfortunately it doesn't work for me. It actually slows things down quite a lot, for reasons I don't understand. Note the adjustment of the pool-max parameter and how it affects the total time. The results are quite reproducible. $ ./nbdkit -r -U - vddk

Rich pointed out that making thread_model dynamic even for plugins makes some sense, so here is the code for doing it for 'sh'. I'm less confident on how to do it for OCaml and Rust (not to mention that those allow the plugin to already compile in their model, rather than the language binding glue forcing a model). The other languages (lua, perl, python, ruby) still need to be

On 8/2/19 2:55 PM, Richard W.M. Jones wrote: >> +++ b/plugins/sh/nbdkit-sh-plugin.pod >> @@ -189,12 +189,11 @@ one of C<"serialize_connections">, C<"serialize_all_requests">, >> C<"serialize_requests">, or C<"parallel">. >> >> This method is I<not> required; if omitted, then the plugin will be >> -executed under the default sh thread model (currently >> -C<"serialize_all_requests">, which implies this callback only makes a &...

...This is a huge pain. +/* The rules on threads and VDDK are here: + * https://code.vmware.com/docs/11750/virtual-disk-development-kit-programming-guide/GUID-6BE903E8-DC70-46D9-98E4-E34A2002C2AD.html + * + * Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS. Since nbdkit + * 1.22 we changed this to SERIALIZE_REQUESTS and added a mutex around + * calls to VixDiskLib_Open and VixDiskLib_Close. This is not quite + * within the letter of the rules, but is within the spirit. + * + * We also enabled multi-conn for readonly connections since it + * appears possible for clients to open multiple handles even if they +...

...9; \ --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 obse...

...ll be made (that is slightly different than SERIALIZE_ALL_REQUESTS, not sure where it would lie conceptually between existing models), and a model that allows parallel requests from a single connection but delays responses to be in order (perhaps named SERIALIZE_RESPONSES, conceptually lies between SERIALIZE_REQUESTS and PARALLEL). We could add a utility helper function that, when given two constants, returns -1/0/1 depending on whether the first parameter is stricter/equal/looser than the second, to at least ease any out-of-order defined integer values back into a hierarchical scheme. I also wonder if we nee...

...39;t help). A plugin that uses SERIALIZE_ALL_REQUESTS is also trivially supported - any flush request will complete (regardless of which connected client made it) prior to any other client being able to make progress on a request that would need to see the results of the flush. Only when we get to SERIALIZE_REQUESTS or PARALLEL can we have the situation where one client's request can outpace the flush operation in another client. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

In theory this patch depends on this series: https://www.redhat.com/archives/libguestfs/2020-August/msg00021.html In practice I believe they're independent of each other, but the above series makes it easier to test. Rich.

On 8/5/20 9:10 AM, Richard W.M. Jones 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 activel...

...better than what we > are doing at the moment. Not sure at present if this will be easy or > will add a lot of complexity. Read on ... > > I found through experimentation that it is possible to open multiple > VDDK handles pointing to the same disk. This would allow us to use > SERIALIZE_REQUESTS (instead of SERIALIZE_ALL_REQUESTS) and have > overlapping calls through different handles all pointing back to the > same server/disk. We should have to change all open/close calls to > make the request through a single background thread - see document > above for why. > > Addin...