Overlap test sometimes failing in Mellanox Jenkins with Yalla

[jsquyres]

Mellanox Jenkins is showing a periodic problem with the overlap test using Yalla. There has been some discussion on open-mpi/ompi-release#1240 about what the actual root cause is, starting around here: open-mpi/ompi-release#1240 (comment).

Please continue the discussion here: @jladd-mlnx @miked-mellanox @artpol84 @nysal @hjelmn

We need to decide if this is a v2.0.0 or v2.0.1 issue. What is the scope of the problem, and how quickly can it be fixed? I'm tentatively marking this as v2.0.0, just so that it comes up on our daily discussions.

I'm opening a separate issue to track the discussion that was occurring on open-mpi/ompi-release#1240 (so that 1240 can go back to its regularly scheduled programming).

[artpol84]

Here is the analysis and the solution for the problem:

• The root cause of the problem is the NEW OMPI request handling code, in particular - locking.
• The bug was introduced in open-mpi/ompi-release@f19731b and unmasked in open-mpi/ompi-release@b4ff061 (I was working with the release originally):

$ git bisect good
b4ff061bd45705eb39a91976fad855315ba990cc is the first bad commit
commit b4ff061bd45705eb39a91976fad855315ba990cc
Author: Nathan Hjelm <hjelmn@lanl.gov>
Date:   Wed May 25 15:42:53 2016 -0600

    pml/yalla: update for request changes

    This commit brings the pml/yalla component up to date with the request
    rework changes.

    Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>

    (cherry picked from open-mpi/ompi@9d439664f074a690ff8c550f9ef489bd70520a0f)

    Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>

:040000 040000 c8047a13774043cf31793730b23f9fcc3cfeb1d6 38e5a2e275229f11f92612f7f1bc541cd9147017 M      ompi

• I guess that Mellanox Jenkins was screaming about it since the end of the May.
• PR with the fix is proposed here: ompi request handling race condition fix (MT-case) #1815. I'm still verifying the code, but would like to here comments from @hjelmn and @bosilca.
• Problem analysis:
  In open-mpi/ompi-release@b4ff061 ompi_request_lock locks around critical yalla regions were removed which made possible the following race condition leading to a deadlock:
  • In multithreaded cases request processing can appear in one thread (will call it T1) and MPI_Wait is called in another (T2).
  • In the T1 we have the following backtrace (see below)
  • In wait_sync_update we are not guaranteed that after decreasing count to 0 in line 79 we won't loose the CPU.
  • In later case T2 can quickly execute and release sync object in line 348 before T1 would get to WAIT_SYNC_SIGNAL in line 87.
  • before open-mpi/ompi-release@b4ff061 the lock in yalla request free callback was holding T2 from releasing sync object before T1's signal.

#2  0x00007fffedec5de0 in wait_sync_update (sync=0x7fffffffd030, updates=1, status=0) at ../../../../opal/threads/wait_sync.h:91
#3  0x00007fffedec5ed3 in ompi_request_complete (request=0xb2c830, with_signal=true) at ../../../../ompi/request/request.h:423
#4  0x00007fffedec6ba0 in mca_pml_yalla_recv_completion_cb (context=0xb2c830) at pml_yalla_request.c:208
#5  0x00007fffedbb64e6 in mxm_proto_progress (context=0x782fc0) at mxm/core/mxm.c:89
#6  mxm_progress (context=0x782fc0) at mxm/core/mxm.c:347
#7  0x00007fffedec31a6 in mca_pml_yalla_progress () at pml_yalla.c:290
#8  0x00007ffff765b8c0 in opal_progress () at runtime/opal_progress.c:225
#9  0x00007fffedec3bc8 in mca_pml_yalla_recv (buf=0x0, count=0, datatype=0x601c40, src=1, tag=34532, comm=0x601640, status=0x0) at pml_yalla.c:383
#10 0x00007ffff7cfdd90 in PMPI_Recv (buf=0x0, count=0, type=0x601c40, source=1, tag=34532, comm=0x601640, status=0x0) at precv.c:77
#11 0x0000000000400c63 in threadfunc ()
#12 0x0000003d698079d1 in start_thread () from /lib64/libpthread.so.0
#13 0x0000003d690e8b6d in clone () from /lib64/libc.so.6

P.S. @hjelmn, Yalla is not guilty and the facts were pointing on the new request code.

[artpol84]

+ @karasevb

[jladd-mlnx]

@artpol84 Outstanding analysis. @hppritcha - LANL, we'll send you a bill for our services ☺️. @karasevb @artpol84 I think we better ensure that this change hasn't negatively impacted our message rates. Please test this with osu_mbw_mr against 1.10.3. @gpaulsen - is IBM keeping track of this in Spectrum MPI?

[jladd-mlnx]

@artpol84 @karasevb - please check the UCX PML as well. Probably the bug was pushed into that component as well.

[rhc54]

@artpol84 thanks - we knew Yalla wasn't guilty of having a new error, but clearly there is some interaction that involved the way Yalla used the request code vs how the other pml's etc do, and that really meant you folks needed to be involved. we'll definitely look forward to your patch.

[artpol84]

@jladd-mlnx this fix is general and fixes this kind of race conditions for all PML's (thanks to centralized approach to locking).
Will check osu_mbw_mr on Monday.

[gpaulsen]

@nysal - has been following the locking changes needed, and should be able to comment. We've hit something similar in our PAMI PML, and are just now reacting to it.