Ceph » CephFS

Patrick,

Do you mean a standby-replay MDS should tolerate missing journal objects? How can it end up in such a situation. (I recall you mentioning this in a BZ somewhere).

Venky Shankar wrote:

Patrick,

Do you mean a standby-replay MDS should tolerate missing journal objects?

Well, it supposed to already:

https://github.com/ceph/ceph/blob/5ab5ff46d6d799c46d8a1c01cc08068a22e4c9b5/src/mds/MDLog.cc#L1320-L1325

Note, ENOENT (-2) got converted to EAGAIN. However, I don't see why the assert on 1381 gets triggered. Maybe the assert is a different bug?

How can it end up in such a situation. (I recall you mentioning this in a BZ somewhere).

It can happen if the standby-replay daemon gets behind the MDS it's following. In that case, the only thing to do is reboot the standby-replay daemon because it cannot know what the missing journal segments contained. It should do that gracefully in a way which does not freak out the cluster admin. But a cluster INFO/WRN would be useful to record the event in case there is a pattern.

Patrick Donnelly wrote:

Venky Shankar wrote:

Patrick,

Do you mean a standby-replay MDS should tolerate missing journal objects?

Well, it supposed to already:

https://github.com/ceph/ceph/blob/5ab5ff46d6d799c46d8a1c01cc08068a22e4c9b5/src/mds/MDLog.cc#L1320-L1325

Note, ENOENT (-2) got converted to EAGAIN. However, I don't see why the assert on 1381 gets triggered. Maybe the assert is a different bug?

is_readable() is guaranteed to be false if we've hit the block at 1322 because of:

if (error != 0) {
return false;

I've seen plenty of instances of standby-replay MDSes respawning when they fall behind the journal, so there are some working code paths, but this doesn't seem like one of them to me.

Greg Farnum wrote:

Patrick Donnelly wrote:

Venky Shankar wrote:

Patrick,

Do you mean a standby-replay MDS should tolerate missing journal objects?

Well, it supposed to already:

https://github.com/ceph/ceph/blob/5ab5ff46d6d799c46d8a1c01cc08068a22e4c9b5/src/mds/MDLog.cc#L1320-L1325

Note, ENOENT (-2) got converted to EAGAIN. However, I don't see why the assert on 1381 gets triggered. Maybe the assert is a different bug?

is_readable() is guaranteed to be false if we've hit the block at 1322 because of:

if (error != 0) {
return false;

If block 1322 is run then 1381 should be unreachable because:

https://github.com/ceph/ceph/blob/5ab5ff46d6d799c46d8a1c01cc08068a22e4c9b5/src/mds/MDLog.cc#L1374

The calls to journaler->is_readable() and journaler->get_error() in MDLog::_replay_thread() will drop Journaler::lock between invocations, so, theoretically, its possible that the initial check:

  // loop
  int r = 0;
  while (1) {
    // wait for read?
    while (!journaler->is_readable() &&
       journaler->get_read_pos() < journaler->get_write_pos() &&
       !journaler->get_error()) {
      C_SaferCond readable_waiter;
      journaler->wait_for_readable(&readable_waiter);
      r = readable_waiter.wait();
    }
    if (journaler->get_error()) {
      r = journaler->get_error();
      dout(0) << "_replay journaler got error " << r << ", aborting" << dendl;

journaler->is_readable() returned true, thereby breaking out of the (inner) while loop and by passing the journaler->get_error() check, and by the time this hits the next set of checks:

    if (!journaler->is_readable() &&
    journaler->get_read_pos() == journaler->get_write_pos())
      break;

    ceph_assert(journaler->is_readable() || mds->is_daemon_stopping());

It's possible that the journal is unreadable due to some error that happened during prefetch. In short, it seems that these checks are racy. Can someone fault my logic?

Venky Shankar wrote:

The calls to journaler->is_readable() and journaler->get_error() in MDLog::_replay_thread() will drop Journaler::lock between invocations, so, theoretically, its possible that the initial check:

[...]

journaler->is_readable() returned true, thereby breaking out of the (inner) while loop and by passing the journaler->get_error() check, and by the time this hits the next set of checks:

[...]

It's possible that the journal is unreadable due to some error that happened during prefetch. In short, it seems that these checks are racy. Can someone fault my logic?

Yes, that makes sense. The "stopping" check originates from 39cf07118583166287ef0faa1811ae8efc9bef85. I think it along with the `is_readable` check can be thrown out. At this point, try to read an entry (which does a readable check) and do normal error handling.

Patrick Donnelly wrote:

Venky Shankar wrote:

The calls to journaler->is_readable() and journaler->get_error() in MDLog::_replay_thread() will drop Journaler::lock between invocations, so, theoretically, its possible that the initial check:

[...]

journaler->is_readable() returned true, thereby breaking out of the (inner) while loop and by passing the journaler->get_error() check, and by the time this hits the next set of checks:

[...]

It's possible that the journal is unreadable due to some error that happened during prefetch. In short, it seems that these checks are racy. Can someone fault my logic?

Yes, that makes sense. The "stopping" check originates from 39cf07118583166287ef0faa1811ae8efc9bef85. I think it along with the `is_readable` check can be thrown out. At this point, try to read an entry (which does a readable check) and do normal error handling.

Yeh, those assert checks are racy. I wonder why we do not run into these failure with our fs suite (maybe out s-r jobs aren't enough or aren't really metadata workload intensive).