two-headBoy opened a new pull request, #3633:
URL: https://github.com/apache/brpc/pull/3633
### What problem does this PR solve?
Issue Number: Fixes #1038
Problem Summary:
When `_rq` is full, `push_rq()` sleeps the worker thread and retries. The
owner worker is the only consumer of `_rq` (other workers only steal), so
once every worker spins there, no one drains any queue and the whole
scheduler livelocks.
### What is changed and the side effects?
Changed:
When `_rq` is full, pop one pending task and run it in place, then retry
the push. The pop frees one slot, which guarantees the push of the task
being scheduled out succeeds, since `_rq` has a single producer and
stealers never push; the original push is then retried until the queue
has room. The task being scheduled out is saved with `current_tid()`
(remained-callbacks re-enter `push_rq()` on the next task's stack).
Pthread-mode tasks cannot be switched out, so they fall back to waiting,
and a pthread-mode task popped by the main task is put back instead of
being silently dropped.
Verified with a recursive spawner repro (every task spawns more tasks until
stopped):
before the fix all workers froze in the usleep loop with CPU ~0%;
after the fix workers kept draining the queues (CPU ~400%) and the echo_c++
smoke test passes.
```cpp
// Repro for brpc issue #1038: all local run queues full -> livelock.
#include <bthread/bthread.h>
#include <stdio.h>
#include <time.h>
#include <unistd.h>
static void* spawner(void*) {
while (true) {
bthread_t tid;
// The spawned task is itself a spawner: a worker that steals it
// is "converted" into a producer.
bthread_start_background(&tid, nullptr, spawner, nullptr);
}
return nullptr;
}
int main() {
// The concurrency floor is BTHREAD_MIN_CONCURRENCY (=3+epoll, 4 on
// Linux); setting 1 fails with EINVAL. Use the floor and seed every
// worker with one spawner.
const int n = BTHREAD_MIN_CONCURRENCY;
bthread_setconcurrency(n);
for (int i = 0; i < n; ++i) {
bthread_t tid;
bthread_start_background(&tid, nullptr, spawner, nullptr);
}
fprintf(stderr, "spawner x%d started, waiting for livelock...\n", n);
time_t start = time(nullptr);
while (true) {
sleep(1);
fprintf(stderr, "main alive, %lds elapsed\n", time(nullptr) - start);
}
return 0;
}
```
Side effects:
- Performance effects: none on the hot path; only runs when the queue is
full.
- Breaking backward compatibility: none.
Note: this fix guarantees scheduler liveness, not per-task completion
deadlines; rate limiting under sustained overload remains the
application's responsibility. The window where every worker is inside a
pthread-mode task when its queue fills is left as-is, consistent with
the existing stance for pthread-mode tasks in `exchange()`.
---
### Check List:
- Please make sure your changes are compilable.
- When providing us with a new feature, it is best to add related tests.
- Please follow
`https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
