question on nbdkit shutdown behavior
I'm observing a difference in timing on nbdkit shutdown that is dependent on client behavior, and I wonder if that's a bug, but I can't figure out where to patch it. When there is no connection present, sending SIGINT to nbdkit terminates immediately. If, on the other hand, there is a single client currently connected, the termination behavior on SIGINT depends on what the client has done: if the client is currently silent with regards to issuing transmission-phase transactions, nbdkit hangs for 5 seconds, then forcibly tears down the connection: term1$ ./nbdkit -f -v -n file file=TODO ... nbdkit: debug: bound to IP address <any>:10809 (2 socket(s)) term2$ qemu-io -f raw nbd://localhost:10809/ -r qemu-io> term1$ ^C nbdkit: debug: waiting for running threads to complete nbdkit: debug: waited 5s for running threads to complete nbdkit: debug: /home/eblake/nbdkit/plugins/file/.libs/nbdkit-file-plugin.so: unload term2$ qemu-io> r 0 1 read failed: Input/output error qemu-io> q But, if the client makes ANY transaction during that window, then nbdkit lets that transaction succeed, then tears down the connection without any further wait: term1$ ./nbdkit -f -v -n file file=TODO ... nbdkit: debug: bound to IP address <any>:10809 (2 socket(s)) term2$ qemu-io -f raw nbd://localhost:10809/ -r qemu-io> r 0 1 # wait to hit Enter until right after ^C in term1 term1$ ^C nbdkit: debug: waiting for running threads to complete term2$ # hit enter term1$ nbdkit: file[1]: debug: acquire per-connection request lock nbdkit: file[1]: debug: pread count=1 offset=0 nbdkit: file[1]: debug: release per-connection request lock nbdkit: file[1]: debug: close nbdkit: debug: waited 2s for running threads to complete nbdkit: debug: /home/eblake/nbdkit/plugins/file/.libs/nbdkit-file-plugin.so: unload term2$ qemu-io> r 0 1 read 1/1 bytes at offset 0 1 bytes, 1 ops; 0.0002 sec (4.138 KiB/sec and 4237.2881 ops/sec) Why does current traffic from the client cause the plugin to be torn down faster? Does it matter? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Mon, Nov 13, 2017 at 12:42:48PM -0600, Eric Blake wrote:
I'm observing a difference in timing on nbdkit shutdown that is dependent on client behavior, and I wonder if that's a bug, but I can't figure out where to patch it.
When there is no connection present, sending SIGINT to nbdkit terminates immediately.
If, on the other hand, there is a single client currently connected, the termination behavior on SIGINT depends on what the client has done: if the client is currently silent with regards to issuing transmission-phase transactions, nbdkit hangs for 5 seconds, then forcibly tears down the connection:
I'm guessing it's because of commit 63f0eb0889c8f8a82ba06a02a8a92d695902baad which I added to fix a race in plugin_cleanup(). See also: https://www.redhat.com/archives/libguestfs/2017-September/msg00226.html [...]
Why does current traffic from the client cause the plugin to be torn down faster? Does it matter?
I believe because the main loop checks the !quit flag if there is traffic on the connection. There is most likely to be a better fix for the race than 63f0eb0889. I added that as a quick workaround for the segfault we saw in the tests. Perhaps we should actively cancel the threads on shutdown instead of waiting? 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
On 11/14/2017 04:25 AM, Richard W.M. Jones wrote:
On Mon, Nov 13, 2017 at 12:42:48PM -0600, Eric Blake wrote:
I'm observing a difference in timing on nbdkit shutdown that is dependent on client behavior, and I wonder if that's a bug, but I can't figure out where to patch it.
When there is no connection present, sending SIGINT to nbdkit terminates immediately.
If, on the other hand, there is a single client currently connected, the termination behavior on SIGINT depends on what the client has done: if the client is currently silent with regards to issuing transmission-phase transactions, nbdkit hangs for 5 seconds, then forcibly tears down the connection:
I'm guessing it's because of commit 63f0eb0889c8f8a82ba06a02a8a92d695902baad which I added to fix a race in plugin_cleanup(). See also: https://www.redhat.com/archives/libguestfs/2017-September/msg00226.html
[...]
Why does current traffic from the client cause the plugin to be torn down faster? Does it matter?
I believe because the main loop checks the !quit flag if there is traffic on the connection.
There is most likely to be a better fix for the race than 63f0eb0889. I added that as a quick workaround for the segfault we saw in the tests. Perhaps we should actively cancel the threads on shutdown instead of waiting?
If the plugin is still responding to an active request, and it is not expecting thread cancellation, that could be bad (writing code that is safe in the presence of pthread_cancel is not always trivial). But the scenario I described is when there are no active requests at the moment, so the threads are under our control. A better way might be to have a read-from-self pipe that we write on SIGINT, and a select() that checks for activity both on the read-from-self pipe and from the client, so that we can react to either situation immediately. But then we still have to be careful to (try) and let the pending plugin active requests complete (and merely reject any new requests from the client during that time), before giving up completely. In other words, I don't have a problem with waiting 5 seconds for SIGINT to have an effect in some cases, so much as a problem of explaining why we still have to wait even when there is no reason for it. At any rate, for now I'm just documenting the issues, rather than planning on tackling the code associated with the problem; it will be a task (for someone else?) down the road. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
A deeper problem I think is that it's ill-specified what is supposed to happen when nbdkit receives a termination signal. We don't specify (for example) that plugin callbacks could or could not be killed in the middle. Killing them in the middle of a callback could be bad for some plugins. Going back to the original problem, perhaps we could have fixed it instead by acquiring a lock around all plugin callbacks, and ensuring that plugin_cleanup also acquires the same lock. The lock would normally be uncontended so it shouldn't cause any performance issues. That would be a fairly simple revert of 63f0eb088 + alternate fix. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
On Tue, Nov 14, 2017 at 03:56:59PM +0000, Richard W.M. Jones wrote:
Going back to the original problem, perhaps we could have fixed it instead by acquiring a lock around all plugin callbacks, and ensuring that plugin_cleanup also acquires the same lock. The lock would normally be uncontended so it shouldn't cause any performance issues. That would be a fairly simple revert of 63f0eb088 + alternate fix.
No actually that would serialize everything. Some sort of mutex primitive, I think a shared/exclusive lock with the exclusive lock being taken in plugin_cleanup. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
participants (2)
-
Eric Blake -
Richard W.M. Jones