[LU-5570] Better router selection in LNet

• Status: Resolved
• Project: Lustre
• Type: Improvement
• Priority: Minor
• Reporter: Liang Zhen (Inactive)
• Assignee: Liang Zhen (Inactive)
• Resolution: Fixed

Description

LNet chooses routers based on queued bytes on routers, at the meanwhile, it normally takes tens of seconds to detect dead routers (we see failed completion event of outstanding tx/rx, then close connection and notify LNet peer is dead) , which means it is still possible to queue more messages to a potentially dead router if all other alive routers have long message queue.

we may need to check aliveness timestamp as part of router evaluation, and avoid to choose those routers that are inactive for certain number of seconds as long as there are other active routers (it takes pretty long to mark a router as dead, we might prefer not to choose it before marking it as dead)

Issue Links

• is related to: LU-7734 LNet Multi-Rail Project

Comments

[Liang Zhen (Inactive)]

Isaac, Amir, I just posted an experimental patch: http://review.whamcloud.com/11748
could you take a look and give me some feedback, thanks.

[Isaac Huang (Inactive)]

How is it different from setting router_ping_timeout to a lower value (and thus detecting dead/congested routers earlier)?

[Liang Zhen (Inactive)]

if so, then more likely we will drop already queued messages even the router is still alive? I also think It's helpful if user can see last_alive of routers on clients/servers.

[Isaac Huang (Inactive)]

In case of a false dead router due to low timeout, we don't abandon messages already queued to it, do we? I thought we just stop giving it more messages, and leave those already queued intact.

[Liang Zhen (Inactive)]

but when we finalise message and return credit, we may drop queued message because router is down? hmm... if without this patch, then this is only happen on router, so you are correct, this is not an issue.
Anyway, I agree changing configuration may prevent upper layer to deliver more messages to potentially dead router, but it may take a few while to recover correct status even it is a false dead (dead_router_check_interval), also, user may not notice that they have to change their configuration. If we have aliveness status for routers, user don't have to change anything?

[Isaac Huang (Inactive)]

The dead_router_check_interval can be changed at run time, not a big deal really. My point is, the added code complexity seemed to out weight the benefits from the patch. If you want to go forward, why not also avoid unnecessary pings (e.g. no need to ping if last_alive is very recent) - then the additional benefit of reduced pings would make it more worthwhile.

[Isaac Huang (Inactive)]

Also, with aliveness for routers, it'd be possible to fix LU-5485 as well. Better plan them all together.

[Liang Zhen (Inactive)]

I would think we should always avoid configuration changes when it's possible, we already have too many tunables which is overkill and very hard for users to make them all corrects.

I totally agree it 's better to fix LU-5485 in this patch (sorry I missed your comment there), and RC ping reduction is also a very good idea, I will have a follow-on patch to implement ping reduction, as it may requires a little more changes to a few different timestamps, so it could be clear to have a separate patch, thanks.

[Alexey Lyashkov]

I think we should don't queue any messages in case negative credits. In that case we will queue only data able to send and easy to put other data to the different routers.

[Liang Zhen (Inactive)]

In current lnet, if there is any router has positive credit, we will queue message to it not to router with negative credits.

[Alexey Lyashkov]

it's not true, i have a lots crash dumps with negative credits per destination when router dead.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/11748/
Subject: LU-5570 lnet: check router aliveness timestamp
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 339c7b2b784a528f41c432e9b90285d3445b7536

[Jodi Levi (Inactive)]

Patch landed to Master. If there is more work to be done in this ticket, please reopen the ticket.

[Gerrit Updater]

Oleg Drokin ([e-mail]) uploaded a new patch: http://review.whamcloud.com/13302
Subject: Revert "LU-5570 lnet: check router aliveness timestamp"
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 2d8b8c9e0149b0fe860983cd2020d9781bd2e548

[Liang Zhen (Inactive)]

I just requested Oleg to revert it, because this patch is not cleanly rebased, also we need Isaac to review it.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/13302/
Subject: Revert "LU-5570 lnet: check router aliveness timestamp"
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: bfaadd73b74da2aca82007ca78a6baf15ea2790c

[Gerrit Updater]

Liang Zhen ([e-mail]) uploaded a new patch: http://review.whamcloud.com/13342
Subject: LU-5570 lnet: check router aliveness timestamp
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 8650e97e5bede98f3bae16cf64a687ae1c07ef4b

[James A Simmons]

Do you think this could help with the ARF problems we have been having? Earlier comment seem to point to that.

[Liang Zhen (Inactive)]

James, probably not, I still need your help to sample NI status on router (my last comment on LU-5758)

[Amir Shehata (Inactive)]

This won't be needed with the Router re-work I have done. LNet Health currently uses the legacy routing code. I reworked the routing code to bring it inline with the Multi-Rail. These changes should resolve the issue described in this ticket.