Skip to content

redis 6.0 doesn't free clients that disconnect during loading #7215

@oranagra

Description

@oranagra

@antirez In 6.0 you changed readQueryFromClient to call freeClientAsync rather than freeClient (it was for threaded IO).

so although redis 6.0 now calls freeClientsInAsyncFreeQueue from beforeSleep rather than serverCron, neither of them is called while loading.

this means that now if a client connects and drops during loading, it is added to clients_to_close and remains alive (listed as a client, and socket not closed) until loading is done.

if there are some stats monitoring tools and watchdogs sampling the loading progress by re-opening and closing the connection repeatedly, over time, on a long loading, thousands of connections accumulate. (i have circumstantial evidence showing that this can also slow down the loading process, i'll post details in the next post)

it is interesting to note that processEventsWhileBlocked does call afterSleep (4 times), but does not call beforeSleep.
this looks very odd, so one possible solution is to move the call to beforeSleep from aeMain to aeProcessEvents (right before aeApiPoll and afterSleep).

we need to carefully consider what side effects such a change can cause, and if we can't afford it, maybe just call freeClientsInAsyncFreeQueue from processEventsWhileBlocked (which is very very specifically crafted function anyway).

it is important to note however, that in some cases (diskless replica) processEventsWhileBlocked is called from within a read event handler (readSyncBulkPayload), but i suppose it won't detect the master disconnected and try to free it from that call path, so maybe it's still ok.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions