Eric Blake
2019-May-21 22:08 UTC
Re: [Libguestfs] [PATCH libnbd] api: Synchronous connect waits til all connections are connected.
On 5/21/19 3:44 PM, Richard W.M. Jones wrote:> If not using multi-conn then obviously the synchronous connection > calls ‘nbd_connect_unix’, ‘nbd_connect_tcp’ and ‘nbd_connect_command’ > should only return when the (one) connection object is connected. > > In the multi-conn case it's not very clear what these synchronous > calls should do. Previously I had it so that they would return as > soon as at least one connection was connected. However this is a > problem if you are using them as a convenient way to set up a > multi-threaded main loop, because it can be that some of them have not > finished connecting, but then you issue commands on those connections > and that will fail. The failure is pernicious because most of the > time you won't see it, only if one connection is slow. So it's > (probably) better that the synchronous ‘nbd_connect_unix’ and > ‘nbd_connect_tcp’ should connect every connection object before > returning.Should this be a bool parameter for the caller to opt-in to? In one direction, returning as soon as possible allows the caller to issue further synchronous commands which will pick the ready connections and skip the ones still initializing, to at least get lower latency on the first commands and better performance later as more connections come up; in the other direction, waiting to return until all connections are made makes it easier to issue async commands on any connection without worrying if the connection is up. The choice of the parameter to pass in depends, then, on whether the caller plans to make future sync calls or async calls. Also, I'm playing with the idea of having some sort of witness whether a connection has ever reached the READY state (even if it is not there at the moment). Prior to that point, we don't want to accept commands (whether sync or async) because we aren't finished negotiating and thus cannot guarantee if the can_FOO() functions are accurate; after that point, it may be nice to queue up the commands for later issuance when we DO come back around to READY, rather than insisting that we be exactly in the READY state when issuing a command.> > For ‘nbd_connect_command’, it essentially ignored multi-conn anyway, > and I changed it so that it waits for conn[0] to get connected and > returns, the other connections (if they exist) being ignored. It > should probably be an error for the user to enable multi-conn on the > handle and then use nbd_connect_command, but I did not make that > change yet.Yes, that change (as a separate patch) makes sense.> --- > generator/generator | 8 +++----- > lib/connect.c | 48 +++++++++++++++++++++++++++++++++++---------- > 2 files changed, 41 insertions(+), 15 deletions(-) >-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2019-May-22 09:28 UTC
Re: [Libguestfs] [PATCH libnbd] api: Synchronous connect waits til all connections are connected.
On Tue, May 21, 2019 at 05:08:05PM -0500, Eric Blake wrote:> On 5/21/19 3:44 PM, Richard W.M. Jones wrote: > > If not using multi-conn then obviously the synchronous connection > > calls ‘nbd_connect_unix’, ‘nbd_connect_tcp’ and ‘nbd_connect_command’ > > should only return when the (one) connection object is connected. > > > > In the multi-conn case it's not very clear what these synchronous > > calls should do. Previously I had it so that they would return as > > soon as at least one connection was connected. However this is a > > problem if you are using them as a convenient way to set up a > > multi-threaded main loop, because it can be that some of them have not > > finished connecting, but then you issue commands on those connections > > and that will fail. The failure is pernicious because most of the > > time you won't see it, only if one connection is slow. So it's > > (probably) better that the synchronous ‘nbd_connect_unix’ and > > ‘nbd_connect_tcp’ should connect every connection object before > > returning. > > Should this be a bool parameter for the caller to opt-in to? In one > direction, returning as soon as possible allows the caller to issue > further synchronous commands which will pick the ready connections and > skip the ones still initializing, to at least get lower latency on the > first commands and better performance later as more connections come up; > in the other direction, waiting to return until all connections are made > makes it easier to issue async commands on any connection without > worrying if the connection is up. The choice of the parameter to pass in > depends, then, on whether the caller plans to make future sync calls or > async calls.It could be. On the other hand if people want to do complicated stuff then there is the AIO interface.> Also, I'm playing with the idea of having some sort of witness whether a > connection has ever reached the READY state (even if it is not there at > the moment). Prior to that point, we don't want to accept commands > (whether sync or async) because we aren't finished negotiating and thus > cannot guarantee if the can_FOO() functions are accurate; after that > point, it may be nice to queue up the commands for later issuance when > we DO come back around to READY, rather than insisting that we be > exactly in the READY state when issuing a command. > > > > > For ‘nbd_connect_command’, it essentially ignored multi-conn anyway, > > and I changed it so that it waits for conn[0] to get connected and > > returns, the other connections (if they exist) being ignored. It > > should probably be an error for the user to enable multi-conn on the > > handle and then use nbd_connect_command, but I did not make that > > change yet. > > Yes, that change (as a separate patch) makes sense.Even more radical, what if we prevent the use of multi-conn at all with the synchronous interface? (I don't think this is a good idea, just putting it out there for discussion). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Eric Blake
2019-May-22 13:07 UTC
Re: [Libguestfs] [PATCH libnbd] api: Synchronous connect waits til all connections are connected.
On 5/22/19 4:28 AM, Richard W.M. Jones wrote:>> Should this be a bool parameter for the caller to opt-in to? In one >> direction, returning as soon as possible allows the caller to issue >> further synchronous commands which will pick the ready connections and >> skip the ones still initializing, to at least get lower latency on the >> first commands and better performance later as more connections come up; >> in the other direction, waiting to return until all connections are made >> makes it easier to issue async commands on any connection without >> worrying if the connection is up. The choice of the parameter to pass in >> depends, then, on whether the caller plans to make future sync calls or >> async calls. > > It could be. On the other hand if people want to do complicated stuff > then there is the AIO interface.True enough. A caller that wants to avoid latency should be using AIO rather than sync. So this change makes sense to me overall.>>> For ‘nbd_connect_command’, it essentially ignored multi-conn anyway, >>> and I changed it so that it waits for conn[0] to get connected and >>> returns, the other connections (if they exist) being ignored. It >>> should probably be an error for the user to enable multi-conn on the >>> handle and then use nbd_connect_command, but I did not make that >>> change yet. >> >> Yes, that change (as a separate patch) makes sense. > > Even more radical, what if we prevent the use of multi-conn at all > with the synchronous interface? (I don't think this is a good idea, > just putting it out there for discussion).multi-conn + synchronous still makes sense (the fact that you round-robin between connections is already nice); it's only the command interface (where you can't make more connections, because you are limited to stdin/stdout) where an error would be appropriate. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Apparently Analagous Threads
- Re: [PATCH libnbd] api: Synchronous connect waits til all connections are connected.
- [PATCH libnbd] api: Synchronous connect waits til all connections are connected.
- [PATCH libnbd v2 1/6] api: Synchronous connect waits til all connections are connected.
- Re: [PATCH libnbd v2 1/6] api: Synchronous connect waits til all connections are connected.
- [PATCH libnbd] api: Synchronous connect waits til all connections are connected.