client: check if a mds rank is up before fetching connection addr
#41875
Conversation
Client segfaults when trying to infer which mds rank a connection reset call is coming from. It does this by iterating `mds_sessions` and checking the mds addr (in mdsmap) to the `Connection *`. However, cases where the mds is blocklisted, the client receives an updated mdsmap in which the corresponding mds is not in `up` set thereby resulting in a segfault when calling `mdsmap->get_addrs` since it expects that the mds should be in `up` state. Note that this leaves the `Connection *` as it is and does not clean it up. That needs to fixed separately probably by maintaining a map of `Connection *` to mds rank for lookup. Fixes: http://tracker.ceph.com/issues/50530 Signed-off-by: Venky Shankar <vshankar@redhat.com>
| for (auto &p : mds_sessions) { | ||
| if (mdsmap->get_addrs(p.first) == con->get_peer_addrs()) { | ||
| if (mdsmap->have_inst(p.first) && mdsmap->get_addrs(p.first) == con->get_peer_addrs()) { | ||
| mds = p.first; | ||
| s = &p.second; | ||
| } |
There was a problem hiding this comment.
I know this is old code and you are just adding a check, but it'd be good if the first and second references were converted to structured binding.
There was a problem hiding this comment.
you probably just need to move mds and s to the for loop initializer so that the have_inst() and get_addrs() calls look more sensible.
There was a problem hiding this comment.
Agree. But the whole client is littered with such .firs/.second semantics. I don't mind converting this to use structured binding, but we will still have the problem at large.
There was a problem hiding this comment.
yeah, I know.
I was just trying to nudge you to do that so that we could get the changes done bit by bit in relevant PRs rather than one huge PR to move the Client to structured bindings :)
There was a problem hiding this comment.
I think it's safe to leave this as-is for now. But Milind is right it'd be nice to do that but it should go in a cleanup PR.
|
@batrick we probably want to start maintaining |
Yes, I think that makes sense. |
Client segfaults when trying to infer which mds rank a connection
reset call is coming from. It does this by iterating
mds_sessionsand checking the mds addr (in mdsmap) to the
Connection *.However, cases where the mds is blocklisted, the client receives
an updated mdsmap in which the corresponding mds is not in
upset thereby resulting in a segfault when calling
mdsmap->get_addrssince it expects that the mds should be in
upstate.Note that this leaves the
Connection *as it is and does not cleanit up. That needs to fixed separately probably by maintaining a map
of
Connection *to mds rank for lookup.Fixes: http://tracker.ceph.com/issues/50530
Signed-off-by: Venky Shankar vshankar@redhat.com
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox