Fix singleton tmp files cleanup

[xbw22109]

In singleton mode, directory cleaning needs to be done by the ompi library.
However, there are issues in the current implementation that prevent complete cleanup of the directory.

Singleton Mode Cleanup Behavior

When the program terminates normally

1. The session dir is only partially cleaned.(Fixed)

The directory /tmp/ompi.<username>.<uid> will still be left behind, along with some subdirectories.
I modified the directory structure and enabled recursive deletion.

2. Segment files created by btl/sm are not properly removed.(Fixed)

In ompi/opal/mca/btl/sm/btl_sm_module.c: function sm_finalize, unlink will never be done, singleton or not. However, in non-singleton mode, the file will be cleaned by pmix after opal_pmix_register_cleanup succeeds.

// shmem has been closed. These two calls will return directly.
opal_shmem_unlink(&mca_btl_sm_component.seg_ds);
opal_shmem_segment_detach(&mca_btl_sm_component.seg_ds);

I registered the segment cleanup with OPAL, which is similar to mca_btl_smcuda_component_init.
Question : Although not directly related to this pr. In opal/mca/btl/sm/btl_sm_component.c around line 421, is this correct? is this need to be removed?

    /* set flag indicating btl not inited */
    mca_btl_sm.btl_inited = false;

When the program terminates by ctrl-C (see issue #13002)

Question : No file cleaning up is possible in this case. Should we explicitly document this behavior or add a signal handling mechanism? It is not easy to add a signal handling mechanism in btl/sm.

[jsquyres]

The btl cleanup looks good; looks like that was a miss that we never noticed because pmix cleaned it up for us.

I'm a little dubious of the session directory cleanup:

1. Isn't this order backwards? Shouldn't we remove the top session directory last (after the proc and session dirs)?
2. It doesn't feel like a bug that the recursive flags were set to "false" in the other opal_os_dirpath_destroy() calls -- it's been that way for (literally) years. I'm curious as to what the reason was (is?) for it to be false.
3. How can we know if we can destroy the top session dir? I see destroy_top_session_dir set to true, below, but is that really true? What if there are other processes using that top session directory -- how can this process know if it is safe to remove the top session directory?

[rhc54]

1. Isn't this order backwards?  Shouldn't we remove the top session directory _last_ (after the proc and session dirs)?

Yes - you remove proc, then job, then session (i.e., work your way upwards)

2. It doesn't feel like a bug that the recursive flags were set to "false" in the other `opal_os_dirpath_destroy()` calls -- it's been that way for (literally) years.  I'm curious as to what the reason was (is?) for it to be `false`.

Can't say - however, PMIx and PRRTE always set that flag to "true". I suppose that in this one special case (a singleton), you could technically just call dirpath_destroy on the top-level session directory with recursive set to "true" since no other job can share the tree. 🤷‍♂️

3. How can we know if we can destroy the top session dir?  I see `destroy_top_session_dir` set to `true`, below, but is that really true?  What if there are other processes using that top session directory -- how can this process know if it is safe to remove the top session directory?

The top session directory's name includes the pid of the singleton process - this is necessary to ensure that some other process doesn't attempt to remove it from underneath you. Accordingly, no other process can use it, so it is safe to remove it.

[jsquyres]

The top session directory's name includes the pid of the singleton process - this is necessary to ensure that some other process doesn't attempt to remove it from underneath you. Accordingly, no other process can use it, so it is safe to remove it.

Is this code specific to the singleton case? If so, I missed that.

[rhc54]

Is this code specific to the singleton case?

Yeah, it's a little convoluted, but it still is restricted to singleton case. You only set the "destroy_xxx" flag if we are a singleton and therefore created the session directory ourselves. So you only execute the destruct code if that flag is set, which means you have to be a singleton.

You might want to double-check that the logic behind that didn't get changed and is correct. Just glancing, it looked like it was still okay.

[xbw22109]

Thank you @rhc54 for the detailed explanation — I completely agree with your points and appreciate the context you provided.

Is this code specific to the singleton case?

Yes. And here is some additional information.

When PRRTE is present, it is responsible for both the creation and cleanup of the session directory.
OMPI retrieves the session directory *path* from PMIx after the directory has been created.

In ompi/runtime/ompi_rte.c:797, function :ompi_rte_init,

    OPAL_MODEX_RECV_VALUE_OPTIONAL(rc, PMIX_TMPDIR, &pname, &val, PMIX_STRING);
    if (OPAL_SUCCESS == rc && NULL != val) {
        opal_process_info.top_session_dir = val;
        val = NULL;  // protect the string
    } else {
        /* we need to create something */
        rc = _setup_top_session_dir(&opal_process_info.top_session_dir);
        if (OPAL_SUCCESS != rc) {
            error = "top session directory";
            goto error;
        }
    }

If PRRTE is running correctly, OPAL_MODEX_RECV_VALUE_OPTIONAL(rc, PMIX_TMPDIR, &pname, &val, PMIX_STRING) is guaranteed to succeed, because PRRTE will always create and register the session directory under all circumstances.

In the else branch, we manually create the session directory. Entering this branch indicates that no PRRTE is available, which I believe, in the Open MPI 5.0.x implementation, only occurs in singleton mode.

In this case, Open MPI should create the session directory itself, handle its cleanup, and use the PID to distinguish the top-level session directory.

These responsibilities were not fully handled in the original implementation. In the original implementation, the top session directory is placed under the system’s temporary directory, and the job session dir is constructed as *top_session_dir*/ompi.*nodename*.*uid*/jf.0/*opal_jobid*.

This structure requires additional directory parsing to perform a full cleanup, which the original implementation does not handle. As a result, the directory top_session_dir/ompi.nodename.uid/jf.0(e.g., /tmp/ompi.x.1000/jf.0 on my machine) , is never deleted after a singleton run.

[xbw22109]

Question : No file cleaning up is possible in this case. Should we explicitly document this behavior or add a signal handling mechanism? It is not easy to add a signal handling mechanism in btl/sm.

In addition, based on my observation, starting from Open MPI 5.0.x, the singleton no longer launches a thread that runs pmix server. This is why singleton processes can no longer clean up their paths upon abnormal termination. In contrast, for non-singleton programs, PRRTE or OpenPMIx takes care of path cleanup after an abnormal termination.

So, what should we do?

Adding a signal handler for singleton programs may not be a good idea, since it would be registered into the "user program" during MPI_Init. On the other hand, we would need an additional "registration system" to track and manage files created by sm.

Personally, I lean toward documenting this behavior—indicating that when a singleton process terminates abnormally, we cannot guarantee the cleanup of temporary files.

[xbw22109]

Question : Although not directly related to this pr. In opal/mca/btl/sm/btl_sm_component.c around line 421, is this correct? is this need to be removed?

I’ve taken a closer look and have now understood it myself — it turns out to be just a variable initialization. There’s no need to follow up on this further.

[rhc54]

the singleton no longer launches a thread that runs pmix server

Errr...that's not true. You initialized PMIx, and so the PMIx library is indeed running its progress thread. Technically, you could follow the same code as in prte.c and setup events (using the PMIx event base) to capture SIGTERM and friends, and then cleanly terminate when fired.

Bit of work, so up to you guys - suspect users will complain if it doesn't clean up, but... 🤷‍♂️

[xbw22109]

Errr...that's not true. You initialized PMIx, and so the PMIx library is indeed running its progress thread. Technically, you could follow the same code as in prte.c and setup events (using the PMIx event base) to capture SIGTERM and friends, and then cleanly terminate when fired.

I hadn't noticed that before — I'll go take a look.

My earlier question was based on the assumption that the PMIx library is not running at all in singleton mode in 5.0.x. If what you said is correct, then following the same code as in prte.c to handle file cleanup — rather than just documenting the behavior — would be the best solution.