Repository navigation
[jaccl] Detect dead RDMA links instead of crashing - #4767
Open
ipazarbasi wants to merge 1 commit into
Open
ipazarbasi wants to merge 1 commit into
ipazarbasi wants to merge 1 commit into
Conversation
After a cable pull, JACCL currently ignores completion errors, posts another receive on the dead QP, and crashes in `tbt_post_recv`. No completion may ever arrive on the peer, and it hangs. `qp->state` only caches the last modify or query, so it never shows the QP going to ERR. To see a dead QP when no completion arrives, JACCL now calls `ibv_query_qp`. It is a syscall, so it only runs after a stall. * `poll()` checks the status of every completion and reports the most severe error. * `poll()` calls `ibv_query_qp` every 100 ms while stalled, and once more when a completion ends a stall longer than 2 ms, because a completion can report success on a dead QP. A QP in `ERR` or `SQE` is treated as dead. * All connections in a group share one failure latch which a failed post also trips * `dispatch_wires` waits for all wires, then rethrows the first exception. Before this patch, exceptions on pool threads were dropped. * Add device-free tests with a fake verbs provider Behavior changes: * A failed `ibv_post_send` or `ibv_post_recv` now throws `std::runtime_error` instead of `std::invalid_argument`. * Groups stay failed after their first error, and every later call on them throws that error. The application must tear the failed group down. Tested with two Macs over Thunderbolt 5 by pulling the cable at either end. Current code crashed in `tbt_post_recv` on every pull. The patched build raised a catchable error on both ranks in under 300 ms on 9 of 9 pulls (mesh and ring, `all_sum` and `all_gather`). `all_sum` timings from 1 KB to 64 MB match the current code within run-to-run noise. Refs ml-explore#4192
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After a cable pull, JACCL currently ignores completion errors, posts another receive on the dead QP, and crashes in
tbt_post_recv. No completion may ever arrive on the peer, and it hangs.qp->stateonly caches the last modify or query, so it never shows the QP going to ERR. To see a dead QP when no completion arrives, JACCL now callsibv_query_qp. It is a syscall, so it only runs after a stall.poll()checks the status of every completion and reports the most severe error.poll()callsibv_query_qpevery 100 ms while stalled, and once more when a completion ends a stall longer than 2 ms, because a completion can report success on a dead QP. A QP inERRorSQEis treated as dead.dispatch_wireswaits for all wires, then rethrows the first exception. Before this patch, exceptions on pool threads were dropped.Behavior changes:
ibv_post_sendoribv_post_recvnow throwsstd::runtime_errorinstead ofstd::invalid_argument.Tested with two Macs over Thunderbolt 5 by pulling the cable at either end. Current code crashed in
tbt_post_recvon every pull. The patched build raised a catchable error on both ranks in under 300 ms on 9 of 9 pulls (mesh and ring,all_sumandall_gather).all_sumtimings from 1 KB to 64 MB match the current code within run-to-run noise.Refs #4192