Rsync and Fuse Issues

[mtelvers]

The rsync folder result-tmp grows uncontrollably, eventually leading to the worker stalling on with the following error:

2023-01-15 13:22.58       obuilder [INFO] Pruning 0 items (of 100 requested)
2023-01-15 13:22.58         worker [INFO] OBuilder partition: 7% free after pruning 0 items
2023-01-15 13:22.58         worker [WARNING] Out of space, but nothing left to prune! (will wait and then retry)

This is caused by the following two considitons:

1. when a job is cancelled, the macOS sandbox does not delete the result-tmp folder, and
2. when a job fails, the rsync backend does not delete the result-tmp.

Furthermore, under normal circumstances, the fuse file system is unmounted between jobs. This is essential for the Docker health check to be able to find the Docker binary which is installed in /usr/local. However, when a job is cancelled, fuse is not unmounted; therefore, we see the health check fail if it occurs immediately after a cancelled job.

[MisterDA]

LGTM. Could you squash the commits when you'll think this is ready for merging?

[tmcgilchrist]

Some minor style suggestions but otherwise the change looks good.
We need an osx rsync stress test to validate we don't leak disk space.

[MisterDA]

I don't understand exactly what's happening here, but it looks wrong as you're not binding the Lwt promise, or am I reading that wrong?

[mtelvers]

I have updated the binding and changed the function a bit as well.

[tmcgilchrist]

The LWT sleep looks curious to me. Does the kill_all_descendants function not wait for the command to terminate?

…

On Wed, 18 Jan 2023 at 6:35 pm, Mark Elvers ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lib/sandbox.macos.ml <#139 (comment)>: > @@ -127,12 +127,11 @@ let run ~cancelled ?stdin:stdin ~log (t : t) config result_tmp = in Lwt.on_termination cancelled (fun () -> let aux () = - post_cancellation ~result_tmp t >>= fun () -> - if Lwt.is_sleeping proc then ( - match !proc_id with + if Lwt.is_sleeping proc then + let _ = match !proc_id with I have updated the binding and changed the function a bit as well. — Reply to this email directly, view it on GitHub <#139 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABJXOPLD6BHOXR3QHVXLZTWS6MNRANCNFSM6AAAAAAT34STOY> . You are receiving this because you commented.Message ID: ***@***.***>

[mtelvers]

kill_all_descendants invokes multiple kill -9 pid commands. The sleep was an attempt to allow the processes time to actually terminate and be cleared up by the operating system before trying to delete any files they had open.

[MisterDA]

The LWT sleep looks curious to me. Does the kill_all_descendants function not wait for the command to terminate?

kill_all_descendants invokes multiple kill -9 pid commands. The sleep was an attempt to allow the processes time to actually terminate and be cleared up by the operating system before trying to delete any files they had open.

I've looked in depth, and I'm convinced that we're correctly waiting until all kill have finished, but maybe macOS does still require some time to catch up.

[MisterDA]

What I meant is that the indentation of post_cancellation is confusing: it is actually part of the else block here. If you want to execute it regardless of the Lwt.is_sleeping proc, you have to enclose the if … else … block in a begin … end (or parenthesis) block.
I think that's what you want.

open Lwt.Infix

let () =
  let x = ref true in
  Lwt_main.run begin
      if !x then
        Lwt_io.print "hello"
      else
        Lwt_io.print "world"
        >>= fun () ->
        Lwt_io.print "else branch"
    end
    
(* prints "hello" *)

let () =
  let x = ref true in
  Lwt_main.run begin
      begin if !x then
        Lwt_io.print "hello"
      else
        Lwt_io.print "world"
      end >>= fun () ->
      Lwt_io.print " common"
    end

(* prints "hello common" *)    

You could configure your editor to use ocp-indent (since we're not applying ocamlformat here).

[mtelvers]

Yes, I have an updated version with that modification which I haven't pushed yet.

[MisterDA]

The returned promise most likely needs to be passed to Lwt.async to be executed by Lwt, just like the function passed to Lwt.on_termination below.

[mtelvers]

I think that on_failure can be removed as on_termination covers both cases.

[mtelvers]

Thanks @MisterDA. Right now, the issue I am seeing is that when a job is cancelled, a new job is started immediately, but new job appears to run concurrently with the on_termination code. For a normal worker, this isn't a problem because jobs can run concurrently, but on the Mac, the new job is starting as the old one is still finishing both using the same home directory.

[MisterDA]

What's the state of this PR now? Are you satisfied with your changes?

[MisterDA]

Merged as of 66f5164, thanks!