[LU-18072] Lock cancel resending overwhelms ldlm canceld thread

• Type: Bug
• Resolution: Unresolved
• Priority: Critical
• Affects Version/s: Lustre 2.16.0
• Severity: 3
• Assignee: Oleg Drokin
• Reporter: Oleg Drokin

Description

Ever since LU-1565 landed it looks like another subtle problem was introduced.

Since LDLM_CANCEL requests don't have any throttling, they could be sent in huge numbers, this is normally not a big problem as processing is relatively lightweight.

But in case there's a resend for such a cancel, suddenly we take this path in ptlrpc_server_check_resend_in_progress() added in LU-793:

ptlrpc_server_check_resend_in_progress(struct ptlrpc_request *req)
{
        struct ptlrpc_request *tmp = NULL;        if (!(lustre_msg_get_flags(req->rq_reqmsg) & MSG_RESENT))
                return NULL;        /*
         * This list should not be longer than max_requests in
         * flights on the client, so it is not all that long.
         * Also we only hit this codepath in case of a resent
         * request which makes it even more rarely hit
         */
        list_for_each_entry(tmp, &req->rq_export->exp_reg_rpcs,
                                rq_exp_list) {
                /* Found duplicate one */
                if (tmp->rq_xid == req->rq_xid)
                        goto found;
        }
        list_for_each_entry(tmp, &req->rq_export->exp_hp_rpcs,
                                rq_exp_list) {
                /* Found duplicate one */
                if (tmp->rq_xid == req->rq_xid)

a case was observed at a customer site where this was entered and the exp_hp_rpc lists for multiple client exports were in tens of thousands as mass cancellations (from lru timing out?) commenced which led to these list iterations under exp_rpc_lock taking really long time and now clogging the ldlm_cn threads so no processing wasreally possible, request delays from network acceptance to req_in handler could reach into tens of seconds as the result for all clients (same thread pool) with the expected disastrous results.

 

I imagine we need to address this from two directions:

1. server side we really need to avoid taking long time for duplicate request search
2. client side we already drop unused and actively cancelled ldlm locks, we need to also drop the cancel resends on replay somehow

Issue Links

• is related to: LU-18077 Do not resend cancel requests over replay boundary
• is related to: LU-18881 MDT overwhelmed by lock cancel requests
• is related to: LU-18111 Don't drop expired cancel request
• is related to: LU-793 Reconnections should not be refused when there is a request in progress from this client.
• is related to: LU-1565 lost LDLM_CANCEL RPCs
• is related to: LU-15190 ptlrpc_server_check_resend_in_progress() can miss duplicate RPC

Comments

[Oleg Drokin]

They are not processing fast because they have the resent flag, so instead of being processed, all the cn threads are hogging the cpu fighting to get the exp_rpc_lock spinlock for their chance to iterate the incoming RPCs list searching for duplicates. Available core files show that there are tens of thousands of requests on those lists from dozen or so clients. This takes a lot of CPU. While the spinlock is separate per client, I imagine those RPCs arrive in batches right after a reconnect.

Dropping them on reconnect server-side certainly makes sense because at least we quickly trim the list, but still need sensible resent handling esp. for portals where it does nto help to reconnect to old requests like the cancel portal.

Personally I think optimizing of duplicates search is not going to buy us as much as not running it where it makes no sense at all.

The other thing is considering duplicates search really only makes sense for requests that are in processing, why don't we have a separate lists for such requests? that one is sure going to be super short (limited by number of server threads at the most).

 

If we go back to LU-793 - the whole idea there was that for long running requests we want to attach resends into the existing processing so the response is sent as a reply to the resent request and is actually delivered, instead of us retryign the real expensive operation anyway.

Doing this for requests that are not yet starting processing is counterproductive for two reasons:

1. the not yet started request is not going to be processed anyway because due to the reconnect it will be dropped thanks to the ptlrpc_check_req() filtering
2. But reconnecting a new resend into the old request like that all we do is ensure even the resent request is not going to be processed because all we do is just tell the client "hey this is old generation request" even though it really was not!

[Mikhail Pershin]

then the question is why they are not processing fast but staying in incoming queue. Is that canceld overload and busy? Or that is result of NRS policy which doesn't prioritize cancels properly? Still, I think if we can't control client flow, then we need to control it on server, why to fill memory with tons incoming cancels from all clients along with all their resent duplicates? Even if they can be dropped, OOM on server is still worse than -EBUSY response for clients with too many requests waiting

Then I'd think about moving to rhash to find duplicates - that way we would avoid duplicates - less memory usage as result and search will be in reasonable time, sort of compromise. I still have no whole picture for this, only some ideas about possible approaches 

[Oleg Drokin]

plus we used to respect max-rpcs-in-flight on the clients in the past?

ldlm cancels never had max RIF, if you have to send a lock cancel and there are no slots, what are you going to do? wait it out? that's the most sure way to get evicted for unresponsiveness there is.

That's why they go to their own separate portal - so other requests don't affect cancellations and the other way around - cancellations don't affect the other requests

[Alex Zhuravlev]

Server cannot stop the flood, we can stop accepting requests in some cases

right, server can reject PRCs. plus we used to respect max-rpcs-in-flight on the clients in the past?

[Oleg Drokin]

Server cannot stop the flood, we can stop accepting requests in some cases, though right now we don't even have a counter of how many requests we have in any particular unprocesses queue it looks like, so making that decision is hard.

[Mikhail Pershin]

I'd say sever needs also own protection from such thing, not capability to serve in timely manner but also limits for such DDOS - we can limit size of incoming queue and just return any new resent without any duplication check with -EBUSY if that client export has too many waiting requests. I.e. 'external' flood control for client on server

[Alex Zhuravlev]

well, enough clients send their huge rpc batches and it's just matter of number of clients to hit timeout-reconnect-resend, right? of course it's server who must stop this flood.

[Oleg Drokin]

While this is indeed not ideal and we definitely need to make client-side improvements, servers should be robust enough in the face of somewhat adversary network traffic anyway so handling it sensibly server side is still important.

[Alex Zhuravlev]

IMO, this is not quite correct that we let clients to DDoS servers essentially - due to lack of any flow control.

[Oleg Drokin]

yes, I agree. Hence why I originally proposed not applying this logic to some services like cancels, but that clearly is not going to be enough alone.

The idea to drop entire queue on reconnect sounds promising here in addition to that?

Should we add a counter for such a "totally unprocessed" queue and if it gets high print something, that way we can see if any other services are affected potentially?

[Alex Zhuravlev]

the original algo was not supposed to handle tens of thousands requests in flight:

	/*
	 * This list should not be longer than max_requests in
	 * flights on the client, so it is not all that long.
	 * Also we only hit this codepath in case of a resent
	 * request which makes it even more rarely hit
	 */

so we have to adapt it to the new requirement..