Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions projects/rccl/test/common/MPIEnvironment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#ifdef MPI_TESTS_ENABLED

#include <chrono>
#include <cstdlib>
#include <thread>
Comment thread
JhaShweta1 marked this conversation as resolved.
Resolved

/**
Expand Down Expand Up @@ -237,12 +238,22 @@ void MPIEnvironment::TearDown()
if(barrier_result == MPI_SUCCESS)
{
// Wait for barrier with a timeout.
// 3 s: ncclCommDestroy is now explicitly called (and MPI-barrier'd) inside
// cleanupTestCommunicator before TearDown runs, so the only remaining rank
// skew here is log-file I/O (RCCL_MPI_LOG_ALL_RANKS) which is fast.
int flag = 0;
auto timeout_start = std::chrono::steady_clock::now();
const auto timeout_duration = std::chrono::seconds(3);
// ncclCommDestroy is now explicitly called (and MPI-barrier'd) inside
// cleanupTestCommunicator before TearDown runs, so the common remaining rank
// skew here is log-file I/O (RCCL_MPI_LOG_ALL_RANKS). However, heavy
// multi-phase tests (e.g. GrowMPITest, NetIbMPITest CtsDepthStress) can have

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we see failures in these tests without this change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes: GrowMPITest, NetIbMPITest are passing now.

// ranks reach TearDown more than a few seconds apart even on success, so a
// 3 s timeout produced false "out of sync" aborts. Use a more tolerant default
// and allow override via RCCL_TEST_TEARDOWN_TIMEOUT_SEC.
int flag = 0;
auto timeout_start = std::chrono::steady_clock::now();
int timeout_sec = 60;
if(const char* env = std::getenv("RCCL_TEST_TEARDOWN_TIMEOUT_SEC"))
{
int v = std::atoi(env);
if(v > 0) timeout_sec = v;
}
const auto timeout_duration = std::chrono::seconds(timeout_sec);

while(!flag)
{
Expand All @@ -254,18 +265,21 @@ void MPIEnvironment::TearDown()
auto elapsed = std::chrono::steady_clock::now() - timeout_start;
if(elapsed > timeout_duration)
{
// Timeout - ranks are out of sync!
// Timeout - ranks are genuinely out of sync (e.g. a rank died or
// deadlocked in the test body). Force abort.
std::fprintf(
stderr,
"Rank %d: TIMEOUT in TearDown barrier - ranks out of sync, forcing abort\n",
world_rank);
"Rank %d: TIMEOUT (%ds) in TearDown barrier - ranks out of sync, forcing abort\n",
world_rank,
timeout_sec);
std::fflush(stderr);

// Cancel the barrier request
MPI_Cancel(&barrier_req);
MPI_Request_free(&barrier_req);

// Force abort - can't safely continue
// NOTE: Do NOT MPI_Cancel(&barrier_req) here. Cancelling a
// nonblocking *collective* request (MPI_Ibarrier) is forbidden by
// the MPI standard; OpenMPI returns MPI_ERR_REQUEST, and because
// MPI_COMM_WORLD uses MPI_ERRORS_ARE_FATAL that call aborts the job
// itself with a misleading error before we reach MPI_Abort. Since
// MPI_Abort tears the whole job down anyway, just call it directly.
MPI_Abort(MPI_COMM_WORLD, 1);
return;
}
Expand Down
Loading