Ceph » CephFS

It aborted in Line#1623. The session->take_ino() may return 0 if the used_preallocated_ino doesn't exist. Then when erasing the ino# 0 it will abort in the interval_set.

...
1604   if (sessionmapv) {          
1605     if (mds->sessionmap.get_version() >= sessionmapv ||
1606         unlikely(type == EVENT_UPDATE && skip_replaying_inotable)) {
1607       dout(10) << "EMetaBlob.replay sessionmap v " << sessionmapv
1608                << " <= table " << mds->sessionmap.get_version() << dendl;
1609       if (used_preallocated_ino)
1610         mds->mdcache->insert_taken_inos(used_preallocated_ino);
1611     } else {
1612       dout(10) << "EMetaBlob.replay sessionmap v " << sessionmapv
1613                << ", table " << mds->sessionmap.get_version()
1614                << " prealloc " << preallocated_inos
1615                << " used " << used_preallocated_ino
1616                << dendl;
1617       Session *session = mds->sessionmap.get_session(client_name);
1618       if (session) {
1619         dout(20) << " (session prealloc " << session->info.prealloc_inos << ")" << dendl;
1620         if (used_preallocated_ino) {        
1621           if (!session->info.prealloc_inos.empty()) {
1622             inodeno_t ino = session->take_ino(used_preallocated_ino);
1623             session->info.prealloc_inos.erase(ino);
1624             ceph_assert(ino == used_preallocated_ino);                                                                                                                                                                            
1625           }
1626           mds->sessionmap.replay_dirty_session(session);
1627         }
1628         if (!preallocated_inos.empty()) {   
1629           session->free_prealloc_inos.insert(preallocated_inos);
1630           session->info.prealloc_inos.insert(preallocated_inos);
1631           mds->sessionmap.replay_dirty_session(session);
1632         }
1633       } else {
...

This should be a similar issue with https://tracker.ceph.com/issues/58489. Just the openc/mknod/mkdir/symblink events flushed but leaving the sessionmap event still in the active mds cache or dropped by the active MDS if it crashed.

So here maybe we should just respawn the mds automatically instead of crashing it, just like:

    -6> 2023-07-26T23:20:59.242+0000 7f65fc15a700  0 mds.71990086.journaler.mdlog(ro) _finish_read got error -2
    -5> 2023-07-26T23:20:59.242+0000 7f65fc15a700  0 mds.71990086.journaler.mdlog(ro) _finish_read got error -2
    -4> 2023-07-26T23:20:59.242+0000 7f65fc15a700  0 mds.71990086.journaler.mdlog(ro) _finish_read got error -2
    -3> 2023-07-26T23:20:59.242+0000 7f65fc15a700  0 mds.71990086.journaler.mdlog(ro) _finish_read got error -2
    -2> 2023-07-26T23:20:59.242+0000 7f65fa957700  0 mds.0.log _replay journaler got error -2, aborting
    -1> 2023-07-26T23:20:59.242+0000 7f65fa957700  0 mds.0.0 boot_start encountered an error CEPHFS_EAGAIN, respawning since we fell behind journal
     0> 2023-07-26T23:20:59.242+0000 7f65fa957700  1 mds.ocs-storagecluster-cephfilesystem-b respawn!

Xiubo Li wrote:

This should be a similar issue with https://tracker.ceph.com/issues/58489. Just the openc/mknod/mkdir/symblink events flushed but leaving the sessionmap event still in the active mds cache or dropped by the active MDS if it crashed.

So here maybe we should just respawn the mds automatically instead of crashing it, just like:

[...]

I'm not exactly recalling the series of events that could cause this. Reading up 58489 again.

Xiubo Li wrote:

It aborted in Line#1623. The session->take_ino() may return 0 if the used_preallocated_ino doesn't exist. Then when erasing the ino# 0 it will abort in the interval_set.

[...]

if it tries to remove ino 0 from interval set, there are are a few conditions that will make sure it doesn't abort. in void erase() we have auto p = find_inc_m(start) which has this code

  auto find_inc_m(T start) {
    auto p = m.lower_bound(start);
    if (p != m.begin() &&
        (p == m.end() || p->first > start)) {
      --p;   // might overlap?
      if (p->first + p->second <= start)
        ++p; // it doesn't.
    }
    return p;
  }

this makes sure we never have p->first > start so the assertion ceph_assert(p->first <= start) should not have failed. This makes me wonder if we are getting some garbage val that overflows and leads to the assertion failure?

i have a concern about this code; if the p->first is still gt than start even after decrementing then it means a single decrement might just not be enough? i.e. keep on decrementing until we are sure it is less than start?

  auto find_inc_m(T start) {
    auto p = m.lower_bound(start);
    if (p != m.begin() &&
        (p == m.end() || p->first > start)) {
      --p;   // might overlap?
      if (p->first + p->second <= start)
        ++p; // it doesn't.
    }
    return p;
  }

Xiubo Li wrote:

This should be a similar issue with https://tracker.ceph.com/issues/58489. Just the openc/mknod/mkdir/symblink events flushed but leaving the sessionmap event still in the active mds cache or dropped by the active MDS if it crashed.

So here maybe we should just respawn the mds automatically instead of crashing it, just like:

[...]

Xiubo, were you able to verify if this issue is similar to tracker #58489?

Venky Shankar wrote:

Xiubo Li wrote:

This should be a similar issue with https://tracker.ceph.com/issues/58489. Just the openc/mknod/mkdir/symblink events flushed but leaving the sessionmap event still in the active mds cache or dropped by the active MDS if it crashed.

So here maybe we should just respawn the mds automatically instead of crashing it, just like:

[...]

Xiubo, were you able to verify if this issue is similar to tracker #58489?

Venky, the issue seems to be the partial flush of log entries where a standby replay would try to replay logs and would fail at line xiubo mentioned in https://tracker.ceph.com/issues/61009#note-6. It tries to compare map elements with 0 and thus fails since the decremented element's key (check https://tracker.ceph.com/issues/61009#note-10) is still gt than 0. This needs to be fixed at multiple points.

1) in journal.cc, first check if the used_preallocated_ino exists in session->info.prealloc_inos and then erase.
2) Make sure to flush the log entries before respawning mds
3) dont decrement just once in erase() in interval_set but instead iterate until we have p>first < start, raised a RFC PR for this https://github.com/ceph/ceph/pull/53063-

I was talking to Dhairya about this today and am not quite sure I understand.

Xiubo, Venky, are we contending the issue is that the mds journals something like:
"adding new preallocated inode(s including) x to session"
"using inode x"

and then later trims the event adding x to the preallocated list, without trimming the use of x? And this causes trouble?

If that's the case, it seems like we messed up journaling when adding preallocated inodes to the protocol — we need to add some kind of tracking structure that we keep persistent.

Greg Farnum wrote:

I was talking to Dhairya about this today and am not quite sure I understand.

Xiubo, Venky, are we contending the issue is that the mds journals something like:
"adding new preallocated inode(s including) x to session"
"using inode x"

and then later trims the event adding x to the preallocated list, without trimming the use of x? And this causes trouble?

If that's the case, it seems like we messed up journaling when adding preallocated inodes to the protocol — we need to add some kind of tracking structure that we keep persistent.

I went through the code again, for example in case:

1, a client mkdir request comes
2, handle_client_mkdir() --> prepare_new_inode(): it will preallocate some ino#*s and save it in *mdr and return one ino# from them.
3, journal_and_reply(): this will submit a EUpdate(mdlog, "mkdir"); log entry to the MDLog.
4, after the above journal log entry is flushed to Rados pool, it will send a safe reply to client
5, for safe reply in respond_to_request() --> reply_client_request() --> apply_allocated_inos(): it will insert the preallocated ino#*s from Step2 to the corresponding *session, and then mark the session as dirty:

 3559 void Server::apply_allocated_inos(MDRequestRef& mdr, Session *session)
 3560 {
 3561   dout(10) << "apply_allocated_inos " << mdr->alloc_ino
 3562            << " / " << mdr->prealloc_inos      
 3563            << " / " << mdr->used_prealloc_ino << dendl; 
 3564  
 3565   if (mdr->alloc_ino) {
 3566     mds->inotable->apply_alloc_id(mdr->alloc_ino);
 3567   }
 3568   if (mdr->prealloc_inos.size()) {
 3569     ceph_assert(session);
 3570     session->pending_prealloc_inos.subtract(mdr->prealloc_inos);
 3571     session->free_prealloc_inos.insert(mdr->prealloc_inos);
 3572     session->info.prealloc_inos.insert(mdr->prealloc_inos);
 3573     mds->sessionmap.mark_dirty(session, !mdr->used_prealloc_ino);
 3574     mds->inotable->apply_alloc_ids(mdr->prealloc_inos);
 3575   }
 3576   if (mdr->used_prealloc_ino) {
 3577     ceph_assert(session);
 3578     session->info.prealloc_inos.erase(mdr->used_prealloc_ino);
 3579     mds->sessionmap.mark_dirty(session);
 3580   }
 3581 }                            

6, till now the journal logs in the Rados pool have include the ino# for the new made dirctory.
7, later when trim/expiring the MDLog segments the MDS will save() the dirty sessions to the pool, which including all the preallocated *ino#*s

What if just before the Step7 the MDS crashed and then failover to the standy-reply MDS and then at the same time starts a new standby-replay daemon ?

Maybe there are other cases like above ?

Xiubo Li wrote:

Greg Farnum wrote:

I was talking to Dhairya about this today and am not quite sure I understand.

Xiubo, Venky, are we contending the issue is that the mds journals something like:
"adding new preallocated inode(s including) x to session"
"using inode x"

and then later trims the event adding x to the preallocated list, without trimming the use of x? And this causes trouble?

If that's the case, it seems like we messed up journaling when adding preallocated inodes to the protocol — we need to add some kind of tracking structure that we keep persistent.

I went through the code again, for example in case:

1, a client mkdir request comes
2, handle_client_mkdir() --> prepare_new_inode(): it will preallocate some ino#*s and save it in *mdr and return one ino# from them.
3, journal_and_reply(): this will submit a EUpdate(mdlog, "mkdir"); log entry to the MDLog.
4, after the above journal log entry is flushed to Rados pool, it will send a safe reply to client
5, for safe reply in respond_to_request() --> reply_client_request() --> apply_allocated_inos(): it will insert the preallocated ino#*s from Step2 to the corresponding *session, and then mark the session as dirty:

[...]

6, till now the journal logs in the Rados pool have include the ino# for the new made dirctory.
7, later when trim/expiring the MDLog segments the MDS will save() the dirty sessions to the pool, which including all the preallocated *ino#*s

What if just before the Step7 the MDS crashed and then failover to the standy-reply MDS and then at the same time starts a new standby-replay daemon ?

if this happens, since the dirty sessions are not saved to the pool, it implies they are now lost? If there is only a single active MDS then it is no more recoverable? i hope i'm wrong because this sounds a lot scary to me if it is true. if this hypothesis of mine is not true, in the case of what xiubo explained, how does our code make sure they are not lost? since the dirty sessions must be stored on the MDS cache right and if it's gone/restarted then the cache is gone too

Maybe there are other cases like above ?

Xiubo Li wrote:

Greg Farnum wrote:

I was talking to Dhairya about this today and am not quite sure I understand.

Xiubo, Venky, are we contending the issue is that the mds journals something like:
"adding new preallocated inode(s including) x to session"
"using inode x"

and then later trims the event adding x to the preallocated list, without trimming the use of x? And this causes trouble?

If that's the case, it seems like we messed up journaling when adding preallocated inodes to the protocol — we need to add some kind of tracking structure that we keep persistent.

I went through the code again, for example in case:

1, a client mkdir request comes
2, handle_client_mkdir() --> prepare_new_inode(): it will preallocate some ino#*s and save it in *mdr and return one ino# from them.
3, journal_and_reply(): this will submit a EUpdate(mdlog, "mkdir"); log entry to the MDLog.
4, after the above journal log entry is flushed to Rados pool, it will send a safe reply to client
5, for safe reply in respond_to_request() --> reply_client_request() --> apply_allocated_inos(): it will insert the preallocated ino#*s from Step2 to the corresponding *session, and then mark the session as dirty:

[...]

6, till now the journal logs in the Rados pool have include the ino# for the new made dirctory.
7, later when trim/expiring the MDLog segments the MDS will save() the dirty sessions to the pool, which including all the preallocated *ino#*s

What if just before the Step7 the MDS crashed and then failover to the standy-reply MDS and then at the same time starts a new standby-replay daemon ?

Do you mean we can trim the journal before the session is updated on disk? That seems like we just need to make the session map flush block journal trimming.

But I see at https://github.com/ceph/ceph/blob/main/src/mds/journal.cc#L279, we are indeed blocking the journal trim on the sessionmap flush. So the issue is just that we let clients use preallocated inodes before we flush them out to disk. We need to rethink this — either by flushing the sessionmap before clients can access new preallocated inodes, or by changing replay so that it can handle this case.

(I got to this rather late - so excuse me for any discussion that were already resolved).

Dhairya Parmar wrote:

Xiubo Li wrote:

Greg Farnum wrote:

I was talking to Dhairya about this today and am not quite sure I understand.

Xiubo, Venky, are we contending the issue is that the mds journals something like:
"adding new preallocated inode(s including) x to session"
"using inode x"

and then later trims the event adding x to the preallocated list, without trimming the use of x? And this causes trouble?

If that's the case, it seems like we messed up journaling when adding preallocated inodes to the protocol — we need to add some kind of tracking structure that we keep persistent.

I went through the code again, for example in case:

1, a client mkdir request comes
2, handle_client_mkdir() --> prepare_new_inode(): it will preallocate some ino#*s and save it in *mdr and return one ino# from them.
3, journal_and_reply(): this will submit a EUpdate(mdlog, "mkdir"); log entry to the MDLog.
4, after the above journal log entry is flushed to Rados pool, it will send a safe reply to client
5, for safe reply in respond_to_request() --> reply_client_request() --> apply_allocated_inos(): it will insert the preallocated ino#*s from Step2 to the corresponding *session, and then mark the session as dirty:

[...]

6, till now the journal logs in the Rados pool have include the ino# for the new made dirctory.
7, later when trim/expiring the MDLog segments the MDS will save() the dirty sessions to the pool, which including all the preallocated *ino#*s

What if just before the Step7 the MDS crashed and then failover to the standy-reply MDS and then at the same time starts a new standby-replay daemon ?

if this happens, since the dirty sessions are not saved to the pool, it implies they are now lost?

The required info is persisted in the mds journal, so they aren't lost.

If there is only a single active MDS then it is no more recoverable?

If someone is running with a single active mds, they chose to shoot in their foot. This isn't particularly related to this crash.

Greg Farnum wrote:

Xiubo Li wrote:

Greg Farnum wrote:

I was talking to Dhairya about this today and am not quite sure I understand.

Xiubo, Venky, are we contending the issue is that the mds journals something like:
"adding new preallocated inode(s including) x to session"
"using inode x"

and then later trims the event adding x to the preallocated list, without trimming the use of x? And this causes trouble?

If that's the case, it seems like we messed up journaling when adding preallocated inodes to the protocol — we need to add some kind of tracking structure that we keep persistent.

I went through the code again, for example in case:

1, a client mkdir request comes
2, handle_client_mkdir() --> prepare_new_inode(): it will preallocate some ino#*s and save it in *mdr and return one ino# from them.
3, journal_and_reply(): this will submit a EUpdate(mdlog, "mkdir"); log entry to the MDLog.
4, after the above journal log entry is flushed to Rados pool, it will send a safe reply to client
5, for safe reply in respond_to_request() --> reply_client_request() --> apply_allocated_inos(): it will insert the preallocated ino#*s from Step2 to the corresponding *session, and then mark the session as dirty:

[...]

6, till now the journal logs in the Rados pool have include the ino# for the new made dirctory.
7, later when trim/expiring the MDLog segments the MDS will save() the dirty sessions to the pool, which including all the preallocated *ino#*s

What if just before the Step7 the MDS crashed and then failover to the standy-reply MDS and then at the same time starts a new standby-replay daemon ?

Do you mean we can trim the journal before the session is updated on disk? That seems like we just need to make the session map flush block journal trimming.

But I see at https://github.com/ceph/ceph/blob/main/src/mds/journal.cc#L279, we are indeed blocking the journal trim on the sessionmap flush. So the issue is just that we let clients use preallocated inodes before we flush them out to disk. We need to rethink this — either by flushing the sessionmap before clients can access new preallocated inodes, or by changing replay so that it can handle this case.

That's indeed problematic. Would hitting this issue even require an active MDS crash? The standby-replay daemon following the active MDS journal would see the inode# before it loads the prealloc_inos interval set.

Greg Farnum wrote:

Xiubo Li wrote:

Greg Farnum wrote:

I was talking to Dhairya about this today and am not quite sure I understand.

Xiubo, Venky, are we contending the issue is that the mds journals something like:
"adding new preallocated inode(s including) x to session"
"using inode x"

and then later trims the event adding x to the preallocated list, without trimming the use of x? And this causes trouble?

If that's the case, it seems like we messed up journaling when adding preallocated inodes to the protocol — we need to add some kind of tracking structure that we keep persistent.

I went through the code again, for example in case:

1, a client mkdir request comes
2, handle_client_mkdir() --> prepare_new_inode(): it will preallocate some ino#*s and save it in *mdr and return one ino# from them.
3, journal_and_reply(): this will submit a EUpdate(mdlog, "mkdir"); log entry to the MDLog.
4, after the above journal log entry is flushed to Rados pool, it will send a safe reply to client
5, for safe reply in respond_to_request() --> reply_client_request() --> apply_allocated_inos(): it will insert the preallocated ino#*s from Step2 to the corresponding *session, and then mark the session as dirty:

[...]

6, till now the journal logs in the Rados pool have include the ino# for the new made dirctory.
7, later when trim/expiring the MDLog segments the MDS will save() the dirty sessions to the pool, which including all the preallocated *ino#*s

What if just before the Step7 the MDS crashed and then failover to the standy-reply MDS and then at the same time starts a new standby-replay daemon ?

Do you mean we can trim the journal before the session is updated on disk? That seems like we just need to make the session map flush block journal trimming.

But I see at https://github.com/ceph/ceph/blob/main/src/mds/journal.cc#L279, we are indeed blocking the journal trim on the sessionmap flush. So the issue is just that we let clients use preallocated inodes before we flush them out to disk. We need to rethink this — either by flushing the sessionmap before clients can access new preallocated inodes, or by changing replay so that it can handle this case.

I can't think of how this can be fixed-up in replay apart from adjusting the prealloc_inos set for the session (EUpdate had client_name to lookup in sessionmap) on-the-fly and later when a ESession event is read from the journal, fix up prealloc_inos set again?

Greg Farnum wrote:

Xiubo Li wrote:

Greg Farnum wrote:

I was talking to Dhairya about this today and am not quite sure I understand.

Xiubo, Venky, are we contending the issue is that the mds journals something like:
"adding new preallocated inode(s including) x to session"
"using inode x"

and then later trims the event adding x to the preallocated list, without trimming the use of x? And this causes trouble?

If that's the case, it seems like we messed up journaling when adding preallocated inodes to the protocol — we need to add some kind of tracking structure that we keep persistent.

I went through the code again, for example in case:

1, a client mkdir request comes
2, handle_client_mkdir() --> prepare_new_inode(): it will preallocate some ino#*s and save it in *mdr and return one ino# from them.
3, journal_and_reply(): this will submit a EUpdate(mdlog, "mkdir"); log entry to the MDLog.
4, after the above journal log entry is flushed to Rados pool, it will send a safe reply to client
5, for safe reply in respond_to_request() --> reply_client_request() --> apply_allocated_inos(): it will insert the preallocated ino#*s from Step2 to the corresponding *session, and then mark the session as dirty:

[...]

6, till now the journal logs in the Rados pool have include the ino# for the new made dirctory.
7, later when trim/expiring the MDLog segments the MDS will save() the dirty sessions to the pool, which including all the preallocated *ino#*s

What if just before the Step7 the MDS crashed and then failover to the standy-reply MDS and then at the same time starts a new standby-replay daemon ?

Do you mean we can trim the journal before the session is updated on disk? That seems like we just need to make the session map flush block journal trimming.

But I see at https://github.com/ceph/ceph/blob/main/src/mds/journal.cc#L279, we are indeed blocking the journal trim on the sessionmap flush. So the issue is just that we let clients use preallocated inodes before we flush them out to disk. We need to rethink this — either by flushing the sessionmap before clients can access new preallocated inodes, or by changing replay so that it can handle this case.

i read this comment in void SessionMap::replay_open_sessions()

  // 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.

so if marking a session dirty can flush all the existing dirty sessions then basically there shouldn't been any issue right? or there could be a possibility that while it was flushing the mds crashed?

Dhairya Parmar wrote:

Greg Farnum wrote:

Xiubo Li wrote:

Greg Farnum wrote:

I was talking to Dhairya about this today and am not quite sure I understand.

Xiubo, Venky, are we contending the issue is that the mds journals something like:
"adding new preallocated inode(s including) x to session"
"using inode x"

and then later trims the event adding x to the preallocated list, without trimming the use of x? And this causes trouble?

If that's the case, it seems like we messed up journaling when adding preallocated inodes to the protocol — we need to add some kind of tracking structure that we keep persistent.

I went through the code again, for example in case:

1, a client mkdir request comes
2, handle_client_mkdir() --> prepare_new_inode(): it will preallocate some ino#*s and save it in *mdr and return one ino# from them.
3, journal_and_reply(): this will submit a EUpdate(mdlog, "mkdir"); log entry to the MDLog.
4, after the above journal log entry is flushed to Rados pool, it will send a safe reply to client
5, for safe reply in respond_to_request() --> reply_client_request() --> apply_allocated_inos(): it will insert the preallocated ino#*s from Step2 to the corresponding *session, and then mark the session as dirty:

[...]

6, till now the journal logs in the Rados pool have include the ino# for the new made dirctory.
7, later when trim/expiring the MDLog segments the MDS will save() the dirty sessions to the pool, which including all the preallocated *ino#*s

What if just before the Step7 the MDS crashed and then failover to the standy-reply MDS and then at the same time starts a new standby-replay daemon ?

Do you mean we can trim the journal before the session is updated on disk? That seems like we just need to make the session map flush block journal trimming.

But I see at https://github.com/ceph/ceph/blob/main/src/mds/journal.cc#L279, we are indeed blocking the journal trim on the sessionmap flush. So the issue is just that we let clients use preallocated inodes before we flush them out to disk. We need to rethink this — either by flushing the sessionmap before clients can access new preallocated inodes, or by changing replay so that it can handle this case.

i read this comment in void SessionMap::replay_open_sessions()
[...]

so if marking a session dirty can flush all the existing dirty sessions then basically there shouldn't been any issue right? or there could be a possibility that while it was flushing the mds crashed?

The comment says it may flush all existing dirty sessions. If you see SessionMap::mark_dirty():

void SessionMap::_mark_dirty(Session *s, bool may_save)
{
  if (dirty_sessions.count(s->info.inst.name))
    return;

  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                                                                                                                                                                                                     
    save(new C_MDSInternalNoop, version);
  }

  null_sessions.erase(s->info.inst.name);
  dirty_sessions.insert(s->info.inst.name);
}

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, may_save);
  version++;
  s->pop_pv(version);
}

It doesn't always flush dirty sessions when marked.

Xiubo/Dhairya, if the MDS can ensure that the sessionmap is persisted before the client starts using the prellocated inode, then the changes in https://github.com/ceph/ceph/pull/49970 aren't needed. Would you agree?

Venky Shankar wrote:

Xiubo/Dhairya, if the MDS can ensure that the sessionmap is persisted before the client starts using the prellocated inode, then the changes in https://github.com/ceph/ceph/pull/49970 aren't needed. Would you agree?

i think yes. if the sessionmap is persisted then we might not need it.

Just FYI - workaround for this crash is to disable async dirops in the (kernel) client (mount option `nowsync`).

Thinking more about this: the mds could delay sending the prealloc inode range to the client till the session metadata (info.prealloc_inos) is persisted.

Question to Xiubo (copied from an internal channel):
Preallocated inode ranges were handed out by the mds to clients starting octopus, however, the mds has been preallocating inodes way before octopus (I'm seeing the jewel branch and the prealloc stuff is there in the MDS and it goes waaaay back). So, if you see prepare_new_inode() in jewel branch, its called with an inode number that the client could send. Do you know if the client was sending that in request head and did it expect a different ino# to be sent back by the MDS?

Venky Shankar wrote:

Thinking more about this: the mds could delay sending the prealloc inode range to the client till the session metadata (info.prealloc_inos) is persisted.

Question to Xiubo (copied from an internal channel):
Preallocated inode ranges were handed out by the mds to clients starting octopus, however, the mds has been preallocating inodes way before octopus (I'm seeing the jewel branch and the prealloc stuff is there in the MDS and it goes waaaay back). So, if you see prepare_new_inode() in jewel branch, its called with an inode number that the client could send. Do you know if the client was sending that in request head and did it expect a different ino# to be sent back by the MDS?

nvm - the ino field in the request head is used by client when replaying ops.

Venky Shankar wrote:

Thinking more about this: the mds could delay sending the prealloc inode range to the client till the session metadata (info.prealloc_inos) is persisted.

I'm testing a fix today and if things go well, I'll submit a change for review.

@here - curious if https://tracker.ceph.com/issues/60241 is actually related to this ticket. The former has got completely different backtrace and the only matching thing is an assertion in interval_set...

Please see https://github.com/ceph/ceph/pull/53752#issuecomment-2056469527 for the status of the change.

This issue hasn't been RCA'd yet.

Explanation of the preallocated machinery which might help in the future:

I played around a bit more with preallocated inodes and how frequently
sessionmap is persisted (since sessionmap carries `prealloc_inos`
interval set). Basically, I tried to explain what causes this
particular check

https://github.com/ceph/ceph/pull/53752/commits/c7dffbf53bb79a71d3e9eb346e01bba2236f1f89#diff-faaa4413e4be5d8bd2b67b3a956c4b26c28d42735a5a19f1b18ed9ad9621b820R1611

to hold true during reply (the above link is from a recent PR that I
pushed to delay delegating inode ranges to clients till the sessionmap
is persisted). The revert commit brings back the check which was
removed under the assumption that the sessionmap version can fall
behind (>2) as compared to the version persisted in the log event. So,
I forcefully made the MDS to crash at a point where the on-disk
sessionmap version would fall behind a lot as compared to the
projected/dirty sessionmap version in-memory. The failed over MDS was
able to cope up with the version lag by utilising the version stored
in ESession and bring the sessionmap version up-to-date with the
latest (actually, during replay, the MDS specially handles the version
lag for ESession/EUpdate/EMetaBlob and two other event types which are
mostly for subtree migration case) . For EMetaBlob, the version lag
can be 1 or 2 depending on preallocated_inos and used_prealloc_ino
being set/unset. So, this got explained. However, I expected the MDS
to assert while deleting a preallocated inode from the preallocated
set

https://github.com/ceph/ceph/pull/53752/commits/c7dffbf53bb79a71d3e9eb346e01bba2236f1f89#diff-faaa4413e4be5d8bd2b67b3a956c4b26c28d42735a5a19f1b18ed9ad9621b820R1623

... since I had synthetically induced the crash at a point where the
preallocated set isn't persisted when it is being handed over to the
client. Well, the MDS copes up with that too! It rebuilds the
preallocated set from the log events (by adding [inserting] into
session->info.prealloc_inos(...)). So, ideally, once can get away from
not persisting the sessionmap for a long time (until trim - which is
what happens). So, a single log event holding the preallocated inode
set (which was filled up during a request) is all that's required to
replay the next 1000 (default number of preallocated inodes) without
persisting the sessionmap (and probably much more).

Greg Farnum wrote in #note-16:

Xiubo Li wrote:

Greg Farnum wrote:

I was talking to Dhairya about this today and am not quite sure I understand.

Xiubo, Venky, are we contending the issue is that the mds journals something like:
"adding new preallocated inode(s including) x to session"
"using inode x"

and then later trims the event adding x to the preallocated list, without trimming the use of x? And this causes trouble?

If that's the case, it seems like we messed up journaling when adding preallocated inodes to the protocol — we need to add some kind of tracking structure that we keep persistent.

I went through the code again, for example in case:

1, a client mkdir request comes
2, handle_client_mkdir() --> prepare_new_inode(): it will preallocate some ino#*s and save it in *mdr and return one ino# from them.
3, journal_and_reply(): this will submit a EUpdate(mdlog, "mkdir"); log entry to the MDLog.
4, after the above journal log entry is flushed to Rados pool, it will send a safe reply to client
5, for safe reply in respond_to_request() --> reply_client_request() --> apply_allocated_inos(): it will insert the preallocated ino#*s from Step2 to the corresponding *session, and then mark the session as dirty:

[...]

6, till now the journal logs in the Rados pool have include the ino# for the new made dirctory.
7, later when trim/expiring the MDLog segments the MDS will save() the dirty sessions to the pool, which including all the preallocated *ino#*s

What if just before the Step7 the MDS crashed and then failover to the standy-reply MDS and then at the same time starts a new standby-replay daemon ?

Do you mean we can trim the journal before the session is updated on disk? That seems like we just need to make the session map flush block journal trimming.

But I see at https://github.com/ceph/ceph/blob/main/src/mds/journal.cc#L279, we are indeed blocking the journal trim on the sessionmap flush. So the issue is just that we let clients use preallocated inodes before we flush them out to disk. We need to rethink this — either by flushing the sessionmap before clients can access new preallocated inodes, or by changing replay so that it can handle this case.

Venky, Greg,

Checked this again and if my understanding is correct the https://github.com/ceph/ceph/blob/main/src/mds/journal.cc#L279 will be the only case that will save the sessionmap to metadata pool, without considering the mark_dirty() case. But this will happen only when expiring the LogSegments, not flushing the MDLogs to metadata pool.

Which means even after the MDLogs are flushed to metadata pool and if the trim never happens the dirty sessionmap still could be in the mds cache. And if the mds is stopped normally we can make sure that all the LogSegments will be trimmed correctly and then the sessionmap will be flushed at the same time.

But what if no trim happens and the MDS crashes and then failover ? My understanding is that the sessionmap could be empty or be partially flushed and then when the new MDS is replaying it could hit this bug ?