Remove potential race conditions

[jgfouca]

IOP tests need extra decoration on their sub test cases in
order to ensure no race condition.

Test cases that need clones should put clones within original
test case directory.

[jeff-cohere]

Is this change only intended for Melvin and Redsky? Do you foresee such race conditions on other machines? (Sorry for the delay in looking at this--was out Friday).

[jgfouca]

The changes to the test case scripts will affect everyone. The RUNDIR and EXEROOT changes are only for melvin and redsky. Other machines will have to make similar changes if they are to avoid races.

[worleyph]

I looked at this briefly and thought that it would affect melvin and redsky only. On Titan, you do not want to put bld and run in your case directory unless your case directory is in the Lustre project-shared or scratch areas. Similar issues probably arise on Mira and at NERSC (limited disk space in home directories). In any case, this change is purely for the test scripts, correct? Will it also affect users building cases?

[jgfouca]

The changes to the test cases, e.g. ERH_script, will affect everyone.
I would think on Titan, you'd want everything, including case directory, on a fast file system. What's the usecase for having case and bld on separate file systems?

[worleyph]

The fast file systems on Titan get swept. People like to put the case directories in their home directories, and the bld and run directories are in scratch (by default with the current scripts). ERH_script would not be called by a user who just wants to build a science new case, correct? This is still limited to testing? If so, being swept after 30 days may not be an issue?

[rljacob]

Could you add a description of the race-condition fail or provide the link to Confluence?

[jgfouca]

@worleyph, yes the ERH_script is a "test case" so only used in ERH tests that are in our test categories.
@rljacob, do you want me to add it to this conversation or to the commit message?

[rljacob]

This conversation.

[jgfouca]

OK, I'll try. If the following conditions are met, you have a race

1. Test A creates a "sub" test case B
2. There is also a test case "B" listed in your test category
3. On a batch system, test A is launched and begins to run
4. As A is running, the create_test script gets to "B" and begin to build it
5. testroot/B/bld is now simultaneously the bld area for two cases and bad things happen

[jeff-cohere]

If any of the acme_developer tests create "sub" tests, I think this means we have to address this race condition on all ACME-supported machines, since that test category is supposed to run everywhere. But it sounds like we don't have a solution that is "elastic" enough for that yet. Can someone correct me if I'm wrong?

[jgfouca]

Yeah, it sounds like, in some cases, the builds need to go in the case directory and in other cases they don't. To me, the key factor is whether the cases are being created by create_test or not. If create_test is the driver, then builds need to go in the case directory to avoid conflicts, otherwise they need to go in the scratch area. Thoughts?

[rljacob]

Do you need both the changes to the scripts AND the change to RUNDIR and EXEROOT to avoid the race condition?

[jgfouca]

The changes to the test case scripts are mostly for cleanliness. Usually, the cloned cases always have a "refN" appended, so I haven't seen a conflict. This just avoids clutter in your test root.

[jgfouca]

Hi all,
Our redsky tests are still on hold until this is resolved. Any thoughts? One idea I had is that we can remove any test case from our suite that is also being created as a sub test case by another case. In other words, in my example above, we can remove test case "B" from acme_integration since "A" will create a "B" test case anyway. I don't like this as much as fixing the test scripts such that test cases cannot conflict with each other, but it might be less controversial. Thoughts?
-Jim

[jeff-cohere]

Removing redundancies in tests is a good short term fix, but it requires intimate knowledge about which tests create subtests. I think this would allow us some time to think about a more general strategy. We should also ask CSEG if they see these race conditions (though they have been putting enough recent effort into their testing system that it's already beginning not to resemble ours[!]).

As an aside, a name-mangling scheme for cases that involved process IDs would be a way to ensure that different tests and subtests don't interfere with each other.

[worleyph]

I agree with @jnjohnsonlbl that a way to generate unique case names is the long term solution. Perhaps the "parent" case name can be prepended to the case name of the sub test case (by adding an optional parameter of some sorts to the scripts)?

[jgfouca]

That's a good idea. Unless there are objections, I'll move forward with the mangling.

[jgfouca]

I struggled with this a bit but eventually settled on mangling the test-id parameter for sub test cases needed by ERS_IOP* tests. The test name itself could not be mangled because the scripts parse that name and infer information about the test from it. Thoughts?

[jeff-cohere]

Is mangling the test-id parameter sufficient to prevent this race condition? It's unfortunate that the test name is tightly coupled, but we might not need to change it to get over this hump.

Thanks for continuing to look into this in any case, Jim!

[jgfouca]

Yes, it should be sufficient. You can see the potential for races if you look at our test suite:
ERS.f19_g16_rx1.A
ERS_IOP.f19_g16_rx1.A
ERS_IOP4c.f19_g16_rx1.A
ERS_IOP4p.f19_g16_rx1.A
... all of these will try to create a ERS.f19_g16_rx1.A-${TEST_ID} directory in the RUNDIR and EXEROOT. By default, the TEST_ID has a timestamp which reduces the odds of a conflict, but batch systems might fire off several of these at the same time so it's not full proof at all. By adding some source test case data into the TEST_ID, it should prevent potential conflicts.

[jeff-cohere]

Okay, I'll do the merge tomorrow. Thanks!

[jgfouca]

I can merge it if you need to head home. I'd like to get it in before tests kick off tonight.

[jeff-cohere]

I'm just in the middle of something else at the moment, so if you can merge, it would be great. For what it's worth, the changes look good, so I can send you an ASCII seal of approval if you want. :-)

[jgfouca]

Let me elaborate; redsky has been a bit busier than usual lately and I was having problems with tests not getting a chance to run in the 24 hour testing window. I bumped the window to 48 hours with tests running on even-number days of the month (*/2 in cron), so if we don't run tonight, we won't run until Thursday and I won't have conclusive evidence of a fix for our Friday meeting.

[jeff-cohere]

No need to elaborate--I am well familiar with the need to push something through a crowded queue! Thanks for the info, though.