add kiro-sdd documents
This commit is contained in:
parent
e847a32546
commit
033f34c07f
10 changed files with 1758 additions and 0 deletions
435
.kiro/specs/resval-id-decoupling/design.md
Normal file
435
.kiro/specs/resval-id-decoupling/design.md
Normal file
|
|
@ -0,0 +1,435 @@
|
|||
# Design: resval-id-decoupling
|
||||
|
||||
> Feature: resval-id-decoupling
|
||||
> Created: 2026-05-12
|
||||
> Status: Design Generated
|
||||
|
||||
## Architecture Pattern & Boundary Map
|
||||
|
||||
**Pattern:** Targeted replacement of an index-keyed cross-subsystem data carrier with a name-keyed one, following the precedent set by `Data_data_resvals` (Path C). No new architectural layers are introduced.
|
||||
|
||||
**Affected boundary:** Controller → Scheduler (initial config push) and Controller → Scheduler/API-server (per-worker data push). The wire protocol (`controller.capnp`) at these two field positions changes from `List(ValueWithIdx)` to `List(NameValue)`.
|
||||
|
||||
**Scope:**
|
||||
|
||||
```
|
||||
src/schema/controller.capnp ← 2 field type edits
|
||||
src/include/xce/types.h ← 1 struct field type change
|
||||
src/include/typemap.h ← 1 struct field type change
|
||||
src/include/utils.h ← swap free function declaration
|
||||
src/controller/include/cluster.h ← remove collect_resval_with_idx declaration
|
||||
src/controller/dataex.c ← replace collect_resval_with_idx with collect_resval
|
||||
src/controller/sched.c ← 3 call-site updates
|
||||
src/controller/apiserver.c ← 2 call-site updates
|
||||
src/controller/reply.c ← 1 free call update
|
||||
src/controller/opget.c ← 1 free call update
|
||||
src/lib/utils.c ← swap free function implementation
|
||||
src/scheduler/src/scheduler.c ← sched_init_sharedInst update
|
||||
src/scheduler/src/schedobj.c ← make_sched_worker update
|
||||
```
|
||||
|
||||
**Not in scope:** `src/scheduler/src/handler.c` (Path C untouched), `src/frontend/apiserver/` Go code (no direct access to `resvals_to_sched` in Go layer), capnp generated files (auto-regenerated by `ninja -C build` after schema edit).
|
||||
|
||||
## Technology Stack & Alignment
|
||||
|
||||
| Aspect | Choice | Rationale |
|
||||
|--------|--------|-----------|
|
||||
| Language | C99 | Matches existing codebase convention |
|
||||
| Wire protocol | Cap'n Proto — `List(NameValue)` replacing `List(ValueWithIdx)` | `NameValue`/`xce_name_val_t` already defined in schema and types; codec already generated |
|
||||
| Build | Meson + capnpc-c (existing) | Schema edit triggers automatic regeneration of `controller.capnp.[ch]` |
|
||||
| Logging | Existing `WARN`/`ERROR` macros | Consistent with codebase; unknown resource → `WARN` + skip |
|
||||
| Iterator pattern | `iter_make` / `iter_next` / `iter_destroy` with read lock | Matches `collect_resources()` pattern already in `dataex.c` |
|
||||
| Backward compatibility | Not required | No public release; breaking wire change is acceptable |
|
||||
|
||||
## Components & Interface Contracts
|
||||
|
||||
---
|
||||
|
||||
### Component 1: `controller.capnp` — Wire type changes (Req 1, 2, 3)
|
||||
|
||||
Two field type replacements in `src/schema/controller.capnp`. Both field ordinal numbers are preserved (no renumbering); only the element type changes.
|
||||
|
||||
```
|
||||
Before:
|
||||
struct Worker { resvals @5: List(ValueWithIdx) ... }
|
||||
struct Conf4Sched { resvals @6: List(ValueWithIdx) ... }
|
||||
|
||||
After:
|
||||
struct Worker { resvals @5: List(NameValue) ... }
|
||||
struct Conf4Sched { resvals @6: List(NameValue) ... }
|
||||
```
|
||||
|
||||
After this edit, `ninja -C build` regenerates `controller.capnp.[ch]`. The generated
|
||||
`encode_Conf4Sched` and `decode_Conf4Sched` functions will automatically call
|
||||
`encode_NameValue_list` / `decode_NameValue_list` (already implemented in the generated
|
||||
code) instead of their `ValueWithIdx` counterparts.
|
||||
|
||||
**Requirement traceability:** 1.1, 2.1, 3.1, 4.2
|
||||
|
||||
---
|
||||
|
||||
### Component 2: Type changes in shared headers (Req 1, 2, 3, 4)
|
||||
|
||||
#### `src/include/xce/types.h` — `xce_worker_t.resvals_to_sched`
|
||||
|
||||
```
|
||||
Before:
|
||||
int nresvals_to_sched;
|
||||
xce_val_with_idx_t** resvals_to_sched;
|
||||
|
||||
After:
|
||||
int nresvals_to_sched;
|
||||
xce_name_val_t** resvals_to_sched;
|
||||
```
|
||||
|
||||
#### `src/include/typemap.h` — `xce_conf4sched_t.resvals`
|
||||
|
||||
```
|
||||
Before:
|
||||
int nresvals;
|
||||
xce_val_with_idx_t** resvals;
|
||||
|
||||
After:
|
||||
int nresvals;
|
||||
xce_name_val_t** resvals;
|
||||
```
|
||||
|
||||
**Requirement traceability:** 1.1, 2.1, 4.2
|
||||
|
||||
---
|
||||
|
||||
### Component 3: `dataex.c` — New collector; remove old (Req 1, 2, 5)
|
||||
|
||||
#### Remove: `collect_resval_with_idx_` and `collect_resval_with_idx`
|
||||
|
||||
Both the static inner function and its public wrapper are deleted. The declaration in
|
||||
`src/controller/include/cluster.h` is also removed.
|
||||
|
||||
#### Add: `collect_resval`
|
||||
|
||||
Iterates `resmgr` (not the zarray directly) so resource names are always available.
|
||||
For each named resource, looks up its value by `r->idx` in the provided `zarray_t`.
|
||||
|
||||
```
|
||||
Signature:
|
||||
int collect_resval(var resmgr, zarray_t* array, xlist_t* l)
|
||||
|
||||
Parameters:
|
||||
resmgr — read-locked resource manager; provides name→idx mapping
|
||||
array — zarray_t holding xce_val_t* values indexed by r->idx (lock held by caller)
|
||||
l — output xlist_t; receives xce_name_val_t* entries (caller owns memory)
|
||||
|
||||
Behavior:
|
||||
1. lock(resmgr, RTTI_LOCK_READ)
|
||||
2. iter_make(&it, resmgr)
|
||||
3. For each xce_resource_t* r from iter_next:
|
||||
a. If r == NULL, continue
|
||||
b. src_val = (xce_val_t*)zarray_get(array, r->idx) [array already caller-locked]
|
||||
c. If src_val == NULL, continue [resource exists but has no value yet]
|
||||
d. Allocate xce_name_val_t* nv = calloc(1, sizeof(*nv))
|
||||
e. nv->name = strdup(r->name)
|
||||
f. nv->val = calloc(1, sizeof(xce_val_t))
|
||||
g. Copy value: if svalue, strdup; else memcpy
|
||||
h. xlist_store(l, nv)
|
||||
4. iter_destroy(&it)
|
||||
5. unlock(resmgr)
|
||||
6. Return l->count
|
||||
|
||||
Return value:
|
||||
>= 0 number of name-val entries collected
|
||||
-ENOMEM allocation failure (partial list may exist; caller responsible for cleanup)
|
||||
|
||||
Note: array lock must be acquired by the caller before calling this function,
|
||||
consistent with how collect_resval_with_idx_ was called from collect_resval_with_idx
|
||||
(which held array->lock). This keeps locking responsibility explicit.
|
||||
```
|
||||
|
||||
#### Update: `collect_shared_resvals`
|
||||
|
||||
```
|
||||
Before:
|
||||
void collect_shared_resvals(cluster_t* c, collect_check_fn_t fn, xlist_t* l) {
|
||||
collect_resval_with_idx(&(c->sharedinst), l);
|
||||
}
|
||||
|
||||
After:
|
||||
void collect_shared_resvals(cluster_t* c, collect_check_fn_t fn, xlist_t* l) {
|
||||
mylock_lock(&(c->sharedinst.lock));
|
||||
collect_resval(c->resmgr, &(c->sharedinst), l);
|
||||
mylock_unlock(&(c->sharedinst.lock));
|
||||
}
|
||||
```
|
||||
|
||||
`fn` parameter is unused (was unused before); kept for signature compatibility with
|
||||
other `collect_*` functions. The `cluster_t*` provides access to both `resmgr` and
|
||||
`sharedinst`.
|
||||
|
||||
#### Update: `collect_resvals_workers` (inlined into `collect_workers`)
|
||||
|
||||
```
|
||||
Before (in collect_resvals_workers):
|
||||
collect_resval_with_idx(&(w->resvals), &lst);
|
||||
|
||||
After:
|
||||
mylock_lock(&(w->resvals.lock));
|
||||
collect_resval(resmgr, &(w->resvals), &lst);
|
||||
mylock_unlock(&(w->resvals.lock));
|
||||
```
|
||||
|
||||
`collect_resvals_workers` requires access to `resmgr`. Pass it as a new parameter, or
|
||||
access it from the containing `cluster_t*` context (the function is only called from
|
||||
`collect_workers` which receives `cluster_t* c`). Preferred: pass `c->resmgr` from
|
||||
`collect_workers`.
|
||||
|
||||
Updated `collect_resvals_workers` signature:
|
||||
|
||||
```
|
||||
static void collect_resvals_workers(var resmgr, xlist_t* l)
|
||||
```
|
||||
|
||||
**Requirement traceability:** 1.1, 2.1, 5.1, 5.2
|
||||
|
||||
---
|
||||
|
||||
### Component 4: `src/lib/utils.c` and `src/include/utils.h` — Free function swap (Req 5)
|
||||
|
||||
#### Remove: `free_resval_with_idx_list`
|
||||
|
||||
The existing implementation and declaration are deleted. No remaining callers after
|
||||
the controller-side changes in Component 5.
|
||||
|
||||
#### Add: `free_name_val_list`
|
||||
|
||||
Mirrors the removed function's structure; frees each `xce_name_val_t*` entry.
|
||||
|
||||
```
|
||||
Signature:
|
||||
void free_name_val_list(xce_name_val_t*** vals, int count)
|
||||
|
||||
Behavior:
|
||||
1. resvals = *vals
|
||||
2. If resvals == NULL or count <= 0, return
|
||||
3. For i in [0, count):
|
||||
a. nv = resvals[i]; if NULL, continue
|
||||
b. FREEUP(nv->name)
|
||||
c. If nv->val != NULL:
|
||||
if nv->val->which == Value_u_svalue: FREEUP(nv->val->u.svalue)
|
||||
FREEUP(nv->val)
|
||||
d. FREEUP(nv)
|
||||
4. FREEUP(*vals)
|
||||
|
||||
Header declaration (utils.h):
|
||||
extern void free_name_val_list(xce_name_val_t***, int);
|
||||
```
|
||||
|
||||
**Requirement traceability:** 5.1
|
||||
|
||||
---
|
||||
|
||||
### Component 5: Controller call sites — `sched.c` and `apiserver.c` (Req 1, 2, 3)
|
||||
|
||||
All call sites that previously called `collect_resval_with_idx` and
|
||||
`free_resval_with_idx_list` are updated to use the new functions and types.
|
||||
|
||||
#### `sched.c` — `xcesched_push_configs` (shared resvals, Req 1)
|
||||
|
||||
```
|
||||
Before:
|
||||
collect_shared_resvals(c, NULL, &xreslist);
|
||||
configdata.data.conf4sched->nresvals = xreslist.count;
|
||||
configdata.data.conf4sched->resvals = (xce_val_with_idx_t**)xreslist.data;
|
||||
...
|
||||
free_resval_with_idx_list(&(configdata.data.conf4sched->resvals),
|
||||
configdata.data.conf4sched->nresvals);
|
||||
|
||||
After:
|
||||
collect_shared_resvals(c, NULL, &xreslist);
|
||||
configdata.data.conf4sched->nresvals = xreslist.count;
|
||||
configdata.data.conf4sched->resvals = (xce_name_val_t**)xreslist.data;
|
||||
...
|
||||
free_name_val_list(&(configdata.data.conf4sched->resvals),
|
||||
configdata.data.conf4sched->nresvals);
|
||||
```
|
||||
|
||||
#### `sched.c` — `xcesched_push_workerlist` (worker resvals, Req 2)
|
||||
|
||||
```
|
||||
Before:
|
||||
collect_resval_with_idx(&(w->host.resvals), &resvalist);
|
||||
w->host.nresvals_to_sched = resvalist.count;
|
||||
w->host.resvals_to_sched = (xce_val_with_idx_t**)resvalist.data;
|
||||
...
|
||||
free_resval_with_idx_list(&(w->host.resvals_to_sched), w->host.nresvals_to_sched);
|
||||
|
||||
After:
|
||||
mylock_lock(&(w->host.resvals.lock));
|
||||
collect_resval(c->resmgr, &(w->host.resvals), &resvalist);
|
||||
mylock_unlock(&(w->host.resvals.lock));
|
||||
w->host.nresvals_to_sched = resvalist.count;
|
||||
w->host.resvals_to_sched = (xce_name_val_t**)resvalist.data;
|
||||
...
|
||||
free_name_val_list(&(w->host.resvals_to_sched), w->host.nresvals_to_sched);
|
||||
```
|
||||
|
||||
#### `apiserver.c` — `apiserver_worker_change` (Req 3)
|
||||
|
||||
Same pattern as the `sched.c` worker case above. Replace collect and free calls;
|
||||
change cast to `(xce_name_val_t**)`.
|
||||
|
||||
#### `sched.c:226`, `reply.c:66`, `opget.c:128` — cleanup-only sites
|
||||
|
||||
Replace `free_resval_with_idx_list` with `free_name_val_list` at each cleanup site
|
||||
where `w->resvals_to_sched` is freed without being re-populated.
|
||||
|
||||
**Requirement traceability:** 1.1, 2.1, 3.1, 5.1
|
||||
|
||||
---
|
||||
|
||||
### Component 6: Scheduler — `sched_init_sharedInst` (Req 1, 6)
|
||||
|
||||
Receives `xce_conf4sched_t.resvals` which is now `xce_name_val_t**`.
|
||||
For each entry, look up the resource by name in `sched->resmgr` to get the local idx,
|
||||
then store into `sched->sharedInst`.
|
||||
|
||||
**Critical ordering constraint:** `sched_init_resources()` must be called before
|
||||
`sched_init_sharedInst()`. This is already guaranteed by `sched_init_config()`
|
||||
(`scheduler.c:603-609`). This call order must not change.
|
||||
|
||||
```
|
||||
Before:
|
||||
static int sched_init_sharedInst(xce_scheduler_t* sched) {
|
||||
for (i = 0; i < sched->config->nresvals; i++) {
|
||||
int idx = sched->config->resvals[i]->idx;
|
||||
xce_val_t* val = NULL;
|
||||
MOVE_PTR(val, sched->config->resvals[i]->val);
|
||||
zarray_put(&(sched->sharedInst), idx, val);
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
After:
|
||||
static int sched_init_sharedInst(xce_scheduler_t* sched) {
|
||||
for (i = 0; i < sched->config->nresvals; i++) {
|
||||
xce_name_val_t* nv = sched->config->resvals[i];
|
||||
xce_resource_t* r = NULL;
|
||||
xce_val_t* val = NULL;
|
||||
|
||||
if (nv == NULL || nv->name == NULL) continue;
|
||||
|
||||
if (key_find(sched->resmgr, (void*)nv->name, (void**)&r) < 0 || r == NULL) {
|
||||
WARN("%s unknown resource '%s' in resvals, skipping",
|
||||
__func__, nv->name);
|
||||
continue;
|
||||
}
|
||||
|
||||
MOVE_PTR(val, nv->val);
|
||||
zarray_put(&(sched->sharedInst), r->idx, val);
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
```
|
||||
|
||||
**Requirement traceability:** 1.2, 1.3, 6.1, 6.2, 6.3
|
||||
|
||||
---
|
||||
|
||||
### Component 7: Scheduler — `make_sched_worker` in `schedobj.c` (Req 2, 6)
|
||||
|
||||
Receives `xce_worker_t.resvals_to_sched` which is now `xce_name_val_t**`.
|
||||
For each entry, look up by name in `sched->resmgr` to get local `r->idx`.
|
||||
|
||||
```
|
||||
Before:
|
||||
for (i = 0; i < w->nresvals_to_sched; i++) {
|
||||
int idx = w->resvals_to_sched[i]->idx;
|
||||
xce_val_t* val = NULL;
|
||||
MOVE_PTR(val, w->resvals_to_sched[i]->val);
|
||||
zarray_put(&(worker->resvals), idx, val);
|
||||
}
|
||||
|
||||
After:
|
||||
for (i = 0; i < w->nresvals_to_sched; i++) {
|
||||
xce_name_val_t* nv = w->resvals_to_sched[i];
|
||||
xce_resource_t* r = NULL;
|
||||
xce_val_t* val = NULL;
|
||||
|
||||
if (nv == NULL || nv->name == NULL) continue;
|
||||
|
||||
if (key_find(sched->resmgr, (void*)nv->name, (void**)&r) < 0 || r == NULL) {
|
||||
WARN("%s unknown resource '%s' in worker resvals, skipping",
|
||||
__func__, nv->name);
|
||||
continue;
|
||||
}
|
||||
|
||||
MOVE_PTR(val, nv->val);
|
||||
zarray_put(&(worker->resvals), r->idx, val);
|
||||
}
|
||||
```
|
||||
|
||||
`make_sched_worker` already receives `xce_scheduler_t* sched` as its first argument
|
||||
(`void* arg`), cast at top of function — `sched->resmgr` is available.
|
||||
|
||||
**Requirement traceability:** 2.2, 2.3, 2.4, 6.3
|
||||
|
||||
---
|
||||
|
||||
## Data Flow
|
||||
|
||||
### Before (fragile index-keyed path)
|
||||
|
||||
```
|
||||
Controller Scheduler
|
||||
---------- ---------
|
||||
c->sharedinst[r->idx] = val
|
||||
│
|
||||
collect_resval_with_idx()
|
||||
emit {idx=i, val} ──────────────► sched_init_sharedInst()
|
||||
zarray_put(&sharedInst, idx, val)
|
||||
↑
|
||||
ASSUMES controller idx == scheduler idx
|
||||
(NO ENFORCEMENT — silent corruption if false)
|
||||
```
|
||||
|
||||
### After (name-keyed path, matches Path C)
|
||||
|
||||
```
|
||||
Controller Scheduler
|
||||
---------- ---------
|
||||
c->sharedinst[r->idx] = val
|
||||
│
|
||||
collect_resval(resmgr, array)
|
||||
iter resmgr → r->name, r->idx
|
||||
val = array->data[r->idx]
|
||||
emit {name=r->name, val} ────────► sched_init_sharedInst()
|
||||
key_find(sched->resmgr, name) → r
|
||||
zarray_put(&sharedInst, r->idx, val)
|
||||
↑
|
||||
Uses scheduler's OWN r->idx
|
||||
No cross-subsystem index assumption
|
||||
```
|
||||
|
||||
## Requirements Traceability
|
||||
|
||||
| Requirement | Component(s) |
|
||||
|------------|--------------|
|
||||
| 1.1 — Name-keyed shared resvals in config push | Comp 1, 2, 3, 5 |
|
||||
| 1.2 — Scheduler resolves by name in resmgr | Comp 6 |
|
||||
| 1.3 — Warn and skip unknown resource names | Comp 6 |
|
||||
| 1.4 — Config path produces same sharedInst state as Path C | Comp 6 (same key_find → r->idx logic as Path C) |
|
||||
| 2.1 — Name-keyed worker resvals | Comp 1, 2, 3, 5 |
|
||||
| 2.2 — Scheduler resolves worker resvals by name | Comp 7 |
|
||||
| 2.3 — Warn and skip unknown worker resource names | Comp 7 |
|
||||
| 2.4 — Dynamic resource catalog changes handled correctly | Comp 7 (no cached idx; lookup on each receive) |
|
||||
| 3.1 — Name-keyed worker resvals to API server | Comp 1, 2, 5 (`apiserver.c`) |
|
||||
| 3.2 — API server match by name without numeric index | Comp 2 (type change), Comp 5 |
|
||||
| 4.1 — Path C (`Data_data_resvals`) unchanged | No changes to `handler.c` or `scheduler.c:770-788` |
|
||||
| 4.2 — All struct consumers updated consistently | Comp 1, 2, 5, 6, 7 |
|
||||
| 5.1 — No cross-subsystem slot-indexed resval path remains | Comp 3 (remove old function), Comp 4, 5, 6, 7 |
|
||||
| 5.2 — `collect_resval_with_idx` removed | Comp 3 |
|
||||
| 6.1 — Correct values when idx assignments differ | Comp 6, 7 |
|
||||
| 6.2 — Identical behavior when idx assignments match | Comp 6, 7 |
|
||||
| 6.3 — Scheduling lookups use scheduler's own indices | Comp 6, 7 |
|
||||
|
||||
## Open Questions
|
||||
|
||||
None — all design decisions resolved in gap analysis and user feedback.
|
||||
95
.kiro/specs/resval-id-decoupling/requirements.md
Normal file
95
.kiro/specs/resval-id-decoupling/requirements.md
Normal file
|
|
@ -0,0 +1,95 @@
|
|||
# Requirements: resval-id-decoupling
|
||||
|
||||
> Generated: 2026-05-12T06:52:00-07:00
|
||||
> Updated: 2026-05-12T07:10:00-07:00
|
||||
> Feature: resval-id-decoupling
|
||||
> Status: Requirements Generated
|
||||
|
||||
## Overview
|
||||
|
||||
The controller passes resource values to the scheduler (initial config push) and to the API server (worker data) using `xce_val_with_idx_t`, a struct carrying a numeric array-slot index. The scheduler and API server then store values at that same index in their own `zarray_t` stores. This works only when both components assign the same slot number to each resource — an assumption that holds when resource registration order is strictly identical but is not enforced by any contract and will silently produce wrong values if it breaks.
|
||||
|
||||
The ongoing update path (`Data_data_resvals` dataset, `xce_resval_t` with `name` field) already resolves this correctly via name-based lookup in the receiver's own resmgr. The goal of this feature is to apply the same name-keyed approach uniformly across all resource-value transfer paths, eliminating the hidden index-identity assumption.
|
||||
|
||||
## Codebase Analysis Summary
|
||||
|
||||
Three distinct paths currently transfer resource values between components:
|
||||
|
||||
**Path A — Shared resvals in initial config push** (`src/controller/sched.c:118-120`)
|
||||
- `collect_shared_resvals()` → `collect_resval_with_idx(&c->sharedinst, ...)` emits `{idx, val}` pairs where `idx` is the controller's `sharedinst` zarray slot (= controller resource `idx`).
|
||||
- Scheduler receives via `xce_conf4sched_t.resvals` (`xce_val_with_idx_t**`) and stores directly at the same index: `zarray_put(&sched->sharedInst, idx, val)` (`src/scheduler/src/scheduler.c:583`).
|
||||
- **Bug**: If `resource->idx` differs between controller and scheduler (different insertion order into each component's independent resmgr), values land in the wrong slot.
|
||||
|
||||
**Path B — Worker resvals to scheduler** (`src/controller/sched.c:442-444`, `src/controller/apiserver.c:367-369`)
|
||||
- `collect_resval_with_idx(&w->host.resvals, ...)` emits `{idx, val}` from the controller's per-worker resvals zarray (slot = controller resource `idx`).
|
||||
- Scheduler stores at the same slot: `zarray_put(&worker->resvals, idx, val)` (`src/scheduler/src/schedobj.c:279`).
|
||||
- Later, `resource.c` indexes `worker->resvals` using the scheduler's own `residx` values — a mismatch if indices differ.
|
||||
|
||||
**Path C — `Data_data_resvals` dataset update** (already correct)
|
||||
- Uses `xce_resval_t` (name + host + val). Scheduler looks up `r = key_find(sched->resmgr, rv->name)` then uses `r->idx` (its own index): `zarray_put(&sched->sharedInst, r->idx, rv->val)` (`src/scheduler/src/scheduler.c:784`).
|
||||
- No index-identity assumption. This is the target pattern.
|
||||
|
||||
The fix must align Paths A and B with Path C's name-based resolution approach. The existing `xce_name_val_t` type (`src/include/xce/types.h:43`) already provides the right shape for shared resvals (name + val). The `xce_resval_t` type already provides the shape for per-host/worker resvals (name + host + val).
|
||||
|
||||
## Requirements
|
||||
|
||||
### Requirement 1: Name-keyed shared resource value transfer in initial config
|
||||
|
||||
The scheduler SHALL receive shared resource values from the controller as name-keyed records (resource name + value) rather than as array-slot-indexed records.
|
||||
|
||||
Upon receiving the initial config push, the scheduler SHALL resolve each resource value to its own slot index by looking up the resource name in its own resource manager (`sched->resmgr`), then store the value at the resolved index in `sched->sharedInst`.
|
||||
|
||||
The scheduler SHALL log a warning and skip (not crash or silently corrupt) any resource value for which the resource name is not found in its resource manager.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- EARS-R1-1: WHEN the controller sends an initial config push (`ConfigData_data_schedconfig`) containing shared resource values, THEN each resource value SHALL be identified by resource name, not by a numeric array slot index.
|
||||
- EARS-R1-2: WHEN the scheduler processes a shared resource value record whose name matches a resource in `sched->resmgr`, THEN the scheduler SHALL store the value at the index assigned by its own resmgr (`r->idx`).
|
||||
- EARS-R1-3: WHEN the scheduler processes a shared resource value record whose name does not match any resource in `sched->resmgr`, THEN the scheduler SHALL emit a warning log and skip that record without modifying `sched->sharedInst`.
|
||||
- EARS-R1-4: WHEN an existing system running Path C updates (`Data_data_resvals`) works correctly, THEN after this change the initial-config path SHALL produce the same `sched->sharedInst` state as Path C would for the same resource values.
|
||||
|
||||
### Requirement 2: Name-keyed per-worker resource value transfer to scheduler
|
||||
|
||||
The scheduler SHALL receive per-worker resource values from the controller as name-keyed records (resource name + value) rather than as array-slot-indexed records.
|
||||
|
||||
Upon receiving worker data, the scheduler SHALL resolve each worker resource value to its own slot index by looking up the resource name in its resource manager, then store the value at the resolved index in the worker's `resvals` zarray.
|
||||
|
||||
The scheduler SHALL log a warning and skip any worker resource value whose resource name is not found.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- EARS-R2-1: WHEN the controller sends worker data containing per-worker resource values, THEN each resource value SHALL be identified by resource name, not by a numeric array slot index.
|
||||
- EARS-R2-2: WHEN the scheduler builds a `sched_worker` from received worker data, THEN it SHALL resolve each resource value name to the scheduler's own `r->idx` before calling `zarray_put(&worker->resvals, ...)`.
|
||||
- EARS-R2-3: WHEN a resource name in a worker's resource values is unknown to the scheduler's resmgr, THEN the scheduler SHALL log a warning and skip that entry without aborting worker construction.
|
||||
- EARS-R2-4: WHILE the scheduler is running and a resource is added or removed from the resource catalog, THEN subsequently received worker data SHALL correctly resolve resource values using the updated resmgr, with no stale index cached from before the change.
|
||||
|
||||
### Requirement 3: Name-keyed per-worker resource value transfer to API server
|
||||
|
||||
The API server SHALL receive per-worker resource values from the controller as name-keyed records (resource name + value) so that its own resource catalog lookup can resolve values independently of the controller's internal index assignment.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- EARS-R3-1: WHEN the controller sends worker data to an API server connection, THEN per-worker resource values SHALL be transmitted using resource names as keys.
|
||||
- EARS-R3-2: WHEN the API server (`src/frontend/apiserver`) converts worker resource values for its models (e.g., `cResvalToDistribution`), THEN it SHALL be able to match values to resources by name without assuming any particular numeric index.
|
||||
|
||||
### Requirement 4: Backward compatibility with the `Data_data_resvals` update path
|
||||
|
||||
The name-keyed changes to the initial config and worker data paths SHALL NOT alter the behavior of the existing `Data_data_resvals` dataset update path (Path C), which is already name-keyed.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- EARS-R4-1: WHEN the scheduler processes a `Data_data_resvals` dataset message, THEN the behavior SHALL be identical before and after this change.
|
||||
- EARS-R4-2: WHEN the `xce_conf4sched_t` struct is updated to use name-keyed resval records, THEN all callers that populate or consume this struct SHALL be updated consistently (controller, scheduler, and any serialization/deserialization layer in `libxce`).
|
||||
|
||||
### Requirement 5: Removal of the index-keyed `collect_resval_with_idx` call sites
|
||||
|
||||
The `collect_resval_with_idx` function (or its callers) SHALL be updated or replaced so that no code path transmits resource values indexed solely by a numeric array slot without an accompanying resource name.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- EARS-R5-1: WHEN this feature is complete, THEN no code path that crosses a subsystem boundary (controller → scheduler, controller → API server) SHALL rely on the receiving subsystem's `zarray_t` slot indices matching the sending subsystem's slot indices.
|
||||
- EARS-R5-2: WHEN `collect_resval_with_idx` is no longer needed for cross-subsystem transfer, THEN it SHOULD be removed or restricted to intra-subsystem use only, with a comment explaining its scope.
|
||||
|
||||
### Requirement 6: No regression in resource value correctness
|
||||
|
||||
The scheduler's scheduling decisions SHALL produce the same resource value lookups after this change as they did when indices happened to be consistent, and SHALL produce correct results when indices are inconsistent (where the old code would silently corrupt scheduling state).
|
||||
|
||||
**Acceptance criteria:**
|
||||
- EARS-R6-1: WHEN the controller and scheduler register resources in a different order (causing their resmgr index assignments to differ), THEN `sched->sharedInst` SHALL still contain the correct value for each resource after receiving the initial config push.
|
||||
- EARS-R6-2: WHEN the controller and scheduler register resources in the same order, THEN the behavior of the scheduler SHALL be identical to the current behavior.
|
||||
- EARS-R6-3: WHEN worker resource values are applied during a scheduling cycle, THEN `calc_wanted_global_resvals` and `calc_wanted_host_resvals` SHALL retrieve values from `sharedInst` and `worker->resvals` using the scheduler's own resource indices, unaffected by the controller's index ordering.
|
||||
154
.kiro/specs/resval-id-decoupling/research.md
Normal file
154
.kiro/specs/resval-id-decoupling/research.md
Normal file
|
|
@ -0,0 +1,154 @@
|
|||
# Gap Analysis: resval-id-decoupling
|
||||
|
||||
> Generated: 2026-05-12T07:25:00-07:00
|
||||
> Feature: resval-id-decoupling
|
||||
|
||||
## Executive Summary
|
||||
|
||||
- **Scope**: Three call sites in the controller use `collect_resval_with_idx()` to produce index-keyed resource values sent to two consumers (scheduler and API server). Fixing all three brings every cross-subsystem resval path into alignment with the already-correct `Data_data_resvals` dataset path.
|
||||
- **Existing Infrastructure**: `xce_name_val_t` (`NameValue` in capnp) already exists in `types.h` and has a **complete capnp codec** (`encode_NameValue_list` / `decode_NameValue_list`) in the generated code — it was defined for this purpose but never wired in at the resval-transfer boundary.
|
||||
- **Wire format impact**: `Conf4Sched.resvals` (field @6) and `Worker.resvals` (field @5) in `controller.capnp` currently declare `List(ValueWithIdx)`. Changing these to `List(NameValue)` is a **breaking wire change** — no intermediate backward-compatibility window unless two field slots are used.
|
||||
- **Build chain**: The capnp schema in `src/schema/controller.capnp` drives generation of `src/build/lib/controller.capnp.[ch]` via `meson` + `capnpc-c`. A schema edit automatically regenerates the C codecs on the next build.
|
||||
- **Key insight**: `makeResource_()` in `resobj.c:137` always overwrites `resobj->idx = rc` (from `key_add`) regardless of the `idx` transmitted in the `Resource` wire struct — so the controller's idx is lost at the scheduler's resmgr and the assumption that they match is never enforced.
|
||||
|
||||
---
|
||||
|
||||
## Existing Components Inventory
|
||||
|
||||
### Types and Structs
|
||||
|
||||
| Symbol | Location | Role | Status |
|
||||
|--------|----------|------|--------|
|
||||
| `xce_val_with_idx_t` | `src/include/xce/types.h:38` | `{int idx, xce_val_t* val}` — current cross-subsystem carrier | **Replace at boundaries** |
|
||||
| `xce_name_val_t` | `src/include/xce/types.h:43` | `{char* name, xce_val_t* val}` — name-keyed carrier | **Ready to use** |
|
||||
| `xce_resval_t` | `src/include/xce/types.h:48` | `{char* name, char* host, xce_val_t* val}` — used by Path C | Already correct |
|
||||
| `xce_conf4sched_t.resvals` | `src/include/typemap.h:50` | `xce_val_with_idx_t**` — shared resvals in initial config | **Change to `xce_name_val_t**`** |
|
||||
| `xce_worker_t.resvals_to_sched` | `src/include/xce/types.h:217` | `xce_val_with_idx_t**` — per-worker resvals | **Change to `xce_name_val_t**`** |
|
||||
|
||||
### Controller — Call Sites (Sender Side)
|
||||
|
||||
| File | Line | Function | Purpose | Change needed |
|
||||
|------|------|----------|---------|---------------|
|
||||
| `src/controller/dataex.c` | 62 | `collect_resval_with_idx()` | Iterates `zarray_t`, emits `{idx, val}` | Replace/augment with name-keyed variant |
|
||||
| `src/controller/dataex.c` | 363 | `collect_shared_resvals()` | Calls `collect_resval_with_idx(&c->sharedinst)` | Change to name-keyed collection |
|
||||
| `src/controller/dataex.c` | 141–163 | `collect_resvals_workers()` | Calls `collect_resval_with_idx(&w->resvals)` per worker | Change to name-keyed collection |
|
||||
| `src/controller/sched.c` | 118–120 | `xcesched_push_configs()` | Puts shared resvals into `conf4sched->resvals` | Change field type; update free |
|
||||
| `src/controller/sched.c` | 442–444 | `xcesched_push_workerlist()` | Puts worker resvals into `w->host.resvals_to_sched` | Change field type; update free |
|
||||
| `src/controller/apiserver.c` | 367–369 | `apiserver_worker_change()` | Same pattern as sched for API server path | Change field type; update free |
|
||||
|
||||
### Free/cleanup Sites
|
||||
|
||||
| File | Line | Call | Must be updated |
|
||||
|------|------|------|-----------------|
|
||||
| `src/controller/sched.c` | 145 | `free_resval_with_idx_list(&conf4sched->resvals, ...)` | → `free_NameValue_list` or equivalent |
|
||||
| `src/controller/sched.c` | 226, 463 | `free_resval_with_idx_list(&w->resvals_to_sched, ...)` | Same |
|
||||
| `src/controller/apiserver.c` | 51, 397–399 | `free_resval_with_idx_list(...)` | Same |
|
||||
| `src/controller/dataex.c` | 404 | `free_resval_with_idx_list(...)` | Same |
|
||||
| `src/controller/reply.c` | 66 | `free_resval_with_idx_list(...)` | Same |
|
||||
| `src/controller/opget.c` | 128 | `free_resval_with_idx_list(...)` | Same |
|
||||
| `src/include/utils.h` | 130 | `free_resval_with_idx_list` declaration | Update or keep alongside new |
|
||||
|
||||
### Scheduler — Consumer Side
|
||||
|
||||
| File | Line | Code | Change needed |
|
||||
|------|------|------|---------------|
|
||||
| `src/scheduler/src/scheduler.c` | 574–586 | `sched_init_sharedInst()` — uses `idx` directly from `config->resvals[i]->idx` | Change to name-lookup in `sched->resmgr` |
|
||||
| `src/scheduler/src/schedobj.c` | 274–280 | `make_sched_worker()` — uses `idx` directly from `w->resvals_to_sched[i]->idx` | Change to name-lookup |
|
||||
|
||||
### Capnp Schema and Generated Code
|
||||
|
||||
| Asset | Location | Current | Change |
|
||||
|-------|----------|---------|--------|
|
||||
| `Conf4Sched.resvals` | `src/schema/controller.capnp:230` | `List(ValueWithIdx)` | → `List(NameValue)` |
|
||||
| `Worker.resvals` | `src/schema/controller.capnp:129` | `List(ValueWithIdx)` | → `List(NameValue)` |
|
||||
| `controller.capnp.c/h` | `src/build/lib/` | Auto-generated — regenerated by `ninja -C build` | Automatically updated after schema edit |
|
||||
| `encode_NameValue_list` | `src/build/lib/controller.capnp.c:3402` | Already exists, unused at these boundaries | Wire in to replace `encode_ValueWithIdx_list` |
|
||||
| `decode_NameValue_list` | `src/build/lib/controller.capnp.c:3426` | Already exists, unused at these boundaries | Wire in to replace `decode_ValueWithIdx_list` |
|
||||
| `free_NameValue_list` | `src/build/lib/controller.capnp.c:3458` | Already exists | Replace `free_ValueWithIdx_list` at these call sites |
|
||||
|
||||
### API Server (Go frontend) — Impact
|
||||
|
||||
The Go API server (`src/frontend/apiserver/backend/clib.go`) includes `controller.capnp.h` and works with `xce_worker_t*` via CGo. It accesses `resvals_to_sched` in `apiserver_worker_change()` (controller side) which populates `xce_worker_t.resvals_to_sched`. The Go frontend's `cResvalToDistribution` in `convert.go:273` already processes `xce_resval_t` (name-keyed), not `xce_val_with_idx_t`. The API server impact is limited to updating any code that reads `resvals_to_sched` field — if it accesses it at all on the Go side (currently no direct access found).
|
||||
|
||||
---
|
||||
|
||||
## Root Cause Deep Dive
|
||||
|
||||
The zarray-based stores (`c->sharedinst`, `w->host.resvals`) use `r->idx` as the slot key. `r->idx` is assigned by `key_add()` in `makeResource_` (`resobj.c:137`) — it is the slot returned by the internal container, which depends on insertion order. The controller and scheduler each call `makeResource()` independently (controller at cluster init, scheduler at `sched_init_resources()`), with no guarantee of identical ordering.
|
||||
|
||||
Crucially, the `Resource` capnp struct **does transmit `r->idx` on the wire** (`encode_Resource` at `build/lib/controller.capnp.c:3889`), but `decode_Resource` writes it into a temporary `xce_resource_t`, which is then passed to `makeResource()`, which **immediately overwrites** the decoded `idx` with the local `key_add` return value (`resobj->idx = rc`). The transmitted idx is therefore discarded at the scheduler.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Options
|
||||
|
||||
### Option A — Replace `ValueWithIdx` with `NameValue` at both capnp fields (Recommended)
|
||||
|
||||
**What changes:**
|
||||
1. `controller.capnp`: change `Conf4Sched.resvals @6` and `Worker.resvals @5` from `List(ValueWithIdx)` to `List(NameValue)`
|
||||
2. Rebuild capnp — generated `encode_Conf4Sched`/`decode_Conf4Sched` and worker codec automatically use `NameValue` functions
|
||||
3. `typemap.h`: `xce_conf4sched_t.resvals` → `xce_name_val_t**`
|
||||
4. `types.h`: `xce_worker_t.resvals_to_sched` → `xce_name_val_t**`
|
||||
5. Controller: new `collect_resval_by_name(zarray_t* vals, var resmgr, xlist_t* l)` that walks `resmgr` entries, reads the value by `r->idx` from the array, and emits `{name, val}` records
|
||||
6. Scheduler `sched_init_sharedInst()`: iterate name-val list, `key_find(sched->resmgr, name)` → `r->idx` → `zarray_put`
|
||||
7. Scheduler `make_sched_worker()`: same pattern
|
||||
8. Update all free/cleanup call sites to use `free_NameValue_list`
|
||||
|
||||
**Pros:** Cleanest — removes the misleading `idx` field entirely; `NameValue` codec already exists; aligns with Path C's design; no backward compatibility concerns given no public release
|
||||
**Cons:** None significant — wire-format break is a non-issue as there is no released version requiring compatibility
|
||||
|
||||
**Affected files (estimate):** 8 C files, 1 capnp schema, 0 Go files
|
||||
|
||||
---
|
||||
|
||||
### Option B — Add `name` field to `xce_val_with_idx_t` / `ValueWithIdx` ~~(Additive wire change)~~ [REJECTED]
|
||||
|
||||
Ruled out: backward compatibility is not a requirement for this project at its current stage. Option B's only advantage over Option A is a softer wire transition — without that constraint, it is strictly worse: it keeps a semantically dead `idx` field, requires more code changes, and leaves misleading structure in the API.
|
||||
|
||||
---
|
||||
|
||||
### Option C — Use Controller's idx as canonical; stop overwriting in `makeResource_` [REJECTED]
|
||||
|
||||
Ruled out: does not fix the design — just moves the source of truth without eliminating the hidden identity assumption. A resource deleted and re-added in the controller gets a new idx, and the scheduler has no way to know its cached idx is stale.
|
||||
|
||||
---
|
||||
|
||||
## Recommendation
|
||||
|
||||
**Option A** is the selected approach. Backward compatibility is not a constraint (no public release), so the wire-format change to `List(NameValue)` is straightforward. The `NameValue` codec already exists in the generated capnp code and is fully functional — this is primarily a wiring exercise. The semantic change aligns the initial-config and worker-data paths with the already-correct `Data_data_resvals` update path (Path C).
|
||||
|
||||
---
|
||||
|
||||
## Files to Change (Option A)
|
||||
|
||||
| File | Change Type |
|
||||
|------|-------------|
|
||||
| `src/schema/controller.capnp` | Schema edit: 2 field type changes |
|
||||
| `src/include/xce/types.h` | Struct field type change: `resvals_to_sched` |
|
||||
| `src/include/typemap.h` | Struct field type change: `resvals` |
|
||||
| `src/controller/dataex.c` | New `collect_resval_by_name()` function; update `collect_shared_resvals()` and `collect_resvals_workers()`; **remove** `collect_resval_with_idx` and `collect_resval_with_idx_` |
|
||||
| `src/controller/sched.c` | Update 3 call sites (populate and free) |
|
||||
| `src/controller/apiserver.c` | Update 2 call sites (populate and free) |
|
||||
| `src/controller/reply.c` | Update free call |
|
||||
| `src/controller/opget.c` | Update free call |
|
||||
| `src/scheduler/src/scheduler.c` | Update `sched_init_sharedInst()` |
|
||||
| `src/scheduler/src/schedobj.c` | Update `make_sched_worker()` |
|
||||
| `src/include/utils.h` | Remove `free_resval_with_idx_list` declaration; add `free_name_val_list` declaration |
|
||||
| `src/lib/utils.c` | Remove `free_resval_with_idx_list`; add `free_name_val_list(xce_name_val_t***, int)` |
|
||||
| `src/controller/include/cluster.h` | Remove `collect_resval_with_idx` declaration |
|
||||
|
||||
**Not changed:** `src/scheduler/src/handler.c` (Path C untouched), `src/frontend/apiserver/` (Go API server does not directly access `resvals_to_sched` in Go code), capnp generated files (auto-regenerated by `ninja -C build`).
|
||||
|
||||
---
|
||||
|
||||
## Decisions (Closed Questions)
|
||||
|
||||
1. **New collector function signature**: `collect_resval_by_name` is controller-internal logic — add it to `dataex.c` alongside `collect_resval_with_idx`. It needs both the `zarray_t` (values indexed by `r->idx`) and the `resmgr` (name→idx mapping) as parameters.
|
||||
|
||||
2. **Handling unknown resources in scheduler**: **Warn and skip.** When `sched_init_sharedInst` or `make_sched_worker` encounters a name not in `sched->resmgr`, emit a `WARN` log and continue. This is safe because the resource list always arrives in the same config push before resvals are applied (see decision 3).
|
||||
|
||||
3. **Ordering guarantee**: **Confirmed — current behavior, must be preserved.** `sched_init_resources()` is called before `sched_init_sharedInst()` in `sched_init_config()` (`scheduler.c:603-609`). The design must not reorder these calls.
|
||||
|
||||
4. **Removal of `collect_resval_with_idx`**: **Remove entirely.** No cross-subsystem callers will remain after this change, and intra-subsystem callers (e.g., `worker.c` reading from `sharedinst` for API get-resval) access the zarray directly by `r->idx` without going through this function. Delete `collect_resval_with_idx`, `collect_resval_with_idx_`, and remove the declaration from `src/controller/include/cluster.h`.
|
||||
|
||||
5. **`free_name_val_list` utility**: **Add to `utils.c`/`utils.h`.** A new `free_name_val_list(xce_name_val_t***, int)` function mirroring `free_resval_with_idx_list` is needed. The capnp-generated `free_NameValue_list` is an option but couples cleanup to the serialization layer — a standalone utility in `utils.c` is cleaner and consistent with the existing pattern.
|
||||
23
.kiro/specs/resval-id-decoupling/spec.json
Normal file
23
.kiro/specs/resval-id-decoupling/spec.json
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
{
|
||||
"feature_name": "resval-id-decoupling",
|
||||
"description": "investigate a better solution to pass resource val from controller to scheduler/api server, the current solution has a very strong assumption that the idx/id of resource in controller and scheduler should keep the same, but this assumption may be too strong, the key function is collect_resval_with_idx() in src/controller/dataex.c",
|
||||
"phase": "tasks-generated",
|
||||
"created_at": "2026-05-12T06:52:00-07:00",
|
||||
"updated_at": "2026-05-12T07:45:00-07:00",
|
||||
"language": "en",
|
||||
"status": "active",
|
||||
"approvals": {
|
||||
"requirements": {
|
||||
"generated": true,
|
||||
"approved": true
|
||||
},
|
||||
"design": {
|
||||
"generated": true,
|
||||
"approved": true
|
||||
},
|
||||
"tasks": {
|
||||
"generated": true,
|
||||
"approved": false
|
||||
}
|
||||
}
|
||||
}
|
||||
224
.kiro/specs/resval-id-decoupling/tasks.md
Normal file
224
.kiro/specs/resval-id-decoupling/tasks.md
Normal file
|
|
@ -0,0 +1,224 @@
|
|||
# Implementation Tasks: resval-id-decoupling
|
||||
|
||||
> Generated: 2026-05-12T07:45:00-07:00
|
||||
> Feature: resval-id-decoupling
|
||||
> Status: Tasks Generated
|
||||
|
||||
## Task Overview
|
||||
|
||||
7 major tasks, 19 sub-tasks. All 6 requirements covered.
|
||||
|
||||
Dependencies: Task 1 must complete first (headers + schema unblock everything).
|
||||
Tasks 2 and 3 can proceed in parallel after Task 1.
|
||||
Tasks 4, 5, 6 can proceed in parallel after Tasks 2 and 3.
|
||||
Task 7 last.
|
||||
|
||||
---
|
||||
|
||||
## Tasks
|
||||
|
||||
### 1. Update shared headers and capnp schema
|
||||
|
||||
Change the wire type and C struct types that carry resource values across subsystem
|
||||
boundaries. This is the structural foundation — all downstream tasks depend on these
|
||||
types compiling correctly.
|
||||
|
||||
- [x] 1.1 In `src/schema/controller.capnp`, change `Worker.resvals @5` and
|
||||
`Conf4Sched.resvals @6` from `List(ValueWithIdx)` to `List(NameValue)`. Preserve
|
||||
field ordinal numbers; only the element type changes.
|
||||
_Requirements: 1, 2, 3, 4_
|
||||
|
||||
- [x] 1.2 In `src/include/xce/types.h`, change `xce_worker_t.resvals_to_sched` from
|
||||
`xce_val_with_idx_t**` to `xce_name_val_t**`.
|
||||
_Requirements: 2, 3_
|
||||
|
||||
- [x] 1.3 In `src/include/typemap.h`, change `xce_conf4sched_t.resvals` from
|
||||
`xce_val_with_idx_t**` to `xce_name_val_t**`.
|
||||
_Requirements: 1, 4_
|
||||
|
||||
- [x] 1.4 Run `nix develop --command ninja -C src/build` to regenerate
|
||||
`controller.capnp.[ch]`. Confirm the generated `encode_Conf4Sched` and
|
||||
`decode_Conf4Sched` now call `encode_NameValue_list` / `decode_NameValue_list`
|
||||
instead of their `ValueWithIdx` counterparts. Expect compiler errors at this stage
|
||||
from old call sites — that is expected and resolved in subsequent tasks.
|
||||
|
||||
---
|
||||
|
||||
### 2. (P) Replace `free_resval_with_idx_list` with `free_name_val_list`
|
||||
|
||||
Swap the heap-cleanup utility function to match the new element type. Old callers in
|
||||
the controller will be updated in Task 4.
|
||||
|
||||
- [x] 2.1 In `src/lib/utils.c`, add `free_name_val_list(xce_name_val_t*** vals, int count)`:
|
||||
iterate the array, free each entry's `name`, free `val->u.svalue` if string-typed,
|
||||
free `val`, free the entry, then null and free the array pointer. Mirror the structure
|
||||
of the existing `free_resval_with_idx_list`.
|
||||
|
||||
- [x] 2.2 In `src/include/utils.h`, remove the `free_resval_with_idx_list` declaration
|
||||
and add `extern void free_name_val_list(xce_name_val_t***, int);`.
|
||||
|
||||
- [x] 2.3 In `src/lib/utils.c`, delete the `free_resval_with_idx_list` implementation.
|
||||
_Requirements: 5_
|
||||
|
||||
---
|
||||
|
||||
### 3. (P) Add `collect_resval` and remove `collect_resval_with_idx`
|
||||
|
||||
Replace the index-keyed collector in the controller with a name-keyed one. The key
|
||||
design inversion: iterate `resmgr` (names) rather than the `zarray_t` (slots), using
|
||||
`r->idx` to look up values. This guarantees every emitted record carries a valid name.
|
||||
|
||||
- [x] 3.1 In `src/controller/dataex.c`, add `collect_resval(var resmgr, zarray_t* array, xlist_t* l)`:
|
||||
acquire a read lock on `resmgr`, iterate with `iter_make`/`iter_next`/`iter_destroy`,
|
||||
for each `xce_resource_t* r` call `zarray_get(array, r->idx)` to get the value (skip
|
||||
if NULL), allocate an `xce_name_val_t`, copy name (strdup) and value (strdup svalue or
|
||||
memcpy), store in the list. Release the resmgr lock. Return `l->count`. The array lock
|
||||
is the caller's responsibility (consistent with old pattern).
|
||||
_Requirements: 1, 2, 5_
|
||||
|
||||
- [x] 3.2 In `src/controller/dataex.c`, rewrite `collect_shared_resvals` to lock
|
||||
`c->sharedinst.lock`, call `collect_resval(c->resmgr, &c->sharedinst, l)`, then
|
||||
unlock. The unused `fn` parameter is retained for signature consistency.
|
||||
_Requirements: 1_
|
||||
|
||||
- [x] 3.3 In `src/controller/dataex.c`, update `collect_resvals_workers` to accept
|
||||
`var resmgr` as a new first parameter. Replace the `collect_resval_with_idx` call with
|
||||
`mylock_lock(&w->resvals.lock)` / `collect_resval(resmgr, &w->resvals, &lst)` /
|
||||
`mylock_unlock`. Update the call site in `collect_workers` to pass `c->resmgr`.
|
||||
_Requirements: 2_
|
||||
|
||||
- [x] 3.4 In `src/controller/dataex.c`, delete both `collect_resval_with_idx_` (static)
|
||||
and `collect_resval_with_idx`. In `src/controller/include/cluster.h`, remove the
|
||||
`collect_resval_with_idx` declaration.
|
||||
_Requirements: 5_
|
||||
|
||||
---
|
||||
|
||||
### 4. (P) Update controller call sites
|
||||
|
||||
Update all controller sites that populate or clean up `resvals_to_sched` /
|
||||
`conf4sched->resvals` to use the new type and free function. Tasks 2 and 3 must be
|
||||
complete before this task.
|
||||
|
||||
- [x] 4.1 In `src/controller/sched.c` — `xcesched_push_configs`: change the cast from
|
||||
`(xce_val_with_idx_t**)` to `(xce_name_val_t**)` when assigning `xreslist.data` to
|
||||
`conf4sched->resvals`, and replace `free_resval_with_idx_list` with
|
||||
`free_name_val_list`.
|
||||
_Requirements: 1_
|
||||
|
||||
- [x] 4.2 In `src/controller/sched.c` — `xcesched_push_workerlist` and its worker-loop
|
||||
cleanup: add `mylock_lock`/`mylock_unlock` around `collect_resval`, change cast to
|
||||
`(xce_name_val_t**)`, replace all `free_resval_with_idx_list` calls with
|
||||
`free_name_val_list`.
|
||||
_Requirements: 2_
|
||||
|
||||
- [x] 4.3 In `src/controller/sched.c:226`, `src/controller/reply.c:66`, and
|
||||
`src/controller/opget.c:128`, replace each `free_resval_with_idx_list` call with
|
||||
`free_name_val_list`.
|
||||
_Requirements: 5_
|
||||
|
||||
- [x] 4.4 In `src/controller/apiserver.c` — `apiserver_worker_change`: add
|
||||
`mylock_lock`/`mylock_unlock` around `collect_resval`, change cast to
|
||||
`(xce_name_val_t**)`, replace `free_resval_with_idx_list` with `free_name_val_list`
|
||||
at both cleanup sites (lines 51 and 397–399).
|
||||
_Requirements: 3_
|
||||
|
||||
---
|
||||
|
||||
### 5. (P) Update scheduler — shared resource value initialisation
|
||||
|
||||
Change `sched_init_sharedInst` to resolve resource values by name via the scheduler's
|
||||
own resmgr instead of using the raw index received from the controller. Task 1 must be
|
||||
complete before this task.
|
||||
|
||||
- [x] 5.1 In `src/scheduler/src/scheduler.c` — `sched_init_sharedInst`: replace the
|
||||
`idx`-direct loop with a name-lookup loop. For each `xce_name_val_t* nv` in
|
||||
`sched->config->resvals`: skip if `nv` or `nv->name` is NULL; call
|
||||
`key_find(sched->resmgr, nv->name, &r)`; if not found emit `WARN` and continue; then
|
||||
`MOVE_PTR(val, nv->val)` and `zarray_put(&sched->sharedInst, r->idx, val)`. Do not
|
||||
change the call order in `sched_init_config` — `sched_init_resources` must remain
|
||||
before `sched_init_sharedInst`.
|
||||
_Requirements: 1, 6_
|
||||
|
||||
---
|
||||
|
||||
### 6. (P) Update scheduler — per-worker resource value initialisation
|
||||
|
||||
Change `make_sched_worker` to resolve worker resource values by name. Task 1 must be
|
||||
complete before this task.
|
||||
|
||||
- [x] 6.1 In `src/scheduler/src/schedobj.c` — `make_sched_worker`: replace the
|
||||
`idx`-direct loop with a name-lookup loop. For each `xce_name_val_t* nv` in
|
||||
`w->resvals_to_sched`: skip if `nv` or `nv->name` is NULL; call
|
||||
`key_find(sched->resmgr, nv->name, &r)`; if not found emit `WARN` and continue; then
|
||||
`MOVE_PTR(val, nv->val)` and `zarray_put(&worker->resvals, r->idx, val)`. The
|
||||
existing `sched` pointer (cast from `void* arg`) is already available at the top of
|
||||
the function.
|
||||
_Requirements: 2, 6_
|
||||
|
||||
---
|
||||
|
||||
### 7. Update Go API server to consume worker resource values
|
||||
|
||||
Update the Go API server frontend to read `resvals_to_sched` (now `xce_name_val_t**`)
|
||||
from received worker data and expose resource values in the `HostInfo` model. Previously
|
||||
the Go layer ignored this field entirely.
|
||||
|
||||
- [x] 7.1 In `src/frontend/apiserver/models/hosts.go`, add
|
||||
`ResourceValues []models.ResourceDistributionInfo` (or
|
||||
`ResourceValues []ResourceDistributionInfo` if within the same package) to `HostInfo`.
|
||||
_Requirements: 3_
|
||||
|
||||
- [x] 7.2 In `src/frontend/apiserver/backend/convert_pure.go`, add a `resvals` field
|
||||
(`[]models.ResourceDistributionInfo`) to `rawWorkerData`, and update `rawWorkerToModel`
|
||||
to assign `info.ResourceValues = r.resvals`.
|
||||
_Requirements: 3_
|
||||
|
||||
- [x] 7.3 In `src/frontend/apiserver/backend/convert.go`, add
|
||||
`cNameValToWorkerResval(workerName string, nv *C.xce_name_val_t) *models.ResourceDistributionInfo`:
|
||||
return nil if `nv` or `nv.val` is nil (with a WARN log); set `ResourceName` from
|
||||
`nv.name`, `Location` from `workerName`, `IsDynamic: true`; decode the value type
|
||||
using the same `xce_val_which` switch as `cResvalToDistribution`.
|
||||
_Requirements: 3_
|
||||
|
||||
- [x] 7.4 In `src/frontend/apiserver/backend/convert.go`, update `cWorkerToModel` to
|
||||
iterate `worker.nresvals_to_sched` entries in `worker.resvals_to_sched`, call
|
||||
`cNameValToWorkerResval` for each, collect non-nil results, and store them in
|
||||
`rawWorkerData.resvals`.
|
||||
_Requirements: 3_
|
||||
|
||||
---
|
||||
|
||||
### 8. Build, verify, and clean up
|
||||
|
||||
Confirm the full system compiles and passes tests with no regressions. All prior tasks
|
||||
must be complete.
|
||||
|
||||
- [x] 8.1 Run a full clean build: `nix develop --command bash -c "cd src && ninja -C build"`.
|
||||
Resolve any remaining compiler errors (expected: none if tasks 1–7 complete). Confirm
|
||||
`controller.capnp.[ch]` uses `NameValue` codecs for both `Conf4Sched.resvals` and
|
||||
`Worker.resvals`.
|
||||
|
||||
- [x] 8.2 Run the scheduler and controller test suites:
|
||||
`nix develop --command bash -c "cd src/frontend && make test"` and any C-level tests
|
||||
under `src/lib/testsuite/`. Confirm all tests pass. Note: `xcecli/common/output`
|
||||
failure (`undefined: parseJobTime`) is pre-existing and unrelated to this feature.
|
||||
|
||||
- [x] 8.3 Verify no remaining references to `collect_resval_with_idx` or
|
||||
`free_resval_with_idx_list` in `src/` (excluding comments or git history):
|
||||
`grep -r "collect_resval_with_idx\|free_resval_with_idx_list" src/`. Expected: zero
|
||||
matches.
|
||||
_Requirements: 5_
|
||||
|
||||
---
|
||||
|
||||
## Requirements Coverage
|
||||
|
||||
| Requirement | Tasks |
|
||||
|------------|-------|
|
||||
| 1 | 1.1, 1.3, 3.2, 4.1, 5.1 |
|
||||
| 2 | 1.1, 1.2, 3.3, 4.2, 4.3, 6.1 |
|
||||
| 3 | 1.1, 1.2, 4.4 |
|
||||
| 4 | 1.1, 1.3 |
|
||||
| 5 | 2.1, 2.2, 2.3, 3.4, 4.3, 7.3 |
|
||||
| 6 | 5.1, 6.1 |
|
||||
Loading…
Add table
Add a link
Reference in a new issue