Arvados

Further testing shows that docker is not the culprit here. I instrumented a version of arvados_fuse that monitors all operations calls (tracking the pid of the calling process) and no operations were performed by docker. It turns out that I was misusing the `fuser -m` option because at that point the fuse filesystem has already been unmounted, so the mount point is just a directory on the scratch filesystem (on which the docker graph database also resides, hence docker daemon having it open).

There is still an issue in that arv-mount fails to cleanly exit (entirely on its own, it would seem). I'm currently experimenting with adding some additonal code to _run_exec to make sure that it exits once the exec'd command completes.

1. I ran into both of these problems on a dev cluster last night.
2. IIRC, Josh mentioned the frequency of "arv-mount gets stuck" seemed to be related to the load on the whole cluster, not just the number of tasks running on a single compute node ... and that high cluster load seemed to be causing websocket failures.

As Josh pointed out in IRC, it looks like we're using operations.destroy() wrong: the llfuse example suggests we should just call llfuse.close() after the filesystem is unmounted and llfuse.main() returns, and let llfuse call our operations.destroy() by itself. (This might explain why we've had to hack mutexes into our shutdown methods: perhaps we wouldn't have to accommodate multiple overlapping shutdown events if we stop going around llfuse's back and calling operations.destroy().)

We start llfuse.main() in a non-daemon thread, and then call operations.destroy() from a different thread. Calling llfuse.close() (not destroy()), and calling it from the same thread that called llfuse.main(), seems like good form if nothing else. And I'm not sure there's any reason that thread should not be a daemon thread: if we're in --exec mode and our main thread ends, that means our child process has exited and we have called fusermount -u, so the expected behavior is for all subsequent IO operations to fail anyway.

1. When someone (like arv-mount) closes its PollClient event listener, don't wait for the current set of "get logs" API calls to complete.
2. In --exec mode, mark the llfuse thread as a daemon thread so Python doesn't stay alive waiting for it to finish after main has decided to exit.
3. In --exec mode, if --unmount-timeout (default 2) seconds pass after "fusermount -u -z" and the llfuse thread is still running, log a warning, call operations.destroy(), and exit. (Otherwise, don't call operations.destroy() at all, just assume llfuse has done the necessary cleanup.)

Would be good to add an "exit within unmount_timeout+1 seconds even if various things are conspiring to keep threads alive" test case.

Split branch into 8288-arv-mount-deadlock and 8288-poll-client-close-timeout

Reviewing 8288-arv-mount-deadlock:

The strategy here seems to be (a) use lazy unmount and (b) make the llfuse main thread a daemon thread so it doesn't prevent the runtime from exiting.

Some notes:

• The llfuse main thread calls fuse_session_loop_mt() in libfuse which spins up more pthreads. I believe these will die quietly when the process as a whole exits.
• The llfuse main thread normally terminates in response to an exit message from the kernel. So if we don't get the exit message, the kernel doesn't think it is done with the mount point (but we knew that already).
• close() calls fuse_session_destroy() which calls op.destroy(), so I don't think we should ever be calling operations.destroy() explicitly.
• It seems very likely to be a race condition to call close() without first stopping the main loop, so we might be signing up for random segfaults.
• The correct way to terminate the main thread seems to be to call fuse_session_exit(), but llfuse doesn't expose that.
• At best, this seems very likely to result in stale mount points in a weird state.

Peter Amstutz wrote:

The strategy here seems to be (a) use lazy unmount and (b) make the llfuse main thread a daemon thread so it doesn't prevent the runtime from exiting.

Right. Also, stop going behind llfuse's back and calling operations.destroy() immediately after fusermount -u and possibly getting there first, which might have been causing subtle problems of its own.

(I suspect the most common reason the llfuse thread was staying open was that our destroy() method called PollClient.close(), which could take a long time -- addressed by the 8288-poll-client-close-timeout branch -- but surely there are also other ways to keep the llfuse thread alive after the --exec proc has exited, and we want to cut those off too.)

• The llfuse main thread calls fuse_session_loop_mt() in libfuse which spins up more pthreads. I believe these will die quietly when the process as a whole exits.
• The llfuse main thread normally terminates in response to an exit message from the kernel. So if we don't get the exit message, the kernel doesn't think it is done with the mount point (but we knew that already).
• close() calls fuse_session_destroy() which calls op.destroy(), so I don't think we should ever be calling operations.destroy() explicitly.

Right. I moved it to "only if the normal way doesn't work normally" because it's clear that's not the right way. But yes, terminating the thread by exiting main is probably better than unsafe-cleanup-then-exit. So, removed the call to operations.destroy() in da7eabf.

• It seems very likely to be a race condition to call close() without first stopping the main loop, so we might be signing up for random segfaults.

Yes, I assume this is why the old "close() from a different thread" code was commented out. The new code is straight from the llfuse example: close() is just what you're supposed to do after main() returns.

• The correct way to terminate the main thread seems to be to call fuse_session_exit(), but llfuse doesn't expose that.

I figure we have to make sure "fusermount -u" works properly (that's the correct way that's exposed to users) so I'm not inclined to look too hard for other ways.

• At best, this seems very likely to result in stale mount points in a weird state.

So far I have not seen a mount point still connected after "fusermount -u -z". But yes, getting rid of operations.destroy() should reduce the possibilities to "normal", "process exited, anything still open gets an IO error", "process won't die, llfuse is still doing its thing without interference"... no more "llfuse is still running after we poked its operations in the eye" option.

If there is still some case where the kernel (or python) keeps our process alive even after we try to exit the main thread, at least we'll have a log message as a clue, and at least we won't have poked llfuse in the eye.

fuse_session_exit() would let us gracefully terminate the main libfuse threads instead of just abandoning them, but as noted, llfuse doesn't expose that. I'll see about filing a feature request.

8288-arv-mount-deadlock @ da7eabf looks good to me.

We could also tell the kernel to abort the connection (and thus terminate the fuse/llfuse process) by either `umount -f` on the mountpoint (which should work after `fusermount -u` but not after `fusermount -u -z`) or signalling the "abort" interface in the fusectl (/sys) filesystem.

Those are the two most effective deadlock-avoidance mechanisms suggested in the linux kernel documentation for fuse:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/filesystems/fuse.txt#n204

I think the implication of these documentation are that there are situations arv-mount can get into in which this will be the only way to kill off the fuse filesystem.

Perhaps we could follow up on the current `fusermount -u` call with a .join() with timeout on the fuse main thread, and if it doesn't exit in the time allotted, hit it harder with a `umount -f` call.

Joshua Randall wrote:

We could also tell the kernel to abort the connection (and thus terminate the fuse/llfuse process) by either `umount -f` on the mountpoint (which should work after `fusermount -u` but not after `fusermount -u -z`) or signalling the "abort" interface in the fusectl (/sys) filesystem.

umount -f definitely works, but requires sudo.

I didn't know about "abort", though. It seems that the permissions are associated with the user that is running the fuse FS, so no sudo needed. However the mounts are identified by a numeric id, I'm not sure where that id comes from.

Reviewing 8288-poll-client-close-timeout

In EventClient:

Calling super(EventClient, self).close() starts a graceful shutdown (WebSocketClient tells the server that we're signing off, waits for an acknowledgement, the acknowledgement is what signals closed()), but if we don't wait for it, won't happen. Suggest the default timeout should be greater than zero, maybe 1s.

In PollClient:

Similar suggestion for default timeout to be greater than zero. Once _closing.set() is called, the thread should exit almost immediately, except with a timeout of zero the thread might not even get a chance to run to completion.

I started out having non-zero default timeouts, but I couldn't think of any advantage.

For example, with EventClient, why do we care whether the server acknowledged our "close"? Either way, the connection will close when the server closes it, it times out, or our process exits.

If there are bugs in ws4py that make it crash during exit hooks if the server hasn't acknowledged shutdown yet, it seems like larger timeouts here would make the resulting problems rarer and harder to reproduce, but wouldn't actually solve them.

Similarly, in PollClient, I don't see an advantage in waiting. OTOH if we close the connection (by exiting the process) there's some chance the server end can notice that we no longer care about the response (in practice I don't think this is a real advantage right now: the nginx proxy might notice, but # nginx connections doesn't tend to be a bottleneck).

• if there are unclean-shutdown bugs, we get to find them sooner/easier, and fix them (instead of "wait 2 seconds every time, and 99.9% of the time that's long enough")
• we just exit as soon as we're done, things run fast

Tom Clegg wrote:

I started out having non-zero default timeouts, but I couldn't think of any advantage.

For example, with EventClient, why do we care whether the server acknowledged our "close"? Either way, the connection will close when the server closes it, it times out, or our process exits.

If there are bugs in ws4py that make it crash during exit hooks if the server hasn't acknowledged shutdown yet, it seems like larger timeouts here would make the resulting problems rarer and harder to reproduce, but wouldn't actually solve them.

Yes, I agree it should just close the socket and not wait for acknowledgement, but ws4py doesn't have an option to do that (I've looked over the code several times). So if you want to poke ws4py in the eye, that's fine, but it's likely to result in intermittent backtraces during shutdown. We should probably fix ws4py at some point.

Similarly, in PollClient, I don't see an advantage in waiting. OTOH if we close the connection (by exiting the process) there's some chance the server end can notice that we no longer care about the response (in practice I don't think this is a real advantage right now: the nginx proxy might notice, but # nginx connections doesn't tend to be a bottleneck).

I agree that for PollClient it shouldn't make much difference one way or the other.

The advantages of zero timeouts are

• if there are unclean-shutdown bugs, we get to find them sooner/easier, and fix them (instead of "wait 2 seconds every time, and 99.9% of the time that's long enough")
• we just exit as soon as we're done, things run fast

In the interests of time, go ahead and merge.

Merged, thanks.

Peter Amstutz wrote:

Yes, I agree it should just close the socket and not wait for acknowledgement, but ws4py doesn't have an option to do that (I've looked over the code several times). So if you want to poke ws4py in the eye, that's fine, but it's likely to result in intermittent backtraces during shutdown. We should probably fix ws4py at some point.

The new default behavior is exactly the same as the old behavior (i.e., don't wait for anything) so if we haven't been having this problem I suppose we will continue to not have it.

I don't think this qualifies as "poking ws4py in the eye" unless ws4py's API tells us we're not allowed to exit our program until we receive a "closed" event on every connection...?

... not wait for acknowledgement, but ws4py doesn't have an option to do that

This seems to be exactly what close() does -- we have to go to extra trouble to wait for acknowledgement via closed() callback.