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