lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(controller): admission-gate OPCode_schedAdmin's blocking wait (Req 5 AC 10)

Browse source 
The schedAdmin dispatch case blocks its vtask handler thread for up to 5s
waiting on a cross-process reply while holding c->sched.lock -- bounded
blocking on a vtask thread, which ha-async-io.md permits only under an
admission reservation. The real risk isn't just the wait itself: since
c->sched.lock serializes concurrent requests one at a time, a burst of N
concurrent schedAdmin calls could already occupy N vtask threads for up to
N*5s each before this fix.

Now acquires vtaskmgr_blocking_acquire(&c->vtaskmgr) before ever locking
c->sched.lock, releasing on every exit path -- the same shared admission
budget commit.c's commit-wait barrier already gates on (wired via
commit_set_pool). Admission-exhausted requests fast-reject with
{ok=false, error="server busy, try again"} rather than queuing. New test:
test_schedadmin_fast_rejects_when_admission_exhausted, using a poison
c->sched.ep sentinel to prove the fast-reject happens before any
lock/forward attempt.
This commit is contained in:
Rongsong Shen 2026-08-19 11:00:49 +00:00
parent a13068dca6
commit e52c68f471
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
4 changed files with 103 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

35
src/controller/cluster.c
View file

@ -860,6 +860,38 @@ static int myreq_handler(vtask_t* task)
break;
}
/* ha-async-io.md Rule 3 (design review follow-up, 2026-08-19):
* bounded blocking on a vtask thread is permitted only under an
* admission reservation. The blocking region below isn't just the
* pthread_cond_timedwait a few lines down -- mylock_lock(&c->sched.
* lock) immediately after this comment is ALSO a bounded wait (up
* to this same ~5s deadline, times however many requests are
* already queued ahead of this one), since c->sched.lock is
* deliberately held for the whole forward+wait duration (see that
* design note below). A burst of concurrent xceadmin schedAdmin
* calls could otherwise occupy N vtask threads for up to N*5s --
* exactly the starvation pattern this reservation exists to
* prevent. Reuses the SAME shared budget commit.c's wait_admit_
* locked()/wait_release_locked() already gate on c->vtaskmgr (wired
* via commit_set_pool(&c->commit, &c->vtaskmgr) below) -- the
* reservation protects a fixed number of vcpus from ANY class of
* admitted bounded-blocking work, not a per-caller quota, so
* schedAdmin and commit-wait traffic intentionally share it. Fast-
* reject (never touch c->sched.lock) rather than queue behind an
* exhausted cap. */
if (vtaskmgr_blocking_acquire(&(c->vtaskmgr)) != 0) {
sched_admin_reply_t busy_reply;
memset(&busy_reply, 0, sizeof(busy_reply));
busy_reply.error = "server busy, try again";
reply.status = 0;
reply.data_which = RequestReply_data_schedadminreplymsg;
reply.data.schedadminreplymsg = &busy_reply;
xce_reply(ep, c->myhost, &reply);
rc = 0;
break;
}
/* c->sched.lock is held for the FULL forward+wait duration below --
* an accepted trade-off (task 10.1 design notes): xceadmin admin
* commands are low-frequency, human-paced traffic, not a hot path.
@ -874,6 +906,7 @@ static int myreq_handler(vtask_t* task)
nc_reply.error = "scheduler not connected";
mylock_unlock(&(c->sched.lock));
vtaskmgr_blocking_release(&(c->vtaskmgr));
reply.status = 0;
reply.data_which = RequestReply_data_schedadminreplymsg;
@ -898,6 +931,7 @@ static int myreq_handler(vtask_t* task)
if (rc < 0) {
schedadmin_unregister(&pending);
mylock_unlock(&(c->sched.lock));
vtaskmgr_blocking_release(&(c->vtaskmgr));
reply.status = rc;
reply.data_which = RequestReply_data_nodata;
@ -982,6 +1016,7 @@ static int myreq_handler(vtask_t* task)
}
mylock_unlock(&(c->sched.lock));
vtaskmgr_blocking_release(&(c->vtaskmgr));
rc = 0;
break;
}