Skip to content

feat: add disk-mode weight update flow to gateway#1237

Merged
rchardx merged 2 commits into
mainfrom
fw/wu2-disk
Apr 23, 2026
Merged

feat: add disk-mode weight update flow to gateway#1237
rchardx merged 2 commits into
mainfrom
fw/wu2-disk

Conversation

@garrett4wade

Copy link
Copy Markdown
Collaborator

Description

Add disk-mode weight updates to the experimental weight-update gateway so training workers can save checkpoints to shared storage and inference workers can load from disk. The update flow now consistently pauses and resumes inference around transfer operations, including error paths, and extends controller/gateway contracts for mode-specific fields.

Related Issue

Fixes #(issue)

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 💥 Breaking change
  • 📝 Documentation update
  • ♻️ Refactoring
  • ⚡ Performance improvement
  • ✅ Test coverage improvement

Checklist

  • I have read the Contributing Guide
  • Pre-commit hooks pass (pre-commit run --all-files)
  • Relevant tests pass; new tests added for new functionality
  • Documentation updated (if applicable; built with ./docs/build_all.sh)
  • Branch is up to date with main
  • Self-reviewed via /review-pr command
  • This PR was created by a coding agent via /create-pr
  • This PR is a breaking change

Breaking Change Details (if applicable):

N/A

Additional Context

Risk areas:

  • Disk path correctness and shared storage permissions in deployment.
  • Inference pause/continue endpoint behavior under transient failures.

Test commands run:

  • uv run pytest tests/experimental/weight_update/test_wu_controller.py
  • uv run pytest tests/experimental/weight_update/test_disk_integration.py -k "not multi_gpu and not slow and not sglang"

Skipped suites:

  • 3 multi-GPU/slow/sglang-marked cases in test_disk_integration.py (environment-dependent E2E coverage).

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces a disk-based weight update mechanism, allowing model weights to be transferred via shared storage as an alternative to the AWEX method. The changes include updates to the API structures, controllers, and the gateway to handle disk-specific parameters such as save paths and LoRA adapter names. The gateway now manages the update lifecycle by pausing inference, triggering saves from training workers, loading weights into inference workers, and resuming generation. Feedback focuses on optimizing the save process for distributed training to avoid redundant operations, validating mandatory fields for disk mode during the connection phase, and improving error logging with tracebacks.

Comment on lines +418 to +423
await asyncio.gather(
*[
_post_json(session, f"{url}/save", timeout_s, json_data=save_payload)
for url in pair_info.train_worker_urls
]
)

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.

high

Calling the /save endpoint on all training workers simultaneously can be problematic if the training engine (e.g., FSDP) performs a collective consolidated save. In such cases, the save operation should typically be triggered on only one rank (usually rank 0) to avoid redundant operations or race conditions when writing to shared storage. If the workers are expected to save shards, they would likely need distinct paths.

        # Trigger save on the first training worker (typically rank 0).
        # For distributed training like FSDP, a single trigger is usually sufficient
        # for a consolidated checkpoint save to shared storage.
        await _post_json(
            session,
            f"{pair_info.train_worker_urls[0]}/save",
            timeout_s,
            json_data=save_payload,
        )

Comment on lines +199 to +208
if body.mode == "disk":
pair_info = PairInfo(
pair_name=pair_name,
train_worker_urls=train_urls,
inference_worker_urls=inference_urls,
mode="disk",
save_path=body.save_path,
use_lora=body.use_lora,
lora_name=body.lora_name,
)

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.

medium

When mode is set to "disk", the save_path is required for the weight update flow to function correctly. It should be validated during the connection phase to return a clear error to the client if it's missing, rather than failing later during the transfer operation.

Suggested change
if body.mode == "disk":
pair_info = PairInfo(
pair_name=pair_name,
train_worker_urls=train_urls,
inference_worker_urls=inference_urls,
mode="disk",
save_path=body.save_path,
use_lora=body.use_lora,
lora_name=body.lora_name,
)
if body.mode == "disk":
if not body.save_path:
return JSONResponse(
status_code=400,
content={"error": "save_path is required for disk mode"},
)
pair_info = PairInfo(
pair_name=pair_name,
train_worker_urls=train_urls,
inference_worker_urls=inference_urls,
mode="disk",
save_path=body.save_path,
use_lora=body.use_lora,
lora_name=body.lora_name,
)

Comment on lines +491 to +495
logger.error(
"Weight update failed for pair '%s': %s",
pair_info.pair_name,
e,
)

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.

medium

Including the exception traceback in the log message will greatly assist in debugging weight update failures.

Suggested change
logger.error(
"Weight update failed for pair '%s': %s",
pair_info.pair_name,
e,
)
logger.error(
"Weight update failed for pair '%s'",
pair_info.pair_name,
exc_info=True,
)

@rchardx rchardx force-pushed the fw/wu2-disk branch 2 times, most recently from aa0094a to 9e33ee0 Compare April 23, 2026 10:36
garrett4wade and others added 2 commits April 23, 2026 18:36
Support disk-based weight transfer for weight-update pairs, including save-to-disk on train workers and load/update on inference workers. Ensure inference pause/continue is wrapped in a context manager so generation resumes even when updates fail.

Key changes:

- add disk mode fields to connect request and pair metadata

- implement disk transfer path with save/update_weights_from_disk and LoRA loading

- add controller and gateway tests for disk mode behavior
Fail fast on disk-mode misconfiguration so weight updates do not silently write to local relative paths instead of shared storage.

Key changes:
- reject empty disk-mode save_path values
- require absolute save_path values for disk mode
- require lora_name when disk-mode LoRA is enabled
- add gateway tests for rejected connect requests

@rchardx rchardx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@rchardx rchardx merged commit ae8c792 into main Apr 23, 2026
6 checks passed
@rchardx rchardx deleted the fw/wu2-disk branch April 23, 2026 10:38
SathyaGnanakumar pushed a commit to danielkiely/AReaL that referenced this pull request Apr 29, 2026
* feat: add disk-mode weight update flow to gateway

Support disk-based weight transfer for weight-update pairs, including save-to-disk on train workers and load/update on inference workers. Ensure inference pause/continue is wrapped in a context manager so generation resumes even when updates fail.

Key changes:

- add disk mode fields to connect request and pair metadata

- implement disk transfer path with save/update_weights_from_disk and LoRA loading

- add controller and gateway tests for disk mode behavior

* fix: validate disk-mode gateway connect inputs

Fail fast on disk-mode misconfiguration so weight updates do not silently write to local relative paths instead of shared storage.

Key changes:
- reject empty disk-mode save_path values
- require absolute save_path values for disk mode
- require lora_name when disk-mode LoRA is enabled
- add gateway tests for rejected connect requests

---------

Co-authored-by: Wentai Zhang <rchardx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants