feat: enhance stability of distributed execution #1580

yottahmd · 2026-01-13T15:19:51Z

Summary by CodeRabbit

Release Notes

New Features
- Added shared-nothing worker mode for distributed deployments.
- Improved remote progress display with terminal width awareness and worker ID truncation.
Bug Fixes
- Enhanced error handling for lost coordinator connections with consecutive error tracking.
- Added file sync for improved data durability in status writes.
Configuration
- Added configurable retry behavior for coordinator connections (MaxRetries and RetryInterval with exponential backoff).
Style
- Removed excess padding from table containers in DAG details UI.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-13T15:20:06Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

The PR implements shared-nothing worker mode, refactors the coordinator handler API from functional options to struct-based config, extends retry configuration for coordinator connections, improves remote progress display with terminal width handling, adds file sync durability to DAG writes, and includes error tracking for sub-DAG polling with consecutive failure thresholds.

Changes

Cohort / File(s)	Summary
Shared-nothing worker implementation `internal/cmd/context.go`	Introduces `isSharedNothingWorker()` detector and early-return context for workers with static coordinators; skips local store initialization (DAGRunStore, ProcStore, QueueStore) and enables coordinator retry config application.
Coordinator handler API refactoring `internal/cmd/coord.go`, `internal/service/coordinator/handler.go`, `internal/service/coordinator/handler_test.go`, `internal/test/coordinator.go`	Migrates from variadic `HandlerOption` pattern to structured `HandlerConfig` with fields (DAGRunStore, LogDir, StaleHeartbeatThreshold); updates all NewHandler() call sites and adds initial status writing for attempt creation.
Configuration schema extensions `internal/cmn/config/config.go`, `internal/cmn/config/definition.go`, `internal/cmn/config/loader.go`	Adds MaxRetries and RetryInterval fields to Peer config; introduces `loadPeerConfig()` helper for parsing and constructing Peer configuration from PeerDef.
Worker remote handling `internal/service/worker/remote_handler.go`, `internal/service/worker/handler.go`	Removes local DAGs directory loading from remote handler; simplifies original target logging; aligns with shared-nothing worker receiving DAG definitions from coordinator.
Remote progress display improvements `internal/cmd/progress_remote.go`	Adds terminal width awareness to RemoteProgressDisplay; implements `truncateWorkerDisplay()` and `truncateWorkerID()` helpers for width-aware header and final-line rendering with ANSI sequences.
Data persistence and error tracking `internal/persis/filedagrun/writer.go`, `internal/runtime/executor/dag_runner.go`	Adds file.Sync() after JSON writes for durability; introduces max consecutive error threshold (10) with counter for sub-DAG status polling, returns error on connection loss after retries.
Test infrastructure updates `internal/service/worker/worker_test.go`	Adds Definition field (YAML string) to Task proto-generated structs across multiple test cases; updates task literals in dispatch, comparison, and mock response scenarios.
UI styling adjustments `ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx`, `DAGStepTable.tsx`, `NodeStatusTable.tsx`	Removes p-px padding utility from container classNames in multiple components; adds overflow-x-hidden to DAGDetailsPanel main content container.

Sequence Diagram(s)

sequenceDiagram
    participant Worker
    participant Context
    participant CoordinatorClient
    participant Coordinator
    participant TaskPayload

    Worker->>Context: NewContext(cmd, config)
    Context->>Context: isSharedNothingWorker()?
    alt Shared-nothing Mode
        Context->>Context: Skip local store init
        Context->>Context: Apply retry config
        Context-->>Worker: Return context (nil stores)
        Worker->>TaskPayload: Extract DAG definition
        Worker->>CoordinatorClient: Send status updates
        CoordinatorClient->>Coordinator: Push status & logs
    else Standard Mode
        Context->>Context: Initialize all local stores
        Context-->>Worker: Return context (ready stores)
    end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~70 minutes

Possibly related PRs

feat: shared-nothing worker #1564: Directly implements the same shared-nothing worker feature with coordinator-related code path changes (context detection, handler config API, retry fields, status streaming).
feat(core): waiting status #1554: Refactors coordinator handler API from functional options to HandlerConfig struct, which mirrors the same API migration pattern in this PR.
worker: fix problems in shared nothing worker #1573: Covers overlapping shared-nothing worker and coordinator changes including context early-return logic, handler config updates, and remote progress handling.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main objective of the PR, which includes adding shared-nothing worker mode support, configurable retry behavior for coordinator connections, error tracking for status polling, and various stability improvements across distributed execution components.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

internal/cmn/config/loader.go (1)
293-311: Consider logging a warning on RetryInterval parse failure for consistency.

Other duration fields in this file (e.g., LockStaleThreshold at lines 740-744) log warnings when parsing fails. The silent failure here is inconsistent and could make configuration issues harder to diagnose.
♻️ Suggested fix
 	if def.RetryInterval != "" {
 		if d, err := time.ParseDuration(def.RetryInterval); err == nil {
 			peer.RetryInterval = d
+		} else {
+			l.warnings = append(l.warnings, fmt.Sprintf("Invalid peer.retryInterval value: %s", def.RetryInterval))
 		}
 	}
internal/cmd/progress_remote.go (1)
251-263: Edge case: truncation may cut into the arrow prefix.

When availableWidth is small (e.g., 5-7), the truncation at line 260 could cut into the " → " prefix, resulting in malformed output like " →…" or " …".

Consider ensuring minimum width accounts for the arrow:
♻️ Suggested improvement
 func (p *RemoteProgressDisplay) truncateWorkerID(availableWidth int) string {
-	if p.workerID == "" || availableWidth <= 5 {
+	// Arrow " → " is 3 chars + at least 1 char of ID + ellipsis = 5 minimum
+	const arrowLen = 4 // " → " with space
+	if p.workerID == "" || availableWidth < arrowLen+2 {
 		return ""
 	}
 
 	workerSuffix := " → " + p.workerID
 	if len(workerSuffix) > availableWidth {
-		return workerSuffix[:availableWidth-1] + "…"
+		// Ensure we keep the arrow and truncate only the worker ID part
+		maxIDLen := availableWidth - arrowLen - 1 // -1 for ellipsis
+		if maxIDLen <= 0 {
+			return ""
+		}
+		return " → " + p.workerID[:maxIDLen] + "…"
 	}
 	return workerSuffix
 }

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 732a891 and 71aa451.

📒 Files selected for processing (17)

internal/cmd/context.go
internal/cmd/coord.go
internal/cmd/progress_remote.go
internal/cmn/config/config.go
internal/cmn/config/definition.go
internal/cmn/config/loader.go
internal/persis/filedagrun/writer.go
internal/runtime/executor/dag_runner.go
internal/service/coordinator/handler.go
internal/service/coordinator/handler_test.go
internal/service/worker/handler.go
internal/service/worker/remote_handler.go
internal/service/worker/worker_test.go
internal/test/coordinator.go
ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx
ui/src/features/dags/components/dag-details/DAGStepTable.tsx
ui/src/features/dags/components/dag-details/NodeStatusTable.tsx

🧰 Additional context used

📓 Path-based instructions (5)

ui/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

ui/**/*.{ts,tsx}: The React + TypeScript frontend resides in ui/, with production bundles copied to internal/service/frontend/assets by make ui
UI code follows ESLint + Prettier (2-space indent) and Tailwind utilities; name React components in PascalCase (JobList.tsx) and hooks with use* (useJobs.ts)

Files:

ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx
ui/src/features/dags/components/dag-details/NodeStatusTable.tsx
ui/src/features/dags/components/dag-details/DAGStepTable.tsx

ui/**/*.{tsx,ts,jsx,js}

📄 CodeRabbit inference engine (ui/CLAUDE.md)

Avoid full-page loading overlays and LoadingIndicator components that hide content - show stale data while fetching updates instead

Files:

ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx
ui/src/features/dags/components/dag-details/NodeStatusTable.tsx
ui/src/features/dags/components/dag-details/DAGStepTable.tsx

ui/**/*.{tsx,jsx}

📄 CodeRabbit inference engine (ui/CLAUDE.md)

ui/**/*.{tsx,jsx}: Keep modal headers small and information-dense with minimal padding (e.g., p-2 or p-3 instead of p-4 or p-6)
Use compact form element heights: select boxes with h-7 or smaller, buttons with h-7 or h-8, inputs with compact padding (py-0.5 or py-1)
Minimize table and list row heights while maintaining readability, merge related columns to save space, and handle long text with whitespace-normal break-words
Use consistent metadata styling with uniform backgrounds (e.g., bg-slate-200 dark:bg-slate-700) and text hierarchy using size/weight over color variation
Use flexbox-first layouts with min-h-0 and overflow-hidden to prevent layout breaks, and account for fixed elements when setting heights
Support keyboard navigation in all interactive components including modals with arrow keys, enter, and escape keys
Avoid auto-focusing first items in modals unless it makes sense for the specific use case
Maintain sufficient color contrast in both light and dark modes, use proper ARIA labels, and ensure text remains readable at smaller sizes
Use transparent backgrounds in navigation elements and keep navigation components small and unobtrusive
Avoid two-line displays for single metadata items, excessive whitespace between elements, decorative elements without purpose, and modals that take up excessive screen space

Files:

ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx
ui/src/features/dags/components/dag-details/NodeStatusTable.tsx
ui/src/features/dags/components/dag-details/DAGStepTable.tsx

**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Backend entrypoint in cmd/ orchestrates the scheduler and CLI; runtime, persistence, and service layers sit under internal/* (for example internal/runtime, internal/persistence)
Keep Go files gofmt/goimports clean; use tabs, PascalCase for exported symbols (SchedulerClient), lowerCamelCase for locals, and Err... names for package-level errors
Repository linting relies on golangci-lint; prefer idiomatic Go patterns, minimal global state, and structured logging helpers in internal/common

Files:

internal/runtime/executor/dag_runner.go
internal/cmn/config/config.go
internal/service/worker/remote_handler.go
internal/service/worker/handler.go
internal/cmn/config/loader.go
internal/persis/filedagrun/writer.go
internal/service/worker/worker_test.go
internal/test/coordinator.go
internal/cmd/coord.go
internal/cmn/config/definition.go
internal/cmd/context.go
internal/service/coordinator/handler_test.go
internal/cmd/progress_remote.go
internal/service/coordinator/handler.go

**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*_test.go: Co-locate Go tests as *_test.go; favour table-driven cases and cover failure paths
Use stretchr/testify/require and shared fixtures from internal/test instead of duplicating mocks

Files:

internal/service/worker/worker_test.go
internal/service/coordinator/handler_test.go

🧠 Learnings (8)