clientsArePaused() has concurrency issues in threaded mode, and #8170 fix it inadvertently, should we backport to 6.0?
Comment From: oranagra
@soloestoy We can't backport that bug feature to 6.0. I'm not sure what you mean by these concurrency issues. Can you submit a PR to 6.0 branch to fix it specifically?
Comment From: soloestoy
@oranagra the stack is IOThreadMain() -> readQueryFromClient() -> processInputBuffer() -> clientsArePaused() and the code in 6.0 is:
int clientsArePaused(void) {
if (server.clients_paused &&
server.clients_pause_end_time < server.mstime)
{
listNode *ln;
listIter li;
client *c;
server.clients_paused = 0;
/* Put all the clients in the unblocked clients queue in order to
* force the re-processing of the input buffer if any. */
listRewind(server.clients,&li);
while ((ln = listNext(&li)) != NULL) {
c = listNodeValue(ln);
/* Don't touch slaves and blocked clients.
* The latter pending requests will be processed when unblocked. */
if (c->flags & (CLIENT_SLAVE|CLIENT_BLOCKED)) continue;
queueClientForReprocessing(c);
}
}
return server.clients_paused;
}
And then io threads can access global variables server.clients_paused iterating server.clients and call queueClientForReprocessing() insert to server.unblocked_clients at the same time.
Comment From: madolson
Btw, I did know about this, it was in the original comment of the PR that was replaced when I updated it for the core group approval.
My opinion is that we should just not fan out while clients are paused, instead of back-porting that hefty PR. It's one of those edge of edge cases, I don't think we should spend a lot of time on it for correctness.
Comment From: oranagra
@madolson @soloestoy can one of you make a PR for the 6.0 branch?
Comment From: madolson
@oranagra @soloestoy https://github.com/redis/redis/pull/8520