Skip to content

mds: notify clients if the session has already opened#45307

Merged
vshankar merged 3 commits intoceph:masterfrom
lxbsz:wip54461
Apr 25, 2022
Merged

mds: notify clients if the session has already opened#45307
vshankar merged 3 commits intoceph:masterfrom
lxbsz:wip54461

Conversation

@lxbsz
Copy link
Member

@lxbsz lxbsz commented Mar 9, 2022

If the connection was accidently closed due to the socket issue or
something else the client will try to open the opened sessions, for
now the MDS will just discard the session open request.

But the client will keep waiting the reply from the mds forever.

We need to tell the clients what has happened instead of discard it
directly. And when the client get the session open reply, it can
do what needed.

Fixes: https://tracker.ceph.com/issues/53911
Signed-off-by: Xiubo Li xiubli@redhat.com

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@github-actions github-actions bot added the cephfs Ceph File System label Mar 9, 2022
@lxbsz lxbsz requested review from a team, anthonyeleven and vshankar and removed request for anthonyeleven March 9, 2022 06:29
@lxbsz lxbsz force-pushed the wip54461 branch 2 times, most recently from 3009978 to fa18e28 Compare March 11, 2022 00:24
@vshankar
Copy link
Contributor

@lxbsz -- I'll revert PR #44638 and merge it for some teuthology tests. (you'll need to rebase this however - I hope that fine?).

I'll plan for reviewing this PR for the fix.

@lxbsz
Copy link
Member Author

lxbsz commented Mar 17, 2022

@lxbsz -- I'll revert PR #44638 and merge it for some teuthology tests. (you'll need to rebase this however - I hope that fine?).

Certainly.

I'll plan for reviewing this PR for the fix.

Sure, thanks. I will rebase it.

@lxbsz lxbsz changed the title Revert "mds: kill session when mds do ms_handle_remote_reset" mds: notify clients if the session has already opened Mar 17, 2022
@vshankar vshankar self-assigned this Mar 17, 2022
dout(10) << "currently open|opening|stale|killing, dropping this req" << dendl;
dout(10) << "currently open|opening|stale|killing" << dendl;
if (m->supported_features.test(CEPHFS_FEATURE_REFRESH_SESSION)) {
auto reply = make_message<MClientSession>(CEPH_SESSION_ALREADY_OPEN,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider sending CEPH_SESSION_OPEN back to the client if the session is already open?

(In client, the case for CEPH_SESSION_ALREADY_OPEN falls-through to CEPH_SESSION_OPEN anyway)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let me try this. Mainly different is updating the seq.

Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lxbsz
Copy link
Member Author

lxbsz commented Apr 14, 2022

Just rebased it to the latest code, nothing changed.

lxbsz added 3 commits April 18, 2022 10:17
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Client side never uses this seq.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
If the connection was accidently closed due to the socket issue or
something else the client will try to open the opened sessions, for
now the MDS will just discard the session open request.

But the client will keep waiting the reply from the mds forever.

We need to tell the clients what has happened instead of discard it
directly. And when the client get the session open reply, it can
do what needed.

Fixes: https://tracker.ceph.com/issues/53911
Signed-off-by: Xiubo Li <xiubli@redhat.com>
@lxbsz
Copy link
Member Author

lxbsz commented Apr 18, 2022

Just rebased it to the latest code, nothing changed.

@vshankar
Copy link
Contributor

@vshankar vshankar merged commit 09704af into ceph:master Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cephfs Ceph File System common

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants