docs: mark Phase 6 complete in phase6-scheduler-integration-plan

This commit is contained in:
Rongsong Shen 2026-04-30 07:17:32 +00:00
parent 9b0e9571d1
commit 177cf45af5
17 changed files with 4226 additions and 361 deletions

View file

@ -105,10 +105,12 @@
## P1-3: Heartbeat Keep-Alive
- [ ] **#13 P1-3: Heartbeat keep-alive implementation**
- `backend/clib.go` — Add `heartbeatIntervalSecs int32` (default 30), `SetHeartbeatInterval()`, `heartbeatFn` var, `StartHeartbeat(ctx context.Context)`
- `backend/handler.go` — `OnClusterUpdated` calls `SetHeartbeatInterval(cluster.HeartbeatInterval)` after store update
- `cmd/main.go` — Call `backend.StartHeartbeat(ctx)` after `backend.RegisterToBackend(...)`
- [x] **#13 P1-3: Heartbeat keep-alive implementation**
- `backend/clib.go` — Added `heartbeatIntervalSecs int32` (default 30), `SetHeartbeatInterval()`, `heartbeatFn` var, `StartHeartbeat(ctx context.Context)` goroutine with ticker
- `backend/handler.go` — `OnClusterUpdated` calls `SetHeartbeatInterval(cluster.HeartbeatInterval)` after store update (zero-preserve: skips if interval==0)
- `cmd/main.go` — Calls `backend.StartHeartbeat(ctx)` after `backend.RegisterToBackend(...)`
- `backend/handler_test.go` — Tests for heartbeat interval update, zero-preserve, timer leak, test drain, store consistency
- Commits: `ffc5b20` (goroutine), `0a3de8b` (leak/drain/consistency fixes)
---
@ -119,4 +121,11 @@
- `ninja -C build` for C components passes — required meson reconfigure + CGO val_helpers.h fix
- RBAC verification — `host:control` (cluster_admin) and `host:view` (all authenticated) confirmed in routes + rbac.yaml
- Input safety — comment limit 512 chars; select expression limit 4094 chars (1024 reduction deferred to P1-2b)
- **CGO fix applied:** extracted `static` val helpers from `clib.go` preamble into `val_helpers.h` shared header; `convert.go` now includes it; added explicit `float64()` cast for `C.double` return
- **CGO fix applied:** extracted `static` val helpers from `clib.go` preamble into `val_helpers.h` shared header; `convert.go` now includes it; added explicit `float64()` cast for `C.double` return
- [x] **#15 Integration test suite** — see [`apiserver-backend-integration-test-plan.md`](apiserver-backend-integration-test-plan.md)
- `backend/integration_test.go` — §1 (config push), §3 (resource values), §4 (heartbeat), §5 (cross-feature): 17 tests
- `controllers/integration_test.go` — §2 (host control), §6 (RBAC), §8 (input safety): 17 tests + 1 skip (§8.1 pending P1-2b)
- `store/concurrency_test.go` — §7 (thread safety): 3 concurrent-access tests
- All use `//go:build !cgo`; run with `CGO_ENABLED=0 go test ./backend/... ./controllers/... ./store/...`
- Commit: `86a65d6`

File diff suppressed because it is too large Load diff

File diff suppressed because it is too large Load diff

8
src/flake.lock generated
View file

@ -117,11 +117,11 @@
"xbuild": "xbuild"
},
"locked": {
"lastModified": 1772607564,
"narHash": "sha256-kBYbiOvM7bBZdIFq2idjQ3eidlAT3w6WS3GRoxHM67s=",
"lastModified": 1777021191,
"narHash": "sha256-hyyJZBX9o1TQLH/7aaEP1/rV3QQTqn70PGs2IDACNnw=",
"ref": "master",
"rev": "10c32f769b4431cb657c3c8f23158f643fad74b4",
"revCount": 19,
"rev": "7212ba170d8db040ac99d0fb47c7b1df70df6e11",
"revCount": 20,
"type": "git",
"url": "ssh://forgejo@gitea.shenrs.eu/rshen/librtti"
},

View file

@ -40,6 +40,11 @@ typedef struct {
xce_val_t* val;
} xce_val_with_idx_t;
typedef struct {
char* name;
xce_val_t* val;
} xce_name_val_t;
typedef struct {
char* name;
char* host;

View file

@ -143,7 +143,7 @@ hent_t* ht_add(htab_t* t, void* key)
e->data = -1;
hc = HASH_KEY_HASH(t, key) % (t->nslots);
mylock_init(&(t->lock));
mylock_lock(&(t->lock));
e->next = t->slots[hc];
t->slots[hc] = e;
mylock_unlock(&(t->lock));
@ -162,7 +162,7 @@ int ht_del(htab_t* t, void* key)
}
hc = HASH_KEY_HASH(t, key) % (t->nslots);
mylock_init(&(t->lock));
mylock_lock(&(t->lock));
e = &(t->slots[hc]);
while ((*e) != NULL) {
if (HASH_KEY_CMP(t, (*e)->key, key) == 0) {

View file

@ -49,11 +49,24 @@ xce_resreq_parser = custom_target(
],
build_by_default: true)
xce_select_parser = custom_target(
'xce select expression parser',
output: ['xce_select_peg.c', 'xce_select_peg.h'],
input: ['xce_select.peg'],
command: [
packcc,
'-l',
'-o',
'lib/xce_select_peg',
'@INPUT@'
],
build_by_default: true)
libxce_src = [ gen_srcs[0],
section_parser[0],
rusage_parser[0],
xce_resreq_parser[0],
xce_select_parser[0],
'client.c',
'dataq.c',
'hook.c',
@ -83,6 +96,7 @@ libxce_src = [ gen_srcs[0],
'xvec.c',
'helper.c',
'resreq.c',
'xce_select.c',
'bitset.c',
'kv.c',
'jobj.c',
@ -141,6 +155,8 @@ libxce_test_src = ['testsuite/test.c',
'testsuite/test_limit.c',
'testsuite/test_xvec.c',
'testsuite/test_resreq.c',
'testsuite/test_resmgr_fixture.c',
'testsuite/test_select.c',
'testsuite/test_bitset.c',
'testsuite/test_kv.c',
'testsuite/test_jobj.c',
@ -156,4 +172,4 @@ libxce_test = executable('xcelib_test',
dependencies: libxce_test_deps,
install: true)
test('libxce test', libxce_test)
test('libxce test', libxce_test, timeout: 120)

View file

@ -46,36 +46,22 @@ select_clause <- "select[" _ s:select_statement _ "]" { $$ = s; }
/ s:select_statement {$$ = s; }
select_statement <- (!("rusage[" / "]") .)* {$$ = strdup($0); }
/ (!("span[" "]" / "]") .)* {$$ = strdup($0); }
/ (!("same[" !"]" / "]") .)* {$$ = strdup($0);}
/ (!("affinity[" / "]") !"]" .)* {$$ = strdup($0);}
select_statement <- (!("rusage[" / "span[" / "same[" / "affinity[" / "]") .)* { $$=strdup($0); }
usage_clause <- "rusage[" _ usage:usage_statement _ "]" { $$ = usage; }
usage_statement <- (!("span[" / "]") .)* { $$ = strdup($0); }
/ (!("same[" / "]") .)* { $$ = strdup($0); }
/ (!("affinity[" / "]") .)* { $$ = strdup($0); }
usage_statement <- (!("span[" / "same[" / "affinity[" / "]") .)* { $$=strdup($0); }
span_clause <- "span[" _ span:span_statement _ "]" { $$ = span; }
span_statement <- (!("same[" / "]") .)* { $$ = strdup($0); }
/ (!("affinity[" / "]") .)* { $$ = strdup($0); }
span_statement <- (!("same[" / "affinity[" / "]") .)* { $$=strdup($0); }
same_clause <- "same[" _ same:same_statement _ "]" { $$ = same; }
same_statement <- (!("affinity[" / "]") .)* { $$ = strdup($0); }
affinity_clause <- "affinity[" _ affinity:affinity_statement _ "]" { $$ = affinity; }
affinity_statement <- (!"]" .)* { $$ = strdup($0); }
EOL <- "\n" / "\r\n" / "\r" / "\0"

View file

@ -22,6 +22,7 @@ extern void test_xlist(void);
extern void test_limit(void);
extern void test_xvec(void);
extern void test_resreq(void);
extern void test_select(void);
extern void test_bitset(void);
extern void test_kv(void);
extern void test_jobobj(void);
@ -119,6 +120,7 @@ int main(int argc, char* argv[])
TEST(test_limit);
TEST(test_xvec);
TEST(test_resreq);
TEST(test_select);
TEST(test_bitset);
TEST(test_kv);
TEST(test_jobobj);

View file

@ -66,6 +66,35 @@ void test_parse_section_fail(void)
FREEUP(errmsg);
}
void test_parse_section3(void)
{
char* resreq = "type == any span[ptile=1]";
char* errmsg = NULL;
int rc = 0;
xce_resreq_data_t data;
rc = parse_section(&data, (char*)resreq, &errmsg);
if (rc < 0) {
if (errmsg != NULL) {
printf("%s", errmsg);
FREEUP(errmsg);
}
}
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_TRUE(data.selectString != NULL);
TEST_ASSERT_TRUE((data.rusageString == NULL) && (data.spanString != NULL)
&& (data.sameString == NULL)
&& (data.affinityString == NULL));
TEST_ASSERT_TRUE(errmsg == NULL);
printf("select: %s\n", data.selectString);
printf("span: %s\n", data.spanString);
FREEUP(data.selectString);
FREEUP(data.spanString);
}
void test_parse_rusage1(void)
{
char* rusageString
@ -173,7 +202,7 @@ void test_resreq_primary(void)
int rc = 0;
xce_primary_resreq_t resreq;
rc = parse_resreq_primary(&resreq, resreqString, &errmsg);
rc = parse_resreq_primary(&resreq, resreqString, NULL, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
if (rc >= 0) {
int i;
@ -220,7 +249,7 @@ void test_resreq_primary2(void)
int rc = 0;
xce_primary_resreq_t resreq;
rc = parse_resreq_primary(&resreq, resreqString, &errmsg);
rc = parse_resreq_primary(&resreq, resreqString, NULL, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
if (rc >= 0) {
int i;
@ -241,7 +270,7 @@ void test_resreq1(void)
int rc = 0;
xce_resreq_t resreq;
rc = parse_resreq(&resreq, resreqString, &errmsg);
rc = parse_resreq(&resreq, resreqString, NULL, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
if (rc >= 0) {
int i;
@ -298,7 +327,7 @@ void test_resreq2(void)
memset(&resreq, 0x0, sizeof(resreq));
rc = parse_resreq(&resreq, resreqString, &errmsg);
rc = parse_resreq(&resreq, resreqString, NULL, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
if (rc >= 0) {
char* buf = NULL;
@ -327,7 +356,7 @@ void test_resreq3(void)
memset(&resreq, 0x0, sizeof(resreq));
rc = parse_resreq(&resreq, resreqString, &errmsg);
rc = parse_resreq(&resreq, resreqString, NULL, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
if (rc >= 0) {
char* buf = NULL;
@ -347,6 +376,111 @@ void test_resreq3(void)
}
}
/*
* Boundary tests for the P0-2 span-bleed fix in section.peg.
*
* Before the fix line 50 read: !("span[]" / "]")
* After the fix line 50 reads: !("span[" / "]")
*
* With the old grammar the negative lookahead would only suppress
* consumption of the literal token "span[]", so any span clause
* with content (e.g. "span[ptile=1]") was silently absorbed into
* selectString. These tests verify the corrected boundary.
*/
/* Test 1: explicit select[...] with a non-empty span[...] clause. */
void test_parse_section_explicit_select_with_span(void)
{
char* resreq = "select[type==linux] span[ptile=1]";
char* errmsg = NULL;
int rc = 0;
xce_resreq_data_t data = {0};
rc = parse_section(&data, (char*)resreq, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
printf("select: %s\n", data.selectString ? data.selectString : "(null)");
printf("span: %s\n", data.spanString ? data.spanString : "(null)");
/* select must contain the predicate, not bleed into the span keyword */
TEST_ASSERT_TRUE(data.selectString != NULL);
TEST_ASSERT_TRUE(strstr(data.selectString, "type==linux") != NULL);
/* span must be parsed separately and hold only its content */
TEST_ASSERT_TRUE(data.spanString != NULL);
TEST_ASSERT_TRUE(strstr(data.spanString, "ptile=1") != NULL);
/* unrelated clauses must be absent */
TEST_ASSERT_TRUE(data.rusageString == NULL);
TEST_ASSERT_TRUE(data.sameString == NULL);
TEST_ASSERT_TRUE(data.affinityString == NULL);
FREEUP(data.selectString);
FREEUP(data.spanString);
FREEUP(errmsg);
}
/* Test 2: all three major clauses — select, rusage, span — in one string. */
void test_parse_section_explicit_select_rusage_span(void)
{
char* resreq = "select[a&&b] rusage[mem=100] span[ptile=2]";
char* errmsg = NULL;
int rc = 0;
xce_resreq_data_t data = {0};
rc = parse_section(&data, (char*)resreq, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
printf("select: %s\n", data.selectString ? data.selectString : "(null)");
printf("rusage: %s\n", data.rusageString ? data.rusageString : "(null)");
printf("span: %s\n", data.spanString ? data.spanString : "(null)");
TEST_ASSERT_TRUE(data.selectString != NULL);
TEST_ASSERT_TRUE(strstr(data.selectString, "a&&b") != NULL);
TEST_ASSERT_TRUE(data.rusageString != NULL);
TEST_ASSERT_TRUE(strstr(data.rusageString, "mem=100") != NULL);
TEST_ASSERT_TRUE(data.spanString != NULL);
TEST_ASSERT_TRUE(strstr(data.spanString, "ptile=2") != NULL);
FREEUP(data.selectString);
FREEUP(data.rusageString);
FREEUP(data.spanString);
FREEUP(errmsg);
}
/*
* Test 3: the core regression guard — selectString must NOT contain the
* substring "span" when a span clause is present. With the old buggy
* grammar the span clause text would bleed into selectString.
*/
void test_parse_section_span_not_in_select(void)
{
char* resreq = "select[gpu] span[ptile=4]";
char* errmsg = NULL;
int rc = 0;
xce_resreq_data_t data = {0};
rc = parse_section(&data, (char*)resreq, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
printf("select: %s\n", data.selectString ? data.selectString : "(null)");
printf("span: %s\n", data.spanString ? data.spanString : "(null)");
/* selectString must not contain the span keyword (bleed check) */
TEST_ASSERT_TRUE(data.selectString != NULL);
TEST_ASSERT_TRUE(strstr(data.selectString, "span") == NULL);
/* span content must be correctly isolated */
TEST_ASSERT_TRUE(data.spanString != NULL);
TEST_ASSERT_TRUE(strstr(data.spanString, "ptile=4") != NULL);
FREEUP(data.selectString);
FREEUP(data.spanString);
FREEUP(errmsg);
}
void test_resreq(void)
{
printf("\n>> resreq test\n\n");
@ -354,6 +488,7 @@ void test_resreq(void)
RUN_TEST(test_parse_section1);
RUN_TEST(test_parse_section2);
RUN_TEST(test_parse_section_fail);
RUN_TEST(test_parse_section3);
RUN_TEST(test_parse_rusage1);
RUN_TEST(test_parse_rusage2);
RUN_TEST(test_resreq_primary);
@ -361,4 +496,7 @@ void test_resreq(void)
RUN_TEST(test_resreq1);
RUN_TEST(test_resreq2);
RUN_TEST(test_resreq3);
RUN_TEST(test_parse_section_explicit_select_with_span);
RUN_TEST(test_parse_section_explicit_select_rusage_span);
RUN_TEST(test_parse_section_span_not_in_select);
}

View file

@ -1,3 +1,4 @@
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@ -6,6 +7,147 @@
#include "utils.h"
#include "xstring.h"
/* ------------------------------------------------------------------ */
/* Concurrent-test helpers */
/* ------------------------------------------------------------------ */
#define NTHREADS (8)
/* Thread argument for the same-key test */
typedef struct {
const char* key;
char* result;
} same_key_arg_t;
static void* thread_make_same_key(void* arg)
{
same_key_arg_t* a = (same_key_arg_t*)arg;
a->result = makeString((char*)a->key);
return a->result;
}
/* Thread argument for the distinct-key test */
typedef struct {
char key[16];
char* result;
} distinct_key_arg_t;
static void* thread_make_distinct_key(void* arg)
{
distinct_key_arg_t* a = (distinct_key_arg_t*)arg;
a->result = makeString(a->key);
return a->result;
}
/* ------------------------------------------------------------------ */
/* Concurrent tests */
/* ------------------------------------------------------------------ */
/*
* test_xstring_concurrent_same_key
*
* Spawn NTHREADS threads, each racing to intern the same key.
* With the TOCTOU fix the lock is held from key_find through key_add,
* so every thread must receive the identical interned pointer.
* Without the fix, two threads could both miss key_find, both call
* makeInstance, and one would leak — and the pointers would differ.
*/
static void test_xstring_concurrent_same_key(void)
{
pthread_t tids[NTHREADS];
same_key_arg_t args[NTHREADS];
int rc;
int i;
rc = xstring_table_init(139);
TEST_ASSERT_TRUE(rc >= 0);
for (i = 0; i < NTHREADS; i++) {
args[i].key = "concurrent_key";
args[i].result = NULL;
rc = pthread_create(&tids[i], NULL, thread_make_same_key, &args[i]);
TEST_ASSERT_TRUE(rc == 0);
}
for (i = 0; i < NTHREADS; i++) {
void* retval = NULL;
rc = pthread_join(tids[i], &retval);
TEST_ASSERT_TRUE(rc == 0);
}
/* All threads must have received a non-NULL pointer */
for (i = 0; i < NTHREADS; i++) {
TEST_ASSERT_TRUE(args[i].result != NULL);
}
/* All pointers must be the same interned object */
for (i = 1; i < NTHREADS; i++) {
TEST_ASSERT_EQUAL_PTR(args[0].result, args[i].result);
}
/*
* makeString increments the refcount on a cache hit. Each thread
* got one reference; drop NTHREADS - 1 extras so xstring_table_fini
* can release the final one cleanly.
*/
for (i = 1; i < NTHREADS; i++) {
dereference((var)args[i].result);
}
xstring_table_fini();
}
/*
* test_xstring_concurrent_distinct_keys
*
* Spawn NTHREADS threads, each interning a unique key.
* Every returned pointer must be non-NULL and distinct from all others,
* confirming that concurrent inserts of different keys do not collide.
*/
static void test_xstring_concurrent_distinct_keys(void)
{
pthread_t tids[NTHREADS];
distinct_key_arg_t args[NTHREADS];
int rc;
int i;
int j;
rc = xstring_table_init(139);
TEST_ASSERT_TRUE(rc >= 0);
for (i = 0; i < NTHREADS; i++) {
snprintf(args[i].key, sizeof(args[i].key), "key_%d", i);
args[i].result = NULL;
rc = pthread_create(&tids[i], NULL, thread_make_distinct_key, &args[i]);
TEST_ASSERT_TRUE(rc == 0);
}
for (i = 0; i < NTHREADS; i++) {
void* retval = NULL;
rc = pthread_join(tids[i], &retval);
TEST_ASSERT_TRUE(rc == 0);
}
/* Every pointer must be non-NULL */
for (i = 0; i < NTHREADS; i++) {
TEST_ASSERT_TRUE(args[i].result != NULL);
}
/* Every pointer must be distinct from all others */
for (i = 0; i < NTHREADS; i++) {
for (j = i + 1; j < NTHREADS; j++) {
TEST_ASSERT_NOT_EQUAL(args[i].result, args[j].result);
}
}
/* Release each thread's reference */
for (i = 0; i < NTHREADS; i++) {
dereference((var)args[i].result);
}
xstring_table_fini();
}
static void test_xstring_create(void)
{
char* s = "Hello, world";
@ -76,6 +218,8 @@ void test_xstrings(void)
RUN_TEST(test_xstring_create);
RUN_TEST(test_xstring_deref);
RUN_TEST(test_xstring_share);
RUN_TEST(test_xstring_concurrent_same_key);
RUN_TEST(test_xstring_concurrent_distinct_keys);
return;
}

View file

@ -106,7 +106,7 @@ primary_resreq <- < (! ('{' / '}' ) .)* > {
xce_primary_resreq_t *primary = (xce_primary_resreq_t *)calloc(1, sizeof(*primary));
if (primary != NULL) {
auxil->primary = primary;
parse_resreq_primary(primary, (char *)$0, &(auxil->errmsg));
parse_resreq_primary(primary, (char *)$0, auxil->resmgr, &(auxil->errmsg));
}
}

View file

@ -35,7 +35,9 @@
}
xce_resource_t* r = NULL;
lock(auxil->resmgr, RTTI_LOCK_READ);
int rc = key_find(auxil->resmgr, (void*)name, (void**)&r);
unlock(auxil->resmgr);
if (rc <= 0 || r == NULL) {
if (strcmp(name, "true") == 0 || strcmp(name, "false") == 0) {
snprintf(auxil->errmsg, sizeof(auxil->errmsg),
@ -182,6 +184,19 @@ comparison <- l:resname_node _ op:cmp_op _ r:value_node {
}
}
}
} else if (l->u.resname.res_type == RES_TYPE_NUMERIC &&
r->type == SEL_STR) {
/* Numeric resource vs string literal: reject — at eval time
* is_str_cmp() would return true and simple_str() would read
* h->strvals[numeric_idx] which is never populated for numeric
* resources (NULL or garbage), causing a strcmp NULL-deref. */
snprintf(auxil->errmsg, sizeof(auxil->errmsg),
"type mismatch: numeric resource cannot be compared "
"with a string literal");
auxil->has_error = 1;
SEL_DISCARD(l);
SEL_DISCARD(r);
$$ = NULL;
} else {
$$ = (xce_select_node_t*)makeSelectNode(op_type,
(uintptr_t)l,

View file

@ -145,30 +145,30 @@ char* makeString(char* s)
}
mylock_lock(&(xstring_table.lock));
rc = key_find(xstring_table.xstr_table, (void*)s, (void**)&string);
if (rc > 0) {
/* add reference */
reference(string);
}
mylock_unlock(&(xstring_table.lock));
if (rc > 0) {
mylock_unlock(&(xstring_table.lock));
return (char*)string;
}
/* Cache miss: allocate and insert under the lock so no concurrent caller
* can insert the same key between our find and our add. key_add cannot
* return -EEXIST here because we hold the lock continuously. */
string = makeInstance(&xstring_type, s);
if (string == NULL) {
mylock_unlock(&(xstring_table.lock));
return NULL;
}
mylock_lock(&(xstring_table.lock));
rc = key_add(xstring_table.xstr_table, (void*)s, string);
mylock_unlock(&(xstring_table.lock));
if (rc < 0) {
destroy_(string, (obj_destroy_fn_t)xstring_destroy);
string = NULL;
errno = -rc;
errno = -rc;
return NULL;
}
return (char*)string;

View file

@ -77,7 +77,7 @@ typedef struct {
char* resreq;
} xce_sched_queue_t;
typedef struct {
typedef struct xce_sched_job_ {
uint64_t jobid;
time_t sub_time;
time_t queue_time;

View file

@ -168,7 +168,7 @@ xce_sched_job_t* make_sched_job(void* arg, xce_job_t* j)
return NULL;
}
rc = parse_resreq(job->resreq, j->submit->resreq, &errmsg);
rc = parse_resreq(job->resreq, j->submit->resreq, sched->resmgr, &errmsg);
if (rc < 0) {
ERROR("%s parse_resreq failed, %s", __func__, errmsg);
FREEUP(errmsg);

View file

@ -161,6 +161,11 @@ struct ValueWithIdx $C.mapname("xce_val_with_idx_t") {
val @1: Value $C.mapname("val");
}
struct NameValue $C.mapname("xce_name_val_t") {
name @0: Text $C.mapname("name");
val @1: Value $C.mapname("val");
}
struct ConfigVar $C.mapname("xce_config_t") {
name @0: Text;
value @1: Text;