Skip to content

Early stop#9

Merged
cheyang merged 8 commits into
rolebasedgroup:mainfrom
diw-zw:early-stop
Jun 22, 2026
Merged

Early stop#9
cheyang merged 8 commits into
rolebasedgroup:mainfrom
diw-zw:early-stop

Conversation

@diw-zw

@diw-zw diw-zw commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Add early termination to the auto-benchmark trial loop, plus failure-log collection and best-trial artifacting so long runs can stop fast and be debugged after the fact.

  • Early termination (strategy.earlyTermination in types.go) with three conditions, evaluated each trial in early_termination.go:
    • maxConsecutiveSLAFailures — gated by minTrials.
    • maxConsecutiveErrors — execution errors only (build/create/startup/runtime), default 3, not gated by minTrials.
    • maxSLAFailureRate — overall failure rate threshold in (0, 1].
    • The trigger reason is persisted on TemplateState.terminationReason and emitted in result.json.
    • Config validation for the new fields and for scenario.maxRequests in parse.go.
  • Trial error semantics: TrialResult.IsSLAFeasible now returns false when Error != ""; BuildResult excludes execution errors from SLA pass/fail counts.
  • Failure diagnostics (new failure_logs.go): snapshot pod RestartCount before each trial; on benchmark failure, dump current logs for Failed pods and previous logs for pods whose containers restarted mid-run.
  • Artifacts: controller logs tee'd to {expDir}/controller.log (cmd/autobenchmark/main.go); genai-bench output tee'd to {trialDir}/benchmark.log (evaluator/genaibench.go); best_trial.yaml written for GlobalBest and ready to kubectl apply.
  • Controller hardening / refactor: cleanup uses ExponentialBackoff (3 retries, treats NotFound as success) and skips when the RBG was never created; on experiment-level timeout the in-flight template is left non-Completed so the next run resumes it; endpoint resolution moved out to endpoint.go; a kubernetes.Interface clientset is added on Controller.
  • Docs: replace remaining kubectl rbg llm ... with llmctl ... in cmd/llmctl/README.md and doc/usage/*.md.

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

Copy link
Copy Markdown

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 transitions the CLI commands from kubectl rbg llm to llmctl across documentation and help strings, and refactors the auto-benchmark controller by modularizing endpoint resolution, early termination, and failure log collection. It also introduces early termination strategies based on SLA failures or execution errors, along with automated log collection for failed trials. The reviewer feedback highlights a critical logic bug in the early termination check where break is used instead of continue when skipping execution errors, and provides suggestions to use defer for more robust resource cleanup of streams and context cancellations.

Comment thread pkg/autobenchmark/controller/early_termination.go
Comment thread pkg/autobenchmark/controller/failure_logs.go
Comment thread pkg/autobenchmark/controller/controller.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds early termination and richer artifact/diagnostic outputs to the auto-benchmark controller so long-running experiments can stop sooner and failures can be debugged after the run. It also refactors endpoint readiness resolution and updates CLI/docs to use llmctl instead of legacy kubectl rbg llm ... examples.

Changes:

  • Add early-termination policy evaluation per template and persist termination reason into checkpoint/result outputs.
  • Add failure log collection for pods (including restart detection) and new artifacts (controller log tee, per-trial benchmark log tee, best_trial.yaml).
  • Refactor controller readiness/cleanup logic and update docs/CLI help text to llmctl.

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
pkg/autobenchmark/types/types.go Tightens SLA feasibility semantics (errors fail SLA) and persists template termination reason.
pkg/autobenchmark/evaluator/genaibench.go Adds benchmark.log tee for genai-bench runs.
pkg/autobenchmark/controller/reporter.go Excludes execution errors from SLA pass/fail counts and writes best_trial.yaml.
pkg/autobenchmark/controller/failure_logs.go New: collect pod logs/restart context to diagnose failed trials.
pkg/autobenchmark/controller/failure_logs_test.go New: unit tests for Pending-pod summarization.
pkg/autobenchmark/controller/endpoint.go New: endpoint/port resolution + readiness probing utilities.
pkg/autobenchmark/controller/endpoint_test.go New: unit tests for endpoint helpers and readiness checks.
pkg/autobenchmark/controller/early_termination.go New: early-termination condition evaluation logic.
pkg/autobenchmark/controller/early_termination_test.go New: unit tests covering termination conditions and execution-error semantics.
pkg/autobenchmark/controller/controller.go Integrates early termination, failure-log collection, resumable template handling, and hardened cleanup retries.
pkg/autobenchmark/config/types.go Adds strategy.earlyTermination config schema.
pkg/autobenchmark/config/parse.go Adds validation for early-termination config and scenario.maxRequests, plus defaults for maxConsecutiveErrors.
pkg/autobenchmark/config/parse_test.go Updates config fixtures to include scenario.maxRequests.
doc/usage/kubectl-rbg.md Updates examples from kubectl rbg llm ... to llmctl ....
doc/usage/kubectl-rbg-llm-svc.md Updates service command docs to llmctl ....
doc/usage/kubectl-rbg-llm-model.md Updates model command docs to llmctl ....
doc/usage/kubectl-rbg-llm-generate.md Updates generate command docs to llmctl ....
doc/usage/kubectl-rbg-llm-config.md Updates config command docs to llmctl ....
doc/usage/kubectl-rbg-benchmark.md Updates benchmark command docs to llmctl ....
cmd/llmctl/svc/run.go Updates user-facing messages/examples to llmctl ....
cmd/llmctl/svc/model_configs.go Updates help text/examples to llmctl ....
cmd/llmctl/svc/list.go Updates help text/examples to llmctl ....
cmd/llmctl/svc/delete.go Updates help text/examples to llmctl ....
cmd/llmctl/svc/chat/chat.go Updates help text/errors/examples to llmctl ....
cmd/llmctl/README.md Updates README command examples to llmctl ....
cmd/llmctl/model/pull.go Updates help text/errors/examples to llmctl ....
cmd/llmctl/model/list.go Updates help text/errors/examples to llmctl ....
cmd/llmctl/config/storage.go Updates help text/errors/examples to llmctl ....
cmd/llmctl/config/source.go Updates help text/errors/examples to llmctl ....
cmd/llmctl/config/misc.go Updates help text/examples to llmctl ....
cmd/llmctl/config/engine.go Updates help text/errors/examples to llmctl ....
cmd/llmctl/config/config.go Updates top-level config command help examples to llmctl ....
cmd/llmctl/benchmark/dashboard_cmd.go Updates help text/examples to llmctl ....
cmd/llmctl/benchmark/benchmark.go Updates user-facing “how to monitor” hint to llmctl ....
cmd/llmctl/autobenchmark/run.go Updates user-facing monitor/log hints to llmctl ....
cmd/autobenchmark/main.go Adds controller log tee to {expDir}/controller.log and injects clientset into controller.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/autobenchmark/controller/endpoint.go Outdated
Comment thread pkg/autobenchmark/config/parse.go Outdated
Comment thread pkg/autobenchmark/evaluator/genaibench.go
Comment thread pkg/autobenchmark/controller/failure_logs.go Outdated
Comment thread cmd/llmctl/svc/run.go Outdated
Comment thread cmd/llmctl/config/config.go Outdated
@cheyang

cheyang commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Reviewed the early-termination work end-to-end. The split into early_termination.go, endpoint.go, and failure_logs.go is a real improvement over the previous monolithic controller, and early_termination_test.go covers the interesting cases (consecutive vs. rate, MinTrials gating, execution errors excluded from SLA accounting, MaxConsecutiveErrors not gated). Resume-after-timeout in Run and the per-trial early-termination check in runTrials look correct, and the cleanup defer guarded by rbgCreated plus the deadline-detection helper (annotateTrialTimeout) close obvious gaps from before.

A few non-blocking notes inline: the if backend == "vllm" branch in extractServedModelName is dead code and reads like an incomplete refactor; the readiness probe should propagate the poll context into the HTTP GET; the cleanup wait.ExponentialBackoffWithContext with Factor: 1 is not actually exponential. Test coverage on the new failure_logs.go decision paths (PodFailed / restart-delta / pending-summary) would be valuable. Also one process question: the kubectl rbg llmllmctl rename is unrelated to the early-stop feature and would be easier to bisect if split out. Otherwise this is in good shape.

Comment thread pkg/autobenchmark/controller/endpoint.go Outdated
Comment thread pkg/autobenchmark/controller/endpoint.go Outdated
Comment thread pkg/autobenchmark/controller/controller.go Outdated
Comment thread pkg/autobenchmark/controller/endpoint.go Outdated
Comment thread pkg/autobenchmark/controller/controller.go
Comment thread pkg/autobenchmark/controller/failure_logs.go
Comment thread pkg/autobenchmark/controller/endpoint.go Outdated
Comment thread pkg/autobenchmark/controller/failure_logs.go Outdated

@cheyang cheyang 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.

/lgtm
/approve

@cheyang
cheyang merged commit 18d3b7f into rolebasedgroup:main Jun 22, 2026
5 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