Don't rmtree(scratchdir)

Issue #860 closed
Barry Wardell created an issue

The mdb in SimFactory has an entry for scratchdir. If scratchdir is a relative path, it is assumed to be relative to the restart output directory. It's not clear exactly what the purpose of this is, but SimFactory does the following things with it:

  1. Create the directory at the start of a simulation if it doesn't exist.
  2. Delete the entire directory using rmtree(scratchdir) during "cleanup". This happens, for example, when an existing simulation is restarted.

The second of these seems very dangerous to me and I think it should be removed. In particular, if you happen to have scratchdir set to be the same for more than one simulation (by setting it to, e.g., /scratch), this scrachdir for all simulations will be erased as soon as one simulation does a cleanup.

I recently had an even worse experience where I was using /scratch as my basedir and /scratch also happened to be set as the scratchdir in the mdb for that machine. All of my simulations on that machine had only a single restart, so all was well until I tried to restart one simulation and instead of doing so SimFactory erased all of my simulations on the machine.

While I can admit to user error in this case (I also wrote the machine entry), I think the goal of SimFactory should be to err on the side of caution and shouldn't do this blind rmtree of whatever scratchdir is set to.

Keyword:

Comments (10)

  1. Erik Schnetter
    • removed comment

    The scratch directory is a directory that "belongs" to the simulation. It is made available at the beginning and is removed at the end, so that unused files don't accumulate. Saving disk space is important in many cases. This also allows using facilities where e.g. the queuing system deletes scratch directories, or where the scratch directories live on the nodes are become unavailable after the run finishes.

    I am sorry if there is a misunderstanding about the meaning of this MDB entry. Yes, setting the scratch directory to "/scratch" makes only sense if these are node-local directories.

    In most cases, the scratch directory contains e.g. the job id, as in

    /scratch/scratchdirs/@USER@/scratch/@JOB_ID@ to guarantee that it is unique.

    One solution could be to check whether the scratch directory is empty before a job starts, or to even ensure that it does not exist. If it exists this would be a fatal error, pointing to a significant inconsistency in the MDB or the system setup.

    Another option would be to deprecate scratchdir (since at least one person misunderstood its meaning), and introduce scratchbasedir instead. The scratchbasedir is supposed to exist, and the scratchdir is created as a subdirectory (that must not yet exist). This subdirectory will then be deleted again when the restart finishes.

    If scratchbasedir is not set, a subdirectory of the simulation directory would be used. Setting scratchbasedir will allow using a faster file system if one is available.

    In the mean time: Please tell me which machine this is, so that we can correct its MDB.

  2. Barry Wardell reporter
    • removed comment

    Replying to [comment:1 eschnett]:

    In most cases, the scratch directory contains e.g. the job id, as in ` /scratch/scratchdirs/@USER@/scratch/@JOB_ID@ ` to guarantee that it is unique.

    One solution could be to check whether the scratch directory is empty before a job starts, or to even ensure that it does not exist. If it exists this would be a fatal error, pointing to a significant inconsistency in the MDB or the system setup.

    I think this is a good idea (checking that the directory doesn't exist before the job starts).

    It's OK to have a per-simulation directory which is created and removed by SimFactory. The problem is the current implementation which is potentially unsafe. The logic ensuring that the scratch directory is unique to a simulation which is currently implemented in each mdb entry should be moved out of the MDB and into the code. This could be achieved as you suggest:

    Another option would be to deprecate scratchdir (since at least one person misunderstood its meaning), and introduce scratchbasedir instead. The scratchbasedir is supposed to exist, and the scratchdir is created as a subdirectory (that must not yet exist). This subdirectory will then be deleted again when the restart finishes.

    If scratchbasedir is not set, a subdirectory of the simulation directory would be used. Setting scratchbasedir will allow using a faster file system if one is available.

    In the mean time: Please tell me which machine this is, so that we can correct its MDB.

    I have already corrected the entry for the relevant machine (tesla). In fact, it wasn't really the mdb at fault, but my own defs.local.ini where I had set basedir = /scratch so that my simulation data would be output to a nice fast filesystem. To allow me to still do this, I changed the MDB entry to be scratchdir = scratchdir.

  3. Barry Wardell reporter
    • removed comment

    This patch looks good to me, buy I will test it anyway and report back.

    This would have caught the and prevented the problem in my case. I still like the idea of having a scratchbasedir and not relying on the mdb to provide a unique path for scratchdir, but that is a lower priority.

  4. Barry Wardell reporter
    • removed comment

    I have tested that this patch would have caught and prevented the problem in my case. I suggest applying the patch along with the attached patch which makes scratchdir an optional entry in the mdb. We will also need to do an audit of the mdb. There are many machines which have:

    scratchdir = scratchdir

    which is no longer valid as it must be an absolute path. For these machines, the entry should just be removed. There are also other machines with dodgy looking values, such as:

    damiana.ini:54:scratchdir = /tmp/${USER}/${JOB_ID} forge: scratchdir = /tmp # what is this for? /scratch/batch/users/${USER}/${PBS_JOBID} mars: scratchdir = /no/scratchdir scinet: scratchdir = /no/scratchdir shc: scratchdir = /scratch supermuc: scratchdir = /scratch/pr32pi/${USER}/${JOB_ID}

    Another separate potential problem I have just noticed is that the scratchdir is created at submit time, not run time. As a result, the directory is created on the head node, not on the compute nodes. It also means that variables such as ${PBS_JOBID} are probably not defined and so scratchdir gets set to something unexpected such as /tmp/${USER} instead of /tmp/${USER}/${PBS_JOBID}.

  5. Barry Wardell reporter
    • removed comment

    An additional problem which is probably a result of the directory being created at submit time is that @JOB_ID@ is not substituted and so a directory called @JOB_ID@ is created instead of a directory named after the actual job id.

  6. Barry Wardell reporter
    • removed comment

    One (hopefully) last issue I've just noticed is that the scratchdir is removed during cleanup. This means that it will probably be running on the head node, not the compute node where the scratch data was written. These may be equivalent, but not necessarily so.

  7. Roland Haas
    • removed comment
    1. 588 related to this same issue of scratch directories ony being visible on the compute nodes but not on the head or submission node though it relates to submit and not cleanup.
  8. Erik Schnetter
    • changed status to resolved
    • removed comment

    Trac ate my previous comment ("missing token", and then the back button doesn't work), where I nicely summarized our discussion on EVO.

    EVO ate the discussion since I already closed the window.

    In summary, we decided to accept the patch.

  9. Log in to comment