From 1feec7d4255ed41be3a2669937652ce0e2e448ea Mon Sep 17 00:00:00 2001 From: "Yan, Zheng" Date: Sat, 8 Jun 2019 13:08:21 +0800 Subject: [PATCH] mds: fix corner case of replaying open sessions Marking a session dirty may flush all existing dirty sessions. MDS calls Server::finish_force_open_sessions() for log event that opens multiple sessions. The function marks sessions dirty one by one. So sessions opened by a log event may get flushed partially. When replaying a log event that opens multiple sessions, mds need to check if some of these sessions have already been flushed. Signed-off-by: "Yan, Zheng" (cherry picked from commit 277327565fae93fad8e03cc5f27b78c98354c0dc) Conflicts: src/mds/SessionMap.cc src/mds/SessionMap.h src/mds/journal.cc --- src/mds/Server.cc | 2 +- src/mds/SessionMap.cc | 11 ++++++----- src/mds/SessionMap.h | 18 +++++++++++++++--- src/mds/journal.cc | 16 +++------------- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/mds/Server.cc b/src/mds/Server.cc index 874b456058c..87db74bfe24 100644 --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -2925,7 +2925,7 @@ void Server::apply_allocated_inos(MDRequestRef& mdr, Session *session) assert(session); session->pending_prealloc_inos.subtract(mdr->prealloc_inos); session->info.prealloc_inos.insert(mdr->prealloc_inos); - mds->sessionmap.mark_dirty(session); + mds->sessionmap.mark_dirty(session, !mdr->used_prealloc_ino); mds->inotable->apply_alloc_ids(mdr->prealloc_inos); } if (mdr->used_prealloc_ino) { diff --git a/src/mds/SessionMap.cc b/src/mds/SessionMap.cc index ae10833c718..286c6056476 100644 --- a/src/mds/SessionMap.cc +++ b/src/mds/SessionMap.cc @@ -669,12 +669,13 @@ void SessionMap::touch_session(Session *session) session->last_cap_renew = clock::now(); } -void SessionMap::_mark_dirty(Session *s) +void SessionMap::_mark_dirty(Session *s, bool may_save) { if (dirty_sessions.count(s->info.inst.name)) return; - if (dirty_sessions.size() >= g_conf->mds_sessionmap_keys_per_op) { + if (may_save && + dirty_sessions.size() >= g_conf->mds_sessionmap_keys_per_op) { // Pre-empt the usual save() call from journal segment trim, in // order to avoid building up an oversized OMAP update operation // from too many sessions modified at once @@ -685,12 +686,12 @@ void SessionMap::_mark_dirty(Session *s) dirty_sessions.insert(s->info.inst.name); } -void SessionMap::mark_dirty(Session *s) +void SessionMap::mark_dirty(Session *s, bool may_save) { dout(20) << __func__ << " s=" << s << " name=" << s->info.inst.name << " v=" << version << dendl; - _mark_dirty(s); + _mark_dirty(s, may_save); version++; s->pop_pv(version); } @@ -700,7 +701,7 @@ void SessionMap::replay_dirty_session(Session *s) dout(20) << __func__ << " s=" << s << " name=" << s->info.inst.name << " v=" << version << dendl; - _mark_dirty(s); + _mark_dirty(s, false); replay_advance_version(); } diff --git a/src/mds/SessionMap.h b/src/mds/SessionMap.h index 23e874831e4..ad054c578b4 100644 --- a/src/mds/SessionMap.h +++ b/src/mds/SessionMap.h @@ -637,11 +637,23 @@ public: get_client_sessions(f); } - void replay_open_sessions(map& client_map) { + void replay_open_sessions(version_t event_cmapv, + map& client_map) { + // Server::finish_force_open_sessions() marks sessions dirty one by one. + // Marking a session dirty may flush all existing dirty sessions. So it's + // possible that some sessions are already saved in sessionmap. + ceph_assert(version + client_map.size() >= event_cmapv); + unsigned already_saved = client_map.size() - (event_cmapv - version); for (map::iterator p = client_map.begin(); p != client_map.end(); ++p) { Session *s = get_or_add_session(p->second); + if (already_saved > 0) { + ceph_assert(s->is_open()); + --already_saved; + continue; + } + set_state(s, Session::STATE_OPEN); replay_dirty_session(s); } @@ -697,7 +709,7 @@ protected: std::set dirty_sessions; std::set null_sessions; bool loaded_legacy = false; - void _mark_dirty(Session *session); + void _mark_dirty(Session *session, bool may_save); public: /** @@ -708,7 +720,7 @@ public: * to the backing store. Must have called * mark_projected previously for this session. */ - void mark_dirty(Session *session); + void mark_dirty(Session *session, bool may_save=true); /** * Advance the projected version, and mark this diff --git a/src/mds/journal.cc b/src/mds/journal.cc index f86d0c2586a..8ad24ca84d8 100644 --- a/src/mds/journal.cc +++ b/src/mds/journal.cc @@ -1857,8 +1857,7 @@ void ESessions::replay(MDSRank *mds) } else { dout(10) << "ESessions.replay sessionmap " << mds->sessionmap.get_version() << " < " << cmapv << dendl; - mds->sessionmap.replay_open_sessions(client_map); - assert(mds->sessionmap.get_version() == cmapv); + mds->sessionmap.replay_open_sessions(cmapv, client_map); } update_segment(); } @@ -2131,8 +2130,7 @@ void EUpdate::replay(MDSRank *mds) map cm; bufferlist::iterator blp = client_map.begin(); ::decode(cm, blp); - mds->sessionmap.replay_open_sessions(cm); - assert(mds->sessionmap.get_version() == cmapv); + mds->sessionmap.replay_open_sessions(cmapv, cm); } } update_segment(); @@ -2957,15 +2955,7 @@ void EImportStart::replay(MDSRank *mds) map cm; bufferlist::iterator blp = client_map.begin(); ::decode(cm, blp); - mds->sessionmap.replay_open_sessions(cm); - if (mds->sessionmap.get_version() != cmapv) - { - derr << "sessionmap version " << mds->sessionmap.get_version() - << " != cmapv " << cmapv << dendl; - mds->clog->error() << "failure replaying journal (EImportStart)"; - mds->damaged(); - ceph_abort(); // Should be unreachable because damaged() calls respawn() - } + mds->sessionmap.replay_open_sessions(cmapv, cm); } update_segment(); } -- 2.47.3