-
Notifications
You must be signed in to change notification settings - Fork 589
fix(recover): warm up NCCL communicators before the first post-recover step #1548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0da16b0
8eaa075
c291222
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,12 @@ | |
| ) | ||
| from areal.engine.core.distributed import ( | ||
| init_custom_process_group, | ||
| nccl_process_groups, | ||
| warmup_all_to_all_transports, | ||
| warmup_collective_transports, | ||
| warmup_p2p_transports, | ||
| warmup_process_groups, | ||
| warmup_sharded_transports, | ||
| ) | ||
| from areal.engine.core.model import ( | ||
| disable_dropout_in_model, | ||
|
|
@@ -1721,6 +1726,53 @@ def _validate_fp8_consistency(self): | |
| def get_device_stats(self) -> DeviceRuntimeInfo: | ||
| return DeviceRuntimeInfo.get_current() | ||
|
|
||
| def warmup_communicators(self) -> None: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the issue only occurs under the conditions of mcore version 0.11 and ppsize=2, add a judgment in the function content to avoid undefined behavior as much as possible. |
||
| """Pre-connect the train step's communicators while memory is light. | ||
|
|
||
| NCCL allocates transport buffers per (communicator, protocol) on first | ||
| use. Left alone, those allocations land inside the first ppo_update at | ||
| peak occupancy, where the per-peer calloc fails on the last pipeline | ||
| stage. The buffers live for the lifetime of the process, so one warmup | ||
| covers every later step. | ||
|
|
||
| Issuing collectives on every registered group means an already unhealthy | ||
| rank surfaces here rather than at the first train step, hence the opt-in. | ||
| """ | ||
| if not self.config.warmup_communicators: | ||
| return | ||
| if not dist.is_initialized() or current_platform.device_type == "cpu": | ||
| return | ||
|
|
||
| warmup_collective_transports(*nccl_process_groups()) | ||
| if mpu.get_expert_model_parallel_world_size() > 1: | ||
| warmup_all_to_all_transports(mpu.get_expert_model_parallel_group()) | ||
| warmup_sharded_transports( | ||
| mpu.get_data_parallel_group(with_context_parallel=True) | ||
| ) | ||
|
|
||
| pp_size = mpu.get_pipeline_model_parallel_world_size() | ||
| if pp_size > 1: | ||
| pp_group = mpu.get_pipeline_model_parallel_group() | ||
| # AReaL turns off batch_p2p_comm (see areal/models/mcore/registry.py), | ||
| # so Megatron takes the unbatched path. That path sends both | ||
| # directions over pp_group, except at pp_size == 2 where one | ||
| # direction moves to WORLD so the two transfers can overlap; the | ||
| # 'ucc' backend is excluded because WORLD is always nccl. See | ||
| # megatron/core/pipeline_parallel/p2p_communication.py::_p2p_ops. | ||
| p2p_groups: list[dist.ProcessGroup] = [pp_group] | ||
| if pp_size == 2 and str(dist.get_backend(pp_group)).lower() != "ucc": | ||
| p2p_groups.append(dist.group.WORLD) | ||
| warmup_p2p_transports( | ||
| *p2p_groups, | ||
| prev_rank=mpu.get_pipeline_model_parallel_prev_rank(), | ||
| next_rank=mpu.get_pipeline_model_parallel_next_rank(), | ||
| has_prev=not mpu.is_pipeline_first_stage(ignore_virtual=True), | ||
| has_next=not mpu.is_pipeline_last_stage(ignore_virtual=True), | ||
| ) | ||
|
|
||
| current_platform.synchronize() | ||
| self.logger.info("Train-step communicator warmup complete") | ||
|
|
||
| def start_memory_profile(self, max_entries: int = 100000) -> None: | ||
| torch.cuda.memory._record_memory_history(max_entries=max_entries) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -371,6 +371,8 @@ def load( | |
| for name, engine_ in normalized_engine.items(): | ||
| self._load_checkpoint(engine_, name=name) | ||
|
|
||
| self._warmup_communicators(normalized_engine) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warmup occurs before SGLang's pause, KV-cache offload, and weight offload. Therefore, the allocation of 32 MiB probe tensors for warmup and the persistence of transport buffers will happen while SGLang is still resident. This is contrary to the goal of "while memory is light" and may retrigger the CUDA allocation failure that this PR attempts to address. Suggested fix: The AWEX colocated path should pause/offload rollout first, then perform communicator warmup, and then load the actor checkpoint. The non-colocated path can maintain an independent timeline. |
||
|
|
||
| if inference_engine is not None: | ||
| assert weight_update_meta is not None | ||
| update_engine = normalized_engine[inference_engine_update_from] | ||
|
|
@@ -440,6 +442,24 @@ def _save_checkpoint( | |
| engine.save(meta) | ||
| logger.info(f"Saved recover checkpoint to {path} (with_optim={with_optim})") | ||
|
|
||
| @staticmethod | ||
| def _warmup_communicators( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ray/Slurm's generic RPC will retry exceptions, including engine HTTP 500. RecoverHandler._warmup_communicators() ultimately catches all exceptions, logs a warning, and then continues to recover. If a rank fails to complete a collective due to allocation or synchronize failure after some work is done, other ranks may have already successfully returned. After the failed rank is retried individually, it will re-enter the first collective, while the other ranks no longer participate, causing a new deadlock. Even if NCCL errors are returned on all ranks, the communicator may already be damaged and cannot safely "degrade" to lazy connect as in the first training. Suggested fixes:
|
||
| normalized_engine: dict[str, TrainEngine | TrainController], | ||
| ) -> None: | ||
| for name, engine_ in normalized_engine.items(): | ||
| warmup = getattr(engine_, "warmup_communicators", None) | ||
| if warmup is None: | ||
| continue | ||
| try: | ||
| warmup() | ||
| except Exception: | ||
| logger.warning( | ||
| "Communicator warmup failed for engine %s; the first " | ||
| "train step will connect lazily instead.", | ||
| name, | ||
| exc_info=True, | ||
| ) | ||
|
|
||
| def _load_checkpoint( | ||
| self, | ||
| engine: TrainEngine | TrainController, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XCCL recovery triggers a single-sided entry into cross-engine all_reduce. Before recovery, the RLTrainer calls actor.connect_engine.XCCL to create a custom NCCL group containing the training PP head and inference workers. This group is registered into the process-level _world.pg_map via _new_process_group_helper. Subsequent recovery occurs in rl_trainer.py, where nccl_process_groups() enumerates the entire _world.pg_map, and warmup_collective_transports() performs all_reduce on each group. Warmup RPC is sent only to training workers, excluding inference workers within the same XCCL group. This results in the training PP head with the weight-update group indefinitely waiting for inference members until a process-group timeout; the exception handling in RecoverHandler cannot take effect before the RPC returns.
This is why the current tests haven't uncovered issues: tests/test_megatron_comm_warmup.py:80-83 always provides an empty pg_map.
Suggested fix: Avoid traversing the process-level NCCL registry. Explicitly enumerate train-step-owned groups, or record reliable owner/purpose for groups and exclude cross-engine weight-update groups. Add real multi-process recovery tests for XCCL connected rollouts.