Skip to content

Commit e5306b5

Browse files
ko3n1gclaude
andcommitted
fix(kubeflow): unique TrainJob name + forward all ranks (deduped)
- TrainJob name is now <basename>-<uuid6> (RFC-1123, <=33 chars) via the new train_job_basename field, decoupled from the experiment name. The uuid makes every launch unique, so concurrent/retried jobs never collide on the API server (the descriptive experiment name is intentionally non-unique). - fetch_logs now forwards every rank to stdout, de-duplicated: torchrun runs the same entrypoint on all ranks so startup/config/NCCL lines are identical; we strip the per-rank [pod/...]/[defaultN] markers and forward each distinct message once. This stops dropping the per-step loss line and wandb URL, which Megatron emits from a single layout-dependent rank (neither rank 0 nor last). The full per-rank stream still goes to log-allranks_0.out untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: oliver könig <okoenig@nvidia.com>
1 parent b597e6c commit e5306b5

1 file changed

Lines changed: 74 additions & 77 deletions

File tree

nemo_run/core/execution/kubeflow.py

Lines changed: 74 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@
1414
# limitations under the License.
1515

1616
import getpass
17-
import json
1817
import logging
1918
import os
2019
import re
2120
import subprocess
2221
import time
22+
import uuid
2323
from dataclasses import dataclass, field
2424
from enum import Enum
2525
from typing import Any, Iterable, Optional
@@ -110,6 +110,10 @@ class KubeflowExecutor(Executor):
110110
# the PVC sync. Use this to include local scripts/files that are not
111111
# generated by the packager (e.g. a hand-written training script).
112112
workdir_local_path: Optional[str] = None
113+
# Human-readable base for the generated TrainJob name. The k8s name becomes
114+
# ``<basename>-<uuid6>`` (RFC-1123 safe, ≤33 chars); the uuid keeps every
115+
# launch unique. Falls back to the launch ``name`` when unset.
116+
train_job_basename: Optional[str] = None
113117

114118
def __post_init__(self):
115119
if not _KUBERNETES_AVAILABLE:
@@ -281,6 +285,26 @@ def get_job_body(self, name: str, command: list[str]) -> dict:
281285

282286
# ── Submit / status / cancel / logs ──────────────────────────────────────
283287

288+
def _trainjob_name(self, fallback: str) -> str:
289+
"""RFC-1123 base name ``<basename>-<uuid6>`` (≤33 chars), generated once.
290+
291+
Shared by the TrainJob and its data-mover pod (created in ``package()``,
292+
before ``launch()``) so both are valid, unique per launch — the uuid
293+
avoids API-server collisions — and consistent. The basename is
294+
``train_job_basename`` (e.g. the model recipe) or the caller's name,
295+
sanitized to lowercase alphanumerics + dashes; capped at 33 chars to
296+
stay under the 63-char label limit with room for the ``-data-mover``
297+
suffix.
298+
"""
299+
cached = getattr(self, "_k8s_job_name", None)
300+
if cached is not None:
301+
return cached
302+
base = re.sub(r"[^a-z0-9-]+", "-", (self.train_job_basename or fallback or "job").lower()).strip("-")
303+
uid = uuid.uuid4().hex[:6]
304+
base = base[: 33 - len(uid) - 1].strip("-") or "job"
305+
self._k8s_job_name = f"{base}-{uid}"
306+
return self._k8s_job_name
307+
284308
def launch(
285309
self,
286310
name: str,
@@ -295,7 +319,7 @@ def launch(
295319
observed ``RUNNING``, ``SUCCEEDED``, or ``FAILED`` state when *wait* is ``True``.
296320
Raises ``RuntimeError`` if the job already exists or *timeout* expires.
297321
"""
298-
name = name.replace("_", "-").replace(".", "-").lower()
322+
name = self._trainjob_name(name)
299323
job_body = self.get_job_body(name, cmd)
300324
try:
301325
self._custom_objects_api.create_namespaced_custom_object(
@@ -413,21 +437,17 @@ def fetch_logs(
413437
*lines* lines from a single ``kubectl logs`` call.
414438
"""
415439
# Tail every rank to <job_dir>/log-allranks_0.out (downstream log
416-
# validation globs log*.out and needs every rank), but forward only
417-
# global rank 0 and the *last* global rank to the caller (stdout / CI
418-
# job log) — streaming all ranks at scale overruns CI job-log limits.
419-
#
420-
# Identifying the last global rank requires the authoritative node rank,
421-
# NOT the pod name. Kubeflow Trainer binds torchrun's PET_NODE_RANK to
422-
# the indexed-Job completion index, stamped on each pod as the
423-
# `batch.kubernetes.io/job-completion-index` label. So:
424-
# global_rank = job_completion_index * nproc_per_node + local_rank
425-
# `--prefix` tags each line with `[pod/<pod>/<container>]`; we map that
426-
# pod name → completion index (refreshed on every (re)connect, since a
427-
# gang restart spawns new pod names) and pair it with torchrun's
428-
# `[defaultN]` local-rank marker. `--tail=-1` replays each pod's full
429-
# history on (re)attach so mid-run lines are never dropped (the previous
430-
# `--tail <lines>` snapshot missed the last rank's per-step lines).
440+
# validation globs log*.out and needs every rank). Forward all ranks to
441+
# the caller (stdout / CI job log) too, but de-duplicated: torchrun runs
442+
# the same entrypoint on every rank, so the bulk of the volume (startup,
443+
# config dump, NCCL init) is byte-identical across ranks. We forward each
444+
# distinct message once — which is also why the rank-specific loss line
445+
# (emitted by a single, parallelism-layout-dependent rank that is usually
446+
# neither rank 0 nor the last rank) and genuine per-rank errors are no
447+
# longer dropped. `--prefix` tags each line with `[pod/<pod>/<container>]`
448+
# and torchrun adds `[defaultN]`; both are stripped to form the dedup key.
449+
# `--tail=-1` replays each pod's full history on (re)attach so mid-run
450+
# lines are never dropped.
431451
label_selector = f"jobset.sigs.k8s.io/jobset-name={job_name}"
432452
cmd = [
433453
"kubectl",
@@ -442,61 +462,40 @@ def fetch_logs(
442462
"--max-log-requests",
443463
str(self.num_nodes),
444464
]
445-
nproc = self.nproc_per_node()
446-
last_rank = max(self.num_nodes * nproc - 1, 0)
447-
pod_re = re.compile(r"pod/([^/]+)/")
448-
local_re = re.compile(r"\[default(\d+)\]")
449-
450-
def _pod_index_map() -> dict[str, int]:
451-
"""Map pod name → job-completion-index (== torchrun node rank)."""
452-
try:
453-
out = subprocess.run(
454-
[
455-
"kubectl",
456-
"get",
457-
"pods",
458-
"-n",
459-
self.namespace,
460-
"-l",
461-
label_selector,
462-
"-o",
463-
"json",
464-
],
465-
capture_output=True,
466-
text=True,
467-
timeout=timeout,
468-
)
469-
items = json.loads(out.stdout).get("items", [])
470-
except Exception as e:
471-
logger.warning("Could not list pods for %s: %s", job_name, e)
472-
return {}
473-
mapping: dict[str, int] = {}
474-
for item in items:
475-
meta = item.get("metadata", {})
476-
name = meta.get("name")
477-
idx = (meta.get("labels", {}) or {}).get("batch.kubernetes.io/job-completion-index")
478-
if name is not None and idx is not None:
479-
mapping[name] = int(idx)
480-
return mapping
481-
482-
def _forward_to_stdout(log_line: str, pod_index: dict[str, int]) -> bool:
483-
"""True for the first and last *global rank* only.
484-
485-
Kubeflow Trainer sets torchrun's PET_NODE_RANK from the JobSet
486-
completion-index label (static), so the global rank is
487-
``node_rank * nproc_per_node + local_rank`` where node_rank is the
488-
pod's completion index and local_rank is torchrun's ``[defaultN]``
489-
marker. We forward global rank 0 and ``world_size - 1`` only.
490-
"""
491-
pod_match = pod_re.search(log_line)
492-
local_match = local_re.search(log_line)
493-
if not pod_match or not local_match:
494-
return False
495-
node = pod_index.get(pod_match.group(1))
496-
if node is None:
465+
# Collapse the near-simultaneous cross-rank burst with a *sliding time
466+
# window* (cf. ClusterShell `clush -b`, which gathers identical output
467+
# across nodes into one line). torchrun runs the same entrypoint on
468+
# every rank, so startup/config/NCCL lines arrive as a burst of
469+
# byte-identical copies; we strip the per-rank `[pod/<pod>/<container>]`
470+
# and `[defaultN]` markers to form a dedup key and suppress a key only
471+
# if an identical line was already forwarded within `dedup_window_s`.
472+
# Unlike a global set this is bounded in both memory and time: a line
473+
# that legitimately recurs later (e.g. a periodic "saving checkpoint")
474+
# is forwarded again once the window passes, and a continuously
475+
# repeating line is rate-limited to once per window rather than
476+
# suppressed for the whole run. Lines whose body differs across ranks
477+
# (per-step loss, `[rankN]` errors) keep distinct keys and are never
478+
# collapsed. The full per-rank stream still goes to log-allranks_0.out.
479+
rank_marker_re = re.compile(r"\[pod/[^\]]+\]\s*|\[default\d+\]:?\s*")
480+
dedup_window_s = 60.0
481+
last_forwarded: dict[str, float] = {}
482+
483+
def _should_forward(log_line: str) -> bool:
484+
key = rank_marker_re.sub("", log_line).strip()
485+
if not key:
486+
return True
487+
now = time.monotonic()
488+
prev = last_forwarded.get(key)
489+
if prev is not None and now - prev < dedup_window_s:
497490
return False
498-
global_rank = node * nproc + int(local_match.group(1))
499-
return global_rank == 0 or global_rank == last_rank
491+
last_forwarded[key] = now
492+
# Bound memory: once the map is large, drop keys older than the
493+
# window (they can no longer suppress anything).
494+
if len(last_forwarded) > 20000:
495+
stale = now - dedup_window_s
496+
for k in [k for k, t in last_forwarded.items() if t < stale]:
497+
del last_forwarded[k]
498+
return True
500499

501500
all_ranks_path = os.path.join(self.job_dir, "log-allranks_0.out")
502501
os.makedirs(self.job_dir, exist_ok=True)
@@ -506,7 +505,6 @@ def _forward_to_stdout(log_line: str, pod_index: dict[str, int]) -> bool:
506505
# Retry kubectl logs -f until the job reaches a terminal state.
507506
# This handles both pods not yet running and transient mid-stream failures.
508507
while True:
509-
pod_index = _pod_index_map()
510508
proc = subprocess.Popen(
511509
cmd, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, text=True, bufsize=1
512510
)
@@ -516,14 +514,14 @@ def _forward_to_stdout(log_line: str, pod_index: dict[str, int]) -> bool:
516514
for line in iter(proc.stdout.readline, ""):
517515
if line:
518516
all_ranks_file.write(line)
519-
if _forward_to_stdout(line, pod_index):
517+
if _should_forward(line):
520518
lines_yielded += 1
521519
yield line
522520
if proc.poll() is not None:
523521
for remaining in proc.stdout:
524522
if remaining:
525523
all_ranks_file.write(remaining)
526-
if _forward_to_stdout(remaining, pod_index):
524+
if _should_forward(remaining):
527525
lines_yielded += 1
528526
yield remaining
529527
break
@@ -543,12 +541,11 @@ def _forward_to_stdout(log_line: str, pod_index: dict[str, int]) -> bool:
543541
)
544542
time.sleep(5)
545543
else:
546-
pod_index = _pod_index_map()
547544
result = subprocess.run(cmd, capture_output=True, text=True, timeout=timeout)
548545
with open(all_ranks_path, "a") as all_ranks_file:
549546
for line in result.stdout.splitlines():
550547
all_ranks_file.write(line + "\n")
551-
if _forward_to_stdout(line, pod_index):
548+
if _should_forward(line):
552549
yield line
553550

554551
def cancel(
@@ -614,7 +611,7 @@ def cancel(
614611
# ── Workdir sync helpers ──────────────────────────────────────────────────
615612

616613
def _data_mover_pod_name(self, job_name: str) -> str:
617-
return f"{job_name}-data-mover"
614+
return f"{self._trainjob_name(job_name)}-data-mover"
618615

619616
def _start_data_mover_pod(self, pod_name: str, timeout: int = 120) -> None:
620617
"""Spin up a throw-away Alpine pod that mounts workdir_pvc and blocks until Running.

0 commit comments

Comments
 (0)