feat(job-array-one-based-index): idgen allocates ordinary IDs only (task 9.1)

idgen() now accepts only a size of 1 (anything else: -EINVAL, context
unchanged) and refuses an exhausted 20-bit sequence with -EOVERFLOW
instead of spilling into the timestamp (also after idgen_array has
taken the last block). Arrays come only from idgen_array; every caller
of idgen/GenerateJobID passes 1. The old multi-element test loop moves
to idgen_array with 1-based assertions.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-01 13:17:59 +00:00
parent d3b12159a4
commit 63eaea215a
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
4 changed files with 110 additions and 26 deletions

View file

@ -205,7 +205,7 @@
- _Depends: 8.2_
- [ ] 9. Integration and cleanup
- [ ] 9.1 Restrict ordinary ID allocation to single jobs
- [x] 9.1 Restrict ordinary ID allocation to single jobs
- Remove the multi-element path from ordinary ID allocation now that the harness and the API server use the array allocator
- Migrate the existing ID-allocation test loop that requests sizes above 1 to the array allocator
- Done when the old multi-element test loop is gone or migrated, ordinary allocation with size 2 returns invalid-argument in the libxce suite, and the harness and API server builds and suites still pass

View file

@ -17,9 +17,16 @@ typedef struct {
} idgen_ctx_t;
extern int idgen_ctx_init(idgen_ctx_t*);
extern int idgen(idgen_ctx_t*, size_t, size_t, size_t, uint64_t*);
extern void idgen_ctx_fini(idgen_ctx_t*);
/* Ordinary (non-array) job ID. arraySize must be 1; array jobs are
* allocated with idgen_array(). On success *pidx is an untagged ID and 1 is
* returned. -EINVAL for bad args or arraySize != 1; -EOVERFLOW if the
* sequence field is exhausted for the current second (the context is left
* unchanged). */
extern int idgen(idgen_ctx_t* ctx, size_t clusterId, size_t apiserverId,
size_t arraySize, uint64_t* pidx);
/* First element ID of a new n-element array, numbered from 1.
* 1 <= n <= MAX_ARRAY_IDX (32767). On success *first = MAKE_JOBID(c, a, ts,
* TAG_JOB_ARRAY, (block << 15) | 1), the context is advanced past the whole

View file

@ -36,34 +36,31 @@ static time_t idgen_tick_locked(idgen_ctx_t* ctx)
int idgen(idgen_ctx_t* ctx, size_t clusterId, size_t apiserverId,
size_t arraySize, uint64_t* pidx)
{
uint64_t idx;
uint64_t ts, tag, seq;
uint64_t ts, seq;
/* ordinary IDs only: arrays are allocated by idgen_array() */
if ((ctx == NULL) || (clusterId > MAX_CLUSTER_ID)
|| (apiserverId > MAX_APISERVER_ID) || (arraySize < 1)
|| (arraySize > MAX_ARRAY_IDX + 1) || (pidx == NULL)) {
|| (apiserverId > MAX_APISERVER_ID) || (arraySize != 1)
|| (pidx == NULL)) {
return -EINVAL;
}
mylock_lock(&(ctx->lock));
ts = idgen_tick_locked(ctx);
ts = idgen_tick_locked(ctx);
seq = ctx->sequence;
if (arraySize == 1) {
/* non array job */
tag = 0x0;
seq = ctx->sequence;
ctx->sequence++;
} else {
tag = TAG_JOB_ARRAY;
seq = ROUNDUP(ctx->sequence, N_ARRAY_IDX_BITS);
ctx->sequence = ROUNDUP(seq + 1, N_ARRAY_IDX_BITS);
if (seq > UMAX_BITS(N_SEQUENCE_BITS)) {
/* the sequence field is exhausted for this second (e.g. after
* idgen_array took the last block); the context is unchanged */
mylock_unlock(&(ctx->lock));
return -EOVERFLOW;
}
ctx->sequence++;
mylock_unlock(&(ctx->lock));
idx = MAKE_JOBID(clusterId, apiserverId, ts, tag, seq);
*pidx = idx;
return arraySize;
*pidx = MAKE_JOBID(clusterId, apiserverId, ts, 0x0, seq);
return 1;
}
int idgen_array(idgen_ctx_t* ctx, size_t clusterId, size_t apiserverId,

View file

@ -50,6 +50,8 @@ static void test_idgen1(void)
idgen_ctx_fini(&ctx);
}
/* Multi-element allocation goes through idgen_array (numbered from 1); this
* replaces the old idgen(..., size > 1, ...) loop that asserted index 0. */
static void test_idgen_array(void)
{
idgen_ctx_t ctx;
@ -65,13 +67,13 @@ static void test_idgen_array(void)
for (k = 0; k < 5; k++) {
int j;
rc = idgen(&ctx, 0x0, 0x0, i + 1, &idx);
TEST_ASSERT_TRUE(rc >= 0);
if (i >= 1) {
TEST_ASSERT_TRUE((idx & MAX_ARRAY_IDX) == 0);
}
rc = idgen_array(&ctx, 0x0, 0x0, i + 1, &idx);
TEST_ASSERT_EQUAL_INT(i + 1, rc);
TEST_ASSERT_EQUAL_UINT64(TAG_JOB_ARRAY, EXTRACT_ID_TAG(idx));
TEST_ASSERT_EQUAL_UINT64(1, EXTRACT_ARRAY_IDX(idx));
TEST_ASSERT_EQUAL_UINT64(i + 1, EXTRACT_ARRAY_IDX(idx + i));
for (j = 0; j < i; j++) {
for (j = 0; j <= i; j++) {
char buf[256];
memset(buf, 0x0, sizeof(buf));
@ -87,6 +89,33 @@ static void test_idgen_array(void)
idgen_ctx_fini(&ctx);
}
/* Ordinary allocation is single-job only: any size other than 1 is
* rejected and leaves the context unchanged; size 1 returns 1 and an
* untagged ID (job-array-one-based-index 1.3, 4.2). */
static void test_idgen_single_only(void)
{
idgen_ctx_t ctx;
uint64_t idx = 0xdeadbeefULL;
size_t bad[] = { 0, 2, 3, 32767, 32768, SIZE_MAX };
size_t i;
idgen_ctx_init(&ctx);
ctx.sequence = 7;
for (i = 0; i < sizeof(bad) / sizeof(bad[0]); i++) {
TEST_ASSERT_EQUAL_INT(-EINVAL, idgen(&ctx, 0, 0, bad[i], &idx));
TEST_ASSERT_EQUAL_UINT64(0xdeadbeefULL, idx);
TEST_ASSERT_EQUAL_UINT32(7, ctx.sequence);
}
TEST_ASSERT_EQUAL_INT(1, idgen(&ctx, 5, 3, 1, &idx));
TEST_ASSERT_EQUAL_UINT64(0, EXTRACT_ID_TAG(idx));
TEST_ASSERT_EQUAL_UINT64(5, EXTRACT_ID_CLUSTER(idx));
TEST_ASSERT_EQUAL_UINT64(3, EXTRACT_ID_APISERVER(idx));
idgen_ctx_fini(&ctx);
}
#define ARRAY_BLOCK ((uint32_t)1 << N_ARRAY_IDX_BITS)
static void test_idgen_array_invalid(void)
@ -238,6 +267,55 @@ static void test_idgen_array_overflow(void)
idgen_ctx_fini(&ctx);
}
/* Plain idgen() must not let the sequence spill into the timestamp field:
* once the 20-bit sequence is exhausted (e.g. after idgen_array took block
* 31) it returns -EOVERFLOW and leaves the context unchanged. */
static void test_idgen_overflow(void)
{
idgen_ctx_t ctx;
uint64_t idx;
uint32_t full = (uint32_t)1 << N_SEQUENCE_BITS;
idgen_ctx_init(&ctx);
for (;;) {
time_t t0 = time(NULL);
int rc_last, rc_over, rc_after_array;
uint64_t last = 0, over = 0xdeadbeefULL, first = 0;
/* the last sequence value is still usable */
ctx.timestamp = t0;
ctx.sequence = full - 1;
rc_last = idgen(&ctx, 5, 3, 1, &last);
/* the next one is not; the context stays as it was */
rc_over = idgen(&ctx, 5, 3, 1, &over);
/* idgen_array taking block 31 exhausts the sequence too */
ctx.sequence = 31 * ARRAY_BLOCK;
rc_after_array = idgen_array(&ctx, 5, 3, 2, &first);
if (rc_after_array == 2) {
rc_after_array = idgen(&ctx, 5, 3, 1, &idx);
}
if (time(NULL) != t0) {
continue; /* the second changed: the sequence was reset */
}
TEST_ASSERT_EQUAL_INT(1, rc_last);
TEST_ASSERT_EQUAL_UINT64(full - 1, EXTRACT_ID_SEQUENCE(last));
TEST_ASSERT_EQUAL_UINT64((uint64_t)t0, EXTRACT_ID_TIMESTAMP(last));
TEST_ASSERT_EQUAL_INT(-EOVERFLOW, rc_over);
TEST_ASSERT_EQUAL_UINT64(0xdeadbeefULL, over);
TEST_ASSERT_EQUAL_INT(-EOVERFLOW, rc_after_array);
TEST_ASSERT_EQUAL_UINT32(full, ctx.sequence);
TEST_ASSERT_EQUAL_INT64((int64_t)t0, (int64_t)ctx.timestamp);
break;
}
idgen_ctx_fini(&ctx);
}
void test_idgen(void)
{
printf("\n>> idgen test\n\n");
@ -245,6 +323,8 @@ void test_idgen(void)
RUN_TEST(test_idgen_invalid);
RUN_TEST(test_idgen1);
RUN_TEST(test_idgen_array);
RUN_TEST(test_idgen_single_only);
RUN_TEST(test_idgen_overflow);
RUN_TEST(test_idgen_array_invalid);
RUN_TEST(test_idgen_array_one_based);
RUN_TEST(test_idgen_array_overflow);