Emil_Shanaty/SciMesh
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix coordinator worker integration

Browse Source
This commit is contained in:
Emil
2026-07-23 21:33:37 +03:00
parent b4a89dd7c2
commit 983c5843ec
25 changed files with 748 additions and 158 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coordinator/Makefile
+18 -4
View File
@@ -1,5 +1,15 @@
.PHONY: build run test test-integration vet lint tidy check migrate-up migrate-down up down down-clean logs ps rebuild psql smoke
# `check` deliberately uses its own Compose project and host ports. This keeps
# it from connecting to or replacing a developer's local PostgreSQL instance.
CHECK_PROJECT ?= scimesh-check
CHECK_POSTGRES_PORT ?= 55432
CHECK_COORDINATOR_PORT ?= 18080
CHECK_HOST ?= http://localhost:$(CHECK_COORDINATOR_PORT)
CHECK_TOKEN ?= dev-token
CHECK_DATABASE_URL ?= postgres://scimesh:scimesh@localhost:$(CHECK_POSTGRES_PORT)/scimesh?sslmode=disable
CHECK_COMPOSE = POSTGRES_PORT=$(CHECK_POSTGRES_PORT) COORDINATOR_PORT=$(CHECK_COORDINATOR_PORT) docker compose -p $(CHECK_PROJECT)
# --- build / run ---------------------------------------------------------
build:
go build ./...
@@ -23,12 +33,16 @@ vet:
# Needs Docker. Hand this to a reviewer.
check: vet lint
go test -race ./...
docker compose up -d --build
$(CHECK_COMPOSE) up -d --build
@echo "waiting for the coordinator to be ready..."
@sleep 6
TEST_DATABASE_URL='postgres://scimesh:scimesh@localhost:5432/scimesh?sslmode=disable' \
@attempt=0; until curl -fsS "$(CHECK_HOST)/health" >/dev/null; do \
attempt=$$((attempt + 1)); \
if [ $$attempt -ge 30 ]; then $(CHECK_COMPOSE) logs coordinator; exit 1; fi; \
sleep 1; \
done
TEST_DATABASE_URL="$(CHECK_DATABASE_URL)" \
go test -tags=integration ./internal/storage/postgres/ -v
./scripts/smoke.sh
HOST="$(CHECK_HOST)" TOKEN="$(CHECK_TOKEN)" ./scripts/smoke.sh
@echo "\nall checks passed ✓"
# Runs golangci-lint without installing it system-wide. Install it for speed:
coordinator/README.md
+11
View File
@@ -169,3 +169,14 @@ make test-integration TEST_DATABASE_URL='postgres://scimesh:scimesh@localhost:54
CI (`.github/workflows/coordinator.yml`) runs vet, gofmt, race tests, lint, and
the integration suite against a Postgres service on every push and PR.
For the complete local verification, including an isolated Docker PostgreSQL
and the HTTP smoke flow, run:
```sh
make check
```
It uses Compose project `scimesh-check` and ports `55432`/`18080` by default,
so it does not connect to a PostgreSQL already running on `5432`. Override
`CHECK_POSTGRES_PORT`, `CHECK_COORDINATOR_PORT`, or `CHECK_PROJECT` if needed.
coordinator/internal/domain/task.go
+12 -7
View File
@@ -117,8 +117,10 @@ const DefaultMaxAttempts = 3
func (t *Task) CanRetry() bool { return t.Attempt < t.MaxAttempts }
// IsLeaseHeldBy reports whether worker currently holds this task at attempt.
func (t *Task) IsLeaseHeldBy(worker string, attempt int) bool {
return t.LeaseOwner != nil && *t.LeaseOwner == worker && t.Attempt == attempt
func (t *Task) IsLeaseHeldBy(worker string, attempt int, now time.Time) bool {
return t.LeaseOwner != nil && t.LeaseExpiresAt != nil && now.Before(*t.LeaseExpiresAt) &&
*t.LeaseOwner == worker && t.Attempt == attempt &&
(t.Status == TaskLeased || t.Status == TaskRunning)
}
// AsClaimed projects the task into the trimmed view handed to a worker:
@@ -146,7 +148,7 @@ func (t *Task) AsClaimed() ClaimedTask {
// verifyLease is the guard every worker-driven transition shares: the caller
// must own the lease and reference the attempt it was granted.
func (t *Task) verifyLease(worker string, attempt int) error {
func (t *Task) verifyLease(worker string, attempt int, now time.Time) error {
// A task is worker-owned while leased or running: the first heartbeat moves
// it from leased to running, but ownership rules are identical for both.
if t.Status != TaskLeased && t.Status != TaskRunning {
@@ -158,13 +160,16 @@ func (t *Task) verifyLease(worker string, attempt int) error {
if t.Attempt != attempt {
return ErrStaleAttempt
}
if t.LeaseExpiresAt == nil || !now.Before(*t.LeaseExpiresAt) {
return ErrLeaseConflict
}
return nil
}
// RenewLease extends the lease of the worker that holds it. The first heartbeat
// also acknowledges start, moving the task from leased to running.
func (t *Task) RenewLease(worker string, attempt int, until time.Time) error {
if err := t.verifyLease(worker, attempt); err != nil {
func (t *Task) RenewLease(worker string, attempt int, now, until time.Time) error {
if err := t.verifyLease(worker, attempt, now); err != nil {
return err
}
t.LeaseExpiresAt = &until
@@ -195,7 +200,7 @@ func (t *Task) CompleteWith(resultArtifactID uuid.UUID, metrics map[string]any,
return ErrResultConflict
}
if err := t.verifyLease(worker, attempt); err != nil {
if err := t.verifyLease(worker, attempt, now); err != nil {
return err
}
@@ -214,7 +219,7 @@ func (t *Task) CompleteWith(resultArtifactID uuid.UUID, metrics map[string]any,
// Fail records a worker-reported failure. A retryable failure with attempts
// left returns the task to the queue; otherwise it terminates as failed.
func (t *Task) Fail(worker string, attempt int, code, message string, retryable bool, now time.Time) error {
if err := t.verifyLease(worker, attempt); err != nil {
if err := t.verifyLease(worker, attempt, now); err != nil {
return err
}
t.ErrorCode = &code
coordinator/internal/domain/task_test.go
+22 -7
View File
@@ -172,14 +172,14 @@ func TestFirstHeartbeatMovesLeasedToRunning(t *testing.T) {
task := leasedTask(1, 3)
until := testLater.Add(time.Hour)
if err := task.RenewLease(testWorker, 1, until); err != nil {
if err := task.RenewLease(testWorker, 1, testNow, until); err != nil {
t.Fatal(err)
}
if task.Status != TaskRunning {
t.Errorf("status = %q, want running after first heartbeat", task.Status)
}
// A second heartbeat keeps it running.
if err := task.RenewLease(testWorker, 1, until); err != nil {
if err := task.RenewLease(testWorker, 1, testNow, until); err != nil {
t.Fatal(err)
}
if task.Status != TaskRunning {
@@ -190,15 +190,15 @@ func TestFirstHeartbeatMovesLeasedToRunning(t *testing.T) {
func TestRunningTaskCanBeCompletedAndExpired(t *testing.T) {
// Complete works from running.
task := leasedTask(1, 3)
_ = task.RenewLease(testWorker, 1, testLater) // -> running
_ = task.RenewLease(testWorker, 1, testNow, testLater) // -> running
if err := task.CompleteWith(testResult, nil, testWorker, 1, testNow); err != nil {
t.Errorf("complete from running: %v", err)
}
// Expire reclaims a running task too.
task2 := leasedTask(1, 3)
_ = task2.RenewLease(testWorker, 1, testLater) // -> running
task2.ExpireLease(testNow)
_ = task2.RenewLease(testWorker, 1, testNow, testLater) // -> running
task2.ExpireLease(testLater)
if task2.Status != TaskPending {
t.Errorf("status = %q, want pending after a running lease expires", task2.Status)
}
@@ -208,14 +208,29 @@ func TestRenewLeaseExtendsOnlyForHolder(t *testing.T) {
task := leasedTask(1, 3)
until := testLater.Add(time.Hour)
if err := task.RenewLease(testWorker, 1, until); err != nil {
if err := task.RenewLease(testWorker, 1, testNow, until); err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !task.LeaseExpiresAt.Equal(until) {
t.Error("lease must be extended")
}
if err := task.RenewLease("worker-2", 1, until); !errors.Is(err, ErrLeaseConflict) {
if err := task.RenewLease("worker-2", 1, testNow, until); !errors.Is(err, ErrLeaseConflict) {
t.Errorf("err = %v, want ErrLeaseConflict", err)
}
}
func TestExpiredLeaseRejectsRenewalCompletionAndFailure(t *testing.T) {
task := leasedTask(1, 3)
expired := testLater.Add(time.Nanosecond)
if err := task.RenewLease(testWorker, 1, expired, expired.Add(time.Minute)); !errors.Is(err, ErrLeaseConflict) {
t.Errorf("renew expired lease: err = %v, want ErrLeaseConflict", err)
}
if err := task.CompleteWith(testResult, nil, testWorker, 1, expired); !errors.Is(err, ErrLeaseConflict) {
t.Errorf("complete expired lease: err = %v, want ErrLeaseConflict", err)
}
if err := task.Fail(testWorker, 1, "timeout", "expired", true, expired); !errors.Is(err, ErrLeaseConflict) {
t.Errorf("fail expired lease: err = %v, want ErrLeaseConflict", err)
}
}
coordinator/internal/storage/postgres/integration_test.go
+1 -1
View File
@@ -223,7 +223,7 @@ func TestUpdateRejectsStaleVersion(t *testing.T) {
if err != nil {
return err
}
if err := fresh.RenewLease("worker-1", fresh.Attempt, now.Add(2*time.Minute)); err != nil {
if err := fresh.RenewLease("worker-1", fresh.Attempt, now, now.Add(2*time.Minute)); err != nil {
return err
}
return repo.Update(ctx, fresh)
coordinator/internal/transport/http/errors.go
+8 -1
View File
@@ -3,6 +3,7 @@ package http
import (
"encoding/json"
"errors"
"io"
"net/http"
"github.com/emil28092005/SciMesh/coordinator/internal/domain"
@@ -24,7 +25,13 @@ func decodeJSON(r *http.Request, dst any) error {
// Reject unknown fields: silently ignoring a misspelled "worker_ID" would
// surface later as a baffling validation failure.
dec.DisallowUnknownFields()
return dec.Decode(dst)
if err := dec.Decode(dst); err != nil {
return err
}
if err := dec.Decode(&struct{}{}); !errors.Is(err, io.EOF) {
return errors.New("request body must contain exactly one JSON value")
}
return nil
}
// writeError translates domain errors into status codes. This mapping is the
coordinator/internal/transport/http/handlers.go
+36 -8
View File
@@ -194,11 +194,14 @@ func (s *Server) handleUploadDataset(w http.ResponseWriter, r *http.Request) {
}
var (
workload string
params map[string]any
rows = defaultChunkRows
result usecase.SubmitDatasetResult
gotDataset bool
workload string
params map[string]any
rows = defaultChunkRows
result usecase.SubmitDatasetResult
gotDataset bool
gotWorkload bool
gotParams bool
gotRows bool
)
for {
@@ -213,9 +216,18 @@ func (s *Server) handleUploadDataset(w http.ResponseWriter, r *http.Request) {
switch part.FormName() {
case "workload":
if gotDataset || gotWorkload {
s.writeError(w, r, domain.ErrInvalidInput)
return
}
b, _ := io.ReadAll(io.LimitReader(part, 1<<10))
workload = strings.TrimSpace(string(b))
gotWorkload = true
case "parameters":
if gotDataset || gotParams {
s.writeError(w, r, domain.ErrInvalidInput)
return
}
b, _ := io.ReadAll(io.LimitReader(part, 1<<16))
if len(b) > 0 {
if err := json.Unmarshal(b, &params); err != nil {
@@ -223,12 +235,25 @@ func (s *Server) handleUploadDataset(w http.ResponseWriter, r *http.Request) {
return
}
}
gotParams = true
case "chunk_rows":
b, _ := io.ReadAll(io.LimitReader(part, 32))
if n, err := strconv.Atoi(strings.TrimSpace(string(b))); err == nil {
rows = n
if gotDataset || gotRows {
s.writeError(w, r, domain.ErrInvalidInput)
return
}
b, _ := io.ReadAll(io.LimitReader(part, 32))
n, err := strconv.Atoi(strings.TrimSpace(string(b)))
if err != nil || n < 1 {
s.writeError(w, r, domain.ErrInvalidInput)
return
}
rows = n
gotRows = true
case "file", "dataset":
if gotDataset || workload == "" {
s.writeError(w, r, domain.ErrInvalidInput)
return
}
filename := part.FileName()
if filename == "" {
filename = "dataset"
@@ -246,6 +271,9 @@ func (s *Server) handleUploadDataset(w http.ResponseWriter, r *http.Request) {
return
}
gotDataset = true
default:
s.writeError(w, r, domain.ErrInvalidInput)
return
}
_ = part.Close()
}
coordinator/internal/transport/http/server_test.go
+31
View File
@@ -259,6 +259,37 @@ func TestErrorMappings(t *testing.T) {
}
}
func TestJSONRejectsTrailingValue(t *testing.T) {
e := newEnv(t, healthy)
if code, _ := e.do(t, "POST", "/workers/register",
`{"name":"lab","capabilities":["w"]} {}`); code != http.StatusBadRequest {
t.Errorf("status = %d, want 400", code)
}
}
func TestUploadDatasetRejectsAmbiguousMultipartInput(t *testing.T) {
e := newEnv(t, healthy)
var buf bytes.Buffer
mw := multipart.NewWriter(&buf)
_ = mw.WriteField("workload", "w")
_ = mw.WriteField("chunk_rows", "not-a-number")
fw, _ := mw.CreateFormFile("file", "chembl.tsv")
_, _ = io.Copy(fw, strings.NewReader("id\tsmiles\nA\tCC\n"))
_ = mw.Close()
req, _ := http.NewRequestWithContext(context.Background(), "POST", e.ts.URL+"/jobs/upload", &buf)
req.Header.Set("Authorization", "Bearer "+token)
req.Header.Set("Content-Type", mw.FormDataContentType())
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatal(err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusBadRequest {
t.Errorf("status = %d, want 400", resp.StatusCode)
}
}
// --- helpers -------------------------------------------------------------
func (e *env) putArtifact(t *testing.T, taskID, worker string, attempt int, data string) string {
coordinator/internal/usecase/artifact.go
+1 -1
View File
@@ -29,7 +29,7 @@ func (uc *UploadArtifact) Execute(ctx context.Context, in UploadArtifactInput) (
}
// Only the worker holding the current lease at this attempt may upload the
// task's output — the coordinator never trusts an ownership claim on faith.
if !task.IsLeaseHeldBy(in.WorkerID, in.Attempt) {
if !task.IsLeaseHeldBy(in.WorkerID, in.Attempt, uc.clk.Now()) {
return nil, domain.ErrLeaseConflict
}
coordinator/internal/usecase/task.go
+2 -1
View File
@@ -91,7 +91,8 @@ func (uc *RenewLease) Execute(ctx context.Context, in RenewLeaseInput) (*domain.
if err != nil {
return err
}
if err := task.RenewLease(in.WorkerID, in.Attempt, uc.clock.Now().Add(uc.leaseDuration)); err != nil {
now := uc.clock.Now()
if err := task.RenewLease(in.WorkerID, in.Attempt, now, now.Add(uc.leaseDuration)); err != nil {
return err
}
if err := uc.tasks.Update(ctx, task); err != nil {