Skip to content

Make multi-host result reporting deterministic - #316

Merged
divyashreepathihalli merged 3 commits into
keras-team:mainfrom
JyotinderSingh:deterministic-multi-host-results
Aug 19, 2026
Merged

Make multi-host result reporting deterministic#316
divyashreepathihalli merged 3 commits into
keras-team:mainfrom
JyotinderSingh:deterministic-multi-host-results

Conversation

@JyotinderSingh

Copy link
Copy Markdown
Collaborator

Description

Every pod of a Pathways/LeaderWorkerSet job got the identical command, including --result-gcs gs://{bucket}/{job_id}/result.pkl, and the runner uploaded its result unconditionally. Two consequences: the value returned to the client was whichever host wrote last, and a worker that raised after the leader uploaded a success payload surfaced as a _false_success_error carrying pod exit codes instead of the worker's exception.

The runner now resolves its process index from TPU_WORKER_ID (falling back to LWS_WORKER_INDEX, then to leader for single-host jobs) and:

  • only the leader writes result.pkl, so the returned value is always process 0's;
  • a non-leader writes result-worker-.pkl only when it fails, and discards its return value without serializing it on success;
  • both paths tag the payload with host_index.

JobHandle.result() aggregates those payloads for Pathways jobs: when the leader claims success on a FAILED job, or wrote no payload at all, the lowest-indexed failing host's exception is re-raised with its remote traceback and a note naming the other failing hosts. Pod exit summaries remain the fallback for hosts the kubelet killed before they could report. Single-pod backends skip the listing entirely.

Adds unit tests for the runner gating and the client aggregation, plus fake-gcs-server contract tests for the new storage helpers, and updates docs/guides/distributed_training.md and AGENTS.md.

Contributor Agreement

Please check all boxes below before submitting your PR for review:

  • I am a human, and not a bot.
  • I will be responsible for responding to review comments in a timely manner.
  • I will work with the maintainers to push this PR forward until submission.
  • I will test the changes on my cloud setup and provide proof of successful validation.

Every pod of a Pathways/LeaderWorkerSet job got the identical command,
including `--result-gcs gs://{bucket}/{job_id}/result.pkl`, and the
runner uploaded its result unconditionally. Two consequences: the value
returned to the client was whichever host wrote last, and a worker that
raised after the leader uploaded a success payload surfaced as a
`_false_success_error` carrying pod exit codes instead of the worker's
exception.

The runner now resolves its process index from TPU_WORKER_ID (falling
back to LWS_WORKER_INDEX, then to leader for single-host jobs) and:

- only the leader writes result.pkl, so the returned value is always
  process 0's;
- a non-leader writes result-worker-<index>.pkl only when it fails, and
  discards its return value without serializing it on success;
- both paths tag the payload with host_index.

`JobHandle.result()` aggregates those payloads for Pathways jobs: when
the leader claims success on a FAILED job, or wrote no payload at all,
the lowest-indexed failing host's exception is re-raised with its remote
traceback and a note naming the other failing hosts. Pod exit summaries
remain the fallback for hosts the kubelet killed before they could
report. Single-pod backends skip the listing entirely.

Adds unit tests for the runner gating and the client aggregation, plus
fake-gcs-server contract tests for the new storage helpers, and updates
docs/guides/distributed_training.md and AGENTS.md.
Rewrites the distributed-training section added with the previous commit
in short active sentences, and removes the idioms and the contractions
it carried. Splits the "read the pod logs" advice into two cases.

Adds the matching entries to the troubleshooting page:

- a new "A multi-host job fails, but the leader is successful" section
  that states which host reports the error and what the local error
  contains;
- a note under "Job failed but no result payload was found" that the
  message now appears only when no host of the job wrote a record;
- a note under "Job FAILS but logs look fine" that `kinetic jobs logs`
  returns the leader pod logs only, which can hide the failing host.
@codecov-commenter

codecov-commenter commented Aug 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.02181% with 215 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@a6adb41). Learn more about missing BASE report.

Files with missing lines Patch % Lines
kinetic/jobs_test.py 0.00% 124 Missing ⚠️
kinetic/runner/remote_runner_test.py 0.00% 84 Missing ⚠️
kinetic/jobs.py 90.19% 5 Missing ⚠️
kinetic/runner/remote_runner.py 96.77% 1 Missing ⚠️
kinetic/utils/storage.py 96.77% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #316   +/-   ##
=======================================
  Coverage        ?   36.73%           
=======================================
  Files           ?       83           
  Lines           ?    15532           
  Branches        ?        0           
=======================================
  Hits            ?     5706           
  Misses          ?     9826           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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 implements multi-host result ownership and failure aggregation for Pathways jobs. Under this new model, only the leader (process 0) writes the successful result payload, while non-leader hosts write failure payloads to worker-specific GCS blobs only if they fail. The client then aggregates these payloads and deterministically raises the exception of the lowest-indexed failing host. The feedback suggests optimizing the worker failure retrieval in kinetic/jobs.py by stopping the download loop as soon as the first valid failure payload is found, preventing sequential download bottlenecks on large multi-host jobs.

Comment thread kinetic/jobs.py Outdated
Review feedback on keras-team#316: the client downloaded every per-host failure
payload but used only the first one's contents — the rest contributed
their index to a note. One collective timeout leaves a payload on every
host, so a large slice paid N sequential downloads to name N-1 hosts.

`_worker_failure_error` now stops at the first payload that reports a
failure and takes the remaining indices straight from the listing, which
already carries them. A non-leader writes its blob only when it fails,
so presence in the listing is the signal. The scan still skips a leading
payload it cannot read, or one that claims success, so a corrupt blob on
the lowest-indexed host does not hide the next host's error.

`_worker_failures` collapses into `_list_worker_results`, which keeps the
guarded listing. The note now reads "Other hosts that also reported a
failure", matching what the listing alone proves.
@divyashreepathihalli
divyashreepathihalli merged commit 6850c3b into keras-team:main Aug 19, 2026
12 checks passed
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.

3 participants